[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL345381: [clang-tidy] Re-commit: Add new 
readability-uppercase-literal-suffix check… (authored by lebedevri, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52670?vs=171279=171290#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52670

Files:
  clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
  clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl16-c.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  
clang-tools-extra/trunk/test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
  clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix.h

Index: clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
+++ clang-tools-extra/trunk/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("cert-dcl03-c");
+CheckFactories.registerCheck(
+"cert-dcl16-c");
 // ENV
 CheckFactories.registerCheck("cert-env33-c");
 // FLP
@@ -78,6 +81,13 @@
 CheckFactories.registerCheck(
 "cert-msc32-c");
   }
+
+  ClangTidyOptions getModuleOptions() override {
+ClangTidyOptions Options;
+ClangTidyOptions::OptionMap  = Options.CheckOptions;
+Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
+return Options;
+  }
 };
 
 } // namespace cert
Index: clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
@@ -24,5 +24,6 @@
   clangTidyGoogleModule
   clangTidyMiscModule
   clangTidyPerformanceModule
+  clangTidyReadabilityModule
   clangTidyUtils
   )
Index: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -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(
 "hicpp-use-override");
+CheckFactories.registerCheck(
+"hicpp-uppercase-literal-suffix");
 CheckFactories.registerCheck(
 "hicpp-vararg");
   }
