[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
Mon Feb 16 13:40:41 PST 2026
https://github.com/Aditya26189 updated https://github.com/llvm/llvm-project/pull/181734
>From 66f98fd973305780d83c3dc68eac294f7c59aa6a 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] [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 +++++++-
.../readability/redundant-preprocessor.cpp | 113 ++++++++++++++++++
2 files changed, 158 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/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
More information about the cfe-commits
mailing list