sammccall added a comment.
In https://reviews.llvm.org/D51314#1215381, @sammccall wrote:
> I'm a bit uncomfortable about the places where we need the type, because this
> is the only thing forcing us to parse before we've picked a command to
> transfer, and the number of commands we need to parse is data-dependent and
> hard to reason about.
>
> Let me think about this a little - I suspect slightly more invasive changes
> (change the concept of type, tweak the heuristics, or do a "lightweight
> parse" to get the type) might make this cleaner and performance more
> predictable.
Having studied the code a bit - we use the parsed type to evaluate candidates,
but we're comparing against the type extracted from the query filename (we have
nothing else!).
So we should be fine just to compare extensions instead here. So either
TransferableCommand would have an Extension field or a TypeFromFilename field -
but we should be careful not to conflate the type inferred from the filename
(fine for *selecting* a command) with the one parsed from the command (needed
to *transfer* the command).
Then we have no need to parse for selection, and `getCompileCommands()` only
needs to parse a single command from the underlying CDB, which should yield
very predictable/consistent performance.
(It also couples nicely with the idea of only grabbing the full command from
the underlying CDB lazily - we'll only do that once, too).
================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:480-481
std::vector<TransferableCommand> Result;
- for (auto Command : Inner->getAllCompileCommands()) {
+ for (auto Command : Inner->getAllCompileCommands())
Result.emplace_back(std::move(Command));
return Result;
----------------
as you pointed out offline, we actually only need the filename for indexing, so
we could ask the underlying DB for the filenames and get their commands on
demand.
(we need to verify that the values returned don't differ from the filenames
stored in CompileCommand, e.g. by filename normalization, in a way that matters)
Repository:
rC Clang
https://reviews.llvm.org/D51314
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
