[clang-tools-extra] [clang-tidy] Fix false positive in readability-redundant-preprocessor for builtin checks (PR #181734)
Aditya Singh via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 18 05:59:06 PST 2026
https://github.com/Aditya26189 updated https://github.com/llvm/llvm-project/pull/181734
>From 36277411068768852abd8c660d5f6571d504b5d8 Mon Sep 17 00:00:00 2001
From: Aditya Singh <139976506+Aditya26189 at users.noreply.github.com>
Date: Tue, 17 Feb 2026 01:17:44 +0530
Subject: [PATCH 1/2] [clang-tidy] Fix false positive in
readability-redundant-preprocessor for builtin checks
The ConditionRange parameter passed to If() callback points to incomplete
token locations after macro expansion. For builtin expressions like
__has_builtin(__remove_cvref), the preprocessor's ExpandBuiltinMacro
replaces the entire expression with a single numeric token, causing the
condition range to collapse.
Implemented getConditionText() helper that reads the condition text
directly from the source buffer using Lexer::getLocForEndOfToken() on
the 'if' keyword location, handling backslash line continuations.
Fixes: llvm/llvm-project#64825
---
.../RedundantPreprocessorCheck.cpp | 49 +++++++-
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../readability/redundant-preprocessor.cpp | 113 ++++++++++++++++++
3 files changed, 163 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp
index 4c503714346f8..8b4342218dd70 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp
@@ -36,10 +36,13 @@ 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());
- checkMacroRedundancy(Loc, Condition, IfStack, DK_If, DK_If, true);
+ // ConditionRange is unreliable for builtin preprocessor expressions like
+ // __has_builtin(...) because ExpandBuiltinMacro replaces the entire
+ // expression with a single token, collapsing the range. Extract the
+ // condition text directly from source instead.
+ const StringRef Condition = getConditionText(Loc);
+ if (!Condition.empty())
+ checkMacroRedundancy(Loc, Condition, IfStack, DK_If, DK_If, true);
}
void Ifdef(SourceLocation Loc, const Token &MacroNameTok,
@@ -69,6 +72,44 @@ class RedundantPreprocessorCallbacks : public PPCallbacks {
}
private:
+ /// Extract the condition text following the 'if' keyword from source.
+ /// Loc points to the 'if' keyword token, so we find its end and read
+ /// forward through the source buffer to the end of the directive,
+ /// handling backslash line continuations.
+ StringRef getConditionText(SourceLocation Loc) {
+ const SourceManager &SM = PP.getSourceManager();
+ const SourceLocation SpellingLoc = SM.getSpellingLoc(Loc);
+
+ const SourceLocation AfterIf =
+ Lexer::getLocForEndOfToken(SpellingLoc, 0, SM, PP.getLangOpts());
+ if (AfterIf.isInvalid())
+ return {};
+
+ bool Invalid = false;
+ auto [FID, Offset] = SM.getDecomposedLoc(AfterIf);
+ const StringRef Buffer = SM.getBufferData(FID, &Invalid);
+ if (Invalid || Offset >= Buffer.size())
+ return {};
+
+ // Read to end-of-line, handling backslash line continuations.
+ size_t End = Offset;
+ while (End < Buffer.size()) {
+ const char C = Buffer[End];
+ if (C == '\r' || C == '\n') {
+ if (End > Offset && Buffer[End - 1] == '\\') {
+ ++End;
+ if (C == '\r' && End < Buffer.size() && Buffer[End] == '\n')
+ ++End;
+ continue;
+ }
+ break;
+ }
+ ++End;
+ }
+
+ return Buffer.slice(Offset, End).trim();
+ }
+
void checkMacroRedundancy(SourceLocation Loc, StringRef MacroName,
SmallVector<PreprocessorEntry, 4> &Stack,
DirectiveKind WarningKind, DirectiveKind NoteKind,
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ee057d9bf0444..8f14c544b501c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -236,6 +236,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 different 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..fbab94f9756bc 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,116 @@ 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 - different 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
>From 3103bc1ff9a822ce67dc938a8ef58ae34804ce26 Mon Sep 17 00:00:00 2001
From: Aditya Singh <139976506+Aditya26189 at users.noreply.github.com>
Date: Wed, 18 Feb 2026 19:27:23 +0530
Subject: [PATCH 2/2] address review: add comment test cases
---
.../readability/redundant-preprocessor.cpp | 40 +++++++++++++++++++
1 file changed, 40 insertions(+)
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 fbab94f9756bc..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
@@ -195,3 +195,43 @@ void k();
# 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 - different comments, condition text differs, 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 - different 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