[PATCH] D115634: [clangd] Cleanup of readability-identifier-naming

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 1 05:03:37 PST 2022


sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks, this is great!
Just a few cases that regex couldn't quite understand, and one bona fide bug!



================
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:103
 
-static clang::clangd::Key<std::string> kFileBeingProcessed;
+static clang::clangd::Key<std::string> KFileBeingProcessed;
 
----------------
I think this got reverted - should just be `FileBeingProcessed`


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1864
       }
-      QualType VisitCoyieldStmt(const CoyieldExpr *S) {
+      QualType visitCoyieldStmt(const CoyieldExpr *S) {
         return type(S->getOperand());
----------------
this was a bug! should be `VisitCoyieldExpr` (capital because it's a CRTP override).

But this is is a functional bugfix, please either leave a fixme or change in a separate patch


================
Comment at: clang-tools-extra/clangd/index/YAMLSerialization.cpp:176
 template <> struct MappingTraits<SymbolInfo> {
-  static void mapping(IO &io, SymbolInfo &SymInfo) {
+  static void mapping(IO &Io, SymbolInfo &SymInfo) {
     // FIXME: expose other fields?
----------------
sammccall wrote:
> We use `IO` rather than `Io` for initialisms.
> There are cases where the type/name can't match, but this doesn't appear to be one of them (we use `IO &IO` in similar mapping function below)
This change also got lost (and in the next function)


================
Comment at: clang-tools-extra/clangd/unittests/SerializationTests.cpp:117
 
-MATCHER_P(ID, I, "") { return arg.ID == cantFail(SymbolID::fromStr(I)); }
-MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P(iD, I, "") { return arg.ID == cantFail(SymbolID::fromStr(I)); }
+MATCHER_P(qName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
----------------
`id`, not `iD`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115634/new/

https://reviews.llvm.org/D115634



More information about the cfe-commits mailing list