[clang-tools-extra] r322854 - [clangd] Always use preamble (even stale) for code completion
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 18 07:17:00 PST 2018
Author: ibiryukov
Date: Thu Jan 18 07:17:00 2018
New Revision: 322854
URL: http://llvm.org/viewvc/llvm-project?rev=322854&view=rev
Log:
[clangd] Always use preamble (even stale) for code completion
Summary:
This improves performance of code completion, because we avoid stating
the files from the preamble and never attempt to parse the files
without using the preamble if it's provided.
However, the change comes at a cost of sometimes providing incorrect
results when doing code completion after making actually considerable
changes to the files used in the preamble or the preamble itself.
Eventually the preamble will get rebuilt and code completion will
be providing the correct results.
Reviewers: bkramer, sammccall, jkorous-apple
Reviewed By: sammccall
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D41991
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Compiler.cpp
clang-tools-extra/trunk/clangd/Compiler.h
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322854&r1=322853&r2=322854&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Jan 18 07:17:00 2018
@@ -517,14 +517,18 @@ bool invokeCodeComplete(const Context &C
std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
llvm::MemoryBuffer::getMemBufferCopy(Contents, FileName);
- // Attempt to reuse the PCH from precompiled preamble, if it was built.
+ // 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.
if (Preamble) {
auto Bounds =
ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
- if (!Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds, VFS.get()))
- Preamble = nullptr;
+ // FIXME(ibiryukov): Remove this call to CanReuse() after we'll fix
+ // clients relying on getting stats for preamble files during code
+ // completion.
+ // Note that results of CanReuse() are ignored, see the comment above.
+ Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds, VFS.get());
}
-
auto Clang = prepareCompilerInstance(
std::move(CI), Preamble, std::move(ContentsBuffer), std::move(PCHs),
std::move(VFS), DummyDiagsConsumer);
Modified: clang-tools-extra/trunk/clangd/Compiler.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.cpp?rev=322854&r1=322853&r2=322854&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Compiler.cpp (original)
+++ clang-tools-extra/trunk/clangd/Compiler.cpp Thu Jan 18 07:17:00 2018
@@ -14,13 +14,6 @@
namespace clang {
namespace clangd {
-/// Creates a CompilerInstance from \p CI, with main buffer overriden to \p
-/// Buffer and arguments to read the PCH from \p Preamble, if \p Preamble is not
-/// null. Note that vfs::FileSystem inside returned instance may differ from \p
-/// VFS if additional file remapping were set in command-line arguments.
-/// On some errors, returns null. When non-null value is returned, it's expected
-/// to be consumed by the FrontendAction as it will have a pointer to the \p
-/// Buffer that will only be deleted if BeginSourceFile is called.
std::unique_ptr<CompilerInstance>
prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
const PrecompiledPreamble *Preamble,
@@ -36,7 +29,7 @@ prepareCompilerInstance(std::unique_ptr<
// NOTE: we use Buffer.get() when adding remapped files, so we have to make
// sure it will be released if no error is emitted.
if (Preamble) {
- Preamble->AddImplicitPreamble(*CI, VFS, Buffer.get());
+ Preamble->OverridePreamble(*CI, VFS, Buffer.get());
} else {
CI->getPreprocessorOpts().addRemappedFile(
CI->getFrontendOpts().Inputs[0].getFile(), Buffer.get());
Modified: clang-tools-extra/trunk/clangd/Compiler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.h?rev=322854&r1=322853&r2=322854&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Compiler.h (original)
+++ clang-tools-extra/trunk/clangd/Compiler.h Thu Jan 18 07:17:00 2018
@@ -28,13 +28,16 @@ public:
const clang::Diagnostic &Info) override {}
};
-/// Creates a CompilerInstance with the main file contens overridden.
-/// The preamble will be reused unless it is null.
-/// Note that the vfs::FileSystem inside returned instance may differ if
-/// additional file remappings occur in command-line arguments.
-/// On some errors, returns null. When non-null value is returned, it's expected
-/// to be consumed by the FrontendAction as it will have a pointer to the
-/// MainFile buffer that will only be deleted if BeginSourceFile is called.
+/// Creates a compiler instance, configured so that:
+/// - Contents of the parsed file are remapped to \p MainFile.
+/// - Preamble is overriden to use PCH passed to this function. It means the
+/// changes to the preamble headers or files included in the preamble are
+/// not visible to this compiler instance.
+/// - vfs::FileSystem is used for all underlying file accesses. The actual
+/// vfs used by the compiler may be an overlay over the passed vfs.
+/// Returns null on errors. When non-null value is returned, it is expected to
+/// be consumed by FrontendAction::BeginSourceFile to properly destroy \p
+/// MainFile.
std::unique_ptr<CompilerInstance> prepareCompilerInstance(
std::unique_ptr<clang::CompilerInvocation>, const PrecompiledPreamble *,
std::unique_ptr<llvm::MemoryBuffer> MainFile,
More information about the cfe-commits
mailing list