Index: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
@@ -27,6 +27,18 @@
 bool exprHasBitFlagWithSpelling(const Expr *Flags, const 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Rerun on clang-tools-extra+lld+clang codebase, nothing broken.
I'm gonna reland.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 171279.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Finally finished creducing the issue i wasn't able to understand even after 
https://reviews.llvm.org/D53719+https://reviews.llvm.org/D53723,
and it turned out to be `hasParent()`vs.`hasAncestor()`. (all tests added)

With that fixed, i do not observe any other bugs.


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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
  
test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
  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-custom-list.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,268 @@
+// 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::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 uppercase
+  // 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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v8 == 1, "");
+
+  // Unsigned Long
+
+  static constexpr auto v9 = 1ul;
+  // 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 171172.
lebedev.ri added a comment.

Reverted in https://reviews.llvm.org/rL345305 due to multiple 
apparently-lurking bugs.


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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
  
test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
  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-custom-list.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,245 @@
+// 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::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 uppercase
+  // 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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::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 uppercase
+  // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul;
+  // CHECK-MESSAGES-NEXT: ^~~
+  // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+  // 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344755: [clang-tidy] Add new 
readability-uppercase-literal-suffix check (CERT DCL16-C… (authored 
by lebedevri, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52670?vs=170108=170111#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52670

Files:
  clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
  clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
  clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl16-c.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  
clang-tools-extra/trunk/test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
  
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
  clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix.h

Index: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -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(
 "hicpp-use-override");
+CheckFactories.registerCheck(
+"hicpp-uppercase-literal-suffix");
 CheckFactories.registerCheck(
 "hicpp-vararg");
   }
Index: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
@@ -27,6 +27,18 @@
 bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager ,
 const LangOptions ,
 StringRef FlagName);
+
+// Check if the range is entirely contained within a macro argument.
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+const SourceManager *SM);
+
+// Check if the range contains any locations from a macro expansion.
+bool rangeContainsMacroExpansion(SourceRange Range, const SourceManager *SM);
+
+// Can a fix-it be issued for this whole Range?
+// FIXME: false-negative if the entire range is fully expanded from a macro.
+bool rangeCanBeFixed(SourceRange Range, const SourceManager *SM);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
@@ -67,6 +67,32 @@
   return true;
 }
 
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+const SourceManager *SM) {
+  // Check if the range is entirely contained within a macro argument.
+  SourceLocation MacroArgExpansionStartForRangeBegin;
+  SourceLocation MacroArgExpansionStartForRangeEnd;
+  bool RangeIsEntirelyWithinMacroArgument =
+  SM &&
+  SM->isMacroArgExpansion(Range.getBegin(),
+  ) &&
+  SM->isMacroArgExpansion(Range.getEnd(),
+  ) &&
+  

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM! Thanks for the changes!


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 170108.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.

CERT: also handle "lu", "llu".


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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
  
test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
  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-custom-list.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,245 @@
+// 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::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 uppercase
+  // 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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::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 uppercase
+  // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul;
+  // CHECK-MESSAGES-NEXT: ^~~
+  // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+  // CHECK-FIXES: 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D52670#1268572, @aaron.ballman wrote:

> In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote:
> >
> > > I talked to someone at CERT responsible for maintaining DCL16-C to get 
> > > their opinion on tightening the wording of the rule and their stated 
> > > intent is:
> >
> >
> > Thank you!
> >
> > > "If the first character is 'ell', it should be capitalized. The other 
> > > ells need not be, and the yew's need not be capitalized either."
> > > 
> > > e.g.,
> > >  11lu -> diagnose
> > >  11ul -> fine
> > >  11llu -> diagnose
> > >  11lLu -> diagnose
> > >  11Llu -> fine
> > >  11ul -> fine
> > > 
> > > That said, the author (and I) agree that it'd be perfectly okay to 
> > > diagnose things like `11Llu` and recommend `11LLU` as a replacement.
> >
> > Ok, nothing unexpected.
> >  So the full revised list is: "L;LL:LU;LLU".
>
>
> Agreed. It might be reasonable to add the above as some extra test cases for 
> the CERT check if they're not already covered elsewhere.


Those exist as the test cases for CERT (although, i only test the integer case 
for CERT, not float.), i will only need to adjust the expected output.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote:

> In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote:
>
> > I talked to someone at CERT responsible for maintaining DCL16-C to get 
> > their opinion on tightening the wording of the rule and their stated intent 
> > is:
>
>
> Thank you!
>
> > "If the first character is 'ell', it should be capitalized. The other ells 
> > need not be, and the yew's need not be capitalized either."
> > 
> > e.g.,
> >  11lu -> diagnose
> >  11ul -> fine
> >  11llu -> diagnose
> >  11lLu -> diagnose
> >  11Llu -> fine
> >  11ul -> fine
> > 
> > That said, the author (and I) agree that it'd be perfectly okay to diagnose 
> > things like `11Llu` and recommend `11LLU` as a replacement.
>
> Ok, nothing unexpected.
>  So the full revised list is: "L;LL:LU;LLU".


Agreed. It might be reasonable to add the above as some extra test cases for 
the CERT check if they're not already covered elsewhere.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote:

> In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote:
> > >
> > > > - Apply minor wording nits.
> > > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not 
> > > > **anything** else (not even `llu`).
> > >
> > >
> > > I'll find out about the DCL16-C recommendation, as I suspect the intent 
> > > is to cover `lu` and `llu` but not `ul` and `ull`.
> >
> >
> > I agree, i've thought so too.
> >
> > That will open an interesting question: in `lu`, `l` should be upper-case. 
> > What about `u`? We can't keep it as-is.
> >  We will either consistently upper-case it, or consistently lower-case it.
> >  I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`?
>
>
> I talked to someone at CERT responsible for maintaining DCL16-C to get their 
> opinion on tightening the wording of the rule and their stated intent is:


Thank you!

> "If the first character is 'ell', it should be capitalized. The other ells 
> need not be, and the yew's need not be capitalized either."
> 
> e.g.,
>  11lu -> diagnose
>  11ul -> fine
>  11llu -> diagnose
>  11lLu -> diagnose
>  11Llu -> fine
>  11ul -> fine
> 
> That said, the author (and I) agree that it'd be perfectly okay to diagnose 
> things like `11Llu` and recommend `11LLU` as a replacement.

Ok, nothing unexpected.
So the full revised list is: "L;LL:LU;LLU".


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote:

> In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote:
> >
> > > - Apply minor wording nits.
> > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not 
> > > **anything** else (not even `llu`).
> >
> >
> > I'll find out about the DCL16-C recommendation, as I suspect the intent is 
> > to cover `lu` and `llu` but not `ul` and `ull`.
>
>
> I agree, i've thought so too.
>
> That will open an interesting question: in `lu`, `l` should be upper-case. 
> What about `u`? We can't keep it as-is.
>  We will either consistently upper-case it, or consistently lower-case it.
>  I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`?


I talked to someone at CERT responsible for maintaining DCL16-C to get their 
opinion on tightening the wording of the rule and their stated intent is:

"If the first character is 'ell', it should be capitalized. The other ells need 
not be, and the yew's need not be capitalized either."

e.g.,
11lu -> diagnose
11ul -> fine
11llu -> diagnose
11lLu -> diagnose
11Llu -> fine
11ul -> fine

That said, the author (and I) agree that it'd be perfectly okay to diagnose 
things like `11Llu` and recommend `11LLU` as a replacement.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote:

> In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote:
>
> > - Apply minor wording nits.
> > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not 
> > **anything** else (not even `llu`).
>
>
> I'll find out about the DCL16-C recommendation, as I suspect the intent is to 
> cover `lu` and `llu` but not `ul` and `ull`.


I agree, i've thought so too.

That will open an interesting question: in `lu`, `l` should be upper-case. What 
about `u`? We can't keep it as-is.
We will either consistently upper-case it, or consistently lower-case it.
I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`?

> I've pinged the authors

Thank you.

> and can hopefully get an answer quickly, but if not, I'm fine with fixing 
> that after the patch goes in.

Thank you.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote:

> - Apply minor wording nits.
> - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not **anything** 
> else (not even `llu`).


I'll find out about the DCL16-C recommendation, as I suspect the intent is to 
cover `lu` and `llu` but not `ul` and `ull`. I've pinged the authors and can 
hopefully get an answer quickly, but if not, I'm fine with fixing that after 
the patch goes in.




Comment at: clang-tidy/cert/CERTTidyModule.cpp:87
+ClangTidyOptions Options;
+auto  = Options.CheckOptions;
+Opts["cert-dcl16-c.NewSuffixes"] = "L;LL";

Don't use `auto` here.



Comment at: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst:6
+
+`cert-dcl16-c` redirects here as an alias for this check.
+

You should consider calling out the CERT behavioral differences.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 170045.
lebedev.ri marked 6 inline comments as done.
lebedev.ri added a comment.

- Apply minor wording nits.
- For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not **anything** 
else (not even `llu`).


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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
  test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
  
test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
  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-custom-list.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,245 @@
+// 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::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 uppercase
+  // 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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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 uppercase
+  // 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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::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 uppercase
+  // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul;

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Aaron Ballman via Phabricator via cfe-commits]

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(

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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 169556.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Bloat with `llvm::find_if()`


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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.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-opencl-half.cpp
  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-custom-list.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,245 @@
+// 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::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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::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;
+  

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:94
+  // Else, find matching suffix, case-*insensitive*ly.
+  for (const auto  : NewSuffixes) {
+if (!OldSuffix.equals_lower(PotentialNewSuffix))

Is this `std::find_if`?


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 169554.
lebedev.ri marked 8 inline comments as done.
lebedev.ri added a comment.

Hopefully address review notes:

- Support more (all?) obscure suffixes (complex, q, h, f16, i{8,16,32,64})
- Improve test coverage. There isn't coverage for every single 
combination/permuation of everything, but hopefully at least the basic coverage.
- Support a list of destination suffixes, thus the replacements can be finely 
controlled.


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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.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-opencl-half.cpp
  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-custom-list.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
  test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,245 @@
+// 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::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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v8 == 1, "");
+
+  // Unsigned Long
+
+  static constexpr auto v9 = 1ul;
+  

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Two main points: I don't think this check is covering all of the suffixes (I 
don't see `q` or `i32` support, for instance), and at least for the CERT rule 
this is covering, it diagnoses more than it should. The CERT rule is specific 
to `l` vs `L` but imposes no requirements on `u` vs `U`.




Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:88-89
 "readability-string-compare");
+CheckFactories.registerCheck(
+"readability-uppercase-literal-suffix");
 CheckFactories.registerCheck(

Please keep this sorted alphabetically.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:25
+
+struct IntegerLiteral {
+  using type = clang::IntegerLiteral;

It's unfortunate that clang::IntegerLiteral and 
clang::tidy::readability::IntegerLiteral are distinct types that are strongly 
related to one another. I'm not keen on this name, as it means reading the 
code, an unqualified IntegerLiteral is hard to reason about.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:30-31
+  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");
+};

There are other suffixes for integer literals. See `NumericLiteralParser`.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:37
+
+struct FloatingLiteral {
+  using type = clang::FloatingLiteral;

Same naming concerns here.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:47-48
+  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");
+};

There are more suffixes here (see `NumericLiteralParser` again), and this 
comment seems to be copy pasta.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:99-100
+ const SourceManager ) {
+  auto Begin = GetMacroAwareLocation(Loc.getBegin(), SM);
+  auto End = GetMacroAwareLocation(Loc.getEnd(), SM);
+  if (!Begin || !End)

Don't use `auto` here.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:122
+  // The literal may have macro expansion, we need the final expanded src 
range.
+  auto Range = GetMacroAwareSourceRange(ReplacementDsc.LiteralLocation, SM);
+  if (!Range)

Don't use `auto` here.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:660-678
-  // Check if the range is entirely contained within a macro argument.
-  SourceLocation MacroArgExpansionStartForRangeBegin;
-  SourceLocation MacroArgExpansionStartForRangeEnd;
-  bool RangeIsEntirelyWithinMacroArgument =
-  SourceMgr &&
-  SourceMgr->isMacroArgExpansion(Range.getBegin(),
- ) &&

@JonasToth that is what this code did.
From a quick look i'm not sure what it would decide without the source manager,
so i simply refactored it as a function, and kept the semantics.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165
+
+  diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case")
+  << LiteralType::Name << S.OldSuffix

JonasToth wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > JonasToth wrote:
> > > > lebedev.ri wrote:
> > > > > JonasToth wrote:
> > > > > > JonasToth wrote:
> > > > > > > Lets move this `diag` into the true branch of the if stmt and 
> > > > > > > drop the ´else`.
> > > > > > I think the warning should point at the suffix, which can be 
> > > > > > retrieved from the replacement range. Then you don't need to 
> > > > > > include the suffix itself in the diagnostic
> > > > > If the warnings are aggregated (i.e. not raw `make` dump), with only 
> > > > > the first line shown, the suffix will still be invisible.
> > > > > 
> > > > > Regarding the pointer direction, i'm not sure.
> > > > > For now i have reworded the diag to justify pointing at the literal 
> > > > > itself.
> > > > I don't understand that. The warning message does include the source 
> > > > location that would be clearly on the literal suffix and the warning 
> > > > without the suffix printed is clear as well. Having this slightly 
> > > > simpler diagnostic would simplify the code significantly.
> > > *location*. which will still be a location, if only the first line of the 
> > > warning is displayed.
> > > 
> > > We won't even win all that much.
> > > Sure, we will return `Optional`, but we will still need to do 
> > > all that internal stuff to find the old suffix, uppercase it, compare 
> > > them, etc.
> > > 
> > > And we lose a very useful ability to check what it considered to be the 
> > > old suffix in the tests.
> > > 
> > > It is basically the same question as in D51949.
> > > If you insist, sure, i can do that, but i *really* do believe this is 
> > > **WRONG**.
> > And one more problem here, where do we point if we don't have a fix-it to 
> > get the suffix location from?
> That point is valid, but if we dont have a suffix location, there is no 
> suffix?
> I dont insist :)
See `test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp`, 
`horrible_macros()`
I guess if we point at the suffix, we will also get less useful `expanded from` 
messages.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 167661.
lebedev.ri marked 5 inline comments as done.
lebedev.ri added a comment.

