[clang-tools-extra] 4b85c13 - [clang-tidy] Fix false positive in readability-redundant-preprocessor for builtin checks (#181734)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 3 19:57:36 PST 2026
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
More information about the cfe-commits
mailing list