[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
berenm added a comment. Yes, please could you commit it for me? Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D39363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
berenm updated this revision to Diff 126177.
berenm added a comment.
Rebase patch on upstream HEAD
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39363
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -405,6 +405,38 @@
using ::FOO_NS::InlineNamespace::CE_function;
// CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}}
+
+ unsigned MY_LOCAL_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for local variable 'MY_LOCAL_array'
+// CHECK-FIXES: {{^}} unsigned my_local_array[] = {1,2,3};{{$}}
+
+ unsigned const MyConstLocal_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'MyConstLocal_array'
+// CHECK-FIXES: {{^}} unsigned const kMyConstLocalArray[] = {1,2,3};{{$}}
+
+ static unsigned MY_STATIC_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for static variable 'MY_STATIC_array'
+// CHECK-FIXES: {{^}} static unsigned s_myStaticArray[] = {1,2,3};{{$}}
+
+ static unsigned const MyConstStatic_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: invalid case style for static constant 'MyConstStatic_array'
+// CHECK-FIXES: {{^}} static unsigned const MY_CONST_STATIC_ARRAY[] = {1,2,3};{{$}}
+
+ char MY_LOCAL_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'MY_LOCAL_string'
+// CHECK-FIXES: {{^}} char my_local_string[] = "123";{{$}}
+
+ char const MyConstLocal_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for local constant 'MyConstLocal_string'
+// CHECK-FIXES: {{^}} char const kMyConstLocalString[] = "123";{{$}}
+
+ static char MY_STATIC_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for static variable 'MY_STATIC_string'
+// CHECK-FIXES: {{^}} static char s_myStaticString[] = "123";{{$}}
+
+ static char const MyConstStatic_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for static constant 'MyConstStatic_string'
+// CHECK-FIXES: {{^}} static char const MY_CONST_STATIC_STRING[] = "123";{{$}}
}
#define MY_TEST_Macro(X) X()
@@ -418,6 +450,27 @@
template struct a {
typename t_t::template b<> c;
+
+ char const MY_ConstMember_string[4] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'MY_ConstMember_string'
+// CHECK-FIXES: {{^}} char const my_const_member_string[4] = "123";{{$}}
+
+ static char const MyConstClass_string[];
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for class constant 'MyConstClass_string'
+// CHECK-FIXES: {{^}} static char const kMyConstClassString[];{{$}}
};
+template
+char const a::MyConstClass_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for class constant 'MyConstClass_string'
+// CHECK-FIXES: {{^}}char const a::kMyConstClassString[] = "123";{{$}}
+
template class A> struct b { A c; };
+
+unsigned MY_GLOBAL_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global variable 'MY_GLOBAL_array'
+// CHECK-FIXES: {{^}}unsigned g_my_global_array[] = {1,2,3};{{$}}
+
+unsigned const MyConstGlobal_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global constant 'MyConstGlobal_array'
+// CHECK-FIXES: {{^}}unsigned const MY_CONST_GLOBAL_ARRAY[] = {1,2,3};{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -449,13 +449,13 @@
if (const auto *Decl = dyn_cast(D)) {
QualType Type = Decl->getType();
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_ConstantMember])
- return SK_ConstantMember;
+if (!Type.isNull() && Type.isConstQualified()) {
+ if (NamingStyles[SK_ConstantMember])
+return SK_ConstantMember;
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_Constant])
- return SK_Constant;
+ if (NamingStyles[SK_Constant])
+return SK_Constant;
+}
if (Decl->getAccess() == AS_private && NamingStyles[SK_PrivateMember])
return SK_PrivateMember;
@@ -478,13 +478,13 @@
if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable])
return SK_ConstexprVariable;
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_ConstantParameter])
- return SK_ConstantParameter;
+if (!Type.isNull() && Type.isConstQualified()) {
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
berenm added a comment. In https://reviews.llvm.org/D39363#948134, @alexfh wrote: > Sorry for the delay. I missed this revision somehow. Please add cfe-commits > to the subscribers list so that others can chime in. No worries, I'll add it in the future. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D39363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
berenm updated this revision to Diff 126017.
berenm added a comment.
Herald added a subscriber: klimek.
Factor !Type.isNull() && Type.isConstQualified() condition
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39363
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -405,6 +405,38 @@
using ::FOO_NS::InlineNamespace::CE_function;
// CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}}
+
+ unsigned MY_LOCAL_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for local variable 'MY_LOCAL_array'
+// CHECK-FIXES: {{^}} unsigned my_local_array[] = {1,2,3};{{$}}
+
+ unsigned const MyConstLocal_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'MyConstLocal_array'
+// CHECK-FIXES: {{^}} unsigned const kMyConstLocalArray[] = {1,2,3};{{$}}
+
+ static unsigned MY_STATIC_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for static variable 'MY_STATIC_array'
+// CHECK-FIXES: {{^}} static unsigned s_myStaticArray[] = {1,2,3};{{$}}
+
+ static unsigned const MyConstStatic_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: invalid case style for static constant 'MyConstStatic_array'
+// CHECK-FIXES: {{^}} static unsigned const MY_CONST_STATIC_ARRAY[] = {1,2,3};{{$}}
+
+ char MY_LOCAL_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'MY_LOCAL_string'
+// CHECK-FIXES: {{^}} char my_local_string[] = "123";{{$}}
+
+ char const MyConstLocal_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for local constant 'MyConstLocal_string'
+// CHECK-FIXES: {{^}} char const kMyConstLocalString[] = "123";{{$}}
+
+ static char MY_STATIC_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for static variable 'MY_STATIC_string'
+// CHECK-FIXES: {{^}} static char s_myStaticString[] = "123";{{$}}
+
+ static char const MyConstStatic_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for static constant 'MyConstStatic_string'
+// CHECK-FIXES: {{^}} static char const MY_CONST_STATIC_STRING[] = "123";{{$}}
}
#define MY_TEST_Macro(X) X()
@@ -418,6 +450,27 @@
template struct a {
typename t_t::template b<> c;
+
+ char const MY_ConstMember_string[4] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'MY_ConstMember_string'
+// CHECK-FIXES: {{^}} char const my_const_member_string[4] = "123";{{$}}
+
+ static char const MyConstClass_string[];
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for class constant 'MyConstClass_string'
+// CHECK-FIXES: {{^}} static char const kMyConstClassString[];{{$}}
};
+template
+char const a::MyConstClass_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for class constant 'MyConstClass_string'
+// CHECK-FIXES: {{^}}char const a::kMyConstClassString[] = "123";{{$}}
+
template class A> struct b { A c; };
+
+unsigned MY_GLOBAL_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global variable 'MY_GLOBAL_array'
+// CHECK-FIXES: {{^}}unsigned g_my_global_array[] = {1,2,3};{{$}}
+
+unsigned const MyConstGlobal_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global constant 'MyConstGlobal_array'
+// CHECK-FIXES: {{^}}unsigned const MY_CONST_GLOBAL_ARRAY[] = {1,2,3};{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -449,13 +449,13 @@
if (const auto *Decl = dyn_cast(D)) {
QualType Type = Decl->getType();
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_ConstantMember])
- return SK_ConstantMember;
+if (!Type.isNull() && Type.isConstQualified()) {
+ if (NamingStyles[SK_ConstantMember])
+return SK_ConstantMember;
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_Constant])
- return SK_Constant;
+ if (NamingStyles[SK_Constant])
+return SK_Constant;
+}
if (Decl->getAccess() == AS_private && NamingStyles[SK_PrivateMember])
return SK_PrivateMember;
@@ -478,13 +478,13 @@
if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable])
return SK_ConstexprVariable;
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_ConstantParameter])
- return
[PATCH] D30931: [clang-tidy] added new identifier naming case type for ignoring case style
berenm added a comment. In https://reviews.llvm.org/D30931#700690, @alexfh wrote: > I understand your use case, but this patch makes the check's behavior more > confusing: having both "any case" and "ignore case" with subtle differences > in behavior seems very misleading. The problem seems to be coming from the > usage of `CT_AnyCase` to denote uninitialized style. Should we just remove > `NamingStyle::isSet` and use `llvm::Optional` instead of > `NamingStyle` where appropriate? I agree https://reviews.llvm.org/D30931 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
berenm added a comment. Pinging @djasper There's https://reviews.llvm.org/D27651 that will conflict with this one. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and to the right
berenm accepted this revision. berenm added a comment. This revision is now accepted and ready to land. Awesome! Pinging @djasper https://reviews.llvm.org/D27651 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and to the right
berenm added a comment. Yes you are right, I believe it's happening for comma-separated declaration list. I think the algorithm tries to keep it clear that the pointer / reference marks are for each declared identifier and not part of the common type. I think that splitting the declarations for the Left and Middle test cases would be better so it actually verifies that the pointer marks are aligned as expected. Also, maybe add a bunch of references and rvalue refs to the test cases so that the tests are exhaustive. Thanks! https://reviews.llvm.org/D27651 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and to the right
berenm added a comment. I'm trying to think of a scenario where *, && or & before tokens to be aligned would not indicate pointers or references, but as the alignment is only done for now on declarations and assignments, I can't find one. Maybe you could add one more test case to check Left pointer alignment, as all the tests are apparently aligned Right by default ? This looks good to me, although it will conflict with https://reviews.llvm.org/D21279, so one or the other has to be merged first and the other rebased. https://reviews.llvm.org/D27651 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