Address review notes.


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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.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-macro.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,213 @@
+// 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::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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::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::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;
+  

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165
+
+  diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case")
+  << LiteralType::Name << S.OldSuffix

lebedev.ri wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > lebedev.ri wrote:
> > > > JonasToth wrote:
> > > > > JonasToth wrote:
> > > > > > Lets move this `diag` into the true branch of the if stmt and drop 
> > > > > > the ´else`.
> > > > > I think the warning should point at the suffix, which can be 
> > > > > retrieved from the replacement range. Then you don't need to include 
> > > > > the suffix itself in the diagnostic
> > > > If the warnings are aggregated (i.e. not raw `make` dump), with only 
> > > > the first line shown, the suffix will still be invisible.
> > > > 
> > > > Regarding the pointer direction, i'm not sure.
> > > > For now i have reworded the diag to justify pointing at the literal 
> > > > itself.
> > > I don't understand that. The warning message does include the source 
> > > location that would be clearly on the literal suffix and the warning 
> > > without the suffix printed is clear as well. Having this slightly simpler 
> > > diagnostic would simplify the code significantly.
> > *location*. which will still be a location, if only the first line of the 
> > warning is displayed.
> > 
> > We won't even win all that much.
> > Sure, we will return `Optional`, but we will still need to do 
> > all that internal stuff to find the old suffix, uppercase it, compare them, 
> > etc.
> > 
> > And we lose a very useful ability to check what it considered to be the old 
> > suffix in the tests.
> > 
> > It is basically the same question as in D51949.
> > If you insist, sure, i can do that, but i *really* do believe this is 
> > **WRONG**.
> And one more problem here, where do we point if we don't have a fix-it to get 
> the suffix location from?
That point is valid, but if we dont have a suffix location, there is no suffix?
I dont insist :)



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:117
+
+  // So was this literal fully spelled,
+  // or is it a product of macro expansion and/or concatenation?

the comma is not necessary



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:119
+  // or is it a product of macro expansion and/or concatenation?
+  const bool RangeCanBeFixed =
+  utils::rangeCanBeFixed(ReplacementDsc.LiteralLocation, );

values are usually not annotated with `const`, please remove it for consistency



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:122
+
+  // Now, this is troubling. The literal may be somehow expanded from macro.
+  // So we can't just use plain getSourceRange(). An assumption is made that 
one

please remove the `Now this is troubling`.
Please use `two or more (what, macros?)` instead of `two+`



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:192
+bool UppercaseLiteralSuffixCheck::checkBoundMatch(
+const ast_matchers::MatchFinder::MatchResult ) {
+  const auto *Literal =

`ast_matchers::` is not necessary, because there is a `using ...` at the top of 
the file



Comment at: clang-tidy/utils/ASTUtils.cpp:72
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+const SourceManager *SM) {
+  // Check if the range is entirely contained within a macro argument.

Does it make sense to have a `nullptr` for SM? I think you can use a reference 
here



Comment at: docs/clang-tidy/checks/list.rst:67
bugprone-virtual-near-miss
cert-dcl03-c (redirects to misc-static-assert) 
cert-dcl21-cpp

hicpp alias is missing here


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 167658.
lebedev.ri marked 16 inline comments as done.
lebedev.ri added a comment.
Herald added subscribers: Sanitizers, llvm-commits.

Addressed remaining review notes:

- Fixed dangling links in docs
- Don't mishandle user-defined literals
- Don't ignore macros, do check them. Thanks to @AaronBallman for the macro 
concatenation example :)
- **Partial** fix-it support for macros.

...


Repository:
  rCRT Compiler Runtime

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/IdentifierNamingCheck.cpp
  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/cert-dcl16-c.rst
  docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.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-macro.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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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,213 @@
+// 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::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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::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 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)
+return false;

JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > Maybe the if could init `T`? It would require a second `return false;` if 
> > > i am not mistaken, but looks more regular to me. No strong opinion from 
> > > my side.
> > Then we will not have an early-return, which is worse than this.
> Ok. Can the `dyn_cast` be null at all? Shouldn't integerliteral always be a 
> `BuiltinType`?
I don't know?
I would guess no.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:85
+template 
+llvm::Optional
+UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix(

JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > These types get really long. Is it possible to put `NewSuffix` into the 
> > > anonymous namespace as well?
> > No, because `shouldReplaceLiteralSuffix()` is a member function which 
> > returns that type.
> > I think it should stay a member function, so theoretically `NewSuffix` 
> > could be a [second] template param, but that is kinda ugly..
> > I also can't simplify it via `using` because the `NewSuffix` is private.
> > 
> > Perhaps we should keep this as-is?
> Why does it need to be a member? It looks like it only accesses `NewSuffix` 
> which does nothing that requires private access to the check class.
Passing `LangOpts` turned out to be sufficient to move it into anon namespace.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165
+
+  diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case")
+  << LiteralType::Name << S.OldSuffix

lebedev.ri wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > JonasToth wrote:
> > > > JonasToth wrote:
> > > > > Lets move this `diag` into the true branch of the if stmt and drop 
> > > > > the ´else`.
> > > > I think the warning should point at the suffix, which can be retrieved 
> > > > from the replacement range. Then you don't need to include the suffix 
> > > > itself in the diagnostic
> > > If the warnings are aggregated (i.e. not raw `make` dump), with only the 
> > > first line shown, the suffix will still be invisible.
> > > 
> > > Regarding the pointer direction, i'm not sure.
> > > For now i have reworded the diag to justify pointing at the literal 
> > > itself.
> > I don't understand that. The warning message does include the source 
> > location that would be clearly on the literal suffix and the warning 
> > without the suffix printed is clear as well. Having this slightly simpler 
> > diagnostic would simplify the code significantly.
> *location*. which will still be a location, if only the first line of the 
> warning is displayed.
> 
> We won't even win all that much.
> Sure, we will return `Optional`, but we will still need to do all 
> that internal stuff to find the old suffix, uppercase it, compare them, etc.
> 
> And we lose a very useful ability to check what it considered to be the old 
> suffix in the tests.
> 
> It is basically the same question as in D51949.
> If you insist, sure, i can do that, but i *really* do believe this is 
> **WRONG**.
And one more problem here, where do we point if we don't have a fix-it to get 
the suffix location from?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D52670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked 8 inline comments as done.
lebedev.ri added inline comments.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165
+
+  diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case")
+  << LiteralType::Name << S.OldSuffix

JonasToth wrote:
> lebedev.ri wrote:
> > JonasToth wrote:
> > > JonasToth wrote:
> > > > Lets move this `diag` into the true branch of the if stmt and drop the 
> > > > ´else`.
> > > I think the warning should point at the suffix, which can be retrieved 
> > > from the replacement range. Then you don't need to include the suffix 
> > > itself in the diagnostic
> > If the warnings are aggregated (i.e. not raw `make` dump), with only the 
> > first line shown, the suffix will still be invisible.
> > 
> > Regarding the pointer direction, i'm not sure.
> > For now i have reworded the diag to justify pointing at the literal itself.
> I don't understand that. The warning message does include the source location 
> that would be clearly on the literal suffix and the warning without the 
> suffix printed is clear as well. Having this slightly simpler diagnostic 
> would simplify the code significantly.
*location*. which will still be a location, if only the first line of the 
warning is displayed.

We won't even win all that much.
Sure, we will return `Optional`, but we will still need to do all 
that internal stuff to find the old suffix, uppercase it, compare them, etc.

And we lose a very useful ability to check what it considered to be the old 
suffix in the tests.

It is basically the same question as in D51949.
If you insist, sure, i can do that, but i *really* do believe this is **WRONG**.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> 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?

I am not suggesting that these should be all uppercase or anything. But the 
tests should demonstrate, that user-defined literals do not impact analysis and 
give no false positives.




Comment at: clang-tidy/hicpp/HICPPTidyModule.cpp:24
 #include "../misc/StaticAssertCheck.h"
-#include "../bugprone/UndelegatedConstructorCheck.h"
 #include "../modernize/DeprecatedHeadersCheck.h"

spurious formatting fix, can be committed separate.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:42
+  // What should be skipped before looking for the Suffixes?
+  // Hexadecimal floating-point literals: skip until exponent first.
+  static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");

lebedev.ri wrote:
> JonasToth wrote:
> > The second line of the comment is slightly confusing, please make a full 
> > sentence out of it.
> Rewrote, hopefully better?
Yes, few nits.

- `15 in decimal and is` (no comma)
- `skip to` instead of `skip until`



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:52
+AST_MATCHER(clang::IntegerLiteral, intHasSuffix) {
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)

lebedev.ri wrote:
> JonasToth wrote:
> > as it hit me in my check: what about `(1)ul`? Is this syntactically correct 
> > and should be diagnosed too? (Please add tests if so).
> > 
> > In this case it should be `Note.getType().IgnoreParens().getTypePtr())`.
> clang says invalid
> https://godbolt.org/z/R8bGe_
fine then.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)
+return false;

lebedev.ri wrote:
> JonasToth wrote:
> > Maybe the if could init `T`? It would require a second `return false;` if i 
> > am not mistaken, but looks more regular to me. No strong opinion from my 
> > side.
> Then we will not have an early-return, which is worse than this.
Ok. Can the `dyn_cast` be null at all? Shouldn't integerliteral always be a 
`BuiltinType`?



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:85
+template 
+llvm::Optional
+UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix(

lebedev.ri wrote:
> JonasToth wrote:
> > These types get really long. Is it possible to put `NewSuffix` into the 
> > anonymous namespace as well?
> No, because `shouldReplaceLiteralSuffix()` is a member function which returns 
> that type.
> I think it should stay a member function, so theoretically `NewSuffix` could 
> be a [second] template param, but that is kinda ugly..
> I also can't simplify it via `using` because the `NewSuffix` is private.
> 
> Perhaps we should keep this as-is?
Why does it need to be a member? It looks like it only accesses `NewSuffix` 
which does nothing that requires private access to the check class.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:152
+  // Ignore literals that aren't fully written in the source code.
+  if (LiteralLocation.isMacroID() ||
+  Result.SourceManager->isMacroBodyExpansion(LiteralLocation) ||

lebedev.ri wrote:
> JonasToth wrote:
> > Wouldn't it make sense to warn for the literals in macros?
> Very obscure example (like all macros is), but i think it shows the point:
> ```
> #include 
> #include 
> 
> #define xstr(s) str(s)
> #define str(s) #s
> 
> #define dump(X) printf("%u is " str(X) "nits", X);
> 
> int main () {
>   dump(1u);
> 
>   return 0;
> }
> ```
> will normally print `1 is 1units`
> But if you uppercase it, it will print `1 is 1Units`.
> 
> I would honestly prefer to give macros a pass here..
Transformation is excluded of course.
The user can still silence the warning in case it is misplaced.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165
+
+  diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case")
+  << LiteralType::Name << S.OldSuffix

lebedev.ri wrote:
> JonasToth wrote:
> > JonasToth wrote:
> > > Lets move this `diag` into the true branch of the if stmt and drop the 
> > > ´else`.
> > I think the warning should point at the suffix, which can be retrieved from 
> > the replacement range. Then you don't need to include the suffix itself in 
> > the diagnostic
> If the warnings are aggregated (i.e. not raw `make` dump), with only the 
> first line shown, the suffix will still be invisible.
> 
> Regarding the pointer direction, i'm not sure.
> For now i have reworded the diag to justify pointing at the literal itself.
I don't understand 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

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 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : 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::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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::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 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:33
+};
+constexpr llvm::StringLiteral IntegerLiteral::Name;
+constexpr llvm::StringLiteral IntegerLiteral::SkipFirst;

