Looking... sorry that I missed the email from this bot. On Wed, May 16, 2018 at 9:43 PM Galina Kistanova <gkistan...@gmail.com> wrote:
> 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@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(collectInclusionsInMainFileCallback( >> - 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.suggestPathToFileForDiagnostics( >> - 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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
