Author: Aditya Singh Date: 2026-03-04T11:57:32+08:00 New Revision: 4b85c1301fb61c6965a5143113f5170a4b062e3a
URL: https://github.com/llvm/llvm-project/commit/4b85c1301fb61c6965a5143113f5170a4b062e3a DIFF: https://github.com/llvm/llvm-project/commit/4b85c1301fb61c6965a5143113f5170a4b062e3a.diff LOG: [clang-tidy] Fix false positive in readability-redundant-preprocessor for builtin checks (#181734) Different `__has_builtin()` checks were incorrectly flagged as redundant because ConditionRange collapsed after macro expansion. It now reads condition text directly from source to fix this. Assisted-by: Claude Fixes #64825 Added: Modified: clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability/redundant-preprocessor.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp index 4c503714346f8..28cdf7e23bcfb 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp @@ -14,7 +14,43 @@ namespace clang::tidy::readability { +static StringRef getConditionText(SourceLocation Loc, const SourceManager &SM, + const LangOptions &LangOpts) { + bool Invalid = false; + const FileID FID = SM.getFileID(Loc); + const StringRef Buffer = SM.getBufferData(FID, &Invalid); + if (Invalid) + return {}; + + // Initialize a raw lexer starting exactly at the condition's location + Lexer RawLexer(SM.getLocForStartOfFile(FID), LangOpts, Buffer.begin(), + SM.getCharacterData(Loc), Buffer.end()); + RawLexer.SetCommentRetentionState(true); + + Token Tok; + // Lex the 'if' token itself + RawLexer.LexFromRawLexer(Tok); + + const unsigned StartOffset = SM.getFileOffset(Tok.getEndLoc()); + unsigned EndOffset = StartOffset; + + // Lex tokens until we hit the start of a new line or EOF. + // The lexer handles backslash line continuations automatically. + while (!RawLexer.LexFromRawLexer(Tok)) { + if (Tok.isAtStartOfLine() || Tok.is(tok::eof)) + break; + EndOffset = SM.getFileOffset(Tok.getLocation()) + Tok.getLength(); + } + + if (EndOffset <= StartOffset) + return {}; + + // Extract the raw text from the buffer to preserve original spacing + return Buffer.substr(StartOffset, EndOffset - StartOffset).trim(); +} + namespace { + /// Information about an opening preprocessor directive. struct PreprocessorEntry { SourceLocation Loc; @@ -37,8 +73,7 @@ class RedundantPreprocessorCallbacks : public PPCallbacks { void If(SourceLocation Loc, SourceRange ConditionRange, ConditionValueKind ConditionValue) override { const StringRef Condition = - Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), - PP.getSourceManager(), PP.getLangOpts()); + getConditionText(Loc, PP.getSourceManager(), PP.getLangOpts()); checkMacroRedundancy(Loc, Condition, IfStack, DK_If, DK_If, true); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index cf74f6f6b34be..52e14d63d7b37 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -290,6 +290,11 @@ Changes in existing checks positives on parameters used in dependent expressions (e.g. inside generic lambdas). +- Improved :doc:`readability-redundant-preprocessor + <clang-tidy/checks/readability/redundant-preprocessor>` check by fixing a + false positive for nested ``#if`` directives using diff erent builtin + expressions such as ``__has_builtin`` and ``__has_cpp_attribute``. + - Improved :doc:`readability-simplify-boolean-expr <clang-tidy/checks/readability/simplify-boolean-expr>` check to provide valid fix suggestions for C23 and later by not using ``static_cast``. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-preprocessor.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-preprocessor.cpp index 5429898e7ccd7..72c05bae1bac2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-preprocessor.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-preprocessor.cpp @@ -82,3 +82,156 @@ void k(); void k(); #endif #endif + +// Different builtin checks should NOT trigger warning +#if __has_builtin(__remove_cvref) +# if __has_cpp_attribute(no_unique_address) +# endif +#endif + +// Redundant nested #if +#if defined(FOO) +// CHECK-NOTES: [[@LINE+1]]:4: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor] +# if defined(FOO) +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here +# endif +#endif + +// Different __has_builtin checks +#if __has_builtin(__builtin_assume) +# if __has_builtin(__builtin_expect) +# endif +#endif + +// Different __has_cpp_attribute checks +#if __has_cpp_attribute(fallthrough) +# if __has_cpp_attribute(nodiscard) +# endif +#endif + +// Same __has_builtin check +#if __has_builtin(__remove_cvref) +// CHECK-NOTES: [[@LINE+1]]:4: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor] +# if __has_builtin(__remove_cvref) +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here +# endif +#endif + +// Different __has_include checks +#if __has_include(<vector>) +# if __has_include(<string>) +# endif +#endif + +// Complex expressions - diff erent conditions +#if __has_builtin(__builtin_clz) && defined(__GNUC__) +# if __has_builtin(__builtin_ctz) && defined(__GNUC__) +# endif +#endif + +#define MACRO1 +#define MACRO2 + +// Redundant #ifdef +#ifdef MACRO1 +// CHECK-NOTES: [[@LINE+1]]:4: warning: nested redundant #ifdef; consider removing it [readability-redundant-preprocessor] +# ifdef MACRO1 +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #ifdef was here +# endif +#endif + +// Different macros - #ifdef +#ifdef MACRO1 +# ifdef MACRO2 +# endif +#endif + +// Redundant #ifndef +#ifndef MACRO3 +// CHECK-NOTES: [[@LINE+1]]:4: warning: nested redundant #ifndef; consider removing it [readability-redundant-preprocessor] +# ifndef MACRO3 +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #ifndef was here +# endif +#endif + +// Different macros - #ifndef +#ifndef MACRO3 +# ifndef MACRO4 +# endif +#endif + +// Different __has_feature checks +#if __has_feature(cxx_rvalue_references) +# if __has_feature(cxx_lambdas) +# endif +#endif + +// Different version checks +#if __cplusplus >= 201103L +# if __cplusplus >= 201402L +# endif +#endif + +// Different builtin functions with similar names +#if __has_builtin(__builtin_addressof) +# if __has_builtin(__builtin_assume_aligned) +# endif +#endif + +// Different compiler checks +#if defined(__clang__) +# if defined(__GNUC__) +# endif +#endif + +// Mixed builtin and regular macro +#if __has_builtin(__make_integer_seq) +# if defined(USE_STD_INTEGER_SEQUENCE) +# endif +#endif + +// Different numeric comparisons +#if __GNUC__ >= 2 +# if __GNUC__ >= 3 +# endif +#endif + +// Test: inline comments - same condition and comment SHOULD warn +#if __has_builtin(__remove_cvref) // same comment +// CHECK-NOTES: [[@LINE+1]]:4: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor] +# if __has_builtin(__remove_cvref) // same comment +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here +# endif +#endif + +// Test: inline comments - diff erent comments, condition text diff ers, no warn +#if defined(FOO) // comment one +# if defined(FOO) // comment two +# endif +#endif + +// Test: block comments - same condition and comment SHOULD warn +#if __has_builtin(__remove_cvref) /* block */ +// CHECK-NOTES: [[@LINE+1]]:4: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor] +# if __has_builtin(__remove_cvref) /* block */ +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here +# endif +#endif + +// Test: multiline block comment spanning lines - same condition SHOULD warn +#if __has_builtin(__remove_cvref) /* multiline + comment */ +// CHECK-NOTES: [[@LINE+2]]:4: warning: nested redundant #if; consider removing it [readability-redundant-preprocessor] +// CHECK-NOTES: [[@LINE-3]]:2: note: previous #if was here +# if __has_builtin(__remove_cvref) /* multiline + comment */ +# endif +#endif + +// Test: multiline block comment - diff erent conditions should NOT warn +#if __has_builtin(__remove_cvref) /* multiline + comment */ +# if __has_cpp_attribute(no_unique_address) /* multiline + comment */ +# endif +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
