lebedev.ri updated this revision to Diff 167620. lebedev.ri marked 13 inline comments as done. lebedev.ri added a comment.
Thank you for taking a look! - Rebased - Added an alias in `hicpp` module - Addressed //most// of the review comments. In https://reviews.llvm.org/D52670#1249898, @JonasToth wrote: > Thanks for working on this! > Related as well: > http://www.codingstandard.com/rule/4-2-1-ensure-that-the-u-suffix-is-applied-to-a-literal-used-in-a-context-requiring-an-unsigned-integral-expression/ > I think its wort a alias is hicpp as well Hmm, after digging but, i think you meant to link to https://www.codingstandard.com/rule/4-3-1-do-not-convert-an-expression-of-wider-floating-point-type-to-a-narrower-floating-point-type/ With some leeway, i think it does talk about the suffix being uppercase. Added. > Please add tests that use user-defined literals and ensure there are no > collision and that they are not diagnosed. Some examples: > https://en.cppreference.com/w/cpp/language/user_literal All those are `UserDefinedLiteral`, so we should be good.. https://godbolt.org/z/PcGi0B Also, it seems the suffix can't be set for these constants: https://godbolt.org/z/YHTqke So i'm not sure what to test. Can you give an example of a test? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/hicpp/HICPPTidyModule.cpp clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MagicNumbersCheck.cpp clang-tidy/readability/MagicNumbersCheck.h clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp clang-tidy/readability/UppercaseLiteralSuffixCheck.h clang-tidy/utils/ASTUtils.cpp clang-tidy/utils/ASTUtils.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp test/clang-tidy/readability-uppercase-literal-suffix.h
Index: test/clang-tidy/readability-uppercase-literal-suffix.h =================================================================== --- /dev/null +++ test/clang-tidy/readability-uppercase-literal-suffix.h @@ -0,0 +1,16 @@ +template <class T, T v> +struct integral_constant { + static constexpr T value = v; + typedef T value_type; + typedef integral_constant type; // using injected-class-name + constexpr operator value_type() const noexcept { return value; } +}; + +using false_type = integral_constant<bool, false>; +using true_type = integral_constant<bool, true>; + +template <class T, class U> +struct is_same : false_type {}; + +template <class T> +struct is_same<T, T> : true_type {}; Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp @@ -0,0 +1,203 @@ +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S + +#include "readability-uppercase-literal-suffix.h" + +void integer_suffix() { + static constexpr auto v0 = __LINE__; // synthetic + static_assert(v0 == 9 || v0 == 5, ""); + + static constexpr auto v1 = __cplusplus; // synthetic, long + + static constexpr auto v2 = 1; // no literal + static_assert(is_same<decltype(v2), const int>::value, ""); + static_assert(v2 == 1, ""); + + // Unsigned + + static constexpr auto v3 = 1u; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'u', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v3 = 1u; + // CHECK-MESSAGES-NEXT: ^~ + // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}} + // CHECK-FIXES: static constexpr auto v3 = 1U; + static_assert(is_same<decltype(v3), const unsigned int>::value, ""); + static_assert(v3 == 1, ""); + + static constexpr auto v4 = 1U; // OK. + static_assert(is_same<decltype(v4), const unsigned int>::value, ""); + static_assert(v4 == 1, ""); + + // Long + + static constexpr auto v5 = 1l; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'l', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1l; + // CHECK-MESSAGES-NEXT: ^~ + // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}} + // CHECK-FIXES: static constexpr auto v5 = 1L; + static_assert(is_same<decltype(v5), const long>::value, ""); + static_assert(v5 == 1, ""); + + static constexpr auto v6 = 1L; // OK. + static_assert(is_same<decltype(v6), const long>::value, ""); + static_assert(v6 == 1, ""); + + // Long Long + + static constexpr auto v7 = 1ll; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'll', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v7 = 1ll; + // CHECK-MESSAGES-NEXT: ^~~ + // CHECK-MESSAGES-NEXT: {{^ *}}LL{{$}} + // CHECK-FIXES: static constexpr auto v7 = 1LL; + static_assert(is_same<decltype(v7), const long long>::value, ""); + static_assert(v7 == 1, ""); + + static constexpr auto v8 = 1LL; // OK. + static_assert(is_same<decltype(v8), const long long>::value, ""); + static_assert(v8 == 1, ""); + + // Unsigned Long + + static constexpr auto v9 = 1ul; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'ul', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul; + // CHECK-MESSAGES-NEXT: ^~~ + // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}} + // CHECK-FIXES: static constexpr auto v9 = 1UL; + static_assert(is_same<decltype(v9), const unsigned long>::value, ""); + static_assert(v9 == 1, ""); + + static constexpr auto v10 = 1uL; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'uL', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v10 = 1uL; + // CHECK-MESSAGES-NEXT: ^~~ + // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}} + // CHECK-FIXES: static constexpr auto v10 = 1UL; + static_assert(is_same<decltype(v10), const unsigned long>::value, ""); + static_assert(v10 == 1, ""); + + static constexpr auto v11 = 1Ul; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'Ul', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v11 = 1Ul; + // CHECK-MESSAGES-NEXT: ^~~ + // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}} + // CHECK-FIXES: static constexpr auto v11 = 1UL; + static_assert(is_same<decltype(v11), const unsigned long>::value, ""); + static_assert(v11 == 1, ""); + + static constexpr auto v12 = 1UL; // OK. + static_assert(is_same<decltype(v12), const unsigned long>::value, ""); + static_assert(v12 == 1, ""); + + // Long Unsigned + + static constexpr auto v13 = 1lu; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'lu', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v13 = 1lu; + // CHECK-MESSAGES-NEXT: ^~~ + // CHECK-MESSAGES-NEXT: {{^ *}}LU{{$}} + // CHECK-FIXES: static constexpr auto v13 = 1LU; + static_assert(is_same<decltype(v13), const unsigned long>::value, ""); + static_assert(v13 == 1, ""); + + static constexpr auto v14 = 1Lu; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'Lu', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v14 = 1Lu; + // CHECK-MESSAGES-NEXT: ^~~ + // CHECK-MESSAGES-NEXT: {{^ *}}LU{{$}} + // CHECK-FIXES: static constexpr auto v14 = 1LU; + static_assert(is_same<decltype(v14), const unsigned long>::value, ""); + static_assert(v14 == 1, ""); + + static constexpr auto v15 = 1lU; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'lU', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v15 = 1lU; + // CHECK-MESSAGES-NEXT: ^~~ + // CHECK-MESSAGES-NEXT: {{^ *}}LU{{$}} + // CHECK-FIXES: static constexpr auto v15 = 1LU; + static_assert(is_same<decltype(v15), const unsigned long>::value, ""); + static_assert(v15 == 1, ""); + + static constexpr auto v16 = 1LU; // OK. + static_assert(is_same<decltype(v16), const unsigned long>::value, ""); + static_assert(v16 == 1, ""); + + // Unsigned Long Long + + static constexpr auto v17 = 1ull; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'ull', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v17 = 1ull; + // CHECK-MESSAGES-NEXT: ^~~~ + // CHECK-MESSAGES-NEXT: {{^ *}}ULL{{$}} + // CHECK-FIXES: static constexpr auto v17 = 1ULL; + static_assert(is_same<decltype(v17), const unsigned long long>::value, ""); + static_assert(v17 == 1, ""); + + static constexpr auto v18 = 1uLL; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'uLL', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v18 = 1uLL; + // CHECK-MESSAGES-NEXT: ^~~~ + // CHECK-MESSAGES-NEXT: {{^ *}}ULL{{$}} + // CHECK-FIXES: static constexpr auto v18 = 1ULL; + static_assert(is_same<decltype(v18), const unsigned long long>::value, ""); + static_assert(v18 == 1, ""); + + static constexpr auto v19 = 1Ull; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'Ull', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v19 = 1Ull; + // CHECK-MESSAGES-NEXT: ^~~~ + // CHECK-MESSAGES-NEXT: {{^ *}}ULL{{$}} + // CHECK-FIXES: static constexpr auto v19 = 1ULL; + static_assert(is_same<decltype(v19), const unsigned long long>::value, ""); + static_assert(v19 == 1, ""); + + static constexpr auto v20 = 1ULL; // OK. + static_assert(is_same<decltype(v20), const unsigned long long>::value, ""); + static_assert(v20 == 1, ""); + + // Long Long Unsigned + + static constexpr auto v21 = 1llu; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'llu', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v21 = 1llu; + // CHECK-MESSAGES-NEXT: ^~~~ + // CHECK-MESSAGES-NEXT: {{^ *}}LLU{{$}} + // CHECK-FIXES: static constexpr auto v21 = 1LLU; + static_assert(is_same<decltype(v21), const unsigned long long>::value, ""); + static_assert(v21 == 1, ""); + + static constexpr auto v22 = 1LLu; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'LLu', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v22 = 1LLu; + // CHECK-MESSAGES-NEXT: ^~~~ + // CHECK-MESSAGES-NEXT: {{^ *}}LLU{{$}} + // CHECK-FIXES: static constexpr auto v22 = 1LLU; + static_assert(is_same<decltype(v22), const unsigned long long>::value, ""); + static_assert(v22 == 1, ""); + + static constexpr auto v23 = 1llU; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal has suffix 'llU', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v23 = 1llU; + // CHECK-MESSAGES-NEXT: ^~~~ + // CHECK-MESSAGES-NEXT: {{^ *}}LLU{{$}} + // CHECK-FIXES: static constexpr auto v23 = 1LLU; + static_assert(is_same<decltype(v23), const unsigned long long>::value, ""); + static_assert(v23 == 1, ""); + + static constexpr auto v24 = 1LLU; // OK. + static_assert(is_same<decltype(v24), const unsigned long long>::value, ""); + static_assert(v24 == 1, ""); +} + +void negatives() { +#define PASSTHROUGH(X) X + + static constexpr auto n0 = PASSTHROUGH(1u); + // CHECK-FIXES: static constexpr auto n0 = PASSTHROUGH(1u); + static_assert(is_same<decltype(n0), const unsigned int>::value, ""); + static_assert(n0 == 1, ""); +} Index: test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp @@ -0,0 +1,74 @@ +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S + +#include "readability-uppercase-literal-suffix.h" + +void integer_suffix() { + static constexpr auto v0 = 0x0p0; // no literal + static_assert(is_same<decltype(v0), const double>::value, ""); + static_assert(v0 == 0, ""); + + // Float + + static constexpr auto v1 = 0xfp0f; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'f', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v1 = 0xfp0f; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}} + // CHECK-FIXES: static constexpr auto v1 = 0xfp0F; + static_assert(is_same<decltype(v1), const float>::value, ""); + static_assert(v1 == 15, ""); + + static constexpr auto v2 = 0xfp0F; // OK + static_assert(is_same<decltype(v2), const float>::value, ""); + static_assert(v2 == 15, ""); + + static constexpr auto v3 = 0xfP0f; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'f', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v3 = 0xfP0f; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}} + // CHECK-FIXES: static constexpr auto v3 = 0xfP0F; + static_assert(is_same<decltype(v3), const float>::value, ""); + static_assert(v3 == 15, ""); + + static constexpr auto v4 = 0xfP0F; // OK + static_assert(is_same<decltype(v4), const float>::value, ""); + static_assert(v4 == 15, ""); + + static constexpr auto v5 = 0xFP0f; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'f', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v5 = 0xFP0f; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}} + // CHECK-FIXES: static constexpr auto v5 = 0xFP0F; + static_assert(is_same<decltype(v5), const float>::value, ""); + static_assert(v5 == 15, ""); + + static constexpr auto v6 = 0xFP0F; // OK + static_assert(is_same<decltype(v6), const float>::value, ""); + static_assert(v6 == 15, ""); + + static constexpr auto v7 = 0xFp0f; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'f', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v7 = 0xFp0f; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}} + // CHECK-FIXES: static constexpr auto v7 = 0xFp0F; + static_assert(is_same<decltype(v7), const float>::value, ""); + static_assert(v7 == 15, ""); + + static constexpr auto v8 = 0xFp0F; // OK + static_assert(is_same<decltype(v8), const float>::value, ""); + static_assert(v8 == 15, ""); +} + +void negatives() { +#define PASSTHROUGH(X) X + + static constexpr auto n0 = PASSTHROUGH(0x0p0f); + static_assert(is_same<decltype(n0), const float>::value, ""); + static_assert(n0 == 0, ""); +} Index: test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp @@ -0,0 +1,80 @@ +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S +// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S + +#include "readability-uppercase-literal-suffix.h" + +void integer_suffix() { + static constexpr auto v0 = 1.; // no literal + static_assert(is_same<decltype(v0), const double>::value, ""); + static_assert(v0 == 1., ""); + + static constexpr auto v1 = 1.e0; // no literal + static_assert(is_same<decltype(v1), const double>::value, ""); + static_assert(v1 == 1., ""); + + // Float + + static constexpr auto v2 = 1.f; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'f', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v2 = 1.f; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}} + // CHECK-FIXES: static constexpr auto v2 = 1.F; + static_assert(is_same<decltype(v2), const float>::value, ""); + static_assert(v2 == 1.0F, ""); + + static constexpr auto v3 = 1.e0f; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'f', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v3 = 1.e0f; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}} + // CHECK-FIXES: static constexpr auto v3 = 1.e0F; + static_assert(is_same<decltype(v3), const float>::value, ""); + static_assert(v3 == 1.0F, ""); + + static constexpr auto v4 = 1.F; // OK. + static_assert(is_same<decltype(v4), const float>::value, ""); + static_assert(v4 == 1.0F, ""); + + static constexpr auto v5 = 1.e0F; // OK. + static_assert(is_same<decltype(v5), const float>::value, ""); + static_assert(v5 == 1.0F, ""); + + // Long double + + static constexpr auto v6 = 1.l; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'l', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v6 = 1.l; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}} + // CHECK-FIXES: static constexpr auto v6 = 1.L; + static_assert(is_same<decltype(v6), const long double>::value, ""); + static_assert(v6 == 1., ""); + + static constexpr auto v7 = 1.e0l; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'l', which is not upper-case + // CHECK-MESSAGES-NEXT: static constexpr auto v7 = 1.e0l; + // CHECK-MESSAGES-NEXT: ^ ~ + // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}} + // CHECK-FIXES: static constexpr auto v7 = 1.e0L; + static_assert(is_same<decltype(v7), const long double>::value, ""); + static_assert(v7 == 1., ""); + + static constexpr auto v8 = 1.L; // OK. + static_assert(is_same<decltype(v8), const long double>::value, ""); + static_assert(v8 == 1., ""); + + static constexpr auto v9 = 1.e0L; // OK. + static_assert(is_same<decltype(v9), const long double>::value, ""); + static_assert(v9 == 1., ""); +} + +void negatives() { +#define PASSTHROUGH(X) X + + static constexpr auto n0 = PASSTHROUGH(1.f); + static_assert(is_same<decltype(n0), const float>::value, ""); + static_assert(n0 == 1.0F, ""); +} Index: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst @@ -0,0 +1,24 @@ +.. title:: clang-tidy - readability-uppercase-literal-suffix + +readability-uppercase-literal-suffix +==================================== + +`cert-dcl16-c` redirects here as an alias for this check. + +`hicpp-uppercase-literal-suffix` redirects here as an alias for this check. + +Detects when the integral literal or floating point (decimal or hexadecimal) +literal has a non-uppercase suffix and provides a fix-it-hint +with the uppercase suffix. + +All valid combinations of suffixes are supported. + +.. code:: c + + auto x = 1; // OK, no suffix. + + auto x = 1u; // warning: integer literal suffix 'u' is not upper-case + + auto x = 1U; // OK, suffix is uppercase. + + ... Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -65,6 +65,7 @@ bugprone-use-after-move bugprone-virtual-near-miss cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c> + cert-dcl16-c (redirects to readability-uppercase-literal-suffix) <cert-dcl16-c> cert-dcl21-cpp cert-dcl50-cpp cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp> @@ -244,4 +245,5 @@ readability-static-definition-in-anonymous-namespace readability-string-compare readability-uniqueptr-delete-release + readability-uppercase-literal-suffix zircon-temporary-objects Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -106,6 +106,23 @@ Detects usage of magic numbers, numbers that are used as literals instead of introduced via constants or symbols. +- New :doc:`readability-uppercase-literal-suffix + <clang-tidy/checks/readability-uppercase-literal-suffix>` check. + + Detects when the integral literal or floating point literal has non-uppercase + suffix, and suggests to make the suffix uppercase. + +- New alias :doc:`cert-dcl16-c + <clang-tidy/checks/cert-dcl16-c>` to :doc:`readability-uppercase-literal-suffix + <clang-tidy/checks/readability-uppercase-literal-suffix>` + added. + +- New alias :doc:`hicpp-uppercase-literal-suffix + <clang-tidy/checks/hicpp-uppercase-literal-suffix>` to + :doc:`readability-uppercase-literal-suffix + <clang-tidy/checks/readability-uppercase-literal-suffix>` + added. + Improvements to include-fixer ----------------------------- Index: clang-tidy/utils/ASTUtils.h =================================================================== --- clang-tidy/utils/ASTUtils.h +++ clang-tidy/utils/ASTUtils.h @@ -27,6 +27,16 @@ bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM, const LangOptions &LangOpts, StringRef FlagName); + +// Check whether given IntegerLiteral is a synthetic value, like __LINE__ +bool isSyntheticValue(const clang::SourceManager *SourceManager, + const IntegerLiteral *Literal); +// No known syntetic fp literals. +inline bool isSyntheticValue(const clang::SourceManager *, + const FloatingLiteral *) { + return false; +} + } // namespace utils } // namespace tidy } // namespace clang Index: clang-tidy/utils/ASTUtils.cpp =================================================================== --- clang-tidy/utils/ASTUtils.cpp +++ clang-tidy/utils/ASTUtils.cpp @@ -67,6 +67,20 @@ return true; } +// Check whether given IntegerLiteral is a synthetic value, like __LINE__ +bool isSyntheticValue(const clang::SourceManager *SourceManager, + const IntegerLiteral *Literal) { + const std::pair<FileID, unsigned> FileOffset = + SourceManager->getDecomposedLoc(Literal->getLocation()); + if (FileOffset.first.isInvalid()) + return false; + + const StringRef BufferIdentifier = + SourceManager->getBuffer(FileOffset.first)->getBufferIdentifier(); + + return BufferIdentifier.empty(); +} + } // namespace utils } // namespace tidy } // namespace clang Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/UppercaseLiteralSuffixCheck.h @@ -0,0 +1,49 @@ +//===--- UppercaseLiteralSuffixCheck.h - clang-tidy -------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Detects when the integral literal or floating point literal has +/// non-uppercase suffix, and suggests to make the suffix uppercase. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-uppercase-literal-suffix.html +class UppercaseLiteralSuffixCheck : public ClangTidyCheck { + struct NewSuffix { + StringRef OldSuffix; + FixItHint Replacement; + }; + + template <typename LiteralType> + llvm::Optional<NewSuffix> + shouldReplaceLiteralSuffix(const clang::Expr &Literal, + const clang::SourceManager &SM); + + template <typename LiteralType> + bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result); + +public: + UppercaseLiteralSuffixCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H Index: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp @@ -0,0 +1,185 @@ +//===--- UppercaseLiteralSuffixCheck.cpp - clang-tidy ---------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "UppercaseLiteralSuffixCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { + +struct IntegerLiteral { + using type = clang::IntegerLiteral; + static constexpr llvm::StringLiteral Name = llvm::StringLiteral("integer"); + // What should be skipped before looking for the Suffixes? (Nothing here.) + static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral(""); + // Integer suffix can only consist of 'u' and 'l' chars. + static constexpr llvm::StringLiteral Suffixes = llvm::StringLiteral("uUlL"); +}; +constexpr llvm::StringLiteral IntegerLiteral::Name; +constexpr llvm::StringLiteral IntegerLiteral::SkipFirst; +constexpr llvm::StringLiteral IntegerLiteral::Suffixes; + +struct FloatingLiteral { + using type = clang::FloatingLiteral; + static constexpr llvm::StringLiteral Name = + llvm::StringLiteral("floating point"); + // 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 until 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 llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP"); + // Floating suffix can only consist of 'u' and 'l' chars. + static constexpr llvm::StringLiteral Suffixes = llvm::StringLiteral("fFlL"); +}; +constexpr llvm::StringLiteral FloatingLiteral::Name; +constexpr llvm::StringLiteral FloatingLiteral::SkipFirst; +constexpr llvm::StringLiteral FloatingLiteral::Suffixes; + +AST_MATCHER(clang::IntegerLiteral, intHasSuffix) { + const auto *T = dyn_cast<BuiltinType>(Node.getType().getTypePtr()); + if (!T) + return false; + switch (T->getKind()) { + default: + case BuiltinType::Kind::Int: + return false; // No suffix. + case BuiltinType::Kind::UInt: // 'u' + case BuiltinType::Kind::Long: // 'l' + case BuiltinType::Kind::ULong: // 'ul' + case BuiltinType::Kind::LongLong: // 'll' + case BuiltinType::Kind::ULongLong: // 'ull' + return true; + } +} + +AST_MATCHER(clang::FloatingLiteral, fpHasSuffix) { + const auto *T = dyn_cast<BuiltinType>(Node.getType().getTypePtr()); + if (!T) + return false; + switch (T->getKind()) { + default: + case BuiltinType::Kind::Double: + return false; // No suffix. + case BuiltinType::Kind::Float: // 'f' + case BuiltinType::Kind::LongDouble: // 'l' + return true; + } +} + +} // namespace + +template <typename LiteralType> +llvm::Optional<UppercaseLiteralSuffixCheck::NewSuffix> +UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix( + const clang::Expr &Literal, const clang::SourceManager &SM) { + const auto &L = cast<typename LiteralType::type>(Literal); + + NewSuffix ReplacementDsc; + + SourceRange Range = L.getSourceRange(); + + // Get the whole Integer Literal from the source buffer. + bool Invalid; + const StringRef LiteralSourceText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Range), SM, getLangOpts(), &Invalid); + assert(!Invalid && "Failed to retrieve the source text."); + + size_t Skip = 0; + + // Do we need to ignore something before actually looking for the suffix? + if (!LiteralType::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); + // We could be in non-hexadecimal flloating-point literal, with no exponent. + if (Skip == StringRef::npos) + Skip = 0; + } + + // 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); + assert( + Skip != StringRef::npos && + "we should not get here unless there was some suffix as per the type " + "system. Also, these two places should accept the same set of suffixes."); + + // Move the cursor in the source range to the beginning of the suffix. + Range.setBegin(Range.getBegin().getLocWithOffset(Skip)); + // And in our textual representation too. + ReplacementDsc.OldSuffix = LiteralSourceText.drop_front(Skip); + assert(!ReplacementDsc.OldSuffix.empty() && + "We still should have some chars left."); + + // And get the replacement suffix, with upper-case chars. + std::string NewSuffix = ReplacementDsc.OldSuffix.upper(); + + if (ReplacementDsc.OldSuffix == NewSuffix) + return llvm::None; // The suffix was already fully uppercase. + + ReplacementDsc.Replacement = FixItHint::CreateReplacement(Range, NewSuffix); + + return ReplacementDsc; +} + +void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + stmt(eachOf(integerLiteral(intHasSuffix()).bind(IntegerLiteral::Name), + floatLiteral(fpHasSuffix()).bind(FloatingLiteral::Name))), + this); +} + +template <typename LiteralType> +bool UppercaseLiteralSuffixCheck::checkBoundMatch( + const ast_matchers::MatchFinder::MatchResult &Result) { + const auto *Literal = + Result.Nodes.getNodeAs<typename LiteralType::type>(LiteralType::Name); + if (!Literal) + return false; + + const SourceLocation LiteralLocation = Literal->getLocation(); + + // Ignore literals that aren't fully written in the source code. + if (LiteralLocation.isMacroID() || + Result.SourceManager->isMacroBodyExpansion(LiteralLocation) || + utils::isSyntheticValue(Result.SourceManager, Literal)) + return true; + + // We won't *always* want to replace. We might have already-uppercase suffix. + if (auto Details = shouldReplaceLiteralSuffix<LiteralType>( + *Literal, *Result.SourceManager)) { + diag(LiteralLocation, "%0 literal has suffix '%1', which is not upper-case") + << LiteralType::Name << Details->OldSuffix << Details->Replacement; + } + + return true; +} + +void UppercaseLiteralSuffixCheck::check( + const MatchFinder::MatchResult &Result) { + if (checkBoundMatch<IntegerLiteral>(Result)) + return; // If it *was* IntegerLiteral, don't check for FloatingLiteral. + checkBoundMatch<FloatingLiteral>(Result); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -38,6 +38,7 @@ #include "StaticDefinitionInAnonymousNamespaceCheck.h" #include "StringCompareCheck.h" #include "UniqueptrDeleteReleaseCheck.h" +#include "UppercaseLiteralSuffixCheck.h" namespace clang { namespace tidy { @@ -84,6 +85,8 @@ "readability-static-definition-in-anonymous-namespace"); CheckFactories.registerCheck<StringCompareCheck>( "readability-string-compare"); + CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>( + "readability-uppercase-literal-suffix"); CheckFactories.registerCheck<readability::NamedParameterCheck>( "readability-named-parameter"); CheckFactories.registerCheck<NonConstParameterCheck>( Index: clang-tidy/readability/MagicNumbersCheck.h =================================================================== --- clang-tidy/readability/MagicNumbersCheck.h +++ clang-tidy/readability/MagicNumbersCheck.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MAGICNUMBERSCHECK_H #include "../ClangTidy.h" +#include "../utils/ASTUtils.h" #include <llvm/ADT/APFloat.h> #include <llvm/ADT/SmallVector.h> #include <vector> @@ -37,14 +38,6 @@ bool isIgnoredValue(const IntegerLiteral *Literal) const; bool isIgnoredValue(const FloatingLiteral *Literal) const; - bool isSyntheticValue(const clang::SourceManager *, - const FloatingLiteral *) const { - return false; - } - - bool isSyntheticValue(const clang::SourceManager *SourceManager, - const IntegerLiteral *Literal) const; - template <typename L> void checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result, const char *BoundName) { @@ -62,7 +55,7 @@ if (isConstant(Result, *MatchedLiteral)) return; - if (isSyntheticValue(Result.SourceManager, MatchedLiteral)) + if (utils::isSyntheticValue(Result.SourceManager, MatchedLiteral)) return; const StringRef LiteralSourceText = Lexer::getSourceText( Index: clang-tidy/readability/MagicNumbersCheck.cpp =================================================================== --- clang-tidy/readability/MagicNumbersCheck.cpp +++ clang-tidy/readability/MagicNumbersCheck.cpp @@ -153,19 +153,6 @@ return false; } -bool MagicNumbersCheck::isSyntheticValue(const SourceManager *SourceManager, - const IntegerLiteral *Literal) const { - const std::pair<FileID, unsigned> FileOffset = - SourceManager->getDecomposedLoc(Literal->getLocation()); - if (FileOffset.first.isInvalid()) - return false; - - const StringRef BufferIdentifier = - SourceManager->getBuffer(FileOffset.first)->getBufferIdentifier(); - - return BufferIdentifier.empty(); -} - } // namespace readability } // namespace tidy } // namespace clang Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -31,6 +31,7 @@ StaticDefinitionInAnonymousNamespaceCheck.cpp StringCompareCheck.cpp UniqueptrDeleteReleaseCheck.cpp + UppercaseLiteralSuffixCheck.cpp LINK_LIBS clangAST Index: clang-tidy/hicpp/HICPPTidyModule.cpp =================================================================== --- clang-tidy/hicpp/HICPPTidyModule.cpp +++ clang-tidy/hicpp/HICPPTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "../bugprone/UndelegatedConstructorCheck.h" #include "../bugprone/UseAfterMoveCheck.h" #include "../cppcoreguidelines/AvoidGotoCheck.h" #include "../cppcoreguidelines/NoMallocCheck.h" @@ -21,7 +22,6 @@ #include "../google/ExplicitConstructorCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" #include "../misc/StaticAssertCheck.h" -#include "../bugprone/UndelegatedConstructorCheck.h" #include "../modernize/DeprecatedHeadersCheck.h" #include "../modernize/UseAutoCheck.h" #include "../modernize/UseEmplaceCheck.h" @@ -35,6 +35,7 @@ #include "../readability/BracesAroundStatementsCheck.h" #include "../readability/FunctionSizeCheck.h" #include "../readability/IdentifierNamingCheck.h" +#include "../readability/UppercaseLiteralSuffixCheck.h" #include "ExceptionBaseclassCheck.h" #include "MultiwayPathsCoveredCheck.h" #include "NoAssemblerCheck.h" @@ -100,6 +101,8 @@ "hicpp-use-nullptr"); CheckFactories.registerCheck<modernize::UseOverrideCheck>( "hicpp-use-override"); + CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>( + "hicpp-uppercase-literal-suffix"); CheckFactories.registerCheck<cppcoreguidelines::ProTypeVarargCheck>( "hicpp-vararg"); } Index: clang-tidy/cert/CMakeLists.txt =================================================================== --- clang-tidy/cert/CMakeLists.txt +++ clang-tidy/cert/CMakeLists.txt @@ -24,5 +24,6 @@ clangTidyGoogleModule clangTidyMiscModule clangTidyPerformanceModule + clangTidyReadabilityModule clangTidyUtils ) Index: clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tidy/cert/CERTTidyModule.cpp +++ clang-tidy/cert/CERTTidyModule.cpp @@ -16,6 +16,7 @@ #include "../misc/StaticAssertCheck.h" #include "../misc/ThrowByValueCatchByReferenceCheck.h" #include "../performance/MoveConstructorInitCheck.h" +#include "../readability/UppercaseLiteralSuffixCheck.h" #include "CommandProcessorCheck.h" #include "DontModifyStdNamespaceCheck.h" #include "FloatLoopCounter.h" @@ -65,6 +66,8 @@ // C checkers // DCL CheckFactories.registerCheck<misc::StaticAssertCheck>("cert-dcl03-c"); + CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>( + "cert-dcl16-c"); // ENV CheckFactories.registerCheck<CommandProcessorCheck>("cert-env33-c"); // FLP
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits