This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340838: Parse compile commands lazily in
InterpolatingCompilationDatabase (authored by ibiryukov, committed by ).
Herald
ilya-biryukov updated this revision to Diff 162868.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.
- Cleanups
Repository:
rC Clang
https://reviews.llvm.org/D51314
Files:
lib/Tooling/InterpolatingCompilationDatabase.cpp
sammccall accepted this revision.
sammccall added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:127
// Language detected from -x or the filename.
- types::ID Type = types::TY_INVALID;
+ // When set, cannot be TY_INVALID.
+ llvm::Optional
ilya-biryukov updated this revision to Diff 162863.
ilya-biryukov added a comment.
- Sort Paths, they are different from OriginalPaths, i.e. lowercased.
Repository:
rC Clang
https://reviews.llvm.org/D51314
Files:
lib/Tooling/InterpolatingCompilationDatabase.cpp
ilya-biryukov updated this revision to Diff 162861.
ilya-biryukov added a comment.
- Handle TransferableCommands with TY_INVALID type (never transfer -x flag for
those)
- Add a test with invalid extensions, seen a crash while experimenting
- Update the test wrt to the new behavior.
Repository:
ilya-biryukov updated this revision to Diff 162855.
ilya-biryukov added a comment.
- Lowercase everything stored in the index.
Repository:
rC Clang
https://reviews.llvm.org/D51314
Files:
lib/Tooling/InterpolatingCompilationDatabase.cpp
Index:
sammccall accepted this revision.
sammccall added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:229
+//
+// Apart from path proximity signals, also takes file extensions into account
+// when scoring the candidates.
(this comment
ilya-biryukov updated this revision to Diff 162851.
ilya-biryukov added a comment.
- Reformat the code
- Minor spelling fix
Repository:
rC Clang
https://reviews.llvm.org/D51314
Files:
lib/Tooling/InterpolatingCompilationDatabase.cpp
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
ilya-biryukov updated this revision to Diff 162849.
ilya-biryukov marked 6 inline comments as done.
ilya-biryukov added a comment.
- Update the comments
- Rename the new class to FileIndex
- Restore an accidentally lost comment
- Store file types in a parallel array instead of recomputing on each
sammccall accepted this revision.
sammccall added a comment.
Awesome :-)
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:220
-// CommandIndex does the real work: given a filename, it produces the best
-// matching TransferableCommand by matching filenames.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:133
+assert(TraitsComputed && "calling transferTo on moved-from object");
+const CommandTraits = getTraits();
+
ilya-biryukov updated this revision to Diff 162817.
ilya-biryukov added a comment.
Herald added a subscriber: mgrang.
- Remove mutexes, recompute every time instead
- Delay creation of TransferableCommand to avoid calling getAllCommands() on
JSONCompilationDatabase
Repository:
rC Clang
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
sammccall added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
struct TransferableCommand {
sammccall added a comment.
Thanks for finding this problem, this fix *mostly* looks good (though I think
we can probably drop memoization).
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
ilya-biryukov added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
struct TransferableCommand {
jfb added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124
+// A CompileCommand that can be applied to another file. Any instance of this
+// object is invalid after std::move() from it.
struct TransferableCommand {
This comment
ilya-biryukov updated this revision to Diff 162702.
ilya-biryukov added a comment.
- Remove #include , it is not used anymore
Repository:
rC Clang
https://reviews.llvm.org/D51314
Files:
lib/Tooling/InterpolatingCompilationDatabase.cpp
Index:
ilya-biryukov added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:201
+
+ CommandTraits computeTraits() const {
+CommandTraits Result;
jfb wrote:
> It's not clear to me that this entire function is safe to call from multiple
ilya-biryukov updated this revision to Diff 162701.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
- Remove computeTraits, put the body inside a lambda
Repository:
rC Clang
https://reviews.llvm.org/D51314
Files:
ilya-biryukov updated this revision to Diff 162699.
ilya-biryukov added a comment.
- Add a comment
- Use std::call_once to compute the lazy value
Repository:
rC Clang
https://reviews.llvm.org/D51314
Files:
lib/Tooling/InterpolatingCompilationDatabase.cpp
Index:
ilya-biryukov added inline comments.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:440
Result.emplace_back(std::move(Command));
- if (Result.back().Type == types::TY_INVALID)
-Result.pop_back();
We can't look at 'Type' at this
jfb requested changes to this revision.
jfb added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:201
+
+ CommandTraits computeTraits() const {
+CommandTraits Result;
It's not
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: jfb.
This greatly reduces the time to read 'compile_commands.json'.
For Chromium on my machine it's now 5 secs vs 30 secs before the
change.
Repository:
rC Clang
24 matches
Mail list logo
