[clang-tools-extra] r332363 - [clangd] Populate #include insertions as additional edits in completion items.
Galina Kistanova via cfe-commits
cfe-commits at lists.llvm.org
Wed May 16 12:43:28 PDT 2018
Hello Eric,
This commit broke one of our builders:
http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test/builds/26399
. . .
387.403 [725/64/4163] Building CXX object
tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/CodeComplete.cpp.o
FAILED:
tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/CodeComplete.cpp.o
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-Itools/clang/tools/extra/clangd
-I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/clangd
-I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/include
-Itools/clang/include -Iinclude
-I/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/include
-fPIC -fvisibility-inlines-hidden -std=c++11 -Wall -W -Wno-unused-parameter
-Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic
-Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor
-Wno-comment -ffunction-sections -fdata-sections -fno-common
-Woverloaded-virtual -fno-strict-aliasing -O3 -UNDEBUG -fno-exceptions
-fno-rtti -MD -MT
tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/CodeComplete.cpp.o
-MF
tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/CodeComplete.cpp.o.d
-o
tools/clang/tools/extra/clangd/CMakeFiles/clangDaemon.dir/CodeComplete.cpp.o
-c
/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp
/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:
In function ‘clang::clangd::SignatureHelp
clang::clangd::signatureHelp(clang::clangd::PathRef, const
clang::tooling::CompileCommand&, const clang::PrecompiledPreamble*,
llvm::StringRef, clang::clangd::Position,
llvm::IntrusiveRefCntPtr<clang::vfs::FileSystem>,
std::shared_ptr<clang::PCHContainerOperations>)’:
/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:1127:37:
error: invalid initialization of reference of type ‘const
clang::clangd::{anonymous}::SemaCompleteInput&’ from expression of type
‘<brace-enclosed initializer list>’
std::move(PCHs)});
^
/home/buildslave/buildslave1a/clang-x86_64-linux-abi-test/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:710:6:
error: in passing argument 3 of ‘bool
clang::clangd::{anonymous}::semaCodeComplete(std::unique_ptr<clang::CodeCompleteConsumer>,
const clang::CodeCompleteOptions&, const
clang::clangd::{anonymous}::SemaCompleteInput&,
std::unique_ptr<clang::clangd::IncludeInserter>*)’
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
^
. . .
Please have a look?
It is not good idea to keep the bot red for too long. This hides new
problem which later hard to track down.
Thanks
Galina
On Tue, May 15, 2018 at 8:29 AM, Eric Liu via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: ioeric
> Date: Tue May 15 08:29:32 2018
> New Revision: 332363
>
> URL: http://llvm.org/viewvc/llvm-project?rev=332363&view=rev
> Log:
> [clangd] Populate #include insertions as additional edits in completion
> items.
>
> Summary:
> o Remove IncludeInsertion LSP command.
> o Populate include insertion edits synchromously in completion items.
> o Share the code completion compiler instance and precompiled preamble to
> get existing inclusions in main file.
> o Include insertion logic lives only in CodeComplete now.
> o Use tooling::HeaderIncludes for inserting new includes.
> o Refactored tests.
>
> Reviewers: sammccall
>
> Reviewed By: sammccall
>
> Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D46497
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdServer.cpp
> clang-tools-extra/trunk/clangd/CodeComplete.cpp
> clang-tools-extra/trunk/clangd/CodeComplete.h
> clang-tools-extra/trunk/clangd/Headers.cpp
> clang-tools-extra/trunk/clangd/Headers.h
> clang-tools-extra/trunk/clangd/Protocol.h
> clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
> clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/ClangdServer.cpp?rev=332363&r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue May 15 08:29:32
> 2018
> @@ -161,6 +161,7 @@ void ClangdServer::codeComplete(PathRef
> // both the old and the new version in case only one of them matches.
> CompletionList Result = clangd::codeComplete(
> File, IP->Command, PreambleData ? &PreambleData->Preamble :
> nullptr,
> + PreambleData ? PreambleData->Inclusions :
> std::vector<Inclusion>(),
> IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
> CB(std::move(Result));
> };
>
> Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/CodeComplete.cpp?rev=332363&r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
> +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue May 15 08:29:32
> 2018
> @@ -18,9 +18,11 @@
> #include "CodeCompletionStrings.h"
> #include "Compiler.h"
> #include "FuzzyMatch.h"
> +#include "Headers.h"
> #include "Logger.h"
> #include "SourceCode.h"
> #include "Trace.h"
> +#include "URI.h"
> #include "index/Index.h"
> #include "clang/Format/Format.h"
> #include "clang/Frontend/CompilerInstance.h"
> @@ -219,6 +221,28 @@ std::string sortText(float Score, llvm::
> return S;
> }
>
> +/// Creates a `HeaderFile` from \p Header which can be either a URI or a
> literal
> +/// include.
> +static llvm::Expected<HeaderFile> toHeaderFile(StringRef Header,
> + llvm::StringRef HintPath) {
> + if (isLiteralInclude(Header))
> + return HeaderFile{Header.str(), /*Verbatim=*/true};
> + auto U = URI::parse(Header);
> + if (!U)
> + return U.takeError();
> +
> + auto IncludePath = URI::includeSpelling(*U);
> + if (!IncludePath)
> + return IncludePath.takeError();
> + if (!IncludePath->empty())
> + return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true};
> +
> + auto Resolved = URI::resolve(*U, HintPath);
> + if (!Resolved)
> + return Resolved.takeError();
> + return HeaderFile{std::move(*Resolved), /*Verbatim=*/false};
> +}
> +
> /// A code completion result, in clang-native form.
> /// It may be promoted to a CompletionItem if it's among the top-ranked
> results.
> struct CompletionCandidate {
> @@ -255,10 +279,10 @@ struct CompletionCandidate {
> }
>
> // Builds an LSP completion item.
> - CompletionItem build(llvm::StringRef FileName,
> - const CompletionItemScores &Scores,
> + CompletionItem build(StringRef FileName, const CompletionItemScores
> &Scores,
> const CodeCompleteOptions &Opts,
> - CodeCompletionString *SemaCCS) const {
> + CodeCompletionString *SemaCCS,
> + const IncludeInserter *Includes) const {
> assert(bool(SemaResult) == bool(SemaCCS));
> CompletionItem I;
> if (SemaResult) {
> @@ -289,6 +313,30 @@ struct CompletionCandidate {
> I.documentation = D->Documentation;
> if (I.detail.empty())
> I.detail = D->CompletionDetail;
> + if (Includes && !D->IncludeHeader.empty()) {
> + auto Edit = [&]() -> Expected<Optional<TextEdit>> {
> + auto ResolvedDeclaring = toHeaderFile(
> + IndexResult->CanonicalDeclaration.FileURI, FileName);
> + if (!ResolvedDeclaring)
> + return ResolvedDeclaring.takeError();
> + auto ResolvedInserted = toHeaderFile(D->IncludeHeader,
> FileName);
> + if (!ResolvedInserted)
> + return ResolvedInserted.takeError();
> + return Includes->insert(*ResolvedDeclaring,
> *ResolvedInserted);
> + }();
> + if (!Edit) {
> + std::string ErrMsg =
> + ("Failed to generate include insertion edits for adding
> header "
> + "(FileURI=\"" +
> + IndexResult->CanonicalDeclaration.FileURI +
> + "\", IncludeHeader=\"" + D->IncludeHeader + "\") into " +
> + FileName)
> + .str();
> + log(ErrMsg + ":" + llvm::toString(Edit.takeError()));
> + } else if (*Edit) {
> + I.additionalTextEdits = {std::move(**Edit)};
> + }
> + }
> }
> }
> I.scoreInfo = Scores;
> @@ -649,6 +697,7 @@ struct SemaCompleteInput {
> PathRef FileName;
> const tooling::CompileCommand &Command;
> PrecompiledPreamble const *Preamble;
> + const std::vector<Inclusion> &PreambleInclusions;
> StringRef Contents;
> Position Pos;
> IntrusiveRefCntPtr<vfs::FileSystem> VFS;
> @@ -656,9 +705,12 @@ struct SemaCompleteInput {
> };
>
> // Invokes Sema code completion on a file.
> +// If \p Includes is set, it will be initialized after a compiler
> instance has
> +// been set up.
> bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
> const clang::CodeCompleteOptions &Options,
> - const SemaCompleteInput &Input) {
> + const SemaCompleteInput &Input,
> + std::unique_ptr<IncludeInserter> *Includes =
> nullptr) {
> trace::Span Tracer("Sema completion");
> std::vector<const char *> ArgStrs;
> for (const auto &S : Input.Command.CommandLine)
> @@ -729,6 +781,28 @@ bool semaCodeComplete(std::unique_ptr<Co
> Input.FileName);
> return false;
> }
> + if (Includes) {
> + // Initialize Includes if provided.
> +
> + // FIXME(ioeric): needs more consistent style support in clangd
> server.
> + auto Style = format::getStyle("file", Input.FileName, "LLVM",
> + Input.Contents, Input.VFS.get());
> + if (!Style) {
> + log("Failed to get FormatStyle for file" + Input.FileName +
> + ". Fall back to use LLVM style. Error: " +
> + llvm::toString(Style.takeError()));
> + Style = format::getLLVMStyle();
> + }
> + *Includes = llvm::make_unique<IncludeInserter>(
> + Input.FileName, Input.Contents, *Style, Input.Command.Directory,
> + Clang->getPreprocessor().getHeaderSearchInfo());
> + for (const auto &Inc : Input.PreambleInclusions)
> + Includes->get()->addExisting(Inc);
> + Clang->getPreprocessor().addPPCallbacks(
> collectInclusionsInMainFileCallback(
> + Clang->getSourceManager(), [Includes](Inclusion Inc) {
> + Includes->get()->addExisting(std::move(Inc));
> + }));
> + }
> if (!Action.Execute()) {
> log("Execute() failed when running codeComplete for " +
> Input.FileName);
> return false;
> @@ -863,7 +937,8 @@ class CodeCompleteFlow {
> CompletionRecorder *Recorder = nullptr;
> int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
> bool Incomplete = false; // Would more be available with a higher limit?
> - llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
> + llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema
> runs.
> + std::unique_ptr<IncludeInserter> Includes; // Initialized once
> compiler runs.
>
> public:
> // A CodeCompleteFlow object is only useful for calling run() exactly
> once.
> @@ -872,20 +947,25 @@ public:
>
> CompletionList run(const SemaCompleteInput &SemaCCInput) && {
> trace::Span Tracer("CodeCompleteFlow");
> +
> // We run Sema code completion first. It builds an AST and calculates:
> // - completion results based on the AST.
> // - partial identifier and context. We need these for the index
> query.
> CompletionList Output;
> auto RecorderOwner = llvm::make_unique<CompletionRecorder>(Opts,
> [&]() {
> assert(Recorder && "Recorder is not set");
> + assert(Includes && "Includes is not set");
> + // If preprocessor was run, inclusions from preprocessor callback
> should
> + // already be added to Inclusions.
> Output = runWithSema();
> + Includes.reset(); // Make sure this doesn't out-live Clang.
> SPAN_ATTACH(Tracer, "sema_completion_kind",
> getCompletionKindString(Recorder->CCContext.getKind())
> );
> });
>
> Recorder = RecorderOwner.get();
> semaCodeComplete(std::move(RecorderOwner),
> Opts.getClangCompleteOpts(),
> - SemaCCInput);
> + SemaCCInput, &Includes);
>
> SPAN_ATTACH(Tracer, "sema_results", NSema);
> SPAN_ATTACH(Tracer, "index_results", NIndex);
> @@ -1011,19 +1091,21 @@ private:
> CodeCompletionString *SemaCCS = nullptr;
> if (auto *SR = Candidate.SemaResult)
> SemaCCS = Recorder->codeCompletionString(*SR,
> Opts.IncludeBriefComments);
> - return Candidate.build(FileName, Scores, Opts, SemaCCS);
> + return Candidate.build(FileName, Scores, Opts, SemaCCS,
> Includes.get());
> }
> };
>
> CompletionList codeComplete(PathRef FileName,
> const tooling::CompileCommand &Command,
> PrecompiledPreamble const *Preamble,
> + const std::vector<Inclusion>
> &PreambleInclusions,
> StringRef Contents, Position Pos,
> IntrusiveRefCntPtr<vfs::FileSystem> VFS,
> std::shared_ptr<PCHContainerOperations> PCHs,
> CodeCompleteOptions Opts) {
> return CodeCompleteFlow(FileName, Opts)
> - .run({FileName, Command, Preamble, Contents, Pos, VFS, PCHs});
> + .run({FileName, Command, Preamble, PreambleInclusions, Contents,
> Pos, VFS,
> + PCHs});
> }
>
> SignatureHelp signatureHelp(PathRef FileName,
> @@ -1040,7 +1122,8 @@ SignatureHelp signatureHelp(PathRef File
> Options.IncludeBriefComments = true;
> semaCodeComplete(llvm::make_unique<SignatureHelpCollector>(Options,
> Result),
> Options,
> - {FileName, Command, Preamble, Contents, Pos,
> std::move(VFS),
> + {FileName, Command, Preamble,
> + /*PreambleInclusions=*/{}, Contents, Pos,
> std::move(VFS),
> std::move(PCHs)});
> return Result;
> }
>
> Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/CodeComplete.h?rev=332363&r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
> +++ clang-tools-extra/trunk/clangd/CodeComplete.h Tue May 15 08:29:32 2018
> @@ -15,6 +15,7 @@
> #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
> #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
>
> +#include "Headers.h"
> #include "Logger.h"
> #include "Path.h"
> #include "Protocol.h"
> @@ -69,6 +70,7 @@ struct CodeCompleteOptions {
> CompletionList codeComplete(PathRef FileName,
> const tooling::CompileCommand &Command,
> PrecompiledPreamble const *Preamble,
> + const std::vector<Inclusion>
> &PreambleInclusions,
> StringRef Contents, Position Pos,
> IntrusiveRefCntPtr<vfs::FileSystem> VFS,
> std::shared_ptr<PCHContainerOperations> PCHs,
>
> Modified: clang-tools-extra/trunk/clangd/Headers.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/Headers.cpp?rev=332363&r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/Headers.cpp (original)
> +++ clang-tools-extra/trunk/clangd/Headers.cpp Tue May 15 08:29:32 2018
> @@ -15,8 +15,6 @@
> #include "clang/Frontend/CompilerInvocation.h"
> #include "clang/Frontend/FrontendActions.h"
> #include "clang/Lex/HeaderSearch.h"
> -#include "clang/Lex/PreprocessorOptions.h"
> -#include "clang/Tooling/CompilationDatabase.h"
> #include "llvm/Support/Path.h"
>
> namespace clang {
> @@ -74,64 +72,13 @@ collectInclusionsInMainFileCallback(cons
>
> /// FIXME(ioeric): we might not want to insert an absolute include path
> if the
> /// path is not shortened.
> -llvm::Expected<std::string>
> -calculateIncludePath(llvm::StringRef File, llvm::StringRef Code,
> - const HeaderFile &DeclaringHeader,
> - const HeaderFile &InsertedHeader,
> - const tooling::CompileCommand &CompileCommand,
> - IntrusiveRefCntPtr<vfs::FileSystem> FS) {
> - assert(llvm::sys::path::is_absolute(File));
> +llvm::Expected<std::string> calculateIncludePath(
> + PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo,
> + const std::vector<Inclusion> &Inclusions, const HeaderFile
> &DeclaringHeader,
> + const HeaderFile &InsertedHeader) {
> assert(DeclaringHeader.valid() && InsertedHeader.valid());
> if (File == DeclaringHeader.File || File == InsertedHeader.File)
> return "";
> - FS->setCurrentWorkingDirectory(CompileCommand.Directory);
> -
> - // Set up a CompilerInstance and create a preprocessor to collect
> existing
> - // #include headers in \p Code. Preprocesor also provides HeaderSearch
> with
> - // which we can calculate the shortest include path for \p Header.
> - std::vector<const char *> Argv;
> - for (const auto &S : CompileCommand.CommandLine)
> - Argv.push_back(S.c_str());
> - IgnoringDiagConsumer IgnoreDiags;
> - auto CI = clang::createInvocationFromCommandLine(
> - Argv,
> - CompilerInstance::createDiagnostics(new DiagnosticOptions(),
> &IgnoreDiags,
> - false),
> - FS);
> - if (!CI)
> - return llvm::make_error<llvm::StringError>(
> - "Failed to create a compiler instance for " + File,
> - llvm::inconvertibleErrorCode());
> - CI->getFrontendOpts().DisableFree = false;
> - // Parse the main file to get all existing #includes in the file, and
> then we
> - // can make sure the same header (even with different include path) is
> not
> - // added more than once.
> - CI->getPreprocessorOpts().SingleFileParseMode = true;
> -
> - // The diagnostic options must be set before creating a
> CompilerInstance.
> - CI->getDiagnosticOpts().IgnoreWarnings = true;
> - auto Clang = prepareCompilerInstance(
> - std::move(CI), /*Preamble=*/nullptr,
> - llvm::MemoryBuffer::getMemBuffer(Code, File),
> - std::make_shared<PCHContainerOperations>(), FS, IgnoreDiags);
> -
> - if (Clang->getFrontendOpts().Inputs.empty())
> - return llvm::make_error<llvm::StringError>(
> - "Empty frontend action inputs empty for file " + File,
> - llvm::inconvertibleErrorCode());
> - PreprocessOnlyAction Action;
> - if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().
> Inputs[0]))
> - return llvm::make_error<llvm::StringError>(
> - "Failed to begin preprocessor only action for file " + File,
> - llvm::inconvertibleErrorCode());
> - std::vector<Inclusion> Inclusions;
> - Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCal
> lback(
> - Clang->getSourceManager(),
> - [&Inclusions](Inclusion Inc) { Inclusions.push_back(std::move(Inc));
> }));
> - if (!Action.Execute())
> - return llvm::make_error<llvm::StringError>(
> - "Failed to execute preprocessor only action for file " + File,
> - llvm::inconvertibleErrorCode());
> llvm::StringSet<> IncludedHeaders;
> for (const auto &Inc : Inclusions) {
> IncludedHeaders.insert(Inc.Written);
> @@ -144,14 +91,13 @@ calculateIncludePath(llvm::StringRef Fil
> if (Included(DeclaringHeader.File) || Included(InsertedHeader.File))
> return "";
>
> - auto &HeaderSearchInfo = Clang->getPreprocessor().
> getHeaderSearchInfo();
> bool IsSystem = false;
>
> if (InsertedHeader.Verbatim)
> return InsertedHeader.File;
>
> std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostic
> s(
> - InsertedHeader.File, CompileCommand.Directory, &IsSystem);
> + InsertedHeader.File, BuildDir, &IsSystem);
> if (IsSystem)
> Suggested = "<" + Suggested + ">";
> else
> @@ -161,5 +107,35 @@ calculateIncludePath(llvm::StringRef Fil
> return Suggested;
> }
>
> +Expected<Optional<TextEdit>>
> +IncludeInserter::insert(const HeaderFile &DeclaringHeader,
> + const HeaderFile &InsertedHeader) const {
> + auto Validate = [](const HeaderFile &Header) {
> + return Header.valid()
> + ? llvm::Error::success()
> + : llvm::make_error<llvm::StringError>(
> + "Invalid HeaderFile: " + Header.File +
> + " (verbatim=" + std::to_string(Header.Verbatim)
> + ").",
> + llvm::inconvertibleErrorCode());
> + };
> + if (auto Err = Validate(DeclaringHeader))
> + return std::move(Err);
> + if (auto Err = Validate(InsertedHeader))
> + return std::move(Err);
> + auto Include =
> + calculateIncludePath(FileName, BuildDir, HeaderSearchInfo,
> Inclusions,
> + DeclaringHeader, InsertedHeader);
> + if (!Include)
> + return Include.takeError();
> + if (Include->empty())
> + return llvm::None;
> + StringRef IncludeRef = *Include;
> + auto Insertion =
> + Inserter.insert(IncludeRef.trim("\"<>"),
> IncludeRef.startswith("<"));
> + if (!Insertion)
> + return llvm::None;
> + return replacementToEdit(Code, *Insertion);
> +}
> +
> } // namespace clangd
> } // namespace clang
>
> Modified: clang-tools-extra/trunk/clangd/Headers.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/Headers.h?rev=332363&r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/Headers.h (original)
> +++ clang-tools-extra/trunk/clangd/Headers.h Tue May 15 08:29:32 2018
> @@ -12,10 +12,14 @@
>
> #include "Path.h"
> #include "Protocol.h"
> +#include "SourceCode.h"
> #include "clang/Basic/VirtualFileSystem.h"
> +#include "clang/Format/Format.h"
> +#include "clang/Lex/HeaderSearch.h"
> #include "clang/Lex/PPCallbacks.h"
> -#include "clang/Tooling/CompilationDatabase.h"
> +#include "clang/Tooling/Core/HeaderIncludes.h"
> #include "llvm/ADT/StringRef.h"
> +#include "llvm/ADT/StringSet.h"
> #include "llvm/Support/Error.h"
>
> namespace clang {
> @@ -51,20 +55,51 @@ collectInclusionsInMainFileCallback(cons
> /// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'.
> ///
> /// \param File is an absolute file path.
> +/// \param Inclusions Existing inclusions in the main file.
> /// \param DeclaringHeader is the original header corresponding to \p
> /// InsertedHeader e.g. the header that declares a symbol.
> /// \param InsertedHeader The preferred header to be inserted. This could
> be the
> /// same as DeclaringHeader but must be provided.
> // \return A quoted "path" or <path>. This returns an empty string if:
> /// - Either \p DeclaringHeader or \p InsertedHeader is already
> (directly)
> -/// included in the file (including those included via different paths).
> +/// in \p Inclusions (including those included via different paths).
> /// - \p DeclaringHeader or \p InsertedHeader is the same as \p File.
> -llvm::Expected<std::string>
> -calculateIncludePath(PathRef File, llvm::StringRef Code,
> - const HeaderFile &DeclaringHeader,
> - const HeaderFile &InsertedHeader,
> - const tooling::CompileCommand &CompileCommand,
> - IntrusiveRefCntPtr<vfs::FileSystem> FS);
> +llvm::Expected<std::string> calculateIncludePath(
> + PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo,
> + const std::vector<Inclusion> &Inclusions, const HeaderFile
> &DeclaringHeader,
> + const HeaderFile &InsertedHeader);
> +
> +// Calculates insertion edit for including a new header in a file.
> +class IncludeInserter {
> +public:
> + IncludeInserter(StringRef FileName, StringRef Code,
> + const format::FormatStyle &Style, StringRef BuildDir,
> + HeaderSearch &HeaderSearchInfo)
> + : FileName(FileName), Code(Code), BuildDir(BuildDir),
> + HeaderSearchInfo(HeaderSearchInfo),
> + Inserter(FileName, Code, Style.IncludeStyle) {}
> +
> + void addExisting(Inclusion Inc) { Inclusions.push_back(std::move(Inc));
> }
> +
> + /// Returns a TextEdit that inserts a new header; if the header is not
> + /// inserted e.g. it's an existing header, this returns None. If any
> header is
> + /// invalid, this returns error.
> + ///
> + /// \param DeclaringHeader is the original header corresponding to \p
> + /// InsertedHeader e.g. the header that declares a symbol.
> + /// \param InsertedHeader The preferred header to be inserted. This
> could be
> + /// the same as DeclaringHeader but must be provided.
> + Expected<Optional<TextEdit>> insert(const HeaderFile &DeclaringHeader,
> + const HeaderFile &InsertedHeader)
> const;
> +
> +private:
> + StringRef FileName;
> + StringRef Code;
> + StringRef BuildDir;
> + HeaderSearch &HeaderSearchInfo;
> + std::vector<Inclusion> Inclusions;
> + tooling::HeaderIncludes Inserter; // Computers insertion replacement.
> +};
>
> } // namespace clangd
> } // namespace clang
>
> Modified: clang-tools-extra/trunk/clangd/Protocol.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/Protocol.h?rev=332363&r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/Protocol.h (original)
> +++ clang-tools-extra/trunk/clangd/Protocol.h Tue May 15 08:29:32 2018
> @@ -99,6 +99,9 @@ struct Position {
> return std::tie(LHS.line, LHS.character) ==
> std::tie(RHS.line, RHS.character);
> }
> + friend bool operator!=(const Position &LHS, const Position &RHS) {
> + return !(LHS == RHS);
> + }
> friend bool operator<(const Position &LHS, const Position &RHS) {
> return std::tie(LHS.line, LHS.character) <
> std::tie(RHS.line, RHS.character);
>
> Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/unittests/clangd/CodeCompleteTests.cpp?rev=
> 332363&r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Tue
> May 15 08:29:32 2018
> @@ -45,6 +45,16 @@ MATCHER_P(Kind, K, "") { return arg.kind
> MATCHER_P(Filter, F, "") { return arg.filterText == F; }
> MATCHER_P(Doc, D, "") { return arg.documentation == D; }
> MATCHER_P(Detail, D, "") { return arg.detail == D; }
> +MATCHER_P(InsertInclude, IncludeHeader, "") {
> + if (arg.additionalTextEdits.size() != 1)
> + return false;
> + const auto &Edit = arg.additionalTextEdits[0];
> + if (Edit.range.start != Edit.range.end)
> + return false;
> + SmallVector<StringRef, 2> Matches;
> + llvm::Regex RE(R"(#include[ ]*(["<][^">]*[">]))");
> + return RE.match(Edit.newText, &Matches) && Matches[1] == IncludeHeader;
> +}
> MATCHER_P(PlainText, Text, "") {
> return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
> arg.insertText == Text;
> @@ -58,6 +68,8 @@ MATCHER(NameContainsFilter, "") {
> return true;
> return llvm::StringRef(arg.insertText).contains(arg.filterText);
> }
> +MATCHER(HasAdditionalEdits, "") { return !arg.additionalTextEdits.empty();
> }
> +
> // Shorthand for Contains(Named(Name)).
> Matcher<const std::vector<CompletionItem> &> Has(std::string Name) {
> return Contains(Named(std::move(Name)));
> @@ -75,9 +87,7 @@ std::unique_ptr<SymbolIndex> memIndex(st
> return MemIndex::build(std::move(Slab).build());
> }
>
> -// Builds a server and runs code completion.
> -// If IndexSymbols is non-empty, an index will be built and passed to
> opts.
> -CompletionList completions(StringRef Text,
> +CompletionList completions(ClangdServer &Server, StringRef Text,
> std::vector<Symbol> IndexSymbols = {},
> clangd::CodeCompleteOptions Opts = {}) {
> std::unique_ptr<SymbolIndex> OverrideIndex;
> @@ -87,10 +97,6 @@ CompletionList completions(StringRef Tex
> Opts.Index = OverrideIndex.get();
> }
>
> - MockFSProvider FS;
> - MockCompilationDatabase CDB;
> - IgnoreDiagnostics DiagConsumer;
> - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
> auto File = testPath("foo.cpp");
> Annotations Test(Text);
> runAddDocument(Server, File, Test.code());
> @@ -101,6 +107,18 @@ CompletionList completions(StringRef Tex
> return CompletionList;
> }
>
> +// Builds a server and runs code completion.
> +// If IndexSymbols is non-empty, an index will be built and passed to
> opts.
> +CompletionList completions(StringRef Text,
> + std::vector<Symbol> IndexSymbols = {},
> + clangd::CodeCompleteOptions Opts = {}) {
> + MockFSProvider FS;
> + MockCompilationDatabase CDB;
> + IgnoreDiagnostics DiagConsumer;
> + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
> + return completions(Server, Text, std::move(IndexSymbols),
> std::move(Opts));
> +}
> +
> std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl)
> {
> std::string Result;
> raw_string_ostream OS(Result);
> @@ -505,6 +523,42 @@ TEST(CompletionTest, SemaIndexMergeWithL
> EXPECT_TRUE(Results.isIncomplete);
> }
>
> +TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) {
> + MockFSProvider FS;
> + MockCompilationDatabase CDB;
> + std::string Subdir = testPath("sub");
> + std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str();
> + CDB.ExtraClangFlags = {SearchDirArg.c_str()};
> + std::string BarHeader = testPath("sub/bar.h");
> + FS.Files[BarHeader] = "";
> +
> + IgnoreDiagnostics DiagConsumer;
> + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
> + Symbol::Details Scratch;
> + auto BarURI = URI::createFile(BarHeader).toString();
> + Symbol Sym = cls("ns::X");
> + Sym.CanonicalDeclaration.FileURI = BarURI;
> + Scratch.IncludeHeader = BarURI;
> + Sym.Detail = &Scratch;
> + // Shoten include path based on search dirctory and insert.
> + auto Results = completions(Server,
> + R"cpp(
> + int main() { ns::^ }
> + )cpp",
> + {Sym});
> + EXPECT_THAT(Results.items,
> + ElementsAre(AllOf(Named("X"), InsertInclude("\"bar.h\""))));
> + // Duplicate based on inclusions in preamble.
> + Results = completions(Server,
> + R"cpp(
> + #include "sub/bar.h" // not shortest, so should only match
> resolved.
> + int main() { ns::^ }
> + )cpp",
> + {Sym});
> + EXPECT_THAT(Results.items,
> + ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits()))));
> +}
> +
> TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
> MockFSProvider FS;
> MockCompilationDatabase CDB;
>
> Modified: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/unittests/clangd/HeadersTests.cpp?rev=332363&
> r1=332362&r2=332363&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp (original)
> +++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp Tue May 15
> 08:29:32 2018
> @@ -8,7 +8,13 @@
> //===-------------------------------------------------------
> ---------------===//
>
> #include "Headers.h"
> +
> +#include "Compiler.h"
> #include "TestFS.h"
> +#include "clang/Frontend/CompilerInstance.h"
> +#include "clang/Frontend/CompilerInvocation.h"
> +#include "clang/Frontend/FrontendActions.h"
> +#include "clang/Lex/PreprocessorOptions.h"
> #include "gmock/gmock.h"
> #include "gtest/gtest.h"
>
> @@ -16,30 +22,83 @@ namespace clang {
> namespace clangd {
> namespace {
>
> +using ::testing::AllOf;
> +using ::testing::UnorderedElementsAre;
> +
> class HeadersTest : public ::testing::Test {
> public:
> HeadersTest() {
> CDB.ExtraClangFlags = {SearchDirArg.c_str()};
> FS.Files[MainFile] = "";
> + // Make sure directory sub/ exists.
> + FS.Files[testPath("sub/EMPTY")] = "";
> + }
> +
> +private:
> + std::unique_ptr<CompilerInstance> setupClang() {
> + auto Cmd = CDB.getCompileCommand(MainFile);
> + assert(static_cast<bool>(Cmd));
> + auto VFS = FS.getFileSystem();
> + VFS->setCurrentWorkingDirectory(Cmd->Directory);
> +
> + std::vector<const char *> Argv;
> + for (const auto &S : Cmd->CommandLine)
> + Argv.push_back(S.c_str());
> + auto CI = clang::createInvocationFromCommandLine(
> + Argv,
> + CompilerInstance::createDiagnostics(new DiagnosticOptions(),
> + &IgnoreDiags, false),
> + VFS);
> + EXPECT_TRUE(static_cast<bool>(CI));
> + CI->getFrontendOpts().DisableFree = false;
> +
> + // The diagnostic options must be set before creating a
> CompilerInstance.
> + CI->getDiagnosticOpts().IgnoreWarnings = true;
> + auto Clang = prepareCompilerInstance(
> + std::move(CI), /*Preamble=*/nullptr,
> + llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile),
> + std::make_shared<PCHContainerOperations>(), VFS, IgnoreDiags);
> +
> + EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty());
> + return Clang;
> }
>
> protected:
> + std::vector<Inclusion> collectIncludes() {
> + auto Clang = setupClang();
> + PreprocessOnlyAction Action;
> + EXPECT_TRUE(
> + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().
> Inputs[0]));
> + std::vector<Inclusion> Inclusions;
> + Clang->getPreprocessor().addPPCallbacks(
> collectInclusionsInMainFileCallback(
> + Clang->getSourceManager(),
> + [&](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); }));
> + EXPECT_TRUE(Action.Execute());
> + Action.EndSourceFile();
> + return Inclusions;
> + }
> +
> // Calculates the include path, or returns "" on error.
> std::string calculate(PathRef Original, PathRef Preferred = "",
> + const std::vector<Inclusion> &Inclusions = {},
> bool ExpectError = false) {
> + auto Clang = setupClang();
> + PreprocessOnlyAction Action;
> + EXPECT_TRUE(
> + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().
> Inputs[0]));
> +
> if (Preferred.empty())
> Preferred = Original;
> - auto VFS = FS.getFileSystem();
> - auto Cmd = CDB.getCompileCommand(MainFile);
> - assert(static_cast<bool>(Cmd));
> - VFS->setCurrentWorkingDirectory(Cmd->Directory);
> auto ToHeaderFile = [](llvm::StringRef Header) {
> return HeaderFile{Header,
> /*Verbatim=*/!llvm::sys::path:
> :is_absolute(Header)};
> };
> - auto Path = calculateIncludePath(MainFile, FS.Files[MainFile],
> - ToHeaderFile(Original),
> - ToHeaderFile(Preferred), *Cmd, VFS);
> +
> + auto Path = calculateIncludePath(
> + MainFile, CDB.getCompileCommand(MainFile)->Directory,
> + Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions,
> + ToHeaderFile(Original), ToHeaderFile(Preferred));
> + Action.EndSourceFile();
> if (!Path) {
> llvm::consumeError(Path.takeError());
> EXPECT_TRUE(ExpectError);
> @@ -49,52 +108,50 @@ protected:
> }
> return std::move(*Path);
> }
> +
> + Expected<Optional<TextEdit>>
> + insert(const HeaderFile &DeclaringHeader, const HeaderFile
> &InsertedHeader,
> + const std::vector<Inclusion> &ExistingInclusions = {}) {
> + auto Clang = setupClang();
> + PreprocessOnlyAction Action;
> + EXPECT_TRUE(
> + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().
> Inputs[0]));
> +
> + IncludeInserter Inserter(MainFile, /*Code=*/"",
> format::getLLVMStyle(),
> + CDB.getCompileCommand(MainFile)->Directory,
> + Clang->getPreprocessor().
> getHeaderSearchInfo());
> + for (const auto &Inc : ExistingInclusions)
> + Inserter.addExisting(Inc);
> +
> + auto Edit = Inserter.insert(DeclaringHeader, InsertedHeader);
> + Action.EndSourceFile();
> + return Edit;
> + }
> +
> MockFSProvider FS;
> MockCompilationDatabase CDB;
> std::string MainFile = testPath("main.cpp");
> std::string Subdir = testPath("sub");
> std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str();
> + IgnoringDiagConsumer IgnoreDiags;
> };
>
> -TEST_F(HeadersTest, InsertInclude) {
> - std::string Path = testPath("sub/bar.h");
> - FS.Files[Path] = "";
> - EXPECT_EQ(calculate(Path), "\"bar.h\"");
> -}
> +MATCHER_P(Written, Name, "") { return arg.Written == Name; }
> +MATCHER_P(Resolved, Name, "") { return arg.Resolved == Name; }
>
> -TEST_F(HeadersTest, DontInsertDuplicateSameName) {
> +TEST_F(HeadersTest, CollectRewrittenAndResolved) {
> FS.Files[MainFile] = R"cpp(
> -#include "bar.h"
> +#include "sub/bar.h" // not shortest
> )cpp";
> std::string BarHeader = testPath("sub/bar.h");
> FS.Files[BarHeader] = "";
> - EXPECT_EQ(calculate(BarHeader), "");
> -}
> -
> -TEST_F(HeadersTest, DontInsertDuplicateDifferentName) {
> - std::string BarHeader = testPath("sub/bar.h");
> - FS.Files[BarHeader] = "";
> - FS.Files[MainFile] = R"cpp(
> -#include "sub/bar.h" // not shortest
> -)cpp";
> - EXPECT_EQ(calculate("\"sub/bar.h\""), ""); // Duplicate rewritten.
> - EXPECT_EQ(calculate(BarHeader), ""); // Duplicate resolved.
> - EXPECT_EQ(calculate(BarHeader, "\"BAR.h\""), ""); // Do not insert
> preferred.
> -}
>
> -TEST_F(HeadersTest, DontInsertDuplicatePreferred) {
> - std::string BarHeader = testPath("sub/bar.h");
> - FS.Files[BarHeader] = "";
> - FS.Files[MainFile] = R"cpp(
> -#include "sub/bar.h" // not shortest
> -)cpp";
> - // Duplicate written.
> - EXPECT_EQ(calculate("\"original.h\"", "\"sub/bar.h\""), "");
> - // Duplicate resolved.
> - EXPECT_EQ(calculate("\"original.h\"", BarHeader), "");
> + EXPECT_THAT(collectIncludes(),
> + UnorderedElementsAre(
> + AllOf(Written("\"sub/bar.h\""), Resolved(BarHeader))));
> }
>
> -TEST_F(HeadersTest, StillInsertIfTrasitivelyIncluded) {
> +TEST_F(HeadersTest, OnlyCollectInclusionsInMain) {
> std::string BazHeader = testPath("sub/baz.h");
> FS.Files[BazHeader] = "";
> std::string BarHeader = testPath("sub/bar.h");
> @@ -104,27 +161,92 @@ TEST_F(HeadersTest, StillInsertIfTrasiti
> FS.Files[MainFile] = R"cpp(
> #include "bar.h"
> )cpp";
> - EXPECT_EQ(calculate(BazHeader), "\"baz.h\"");
> + EXPECT_THAT(
> + collectIncludes(),
> + UnorderedElementsAre(AllOf(Written("\"bar.h\""),
> Resolved(BarHeader))));
> +}
> +
> +TEST_F(HeadersTest, UnResolvedInclusion) {
> + FS.Files[MainFile] = R"cpp(
> +#include "foo.h"
> +)cpp";
> +
> + EXPECT_THAT(collectIncludes(),
> + UnorderedElementsAre(AllOf(Written("\"foo.h\""),
> Resolved(""))));
> +}
> +
> +TEST_F(HeadersTest, InsertInclude) {
> + std::string Path = testPath("sub/bar.h");
> + FS.Files[Path] = "";
> + EXPECT_EQ(calculate(Path), "\"bar.h\"");
> }
>
> TEST_F(HeadersTest, DoNotInsertIfInSameFile) {
> MainFile = testPath("main.h");
> - FS.Files[MainFile] = "";
> EXPECT_EQ(calculate(MainFile), "");
> }
>
> +TEST_F(HeadersTest, ShortenedInclude) {
> + std::string BarHeader = testPath("sub/bar.h");
> + EXPECT_EQ(calculate(BarHeader), "\"bar.h\"");
> +}
> +
> +TEST_F(HeadersTest, NotShortenedInclude) {
> + std::string BarHeader = testPath("sub-2/bar.h");
> + EXPECT_EQ(calculate(BarHeader, ""), "\"" + BarHeader + "\"");
> +}
> +
> TEST_F(HeadersTest, PreferredHeader) {
> - FS.Files[MainFile] = "";
> std::string BarHeader = testPath("sub/bar.h");
> - FS.Files[BarHeader] = "";
> EXPECT_EQ(calculate(BarHeader, "<bar>"), "<bar>");
>
> std::string BazHeader = testPath("sub/baz.h");
> - FS.Files[BazHeader] = "";
> EXPECT_EQ(calculate(BarHeader, BazHeader), "\"baz.h\"");
> }
>
> +TEST_F(HeadersTest, DontInsertDuplicatePreferred) {
> + std::vector<Inclusion> Inclusions = {
> + {Range(), /*Written*/ "\"bar.h\"", /*Resolved*/ ""}};
> + EXPECT_EQ(calculate(testPath("sub/bar.h"), "\"bar.h\"", Inclusions),
> "");
> + EXPECT_EQ(calculate("\"x.h\"", "\"bar.h\"", Inclusions), "");
> +}
> +
> +TEST_F(HeadersTest, DontInsertDuplicateResolved) {
> + std::string BarHeader = testPath("sub/bar.h");
> + std::vector<Inclusion> Inclusions = {
> + {Range(), /*Written*/ "fake-bar.h", /*Resolved*/ BarHeader}};
> + EXPECT_EQ(calculate(BarHeader, "", Inclusions), "");
> + // Do not insert preferred.
> + EXPECT_EQ(calculate(BarHeader, "\"BAR.h\"", Inclusions), "");
> +}
> +
> +HeaderFile literal(StringRef Include) {
> + HeaderFile H{Include, /*Verbatim=*/true};
> + assert(H.valid());
> + return H;
> +}
> +
> +TEST_F(HeadersTest, PreferInserted) {
> + auto Edit = insert(literal("<x>"), literal("<y>"));
> + EXPECT_TRUE(static_cast<bool>(Edit));
> + EXPECT_TRUE(llvm::StringRef((*Edit)->newText).contains("<y>"));
> +}
> +
> +TEST_F(HeadersTest, ExistingInclusion) {
> + Inclusion Existing{Range(), /*Written=*/"<c.h>", /*Resolved=*/""};
> + auto Edit = insert(literal("<c.h>"), literal("<c.h>"), {Existing});
> + EXPECT_TRUE(static_cast<bool>(Edit));
> + EXPECT_FALSE(static_cast<bool>(*Edit));
> +}
> +
> +TEST_F(HeadersTest, ValidateHeaders) {
> + HeaderFile InvalidHeader{"a.h", /*Verbatim=*/true};
> + assert(!InvalidHeader.valid());
> + auto Edit = insert(InvalidHeader, literal("\"c.h\""));
> + EXPECT_FALSE(static_cast<bool>(Edit));
> + llvm::consumeError(Edit.takeError());
> +}
> +
> } // namespace
> } // namespace clangd
> } // namespace clang
> -
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180516/a5ef9145/attachment-0001.html>
More information about the cfe-commits
mailing list