ioeric added inline comments.
================
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);
----------------
sammccall wrote:
> 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)
`ImplementationDetail` sounds great.


================
Comment at: clangd/Quality.cpp:192
   Category = categorize(IndexResult.SymInfo);
   ReservedName = ReservedName || isReserved(IndexResult.Name);
 }
----------------
sammccall wrote:
> 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).
Yeah, I wasn't sure what the name to use here and wanted to get a a second 
opinion on adding the new flag. Thanks for the name!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53374



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to