aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from minor wording nits.
================
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:31-32
+ static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("");
+ // Suffix can only consist of 'u' and 'l' chars, and can be a complex number.
+ // Also, in MS compatibility mode, suffixes like i32 are supported.
+ static constexpr llvm::StringLiteral Suffixes =
----------------
The comment is somewhat out of sync with the code because of the i and j
suffixes as well.
================
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:45
+ // C++17 introduced hexadecimal floating-point literals, and 'f' is both
+ // 15 in decimal and is 'f' as in 'floating point suffix'.
+ // So we can't just "skip to the chars that can be in the suffix".
----------------
I think you mean f is both a valid hexadecimal digit in a hex float literal and
a valid floating-point literal suffix.
================
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:101
+ return *NewSuffix;
+ // Nope, i guess we have to keep it as-is.
+ return llvm::None;
----------------
i -> I
================
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:212
+
+ // We won't *always* want to diagnose. We might have already-uppercase
suffix.
+ if (auto Details = shouldReplaceLiteralSuffix<LiteralType>(
----------------
have already-uppercase suffix -> have a suffix that is already uppercase
================
Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:216
+ auto Complaint = diag(Details->LiteralLocation,
+ "%0 literal has suffix '%1', which is not
upper-case")
+ << LiteralType::Name << Details->OldSuffix;
----------------
upper-case -> uppercase
================
Comment at: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst:28
+When the suffix is found, a case-insensitive lookup in that list is made,
+and if replacement is found, and it is different from the current suffix,
+only then the diagnostic is issued.
----------------
I'd reword this slightly:
"and if a replacement is found that is different from the current suffix, then
the diagnostic is issued. This allows for fine-grained control of what suffixes
to consider and what their replacements should be."
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52670
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
