Author: Victor Chernyakin Date: 2026-02-12T10:04:59-07:00 New Revision: beac6260c41975d4b7677872a8456737a5b35e6f
URL: https://github.com/llvm/llvm-project/commit/beac6260c41975d4b7677872a8456737a5b35e6f DIFF: https://github.com/llvm/llvm-project/commit/beac6260c41975d4b7677872a8456737a5b35e6f.diff LOG: [clang-tidy] Speed up `readability-uppercase-literal-suffix` (#178149) As usual, this is one of our most expensive checks according to `--enable-check-profile`. Measuring overall runtime: ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-uppercase-literal-suffix all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` Status quo: ```txt Time (mean ± σ): 4.435 s ± 0.012 s [User: 4.158 s, System: 0.275 s] Range (min … max): 4.409 s … 4.455 s 10 runs ``` With this change: ```txt Time (mean ± σ): 3.549 s ± 0.010 s [User: 3.328 s, System: 0.223 s] Range (min … max): 3.540 s … 3.569 s 10 runs ``` Measuring with `--enable-check-profile`: Status quo: ```txt ---User Time--- --System Time-- --User+System-- ---Wall Time--- --- Name --- 1.1875 (100.0%) 0.1406 (100.0%) 1.3281 (100.0%) 1.3147 (100.0%) readability-uppercase-literal-suffix ``` With this change: ```txt ---User Time--- --System Time-- --User+System-- ---Wall Time--- --- Name --- 0.2500 (100.0%) 0.0469 (100.0%) 0.2969 (100.0%) 0.2904 (100.0%) readability-uppercase-literal-suffix ``` However, subtracting 200 ms to account for the "[`hasParent` tax](https://github.com/llvm/llvm-project/pull/178149#discussion_r2736652174)", the "true" numbers are probably closer to: ``` ---User Time--- --System Time-- --User+System-- ---Wall Time--- --- Name --- Status quo: 0.9875 (100.0%) 0.1406 (100.0%) 1.1281 (100.0%) 1.1147 (100.0%) readability-uppercase-literal-suffix With this change: 0.0500 (100.0%) 0.0469 (100.0%) 0.0969 (100.0%) 0.0904 (100.0%) readability-uppercase-literal-suffix Added: Modified: clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.h clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-floating-point.cpp clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp index 6463a82ff68f1..812ade0df42c1 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,33 @@ 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())))), + integerLiteral(unless(hasParent(userDefinedLiteral()))).bind("expr"), this); + Finder->addMatcher( + floatLiteral(unless(hasParent(userDefinedLiteral()))).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"); + 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..5df24241d1fb7 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,9 +30,6 @@ class UppercaseLiteralSuffixCheck : public ClangTidyCheck { } private: - template <typename LiteralType> - bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result); - const std::vector<StringRef> NewSuffixes; const bool IgnoreMacros; }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-floating-point.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-floating-point.cpp index fc1976b0e6b22..07c741c4c7afc 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-floating-point.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-floating-point.cpp @@ -132,3 +132,8 @@ void macros() { static_assert(is_same<decltype(m0), const float>::value, ""); static_assert(m0 == 1.0F, ""); } + +long double operator""_f(long double); +void user_defined_literals() { + 1.0_f; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer.cpp index e4dd968f49c12..fd5e4a529d99c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/uppercase-literal-suffix-integer.cpp @@ -185,7 +185,7 @@ void macros() { // Check that user-defined literals do not cause any diags. -unsigned long long int operator"" _ull(unsigned long long int); +unsigned long long int operator""_ull(unsigned long long int); void user_defined_literals() { 1_ull; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
