sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a reviewer: jkorous-apple.
Big +1 to the change here.
Just suggestions for the comments.
================
Comment at: clangd/CodeComplete.cpp:522
- // Attempt to reuse the PCH from precompiled preamble, if it was built.
- if (Preamble) {
- auto Bounds =
- ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
- if (!Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds, VFS.get()))
- Preamble = nullptr;
- }
-
+ // Note that we delibirately don't check if preamble is up-to-date. This
+ // operation is very expensive and we feel the right trade-off here is to
----------------
delibirately -> deliberately
Thanks for adding the motivation here! I think this can be a bit terser, but up
to you.
e.g.
// We reuse the preamble whether it's valid or not. This is a
correctness/performance
// tradeoff: building without a preamble is slow, and completion is
latency-sensitive.
================
Comment at: clangd/Compiler.h:39
+/// be consumed by the FrontendAction as it will have a pointer to the MainFile
+/// buffer that will only be deleted if BeginSourceFile is called.
std::unique_ptr<CompilerInstance> prepareCompilerInstance(
----------------
bkramer wrote:
> This comment is somewhat messy now. Can you rephrase it a bit?
+1, this has grown a bit unwieldy. Maybe:
/// Creates a compiler which will build the provided file.
/// The preamble will not be checked for validity - caller should do that if
needed.
/// The returned compiler's VFS may differ due to command-line flags.
/// The returned value must be consumed by a FrontendAction to avoid leaking
MainFile.
/// May return null in error cases.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41991
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
