ioeric accepted this revision.
ioeric added a comment.
Lg
================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67
+ // XXX this is just to make running the tool fast during dev!
+ bool BeginInvocation(CompilerInstance &CI) override {
+ const auto &Inputs = CI.getInvocation().getFrontendOpts().Inputs;
----------------
hokein wrote:
> ioeric wrote:
> > sammccall wrote:
> > > hokein wrote:
> > > > It is fine for debugging, but I think we don't want this behavior by
> > > > default.
> > > >
> > > > global-symbol-builder also supports running a single TU
> > > > (`global-symbol-builder /path/to/file`), which is sufficient for
> > > > debugging, I think?
> > > >
> > > Yeah, I'm not going to check this in, thus the XXX comment :-)
> > >
> > > Single TU isn't enough - it doesn't test the reducer. On the other hand
> > > the full compilation database is too big. So this option would actually
> > > be useful! But it doesn't belong here.
> > Drive-by: you could also run the tool in the default standalone mode and
> > provide a list of files.
> However the default standalone mode is not multiple thread :(.
You are right :) But I don't think this matters much if you only want to test
reduction.
================
Comment at: clangd/index/SymbolCollector.cpp:210
+ BasicSymbol = addDeclaration(*ND, std::move(ID));
+ if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
+ addDefinition(*cast<NamedDecl>(ASTNode.OrigD), *BasicSymbol);
----------------
sammccall wrote:
> hokein wrote:
> > ioeric wrote:
> > > It seems that we store definition even if it's the same as declaration,
> > > which might be true for most classes? This is probably fine, but it would
> > > be worth documenting. Or maybe consider not storing the same location
> > > twice?
> > I think it is fine to store the same location twice. We can't tell whether
> > the CanonicalLoc is a definition or not.
> Documented that these may be the same.
> We wouldn't actually save any memory by avoiding saving this twice - the
> filename is a stringref to the same data, and the offsets get stored even if
> they're null.
I'm less concerned about space. I think this would also make it easy to check
whether a symbol has a separate definition (e.g. we might want to do this for
go-to-definition/go-to-declaration). A comparison would also work but seems
less natural. But I'm not sure, up tp you :)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42942
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
