https://github.com/localspook updated https://github.com/llvm/llvm-project/pull/178149
>From 3a7e5b665de8aa159399f029f830174d88e52d9c Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <[email protected]> Date: Tue, 27 Jan 2026 01:47:07 -0800 Subject: [PATCH] [clang-tidy] Speed up `readability-uppercase-literal-suffix` --- .../UppercaseLiteralSuffixCheck.cpp | 118 +++++++++--------- .../readability/UppercaseLiteralSuffixCheck.h | 5 +- 2 files changed, 57 insertions(+), 66 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp index 6463a82ff68f1..6a5eb90f14155 100644 --- a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp @@ -8,10 +8,10 @@ #include "UppercaseLiteralSuffixCheck.h" #include "../utils/ASTUtils.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" -#include <cctype> #include <optional> using namespace clang::ast_matchers; @@ -20,40 +20,42 @@ namespace clang::tidy::readability { namespace { -struct IntegerLiteralCheck { - using type = clang::IntegerLiteral; - static constexpr StringRef Name = "integer"; - // What should be skipped before looking for the Suffixes? (Nothing here.) - static constexpr StringRef SkipFirst = ""; - // Suffix can only consist of 'u', 'l', and 'z' chars, can be a bit-precise - // integer (wb), and can be a complex number ('i', 'j'). In MS compatibility - // mode, suffixes like i32 are supported. - static constexpr StringRef Suffixes = "uUlLzZwWiIjJ"; -}; - -struct FloatingLiteralCheck { - using type = clang::FloatingLiteral; - static constexpr StringRef Name = "floating point"; - // C++17 introduced hexadecimal floating-point literals, and 'f' is both a - // valid hexadecimal digit in a hex float literal and a valid floating-point - // literal suffix. - // So we can't just "skip to the chars that can be in the suffix". - // Since the exponent ('p'/'P') is mandatory for hexadecimal floating-point - // literals, we first skip everything before the exponent. - static constexpr StringRef SkipFirst = "pP"; - // Suffix can only consist of 'f', 'l', "f16", "bf16", "df", "dd", "dl", - // 'h', 'q' chars, and can be a complex number ('i', 'j'). - static constexpr StringRef Suffixes = "fFlLbBdDhHqQiIjJ"; -}; - struct NewSuffix { SourceRange LiteralLocation; StringRef OldSuffix; std::optional<FixItHint> FixIt; }; +struct LiteralParameters { + // What characters should be skipped before looking for the Suffixes? + StringRef SkipFirst; + // What characters can a suffix start with? + StringRef Suffixes; +}; + } // namespace +static constexpr LiteralParameters IntegerParameters = { + "", + // Suffix can only consist of 'u', 'l', and 'z' chars, can be a + // bit-precise integer (wb), and can be a complex number ('i', 'j'). In MS + // compatibility mode, suffixes like i32 are supported. + "uUlLzZwWiIjJ", +}; + +static constexpr LiteralParameters FloatParameters = { + // C++17 introduced hexadecimal floating-point literals, and 'f' is both a + // valid hexadecimal digit in a hex float literal and a valid floating-point + // literal suffix. + // So we can't just "skip to the chars that can be in the suffix". + // Since the exponent ('p'/'P') is mandatory for hexadecimal floating-point + // literals, we first skip everything before the exponent. + "pP", + // Suffix can only consist of 'f', 'l', "f16", "bf16", "df", "dd", "dl", + // 'h', 'q' chars, and can be a complex number ('i', 'j'). + "fFlLbBdDhHqQiIjJ", +}; + static std::optional<SourceLocation> getMacroAwareLocation(SourceLocation Loc, const SourceManager &SM) { // Do nothing if the provided location is invalid. @@ -77,8 +79,7 @@ getMacroAwareSourceRange(SourceRange Loc, const SourceManager &SM) { } static std::optional<std::string> -getNewSuffix(llvm::StringRef OldSuffix, - const std::vector<StringRef> &NewSuffixes) { +getNewSuffix(StringRef OldSuffix, const std::vector<StringRef> &NewSuffixes) { // If there is no config, just uppercase the entirety of the suffix. if (NewSuffixes.empty()) return OldSuffix.upper(); @@ -94,17 +95,15 @@ getNewSuffix(llvm::StringRef OldSuffix, return std::nullopt; } -template <typename LiteralType> static std::optional<NewSuffix> shouldReplaceLiteralSuffix(const Expr &Literal, + const LiteralParameters &Parameters, const std::vector<StringRef> &NewSuffixes, const SourceManager &SM, const LangOptions &LO) { NewSuffix ReplacementDsc; - const auto &L = cast<typename LiteralType::type>(Literal); - // The naive location of the literal. Is always valid. - ReplacementDsc.LiteralLocation = L.getSourceRange(); + ReplacementDsc.LiteralLocation = Literal.getSourceRange(); // Was this literal fully spelled or is it a product of macro expansion? const bool RangeCanBeFixed = @@ -134,11 +133,11 @@ shouldReplaceLiteralSuffix(const Expr &Literal, size_t Skip = 0; // Do we need to ignore something before actually looking for the suffix? - if (!LiteralType::SkipFirst.empty()) { + if (!Parameters.SkipFirst.empty()) { // E.g. we can't look for 'f' suffix in hexadecimal floating-point literals // until after we skip to the exponent (which is mandatory there), // because hex-digit-sequence may contain 'f'. - Skip = LiteralSourceText.find_first_of(LiteralType::SkipFirst); + Skip = LiteralSourceText.find_first_of(Parameters.SkipFirst); // We could be in non-hexadecimal floating-point literal, with no exponent. if (Skip == StringRef::npos) Skip = 0; @@ -147,7 +146,7 @@ shouldReplaceLiteralSuffix(const Expr &Literal, // Find the beginning of the suffix by looking for the first char that is // one of these chars that can be in the suffix, potentially starting looking // in the exponent, if we are skipping hex-digit-sequence. - Skip = LiteralSourceText.find_first_of(LiteralType::Suffixes, /*From=*/Skip); + Skip = LiteralSourceText.find_first_of(Parameters.Suffixes, /*From=*/Skip); // We can't check whether the *Literal has any suffix or not without actually // looking for the suffix. So it is totally possible that there is no suffix. @@ -191,43 +190,38 @@ void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) { // Sadly, we can't check whether the literal has suffix or not. // E.g. i32 suffix still results in 'BuiltinType::Kind::Int'. // And such an info is not stored in the *Literal itself. - Finder->addMatcher( - stmt(eachOf(integerLiteral().bind(IntegerLiteralCheck::Name), - floatLiteral().bind(FloatingLiteralCheck::Name)), - unless(anyOf(hasParent(userDefinedLiteral()), - hasAncestor(substNonTypeTemplateParmExpr())))), - this); + + Finder->addMatcher(userDefinedLiteral().bind("expr"), this); + Finder->addMatcher(integerLiteral().bind("expr"), this); + Finder->addMatcher(floatLiteral().bind("expr"), this); } -template <typename LiteralType> -bool UppercaseLiteralSuffixCheck::checkBoundMatch( +void UppercaseLiteralSuffixCheck::check( const MatchFinder::MatchResult &Result) { - const auto *Literal = - Result.Nodes.getNodeAs<typename LiteralType::type>(LiteralType::Name); - if (!Literal) - return false; + const auto *const Literal = Result.Nodes.getNodeAs<Expr>("expr"); + if (isa<UserDefinedLiteral>(Literal)) { + LatestUserDefinedLiteralLoc = Literal->getBeginLoc(); + return; + } + if (Literal->getBeginLoc() == LatestUserDefinedLiteralLoc) + return; + + const bool IsInteger = isa<IntegerLiteral>(Literal); // We won't *always* want to diagnose. // We might have a suffix that is already uppercase. - if (auto Details = shouldReplaceLiteralSuffix<LiteralType>( - *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) { + if (auto Details = shouldReplaceLiteralSuffix( + *Literal, IsInteger ? IntegerParameters : FloatParameters, + NewSuffixes, *Result.SourceManager, getLangOpts())) { if (Details->LiteralLocation.getBegin().isMacroID() && IgnoreMacros) - return true; + return; auto Complaint = diag(Details->LiteralLocation.getBegin(), - "%0 literal has suffix '%1', which is not uppercase") - << LiteralType::Name << Details->OldSuffix; + "%select{floating point|integer}0 literal has suffix " + "'%1', which is not uppercase") + << IsInteger << Details->OldSuffix; if (Details->FixIt) // Similarly, a fix-it is not always possible. Complaint << *(Details->FixIt); } - - return true; -} - -void UppercaseLiteralSuffixCheck::check( - const MatchFinder::MatchResult &Result) { - if (checkBoundMatch<IntegerLiteralCheck>(Result)) - return; // If it *was* IntegerLiteral, don't check for FloatingLiteral. - checkBoundMatch<FloatingLiteralCheck>(Result); } } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.h b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.h index e1eef3d5b58ee..f8deee399565d 100644 --- a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.h @@ -10,7 +10,6 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H #include "../ClangTidyCheck.h" -#include "../utils/OptionsUtils.h" namespace clang::tidy::readability { @@ -31,11 +30,9 @@ class UppercaseLiteralSuffixCheck : public ClangTidyCheck { } private: - template <typename LiteralType> - bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result); - const std::vector<StringRef> NewSuffixes; const bool IgnoreMacros; + SourceLocation LatestUserDefinedLiteralLoc; }; } // namespace clang::tidy::readability _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
