stephanemoore added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:45
+
+ auto NewName = (IsConst? "k": "g") + llvm::StringRef(std::string(1,
FC)).upper()) +
+ Decl->getName().substr(1).str());
----------------
Can you make sure this is formatted correctly? I would expect a space before
the `?` but I am not familiar with LLVM style on every detail.
================
Comment at:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:45
+
+ auto NewName = (IsConst? "k": "g") + llvm::StringRef(std::string(1,
FC)).upper()) +
+ Decl->getName().substr(1).str());
----------------
stephanemoore wrote:
> Can you make sure this is formatted correctly? I would expect a space before
> the `?` but I am not familiar with LLVM style on every detail.
Do the tests pass with the added `GlobalConstant` test case? I would expect
that we need to add an early return to handle extern global constants but I
could be mistaken.
================
Comment at:
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:74
+ unless(isLocalVariable()),
+ unless(matchesName("::(k[A-Z].*)|([A-Z][A-Z0-9].*)")))
+ .bind("global_const"),
----------------
What is the reason for changing the regex to match the entire name?
(I do not object to the change but I want to know if there is a policy that I
am not aware of)
================
Comment at:
clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:28
-static NSString* const k_Alpha = @"SecondNotAlpha";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha'
must have a name which starts with an appropriate prefix
[google-objc-global-variable-declaration]
----------------
I would keep a `k_Alpha` test case so that we know that an underscore after the
`k` is not accepted.
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
