stephanemoore accepted this revision.
stephanemoore marked 2 inline comments as done.
stephanemoore added a comment.
(sorry I forgot to send this earlier)
Two small things and then I think everything is good.
================
Comment at:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:29
FixItHint generateFixItHint(const VarDecl *Decl, bool IsConst) {
+ if (IsConst && Decl->hasExternalStorage()) {
+ // No fix available if it is a extern global constant, since it is
difficult
----------------
This condition is probably technically fine since I don't believe it's possible
for variables with auto or register storage classes to get here but if we were
worried about that then I think we could change the condition to something like
this:
```
if (IsConst && (Decl->getStorageClass() != SC_Static)) {
```
================
Comment at:
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:16-18
NSString* globalString = @"test";
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: non-const global variable
'globalString' must have a name which starts with 'g[A-Z]'
[google-objc-global-variable-declaration]
// CHECK-FIXES: NSString* gGlobalString = @"test";
----------------
This is definitely outside the scope of this commit but this fix seems like it
would only be appropriate in an implementation file 🤔
================
Comment at:
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:32
+static NSString* const notCap = @"NotBeginWithCap";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'notCap'
must have a name which starts with an appropriate prefix
[google-objc-global-variable-declaration]
----------------
Isn't this a case where we would expect `kNotCap` as a suggested fixit?
================
Comment at:
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:39
+static NSString* const SecondNotCap = @"SecondNotCapOrNumber";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable
'SecondNotCap' must have a name which starts with an appropriate prefix
[google-objc-global-variable-declaration]
----------------
Isn't this a case where we would expect `kSecondNotCap` as a suggested fixit?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62045/new/
https://reviews.llvm.org/D62045
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
