[clang-tools-extra] beac626 - [clang-tidy] Speed up `readability-uppercase-literal-suffix` (#178149)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 12 09:05:09 PST 2026
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;
}
More information about the cfe-commits
mailing list