JonasToth wrote:
> why are these declarations necessary?
Well, because these are *declarations*.
Else the linker complains, since i'm using these in non-`constexpr` context, 
but only provided definitions.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:42
+  // What should be skipped before looking for the Suffixes?
+  // Hexadecimal floating-point literals: skip until exponent first.
+  static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");

JonasToth wrote:
> The second line of the comment is slightly confusing, please make a full 
> sentence out of it.
Rewrote, hopefully better?



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:52
+AST_MATCHER(clang::IntegerLiteral, intHasSuffix) {
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)

JonasToth wrote:
> as it hit me in my check: what about `(1)ul`? Is this syntactically correct 
> and should be diagnosed too? (Please add tests if so).
> 
> In this case it should be `Note.getType().IgnoreParens().getTypePtr())`.
clang says invalid
https://godbolt.org/z/R8bGe_



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)
+return false;

JonasToth wrote:
> Maybe the if could init `T`? It would require a second `return false;` if i 
> am not mistaken, but looks more regular to me. No strong opinion from my side.
Then we will not have an early-return, which is worse than this.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:85
+template 
+llvm::Optional
+UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix(

JonasToth wrote:
> These types get really long. Is it possible to put `NewSuffix` into the 
> anonymous namespace as well?
No, because `shouldReplaceLiteralSuffix()` is a member function which returns 
that type.
I think it should stay a member function, so theoretically `NewSuffix` could be 
a [second] template param, but that is kinda ugly..
I also can't simplify it via `using` because the `NewSuffix` is private.

Perhaps we should keep this as-is?



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:95
+  // Get the whole Integer Literal from the source buffer.
+  const StringRef LiteralSourceText = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(S.Range), SM, getLangOpts());

