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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
