sammccall added inline comments.
================
Comment at: clangd/index/SymbolCollector.cpp:309
+ llvm::SmallString<128> USR;
+ if (index::generateUSRForDecl(ND, USR))
----------------
why this change? I think this makes us run generateUSR much more often (once
per non-unique reference in *any* file, vs unique refeneces in main file),
keeping track of a few extra referenced symbols by pointer should be much
cheaper
================
Comment at: clangd/index/SymbolCollector.cpp:349
+ SourceLocation Loc) {
+ assert(PP.get() && "Preprocessor must be set.");
+ if (!Opts.CollectMacro)
----------------
drop the message unless it has something new to say
================
Comment at: clangd/index/SymbolCollector.cpp:349
+ SourceLocation Loc) {
+ assert(PP.get() && "Preprocessor must be set.");
+ if (!Opts.CollectMacro)
----------------
sammccall wrote:
> drop the message unless it has something new to say
(why) do we require PP to be set if CollectMacro is false?
================
Comment at: clangd/index/SymbolCollector.cpp:356
+ return true;
+ // Builtin macro should already be available in sema.
+ if (MI.isUsedForHeaderGuard() || MI.isBuiltinMacro())
----------------
not sure what this means. Are you talking about code completion? This isn't
CodeComplete :-)
Header guard macros are clearly not useful in the index, but are probably worth
a comment. Builtin macros don't have useful locations, and also aren't needed
for code completion as you say.
================
Comment at: clangd/index/SymbolCollector.cpp:360
+
+ llvm::SmallString<128> USR;
+ if (index::generateUSRForMacro(Name.getName(), MI.getDefinitionLoc(), SM,
----------------
as above, can we avoid generating the USR for every reference?
The macro name or hash thereof should be enough to identify it, right?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49028
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