JonasToth wrote:
> Please check if the source text could be retrieved, with a final `bool` 
> parameter, thats in/out and at least `assert` on that.
I looked at a randomly-selected few dozen calls to this function within 
clang-tidy, and none of those do this.
But `assert` added, better than nothing.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:129
+  if (S.OldSuffix == S.NewSuffix)
+return llvm::None; // The suffix was already fully uppercase.
+

JonasToth wrote:
> Could this function return a `Optional`? That would include the 
> range and the relacement-text. I feel that is would simplify the code, 
> especially the amount of state that one has to keep track of.
I still want to see the old suffix, but i think we can form the FixitHint here, 
and store it.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:135
+void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  stmt(integerLiteral(intHasSuffix())).bind(IntegerLiteral::Name), this);

JonasToth wrote:
> I think you can merge this matcher to 
> `stmt(eachOf(integerLiteral(intHasSuffix()).bind(), 
> floatLiteral(fpHasSuffix()).bind()))`
> 
> `eachOf` because we want to match all, and not short-circuit.
`bind()` still wants the name though.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:152
+  // Ignore literals that aren't fully written in the source code.
+  if (LiteralLocation.isMacroID() ||
+  Result.SourceManager->isMacroBodyExpansion(LiteralLocation) ||

JonasToth wrote:
> Wouldn't it make sense to warn for the literals in macros?
Very obscure example (like all macros is), but i think it shows the point:
```
#include 
#include 

#define xstr(s) str(s)
#define str(s) #s

#define dump(X) printf("%u is " str(X) "nits", X);

int main () {
  dump(1u);

  return 0;
}
```
will normally print `1 is 1units`
But if you uppercase it, it will print `1 is 1Units`.

I would honestly prefer to give macros a pass here..



Comment at: 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: docs/ReleaseNotes.rst:96
 
+- New alias :doc:`cert-dcl16-c
+  ` to 
:doc:`readability-uppercase-literal-suffix

JonasToth wrote:
> lebedev.ri wrote:
> > Eugene.Zelenko wrote:
> > > Please move alias after new checks.
> > BTW, is there some tool to actually add this alias? I didn't find it, and 
> > had to do it by hand..
> So far there is nothing, but would be a nice addition :)
Oh, i was really hoping i simply did not find it :/


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

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

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




Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:26
+struct IntegerLiteral {
+  using ClangType = clang::IntegerLiteral;
+  static constexpr llvm::StringLiteral Name = llvm::StringLiteral("integer");

maybe `ClangType` is not a good name, how about just `type` to be consistent 
with e.g. std::vector convention.
The use-case in your template makes it clear, that we are talking about 
`LiteralType`s.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:33
+};
+constexpr llvm::StringLiteral IntegerLiteral::Name;
+constexpr llvm::StringLiteral IntegerLiteral::SkipFirst;

why are these declarations necessary?



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:42
+  // What should be skipped before looking for the Suffixes?
+  // Hexadecimal floating-point literals: skip until exponent first.
+  static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");

The second line of the comment is slightly confusing, please make a full 
sentence out of it.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:52
+AST_MATCHER(clang::IntegerLiteral, intHasSuffix) {
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)

as it hit me in my check: what about `(1)ul`? Is this syntactically correct and 
should be diagnosed too? (Please add tests if so).

In this case it should be `Note.getType().IgnoreParens().getTypePtr())`.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)
+return false;

Maybe the if could init `T`? It would require a second `return false;` if i am 
not mistaken, but looks more regular to me. No strong opinion from my side.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:69
+AST_MATCHER(clang::FloatingLiteral, fpHasSuffix) {
+  const auto *T = dyn_cast(Node.getType().getTypePtr());
+  if (!T)

Same comment as above



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:85
+template 
+llvm::Optional
+UppercaseLiteralSuffixCheck::shouldReplaceLiteralSuffix(

These types get really long. Is it possible to put `NewSuffix` into the 
anonymous namespace as well?



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:90
+
+  NewSuffix S;
+

GIven that this variable is used mutliple times in a longer function, i feel it 
shuold get a longer, more descriptive name



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:95
+  // Get the whole Integer Literal from the source buffer.
+  const StringRef LiteralSourceText = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(S.Range), SM, getLangOpts());

Please check if the source text could be retrieved, with a final `bool` 
parameter, thats in/out and at least `assert` on that.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:106
+Skip = LiteralSourceText.find_first_of(LiteralType::SkipFirst);
+if (Skip == StringRef::npos) // We could be in non-hexadecimal fp literal.
+  Skip = 0;

please use `flloating-point` instead of `fp` (same in other places)



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:129
+  if (S.OldSuffix == S.NewSuffix)
+return llvm::None; // The suffix was already fully uppercase.
+

Could this function return a `Optional`? That would include the 
range and the relacement-text. I feel that is would simplify the code, 
especially the amount of state that one has to keep track of.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:135
+void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  stmt(integerLiteral(intHasSuffix())).bind(IntegerLiteral::Name), this);

I think you can merge this matcher to 
`stmt(eachOf(integerLiteral(intHasSuffix()).bind(), 
floatLiteral(fpHasSuffix()).bind()))`

`eachOf` because we want to match all, and not short-circuit.



Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:152
+  // Ignore literals that aren't fully written in the source code.
+  if 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 167552.
lebedev.ri added a comment.

- Deduplicate one test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52670

Files:
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  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

Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -0,0 +1,218 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix'
+
+template 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : true_type {};
+
+void integer_suffix() {
+  static constexpr auto v0 = __LINE__; // synthetic
+  static_assert(v0 == 24 || v0 == 20, "");
+
+  static constexpr auto v1 = __cplusplus; // synthetic, long
+
+  static constexpr auto v2 = 1; // no literal
+  static_assert(is_same::value, "");
+  static_assert(v2 == 1, "");
+
+  // Unsigned
+
+  static constexpr auto v3 = 1u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'u' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v4 == 1, "");
+
+  // Long
+
+  static constexpr auto v5 = 1l;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'l' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v6 == 1, "");
+
+  // Long Long
+
+  static constexpr auto v7 = 1ll;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'll' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v8 == 1, "");
+
+  // Unsigned Long
+
+  static constexpr auto v9 = 1ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'ul' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v9 == 1, "");
+
+  static constexpr auto v10 = 1uL;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal suffix 'uL' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v10 == 1, "");
+
+  static constexpr auto v11 = 1Ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal suffix 'Ul' is not upper-case [readability-uppercase-literal-suffix]
+  // CHECK-MESSAGES-NEXT: static constexpr auto v11 = 1Ul;
+  // 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: docs/ReleaseNotes.rst:96
 
+- New alias :doc:`cert-dcl16-c
+  ` to 
:doc:`readability-uppercase-literal-suffix

