[clang-tidy] Add readability-use-builtin-literals check #76065
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Finds literals explicitly casted to a type that could be expressed using builtin prefixes or suffixes. In elementary cases, provides automated fix-it hints. Defined for character, integer and floating literals.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-tidy Author: None (BenBlaise) ChangesFinds literals explicitly casted to a type that could be expressed using builtin prefixes or suffixes. In elementary cases, provides automated fix-it hints. (char)'a'; // -> 'a'
(char16_t)U'a'; // -> u'a'
static_cast<unsigned int>(0x1ul); // -> 0x1u
reinterpret_cast<long long int>(3ll); // -> 3LL
float(2.); // -> 2.f
double{2.}; // -> 2.Defined for character, integer and floating literals. Full diff: https://github.com/llvm/llvm-project/pull/76065.diff 10 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 5452c2d48a4617..3c5e12aaeb3bd4 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -51,6 +51,7 @@ add_clang_library(clangTidyReadabilityModule
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp
UseAnyOfAllOfCheck.cpp
+ UseBuiltinLiteralsCheck.cpp
LINK_LIBS
clangTidy
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index b8e6e641432060..b0ec5a906cac55 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -54,6 +54,7 @@
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"
#include "UseAnyOfAllOfCheck.h"
+#include "UseBuiltinLiteralsCheck.h"
namespace clang::tidy {
namespace readability {
@@ -151,6 +152,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-uppercase-literal-suffix");
CheckFactories.registerCheck<UseAnyOfAllOfCheck>(
"readability-use-anyofallof");
+ CheckFactories.registerCheck<UseBuiltinLiteralsCheck>(
+ "readability-use-builtin-literals");
}
};
diff --git a/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp
new file mode 100644
index 00000000000000..ede46517898d87
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp
@@ -0,0 +1,161 @@
+//===--- UseBuiltinLiteralsCheck.cpp - clang-tidy -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseBuiltinLiteralsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include <optional>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+using RuleOnStd = bool (*)(const LangStandard &LS);
+
+struct Replacement {
+ Replacement(StringRef Seq, const RuleOnStd Std = nullptr)
+ : Seq(Seq), Std(Std) {}
+ bool operator()(const LangOptions &LO) const {
+ return Std ? Std(LangStandard::getLangStandardForKind(LO.LangStd)) : true;
+ }
+ StringRef Seq;
+ RuleOnStd Std;
+};
+
+} // namespace
+
+static const llvm::Regex CharRegex("^(u8|u|U|L)?");
+static const llvm::StringMap<Replacement> CharPrefix({
+ {"char", {""}},
+ {"char8_t", {"u8"}},
+ {"char16_t", {"u"}},
+ {"char32_t", {"U"}},
+ {"wchar_t", {"L"}},
+});
+
+static const llvm::Regex
+ IntRegex("(([uU]?[lL]{0,2})|([lL]{0,2}[uU]?)|([uU]?[zZ]?)|([zZ]?[uU]?))?$");
+static const llvm::StringMap<Replacement> IntSuffix({
+ {"int", {""}},
+ {"unsigned int", {"u"}},
+ {"long", {"L"}},
+ {"unsigned long", {"uL"}},
+ {"long long", {"LL"}},
+ {"unsigned long long", {"uLL"}},
+ {"size_t", {"uz", [](const auto &LS) { return LS.isCPlusPlus23(); }}},
+ {"std::size_t", {"uz", [](const auto &LS) { return LS.isCPlusPlus23(); }}},
+});
+
+static const llvm::Regex FloatRegex(
+ "([fF]|[lL]|([fF]16)|([fF]32)|([fF]64)|([fF]128)|((bf|BF)16))?$");
+static const llvm::StringMap<Replacement> FloatSuffix({
+ {"double", {""}},
+ {"float", {"f"}},
+ {"long double", {"L"}},
+ {"std::float16_t", {"f16"}},
+ {"std::float32_t", {"f32"}},
+ {"std::float64_t", {"f64"}},
+ {"std::float128_t", {"f128"}},
+ {"std::bfloat16_t", {"bf16"}},
+ {"float16_t", {"f16"}},
+ {"float32_t", {"f32"}},
+ {"float64_t", {"f64"}},
+ {"float128_t", {"f128"}},
+ {"bfloat16_t", {"bf16"}},
+});
+
+void UseBuiltinLiteralsCheck::registerMatchers(MatchFinder *Finder) {
+ static const auto Literal = has(ignoringParenImpCasts(
+ expr(anyOf(characterLiteral().bind("char"), integerLiteral().bind("int"),
+ floatLiteral().bind("float")))
+ .bind("lit")));
+ Finder->addMatcher(
+ traverse(TK_IgnoreUnlessSpelledInSource,
+ explicitCastExpr(anyOf(Literal, has(initListExpr(Literal))))
+ .bind("expr")),
+ this);
+}
+
+static StringRef getRawStringRef(const SourceRange &Range,
+ const SourceManager &Sources,
+ const LangOptions &LangOpts) {
+ CharSourceRange TextRange = Lexer::getAsCharRange(Range, Sources, LangOpts);
+ return Lexer::getSourceText(TextRange, Sources, LangOpts);
+}
+
+void UseBuiltinLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
+
+ const auto &SM = *Result.SourceManager;
+ const auto &Nodes = Result.Nodes;
+
+ const auto *MatchedCast = Nodes.getNodeAs<ExplicitCastExpr>("expr");
+ const auto *Lit = Nodes.getNodeAs<Expr>("lit");
+ assert(MatchedCast && Lit);
+
+ StringRef LitText = getRawStringRef(Lit->getExprLoc(), SM, getLangOpts());
+ std::string CastType = MatchedCast->getTypeAsWritten().getAsString();
+ std::string Fix; // Replacement string for the fix-it hint.
+ std::optional<StringRef> Seq; // Literal sequence, prefix or suffix.
+
+ if (const auto *CharLit = Nodes.getNodeAs<CharacterLiteral>("char");
+ CharLit && CharPrefix.contains(CastType)) {
+ if (const Replacement &Rep = CharPrefix.at(CastType); Rep(getLangOpts())) {
+
+ Seq = Rep.Seq;
+ if (!CharLit->getLocation().isMacroID()) {
+ Fix.append(Rep.Seq);
+ Fix.append(CharRegex.sub("", LitText.str()));
+ }
+ }
+ } else if (const auto *IntLit = Nodes.getNodeAs<IntegerLiteral>("int");
+ IntLit && IntSuffix.contains(CastType)) {
+ if (const Replacement &Rep = IntSuffix.at(CastType); Rep(getLangOpts())) {
+
+ Seq = Rep.Seq;
+ if (!IntLit->getLocation().isMacroID()) {
+ Fix.append(IntRegex.sub("", LitText.str()));
+ Fix.append(Rep.Seq);
+ }
+ }
+ } else if (const auto *FloatLit = Nodes.getNodeAs<FloatingLiteral>("float");
+ FloatLit && FloatSuffix.contains(CastType)) {
+ if (const Replacement &Rep = FloatSuffix.at(CastType); Rep(getLangOpts())) {
+
+ Seq = Rep.Seq;
+ if (!FloatLit->getLocation().isMacroID()) {
+ Fix.append(FloatRegex.sub("", LitText.str()));
+ Fix.append(Rep.Seq);
+ }
+ }
+ }
+
+ const TypeLoc CastTypeLoc = MatchedCast->getTypeInfoAsWritten()->getTypeLoc();
+
+ if (!Fix.empty() && !CastTypeLoc.getBeginLoc().isMacroID()) {
+
+ // Recommend fix-it when no part of the explicit cast comes from a macro.
+ diag(MatchedCast->getExprLoc(),
+ "use built-in literal instead of explicit cast")
+ << FixItHint::CreateReplacement(MatchedCast->getSourceRange(), Fix);
+ } else if (Seq && MatchedCast->getExprLoc().isMacroID()) {
+
+ // Recommend manual fix when the entire explicit cast is within a macro.
+ diag(MatchedCast->getExprLoc(),
+ "use built-in '%0' %1 instead of explicit cast to '%2'")
+ << *Seq
+ << (Nodes.getNodeAs<CharacterLiteral>("char") ? "prefix" : "suffix")
+ << CastType;
+ }
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.h b/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.h
new file mode 100644
index 00000000000000..7b90786408d9ba
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.h
@@ -0,0 +1,37 @@
+//===--- UseBuiltinLiteralsCheck.h - clang-tidy -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEBUILTINLITERALSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEBUILTINLITERALSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Finds literals explicitly casted to a type that could be expressed using
+/// builtin prefixes or suffixes. Defined for character, integer, and floating
+/// literals. Removes any suffixes or prefixes before applying the one that
+/// corresponds to the type of the cast.
+///
+/// An explicit cast within a macro will be matched, but will only yield a
+/// suggestion for a manual fix. Otherwise, if either the destination type or
+/// the literal was substituted from a macro, no warning will be produced.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/use-builtin-literals.html
+class UseBuiltinLiteralsCheck : public ClangTidyCheck {
+public:
+ UseBuiltinLiteralsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEBUILTINLITERALSCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef18..5e144d94ccc297 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -218,6 +218,12 @@ New checks
Detects C++ code where a reference variable is used to extend the lifetime
of a temporary object that has just been constructed.
+- New :doc:`readability-use-builtin-literals
+ <clang-tidy/checks/readability/use-builtin-literals>` check.
+
+ Finds literals explicitly casted to a type that could be expressed using
+ builtin prefixes or suffixes.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 31f0e090db1d7d..e50562a22bc59e 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -380,6 +380,7 @@ Clang-Tidy Checks
:doc:`readability-uniqueptr-delete-release <readability/uniqueptr-delete-release>`, "Yes"
:doc:`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix>`, "Yes"
:doc:`readability-use-anyofallof <readability/use-anyofallof>`,
+ :doc:`readability-use-builtin-literals <readability/use-builtin-literals>`, "Yes"
:doc:`zircon-temporary-objects <zircon/temporary-objects>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-builtin-literals.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-builtin-literals.rst
new file mode 100644
index 00000000000000..044b14442eb5ee
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-builtin-literals.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - readability-use-builtin-literals
+
+readability-use-builtin-literals
+================================
+
+Finds literals explicitly casted to a type that could be expressed using builtin prefixes or suffixes.
+
+In elementary cases, provides automated fix-it hints.
+
+.. code-block:: c++
+
+ (char)'a'; // -> 'a'
+ (char16_t)U'a'; // -> u'a'
+ static_cast<unsigned int>(0x1ul); // -> 0x1u
+ reinterpret_cast<long long int>(3ll); // -> 3LL
+ float(2.); // -> 2.f
+ double{2.}; // -> 2.
+
+Defined for character, integer and floating literals. Removes any suffixes or prefixes before applying the one that corresponds to the type of the cast. Long values will always have uppercase ``L`` recommended, conforming to :doc:`readability-uppercase-literal-suffix <../readability/uppercase-literal-suffix>`.
+
+An explicit cast within a macro is matched, but yields only a suggestion for a manual fix.
+
+.. code-block:: c++
+
+ #define OPSHIFT ((unsigned)27)
+ OPSHIFT; // warning: use built-in 'u' instead of explicit cast to 'unsigned int'
+
+Otherwise, if either the destination type or the literal was substituted from a macro, no warnings are produced.
+
+.. code-block:: c++
+
+ #define INT_MAX 2147483647
+ static_cast<unsigned>(INT_MAX); // no warning
+
+ #define INTTYPE unsigned
+ (INTTYPE)31; // no warning
+
+Explicit casts within templates are also ignored.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp17.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp17.cpp
new file mode 100644
index 00000000000000..94f7bfbf3ccfba
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp17.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s readability-use-builtin-literals %t
+
+void warn_and_fix() {
+
+ (char)u8'a';
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 'a';
+ (wchar_t)u8'a';
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: L'a';
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp23.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp23.cpp
new file mode 100644
index 00000000000000..e9a790e624d8f2
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp23.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy -std=c++23-or-later %s readability-use-builtin-literals %t
+
+using size_t = decltype(sizeof(void*));
+namespace std {
+ using size_t = size_t;
+}
+
+void warn_and_fix() {
+
+ (size_t)6;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 6uz;
+ (std::size_t)6;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 6uz;
+ (size_t)6zu;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 6uz;
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals.cpp
new file mode 100644
index 00000000000000..df78ce3f1b9e81
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals.cpp
@@ -0,0 +1,105 @@
+// RUN: %check_clang_tidy %s readability-use-builtin-literals %t
+
+void warn_and_fix() {
+
+ (char16_t)U'a';
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: u'a';
+ (char32_t)u'a';
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: U'a';
+
+ (int)1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 1;
+ (unsigned int)0x1ul;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 0x1u;
+ (long int)2l;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 2L;
+ (unsigned long int)0x2lu;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 0x2uL;
+ (long long int)3ll;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 3LL;
+ (unsigned long long int)0x3llu;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 0x3uLL;
+
+ (double)1.f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 1.;
+ (float)2.;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 2.f;
+ (long double)3e0f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 3e0L;
+
+ float(2.);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 2.f;
+ double{2.};
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 2.;
+
+ static_cast<double>(2.f);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 2.;
+
+ reinterpret_cast<int>(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals]
+ // CHECK-FIXES: 1;
+}
+
+#define OPSHIFT ((unsigned)27)
+#define OCHAR (2LU<<OPSHIFT)
+#define OPSHIFT2 (27)
+#define OCHAR2 (2LU<<(unsigned)OPSHIFT2)
+#define SUBT unsigned
+#define OCHAR3 (2LU<<(SUBT)27)
+#define SUBT2 char
+#define OCHAR4 (2LU<<(SUBT2)'a')
+
+void warn_and_recommend_fix() {
+
+ OPSHIFT;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in 'u' suffix instead of explicit cast to 'unsigned int' [readability-use-builtin-literals]
+ OCHAR;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in 'u' suffix instead of explicit cast to 'unsigned int' [readability-use-builtin-literals]
+ OCHAR2;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in 'u' suffix instead of explicit cast to 'unsigned int' [readability-use-builtin-literals]
+ OCHAR3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in 'u' suffix instead of explicit cast to 'unsigned int' [readability-use-builtin-literals]
+ OCHAR4;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in '' prefix instead of explicit cast to 'char' [readability-use-builtin-literals]
+}
+
+#define INT_MAX 2147483647
+#define MAXCOL 2
+
+#ifdef CHECKED
+#define SUINT int
+#else
+#define SUINT unsigned
+#endif
+
+template <typename T>
+T f() {
+ return T(1);
+}
+
+int no_warn() {
+
+(void)0;
+(unsigned*)0;
+
+static_cast<unsigned>(INT_MAX);
+(unsigned)MAXCOL;
+
+(SUINT)31;
+
+return f<int>();
+}
|
You can test this locally with the following command:git-clang-format --diff 7767c5856d85cd1acf2efc32f77fdf07f00f9ff4 b4d7dbcf67117471a2d7025013fb1fcd188b33b6 -- clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.h clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp17.cpp clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals-cpp23.cpp clang-tools-extra/test/clang-tidy/checkers/readability/use-builtin-literals.cpp clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cppView the diff from clang-format here.diff --git a/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp
index ede4651789..6a10fbd51c 100644
--- a/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseBuiltinLiteralsCheck.cpp
@@ -104,7 +104,7 @@ void UseBuiltinLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
StringRef LitText = getRawStringRef(Lit->getExprLoc(), SM, getLangOpts());
std::string CastType = MatchedCast->getTypeAsWritten().getAsString();
- std::string Fix; // Replacement string for the fix-it hint.
+ std::string Fix; // Replacement string for the fix-it hint.
std::optional<StringRef> Seq; // Literal sequence, prefix or suffix.
if (const auto *CharLit = Nodes.getNodeAs<CharacterLiteral>("char");
@@ -153,8 +153,8 @@ void UseBuiltinLiteralsCheck::check(const MatchFinder::MatchResult &Result) {
diag(MatchedCast->getExprLoc(),
"use built-in '%0' %1 instead of explicit cast to '%2'")
<< *Seq
- << (Nodes.getNodeAs<CharacterLiteral>("char") ? "prefix" : "suffix")
- << CastType;
+ << (Nodes.getNodeAs<CharacterLiteral>("char") ? "prefix" : "suffix")
+ << CastType;
}
}
|
EugeneZelenko
requested changes
Dec 20, 2023
|
|
||
| void UseBuiltinLiteralsCheck::check(const MatchFinder::MatchResult &Result) { | ||
|
|
||
| const auto &SM = *Result.SourceManager; |
There was a problem hiding this comment.
Please don't use auto unless type is explicitly stated in same statement or iterator. Same in other places.
| readability-use-builtin-literals | ||
| ================================ | ||
|
|
||
| Finds literals explicitly casted to a type that could be expressed using builtin prefixes or suffixes. |
There was a problem hiding this comment.
Please follow 80-characters limit.
PiotrZSL
requested changes
Jan 5, 2024
There was a problem hiding this comment.
Overall what is now is +- working.
Few fixes are still needed to tests, code, maybe some tiny refactoring and code formatting.
Consider adding support for implicit casts:
unsigned X = 10;
`-VarDecl <line:1:1, col:14> col:10 X 'unsigned int' cinit
`-ImplicitCastExpr <col:14> 'unsigned int' <IntegralCast>
`-IntegerLiteral <col:14> 'int' 10
To:
unsigned X = 10U;
`-VarDecl <line:1:1, col:14> col:10 X 'unsigned int' cinit
`-IntegerLiteral <col:14> 'unsigned int' 10
I worry also a little bit about integer overflow issues.
| }); | ||
|
|
||
| void UseBuiltinLiteralsCheck::registerMatchers(MatchFinder *Finder) { | ||
| static const auto Literal = has(ignoringParenImpCasts( |
There was a problem hiding this comment.
Do not make matchers static, this can be problematic
| }); | ||
|
|
||
| void UseBuiltinLiteralsCheck::registerMatchers(MatchFinder *Finder) { | ||
| static const auto Literal = has(ignoringParenImpCasts( |
There was a problem hiding this comment.
ignoringParenImpCasts doesn't look to be needed because TK_IgnoreUnlessSpelledInSource is used, and without it all tests still pass. Remove or add tests to cover this.
| floatLiteral().bind("float"))) | ||
| .bind("lit"))); | ||
| Finder->addMatcher( | ||
| traverse(TK_IgnoreUnlessSpelledInSource, |
There was a problem hiding this comment.
move TK_IgnoreUnlessSpelledInSource to separate method:
std::optional<TraversalKind> getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}
Like its done in other checks
| Finder->addMatcher( | ||
| traverse(TK_IgnoreUnlessSpelledInSource, | ||
| explicitCastExpr(anyOf(Literal, has(initListExpr(Literal)))) | ||
| .bind("expr")), |
There was a problem hiding this comment.
change this "expr" into "cast", as later in check method is not so clear what it refers.
| IntRegex("(([uU]?[lL]{0,2})|([lL]{0,2}[uU]?)|([uU]?[zZ]?)|([zZ]?[uU]?))?$"); | ||
| static const llvm::StringMap<Replacement> IntSuffix({ | ||
| {"int", {""}}, | ||
| {"unsigned int", {"u"}}, |
There was a problem hiding this comment.
use "U" instead of "u", to be in sync with readability-uppercase-literal-suffix check.
Verify also other suffixes.
| diag(MatchedCast->getExprLoc(), | ||
| "use built-in '%0' %1 instead of explicit cast to '%2'") | ||
| << *Seq | ||
| << (Nodes.getNodeAs<CharacterLiteral>("char") ? "prefix" : "suffix") |
There was a problem hiding this comment.
use 1%select{prefix|suffix}, look into other checks how its done, and no need to call getNodeAs again, thats not chep, you already know if this is char or not, just use it, simply add some bool like IsPrefix
|
|
||
| .. code-block:: c++ | ||
|
|
||
| (char)'a'; // -> 'a' |
There was a problem hiding this comment.
list here all supported types, for example in form of table, like type, example, replacement.
| (SUINT)31; | ||
|
|
||
| return f<int>(); | ||
| } |
There was a problem hiding this comment.
Missing test for size_t and lack of warning & fix.
Comment on lines
+40
to
+42
| {"char8_t", {"u8"}}, | ||
| {"char16_t", {"u"}}, | ||
| {"char32_t", {"U"}}, |
| (long double)3e0f; | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use built-in literal instead of explicit cast [readability-use-builtin-literals] | ||
| // CHECK-FIXES: 3e0L; | ||
|
|
There was a problem hiding this comment.
missing tests for:
- float16_t
- float32_t
- float64_t
- float128_t
- bfloat16
|
@BenBlaise Any plans to finish this ? Clang-tidy 18 branch-out is in ~3 days. |
|
I would like to, but I won't get around to it by then (concentrating on a thesis defense for tomorrow). |
|
@BenBlaise Ok, no problem, then it will get into next release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finds literals explicitly casted to a type that could be expressed using builtin prefixes or suffixes.
In elementary cases, provides automated fix-it hints.
Defined for character, integer and floating literals.