stephanemoore requested changes to this revision. stephanemoore marked 4 inline comments as done. stephanemoore added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:51-52 + + auto NewName = "g" + llvm::StringRef(std::string(1, FC)).upper()) + + Decl->getName().substr(1).str()); + ---------------- benhamilton wrote: > I don't see any guidance in the style guide about prefixing with `g` either. > I assume we should remove that? > Non-const static variable declaration names should have the prefix `g`: "File scope or global variables have a prefix g." https://google.github.io/styleguide/objcguide.html#variable-names The prefixes section in the style guide (https://google.github.io/styleguide/objcguide.html#prefixes) is targeted towards prefixes for classes, protocols, global functions, and global constants (i.e., const qualified variable declaration names). ================ Comment at: clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:79 unless(isLocalVariable()), - unless(matchesName("::(k[A-Z]|[A-Z]{2,})"))) + unless(matchesName("::[A-Z][A-Z0-9].*"))) .bind("global_const"), ---------------- For legacy code, I think we still need to allow variable declarations of the following form: ``` static const int kFileCount = 12; static const int kFOOFileCount = 12; ``` ================ Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:26 // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration] -// CHECK-FIXES: static NSString* const _notAlpha = @"NotBeginWithAlpha"; ---------------- Thanks for removing the fixit on this case 😄 The fix was not compliant with Google Objective-C style. ================ Comment at: clang-tools-extra/test/clang-tidy/google-objc-global-variable-declaration.m:30 -// 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] -// CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha"; ---------------- Thanks for removing the fixit on this case 😄 The fix was not compliant with Google Objective-C style. 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
