https://github.com/localspook updated
https://github.com/llvm/llvm-project/pull/156220
>From 5baddd86094b185aeb5ecd34e13a3c715d6c1ee3 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victo...@outlook.com>
Date: Sat, 30 Aug 2025 19:32:56 -0700
Subject: [PATCH 1/2] [clang-tidy] add option to
`readability-use-concise-preprocessor-directives` to take consistency into
account
---
.../UseConcisePreprocessorDirectivesCheck.cpp | 164 ++++++++++++++----
.../UseConcisePreprocessorDirectivesCheck.h | 10 +-
clang-tools-extra/docs/ReleaseNotes.rst | 6 +
.../use-concise-preprocessor-directives.rst | 38 ++++
...cessor-directives-preserve-consistency.cpp | 108 ++++++++++++
5 files changed, 293 insertions(+), 33 deletions(-)
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/readability/use-concise-preprocessor-directives-preserve-consistency.cpp
diff --git
a/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp
b/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp
index 05c0088e6b41b..a8c92fbc619a0 100644
---
a/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp
+++
b/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/Lex/Lexer.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/STLForwardCompat.h"
#include <array>
@@ -18,44 +19,116 @@ namespace clang::tidy::readability {
namespace {
-class IfPreprocessorCallbacks final : public PPCallbacks {
+enum class DirectiveKind : bool { If, Elif };
+
+struct Rewrite {
+ SourceLocation DirectiveLoc;
+ SourceRange ConditionRange;
+ StringRef Macro;
+ bool Negated;
+ DirectiveKind Directive;
+};
+
+struct StackEntry {
+ SmallVector<Rewrite, 2> Rewrites;
+ bool ApplyingRewritesWouldBreakConsistency = false;
+};
+
+class UseConciseDirectivesPPCallbacks final : public PPCallbacks {
public:
- IfPreprocessorCallbacks(ClangTidyCheck &Check, const Preprocessor &PP)
- : Check(Check), PP(PP) {}
+ UseConciseDirectivesPPCallbacks(UseConcisePreprocessorDirectivesCheck &Check,
+ const Preprocessor &PP,
+ bool PreserveConsistency)
+ : Check(Check), PP(PP), PreserveConsistency(PreserveConsistency) {}
+
+ void Ifdef(SourceLocation, const Token &, const MacroDefinition &) override {
+ Stack.emplace_back();
+ }
+
+ void Ifndef(SourceLocation, const Token &, const MacroDefinition &) override
{
+ Stack.emplace_back();
+ }
void If(SourceLocation Loc, SourceRange ConditionRange,
ConditionValueKind) override {
- impl(Loc, ConditionRange, {"ifdef", "ifndef"});
+ Stack.emplace_back();
+ handleCondition(Loc, ConditionRange, DirectiveKind::If);
}
void Elif(SourceLocation Loc, SourceRange ConditionRange, ConditionValueKind,
SourceLocation) override {
- if (PP.getLangOpts().C23 || PP.getLangOpts().CPlusPlus23)
- impl(Loc, ConditionRange, {"elifdef", "elifndef"});
+ handleCondition(Loc, ConditionRange, DirectiveKind::Elif);
+ }
+
+ void Endif(SourceLocation, SourceLocation) override {
+ static constexpr std::array<std::array<StringRef, 2>, 2> Replacements = {{
+ {"ifdef", "ifndef"},
+ {"elifdef", "elifndef"},
+ }};
+
+ const auto &[Rewrites, ApplyingRewritesWouldBreakConsistency] =
+ Stack.back();
+ if (!(PreserveConsistency && ApplyingRewritesWouldBreakConsistency)) {
+ for (const auto &[DirectiveLoc, ConditionRange, Macro, Negated,
+ Directive] : Rewrites) {
+ const StringRef Replacement =
+ Replacements[llvm::to_underlying(Directive)][Negated];
+ Check.diag(
+ DirectiveLoc,
+ "preprocessor condition can be written more concisely using '#%0'")
+ << Replacement
+ << FixItHint::CreateReplacement(DirectiveLoc, Replacement)
+ << FixItHint::CreateReplacement(ConditionRange, Macro);
+ }
+ }
+
+ Stack.pop_back();
}
private:
- void impl(SourceLocation DirectiveLoc, SourceRange ConditionRange,
- const std::array<llvm::StringLiteral, 2> &Replacements) {
+ struct RewriteInfo {
+ StringRef Macro;
+ bool Negated;
+ };
+
+ void handleCondition(SourceLocation DirectiveLoc, SourceRange ConditionRange,
+ DirectiveKind Directive) {
+ if (Directive != DirectiveKind::Elif || PP.getLangOpts().C23 ||
+ PP.getLangOpts().CPlusPlus23)
+ if (const std::optional<RewriteInfo> Rewrite =
+ tryRewrite(ConditionRange)) {
+ Stack.back().Rewrites.push_back({DirectiveLoc, ConditionRange,
+ Rewrite->Macro, Rewrite->Negated,
+ Directive});
+ return;
+ }
+
+ if (!Stack.back().ApplyingRewritesWouldBreakConsistency)
+ Stack.back().ApplyingRewritesWouldBreakConsistency =
+ conditionContainsDefinedOperator(ConditionRange);
+ }
+
+ std::optional<RewriteInfo> tryRewrite(SourceRange ConditionRange) {
// Lexer requires its input range to be null-terminated.
- SmallString<128> Condition =
+ const StringRef SourceText =
Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
PP.getSourceManager(), PP.getLangOpts());
+ SmallString<128> Condition = SourceText;
Condition.push_back('\0');
- Lexer Lex(DirectiveLoc, PP.getLangOpts(), Condition.data(),
- Condition.data(), Condition.data() + Condition.size() - 1);
+ Lexer Lex({}, PP.getLangOpts(), Condition.data(), Condition.data(),
+ Condition.data() + Condition.size() - 1);
Token Tok;
- bool Inverted = false; // The inverted form of #*def is #*ndef.
+ bool Negated = false;
std::size_t ParensNestingDepth = 0;
for (;;) {
if (Lex.LexFromRawLexer(Tok))
- return;
+ return {};
if (Tok.is(tok::TokenKind::exclaim) ||
(PP.getLangOpts().CPlusPlus &&
Tok.is(tok::TokenKind::raw_identifier) &&
Tok.getRawIdentifier() == "not"))
- Inverted = !Inverted;
+ Negated = !Negated;
else if (Tok.is(tok::TokenKind::l_paren))
++ParensNestingDepth;
else
@@ -64,47 +137,80 @@ class IfPreprocessorCallbacks final : public PPCallbacks {
if (Tok.isNot(tok::TokenKind::raw_identifier) ||
Tok.getRawIdentifier() != "defined")
- return;
+ return {};
bool NoMoreTokens = Lex.LexFromRawLexer(Tok);
if (Tok.is(tok::TokenKind::l_paren)) {
if (NoMoreTokens)
- return;
+ return {};
++ParensNestingDepth;
NoMoreTokens = Lex.LexFromRawLexer(Tok);
}
if (Tok.isNot(tok::TokenKind::raw_identifier))
- return;
- const StringRef Macro = Tok.getRawIdentifier();
+ return {};
+
+ // We need a stable StringRef into the ConditionRange, but because Lexer
+ // forces us to work on a temporary null-terminated copy of the
+ // ConditionRange, we need to do this ugly translation.
+ const StringRef Macro = {
+ SourceText.data() + (Tok.getRawIdentifier().data() - Condition.data()),
+ Tok.getRawIdentifier().size()};
while (!NoMoreTokens) {
NoMoreTokens = Lex.LexFromRawLexer(Tok);
if (Tok.isNot(tok::TokenKind::r_paren))
- return;
+ return {};
--ParensNestingDepth;
}
if (ParensNestingDepth != 0)
- return;
-
- Check.diag(
- DirectiveLoc,
- "preprocessor condition can be written more concisely using '#%0'")
- << FixItHint::CreateReplacement(DirectiveLoc, Replacements[Inverted])
- << FixItHint::CreateReplacement(ConditionRange, Macro)
- << Replacements[Inverted];
+ return {};
+
+ return {{Macro, Negated}};
}
- ClangTidyCheck &Check;
+ bool conditionContainsDefinedOperator(SourceRange ConditionRange) {
+ // Lexer requires its input range to be null-terminated.
+ SmallString<128> Condition =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
+ PP.getSourceManager(), PP.getLangOpts());
+ Condition.push_back('\0');
+ Lexer Lex({}, PP.getLangOpts(), Condition.data(), Condition.data(),
+ Condition.data() + Condition.size() - 1);
+
+ for (Token Tok;;) {
+ const bool NoMoreTokens = Lex.LexFromRawLexer(Tok);
+ if (Tok.is(tok::TokenKind::raw_identifier) &&
+ Tok.getRawIdentifier() == "defined")
+ return true;
+ if (NoMoreTokens)
+ return false;
+ }
+ }
+
+ UseConcisePreprocessorDirectivesCheck &Check;
const Preprocessor &PP;
+ const bool PreserveConsistency;
+ SmallVector<StackEntry, 4> Stack;
};
} // namespace
+UseConcisePreprocessorDirectivesCheck::UseConcisePreprocessorDirectivesCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ PreserveConsistency(Options.get("PreserveConsistency", false)) {}
+
+void UseConcisePreprocessorDirectivesCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "PreserveConsistency", PreserveConsistency);
+}
+
void UseConcisePreprocessorDirectivesCheck::registerPPCallbacks(
const SourceManager &, Preprocessor *PP, Preprocessor *) {
- PP->addPPCallbacks(std::make_unique<IfPreprocessorCallbacks>(*this, *PP));
+ PP->addPPCallbacks(std::make_unique<UseConciseDirectivesPPCallbacks>(
+ *this, *PP, PreserveConsistency));
}
} // namespace clang::tidy::readability
diff --git
a/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.h
b/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.h
index e65b16876a89a..c223987a44b73 100644
---
a/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.h
+++
b/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.h
@@ -21,12 +21,14 @@ namespace clang::tidy::readability {
///
https://clang.llvm.org/extra/clang-tidy/checks/readability/use-concise-preprocessor-directives.html
class UseConcisePreprocessorDirectivesCheck : public ClangTidyCheck {
public:
- using ClangTidyCheck::ClangTidyCheck;
+ UseConcisePreprocessorDirectivesCheck(StringRef Name,
+ ClangTidyContext *Context);
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
- bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
- return true;
- }
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ const bool PreserveConsistency;
};
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 4fee4f93908da..5a2345877c934 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -243,6 +243,12 @@ Changes in existing checks
<clang-tidy/checks/readability/qualified-auto>` check by adding the option
`IgnoreAliasing`, that allows not looking at underlying types of type
aliases.
+- Improved :doc:`readability-use-concise-preprocessor-directives
+ <clang-tidy/checks/readability/use-concise-preprocessor-directives>` check by
+ adding the option `PreserveConsistency`, which prevents shortening
+ directives if it would break visual consistency with other directives in the
+ ``#if`` chain.
+
Removed checks
^^^^^^^^^^^^^^
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
b/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
index 30ec7e6b89936..5b9970488d1f7 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
@@ -28,3 +28,41 @@ Since C23 and C++23:
#elifdef MEOW
#elifndef MEOW
+
+Options
+-------
+
+.. option:: PreserveConsistency
+
+ If `true`, directives will not be simplified if doing so would break
+ consistency with other directives in a chain. Default is `false`.
+
+Example
+^^^^^^^
+
+.. code-block:: c++
+
+ // Not simplified.
+ #if defined(FOO)
+ #elif defined(BAR) || defined(BAZ)
+ #endif
+
+ // Only simplified in C23 or C++23.
+ #if defined(FOO)
+ #elif defined(BAR)
+ #endif
+
+ // Consistency among *different* chains is not taken into account.
+ #if defined(FOO)
+ #if defined(BAR) || defined(BAZ)
+ #endif
+ #elif defined(HAZ)
+ #endif
+
+ // becomes
+
+ #ifdef FOO
+ #if defined(BAR) || defined(BAZ)
+ #endif
+ #elifdef HAZ
+ #endif
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/use-concise-preprocessor-directives-preserve-consistency.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/use-concise-preprocessor-directives-preserve-consistency.cpp
new file mode 100644
index 0000000000000..3959f5b7dd037
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/use-concise-preprocessor-directives-preserve-consistency.cpp
@@ -0,0 +1,108 @@
+// RUN: %check_clang_tidy -std=c++98,c++11,c++14,c++17,c++20 %s
readability-use-concise-preprocessor-directives %t -- \
+// RUN: -config='{ CheckOptions:
{readability-use-concise-preprocessor-directives.PreserveConsistency: true} }'
+// RUN: %check_clang_tidy -std=c++23-or-later -check-suffixes=,23 %s
readability-use-concise-preprocessor-directives %t -- \
+// RUN: -config='{ CheckOptions:
{readability-use-concise-preprocessor-directives.PreserveConsistency: true} }'
+
+// RUN: %check_clang_tidy -std=c99,c11,c17 %s
readability-use-concise-preprocessor-directives %t -- \
+// RUN: -config='{ CheckOptions:
{readability-use-concise-preprocessor-directives.PreserveConsistency: true} }'
-- -x c
+// RUN: %check_clang_tidy -std=c23-or-later -check-suffixes=,23 %s
readability-use-concise-preprocessor-directives %t -- \
+// RUN: -config='{ CheckOptions:
{readability-use-concise-preprocessor-directives.PreserveConsistency: true} }'
-- -x c
+
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifndef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES: #ifndef HEADER_GUARD
+#if !defined(HEADER_GUARD)
+#define HEADER_GUARD
+
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifdef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES: #ifdef FOO
+#if defined(FOO)
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifndef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES: #ifndef FOO
+#if !defined(FOO)
+#endif
+
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifndef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #ifndef FOO
+#if !defined(FOO)
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#elifdef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #elifdef BAR
+#elif defined(BAR)
+#endif
+
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifdef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #ifdef FOO
+#if defined FOO
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#elifdef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #elifdef BAR
+#elif defined BAR
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#elifndef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #elifndef BAZ
+#elif !defined BAZ
+#endif
+
+#ifdef FOO
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#elifdef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #elifdef BAR
+#elif defined BAR
+#endif
+
+#if ( defined(__cplusplus) && __cplusplus >= 202302L) || \
+ (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L)
+
+
+// Existing code can't decide between concise and verbose form, but
+// we can rewrite it to be consistent.
+//
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifndef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #ifndef FOO
+#if !defined(FOO)
+#elifdef BAR
+#endif
+
+#endif
+
+// Existing code can't decide between concise and verbose form, and rewriting
+// the '#elif defined(BAR)' won't make it more consistent, so leave it alone.
+#ifdef FOO
+#elif defined(BAR)
+#elif defined(BAZ) || defined(HAZ)
+#endif
+
+#if defined(FOO)
+#elif defined(BAR) || defined(BAZ)
+#endif
+
+#if defined(FOO)
+#elif defined(BAR) && BAR == 0xC0FFEE
+#endif
+
+#if defined(FOO)
+#elif BAR == 10 || defined(BAZ)
+#endif
+
+#if FOO
+#elif BAR
+#endif
+
+#if defined FOO && BAR
+#elif defined BAZ
+#endif
+
+// CHECK-MESSAGES: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifdef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES: #ifdef FOO
+#if defined(FOO)
+#elif BAR
+#endif
+
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#ifdef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #ifdef FOO
+#if defined(FOO)
+#elif 1 == 1
+// CHECK-MESSAGES-23: :[[@LINE+2]]:2: warning: preprocessor condition can be
written more concisely using '#elifndef'
[readability-use-concise-preprocessor-directives]
+// CHECK-FIXES-23: #elifndef BAR
+#elif !defined(BAR)
+#endif
+
+#endif // HEADER_GUARD
>From adcc2e19f79777a5f8c82bcdcef5c1651fd95c7d Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victo...@outlook.com>
Date: Thu, 11 Sep 2025 07:48:16 -0700
Subject: [PATCH 2/2] Indent block in docs
---
.../use-concise-preprocessor-directives.rst | 58 +++++++++----------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
b/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
index 5b9970488d1f7..948e94106ad08 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/readability/use-concise-preprocessor-directives.rst
@@ -37,32 +37,32 @@ Options
If `true`, directives will not be simplified if doing so would break
consistency with other directives in a chain. Default is `false`.
-Example
-^^^^^^^
-
-.. code-block:: c++
-
- // Not simplified.
- #if defined(FOO)
- #elif defined(BAR) || defined(BAZ)
- #endif
-
- // Only simplified in C23 or C++23.
- #if defined(FOO)
- #elif defined(BAR)
- #endif
-
- // Consistency among *different* chains is not taken into account.
- #if defined(FOO)
- #if defined(BAR) || defined(BAZ)
- #endif
- #elif defined(HAZ)
- #endif
-
- // becomes
-
- #ifdef FOO
- #if defined(BAR) || defined(BAZ)
- #endif
- #elifdef HAZ
- #endif
+ Example
+ ^^^^^^^
+
+ .. code-block:: c++
+
+ // Not simplified.
+ #if defined(FOO)
+ #elif defined(BAR) || defined(BAZ)
+ #endif
+
+ // Only simplified in C23 or C++23.
+ #if defined(FOO)
+ #elif defined(BAR)
+ #endif
+
+ // Consistency among *different* chains is not taken into account.
+ #if defined(FOO)
+ #if defined(BAR) || defined(BAZ)
+ #endif
+ #elif defined(HAZ)
+ #endif
+
+ // becomes
+
+ #ifdef FOO
+ #if defined(BAR) || defined(BAZ)
+ #endif
+ #elifdef HAZ
+ #endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