Eugene.Zelenko wrote:
> Please move alias after new checks.
BTW, is there some tool to actually add this alias? I didn't find it, and had 
to do it by hand..


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 167547.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Move alias to after the new check,


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52670

Files:
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  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

Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -0,0 +1,218 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix'
+
+template 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : true_type {};
+
+void integer_suffix() {
+  static constexpr auto v0 = __LINE__; // synthetic
+  static_assert(v0 == 24 || v0 == 20, "");
+
+  static constexpr auto v1 = __cplusplus; // synthetic, long
+
+  static constexpr auto v2 = 1; // no literal
+  static_assert(is_same::value, "");
+  static_assert(v2 == 1, "");
+
+  // Unsigned
+
+  static constexpr auto v3 = 1u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'u' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v4 == 1, "");
+
+  // Long
+
+  static constexpr auto v5 = 1l;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'l' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v6 == 1, "");
+
+  // Long Long
+
+  static constexpr auto v7 = 1ll;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'll' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v8 == 1, "");
+
+  // Unsigned Long
+
+  static constexpr auto v9 = 1ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'ul' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v9 == 1, "");
+
+  static constexpr auto v10 = 1uL;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal suffix 'uL' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v10 == 1, "");
+
+  static constexpr auto v11 = 1Ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: integer literal suffix 'Ul' is not upper-case [readability-uppercase-literal-suffix]
+  // 

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

