[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 17 23:33:49 PDT 2018


sammccall added a comment.

Idea looks good, I think it needs some renames and index support.



================
Comment at: clangd/AST.h:33
+//     `-DName=foo`, the spelling location will be "<command line>".
+bool SpelledInSourceCode(const Decl *D);
+
----------------
nit: should start with lowercase: `isSpelledInSourceCode`?


================
Comment at: clangd/Quality.cpp:182
     if (auto *ID = SemaCCResult.Declaration->getIdentifier())
-      ReservedName = ReservedName || isReserved(ID->getName());
+      ReservedName = ReservedName || isReserved(ID->getName()) ||
+                     !SpelledInSourceCode(SemaCCResult.Declaration);
----------------
This doesn't match the current definition of `ReservedName`.
I'd suggest either:
 - adding a new boolean (`ImplementationDetail`? maybe we'll add other heuristics) and treating this as independent of reserved-ness
 - renaming the current `ReservedName` flag to cover this expanded scope (again, `ImplementationDetail` is a reasonable name)


================
Comment at: clangd/Quality.cpp:192
   Category = categorize(IndexResult.SymInfo);
   ReservedName = ReservedName || isReserved(IndexResult.Name);
 }
----------------
The new `ReservedName` cases don't survive a round-trip through the index (unlike the existing ones, which get recomputed from the name).

I think you want to add a new flag bit to `Symbol`, set it in `SymbolCollector`, and read it here. (IIRC new flags in the `Flags` field are handled transparently by yaml and binary serialization).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53374





More information about the cfe-commits mailing list