ilya-biryukov added inline 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
----------------
sammccall wrote:
> 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.
Looks much better, thanks.
================
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(
----------------
sammccall wrote:
> 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.
>
I rewrote the comment before reading your suggestion, it should be better now.
If it's still unclear, let me know.
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