May be we should also create MISRA module?


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:96
 
+- New alias :doc:`cert-dcl16-c
+  ` to 
:doc:`readability-uppercase-literal-suffix

Please move alias after new checks.


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

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun.
lebedev.ri added a project: clang-tools-extra.
Herald added subscribers: rnkovacs, mgorny.

Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.

All valid combinations of suffixes are supported.

  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.
  
  ...

References:

- CERT DCL16-C 

- MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a 
literal suffix
- MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52670

Files:
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  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

Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -0,0 +1,218 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix'
+
+template 
+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;
+using true_type = integral_constant;
+
+template 
+struct is_same : false_type {};
+
+template 
+struct is_same : true_type {};
+
+void integer_suffix() {
+  static constexpr auto v0 = __LINE__; // synthetic
+  static_assert(v0 == 24 || v0 == 20, "");
+
+  static constexpr auto v1 = __cplusplus; // synthetic, long
+
+  static constexpr auto v2 = 1; // no literal
+  static_assert(is_same::value, "");
+  static_assert(v2 == 1, "");
+
+  // Unsigned
+
+  static constexpr auto v3 = 1u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'u' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v3 == 1, "");
+
+  static constexpr auto v4 = 1U; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v4 == 1, "");
+
+  // Long
+
+  static constexpr auto v5 = 1l;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'l' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v5 == 1, "");
+
+  static constexpr auto v6 = 1L; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v6 == 1, "");
+
+  // Long Long
+
+  static constexpr auto v7 = 1ll;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'll' is not upper-case [readability-uppercase-literal-suffix]
+  // 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::value, "");
+  static_assert(v7 == 1, "");
+
+  static constexpr auto v8 = 1LL; // OK.
+  static_assert(is_same::value, "");
+  static_assert(v8 == 1, "");
+
+  // Unsigned Long
+
+  static constexpr auto v9 = 1ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal suffix 'ul' is not upper-case [readability-uppercase-literal-suffix]
+  // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul;
+  // CHECK-MESSAGES-NEXT: ^~~
+  // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+  // CHECK-FIXES: static constexpr