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