Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -45,6 +45,7 @@ #include "clang/Frontend/FrontendActions.h" #include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/CodeCompleteConsumer.h" +#include "clang/Sema/DeclSpec.h" #include "clang/Sema/Sema.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/None.h" @@ -539,54 +540,59 @@ // (e.g. enclosing namespace). std::pair, bool> getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema, + const CompletionPrefix &HeuristicPrefix, const CodeCompleteOptions &Opts) { - auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) { - SpecifiedScope Info; - for (auto *Context : CCContext.getVisitedContexts()) { - if (isa(Context)) - Info.AccessibleScopes.push_back(""); // global namespace - else if (isa(Context)) - Info.AccessibleScopes.push_back(printNamespaceScope(*Context)); - } - return Info; - }; - - auto SS = CCContext.getCXXScopeSpecifier(); + SpecifiedScope Scopes; + for (auto *Context : CCContext.getVisitedContexts()) { + if (isa(Context)) + Scopes.AccessibleScopes.push_back(""); // global namespace + else if (isa(Context)) + Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context)); + } - // Unqualified completion (e.g. "vec^"). - if (!SS) { - std::vector Scopes; + const CXXScopeSpec *SemaSpecifier = + CCContext.getCXXScopeSpecifier().getValueOr(nullptr); + // Case 1: unqualified completion. + if (!SemaSpecifier) { + // Case 2 (exception): sema saw no qualifier, but there appears to be one! + // This can happen e.g. in incomplete macro expansions. Use heuristics. + if (!HeuristicPrefix.Qualifier.empty()) { + vlog("Sema said no scope specifier, but we saw {0} in the source code", + HeuristicPrefix.Qualifier); + StringRef SpelledSpecifier = HeuristicPrefix.Qualifier; + if (SpelledSpecifier.consume_front("::")) + Scopes.AccessibleScopes = {""}; + Scopes.UnresolvedQualifier = SpelledSpecifier; + return {Scopes.scopesForIndexQuery(), false}; + } + // The enclosing namespace must be first, it gets a quality boost. + std::vector EnclosingAtFront; std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext); - Scopes.push_back(EnclosingScope); - for (auto &S : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) { + EnclosingAtFront.push_back(EnclosingScope); + for (auto &S : Scopes.scopesForIndexQuery()) { if (EnclosingScope != S) - Scopes.push_back(std::move(S)); + EnclosingAtFront.push_back(std::move(S)); } - // Allow AllScopes completion only for there is no explicit scope qualifier. - return {Scopes, Opts.AllScopes}; + // Allow AllScopes completion as there is no explicit scope qualifier. + return {EnclosingAtFront, Opts.AllScopes}; } - - // Qualified completion ("std::vec^"), we have two cases depending on whether - // the qualifier can be resolved by Sema. - if ((*SS)->isValid()) { // Resolved qualifier. - return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false}; - } - - // Unresolved qualifier. - SpecifiedScope Info = GetAllAccessibleScopes(CCContext); - Info.AccessibleScopes.push_back(""); // Make sure global scope is included. - - llvm::StringRef SpelledSpecifier = - Lexer::getSourceText(CharSourceRange::getCharRange((*SS)->getRange()), - CCSema.SourceMgr, clang::LangOptions()); + // Case 3: sema saw and resolved a scope qualifier. + if (SemaSpecifier && SemaSpecifier->isValid()) + return {Scopes.scopesForIndexQuery(), false}; + + // Case 4: There was a qualifier, and Sema didn't resolve it. + Scopes.AccessibleScopes.push_back(""); // Make sure global scope is included. + llvm::StringRef SpelledSpecifier = Lexer::getSourceText( + CharSourceRange::getCharRange(SemaSpecifier->getRange()), + CCSema.SourceMgr, clang::LangOptions()); if (SpelledSpecifier.consume_front("::")) - Info.AccessibleScopes = {""}; - Info.UnresolvedQualifier = SpelledSpecifier; + Scopes.AccessibleScopes = {""}; + Scopes.UnresolvedQualifier = SpelledSpecifier; // Sema excludes the trailing "::". - if (!Info.UnresolvedQualifier->empty()) - *Info.UnresolvedQualifier += "::"; + if (!Scopes.UnresolvedQualifier->empty()) + *Scopes.UnresolvedQualifier += "::"; - return {Info.scopesForIndexQuery(), false}; + return {Scopes.scopesForIndexQuery(), false}; } // Should we perform index-based completion in a context of the specified kind? @@ -984,7 +990,7 @@ const tooling::CompileCommand &Command; const PreambleData *Preamble; llvm::StringRef Contents; - Position Pos; + size_t Offset; llvm::IntrusiveRefCntPtr VFS; }; @@ -1015,14 +1021,9 @@ // Setup code completion. FrontendOpts.CodeCompleteOpts = Options; FrontendOpts.CodeCompletionAt.FileName = Input.FileName; - auto Offset = positionToOffset(Input.Contents, Input.Pos); - if (!Offset) { - elog("Code completion position was invalid {0}", Offset.takeError()); - return false; - } std::tie(FrontendOpts.CodeCompletionAt.Line, FrontendOpts.CodeCompletionAt.Column) = - offsetToClangLineColumn(Input.Contents, *Offset); + offsetToClangLineColumn(Input.Contents, Input.Offset); std::unique_ptr ContentsBuffer = llvm::MemoryBuffer::getMemBufferCopy(Input.Contents, Input.FileName); @@ -1036,7 +1037,7 @@ // skip all includes in this case; these completions are really simple. bool CompletingInPreamble = ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size > - *Offset; + Input.Offset; // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise // the remapped buffers do not get freed. IgnoreDiagnostics DummyDiagsConsumer; @@ -1107,16 +1108,9 @@ // Creates a `FuzzyFindRequest` based on the cached index request from the // last completion, if any, and the speculated completion filter text in the // source code. -llvm::Optional -speculativeFuzzyFindRequestForCompletion(FuzzyFindRequest CachedReq, - PathRef File, llvm::StringRef Content, - Position Pos) { - auto Offset = positionToOffset(Content, Pos); - if (!Offset) { - elog("No speculative filter: bad offset {0} in {1}", Pos, File); - return llvm::None; - } - CachedReq.Query = guessCompletionPrefix(Content, *Offset).Name; +FuzzyFindRequest speculativeFuzzyFindRequestForCompletion( + FuzzyFindRequest CachedReq, const CompletionPrefix &HeuristicPrefix) { + CachedReq.Query = HeuristicPrefix.Name; return CachedReq; } @@ -1159,6 +1153,7 @@ CompletionRecorder *Recorder = nullptr; int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging. bool Incomplete = false; // Would more be available with a higher limit? + CompletionPrefix HeuristicPrefix; llvm::Optional Filter; // Initialized once Sema runs. std::vector QueryScopes; // Initialized once Sema runs. // Initialized once QueryScopes is initialized, if there are scopes. @@ -1186,12 +1181,13 @@ CodeCompleteResult run(const SemaCompleteInput &SemaCCInput) && { trace::Span Tracer("CodeCompleteFlow"); + HeuristicPrefix = + guessCompletionPrefix(SemaCCInput.Contents, SemaCCInput.Offset); if (Opts.Index && SpecFuzzyFind && SpecFuzzyFind->CachedReq.hasValue()) { assert(!SpecFuzzyFind->Result.valid()); - if ((SpecReq = speculativeFuzzyFindRequestForCompletion( - *SpecFuzzyFind->CachedReq, SemaCCInput.FileName, - SemaCCInput.Contents, SemaCCInput.Pos))) - SpecFuzzyFind->Result = startAsyncFuzzyFind(*Opts.Index, *SpecReq); + SpecReq = speculativeFuzzyFindRequestForCompletion( + *SpecFuzzyFind->CachedReq, HeuristicPrefix); + SpecFuzzyFind->Result = startAsyncFuzzyFind(*Opts.Index, *SpecReq); } // We run Sema code completion first. It builds an AST and calculates: @@ -1285,8 +1281,8 @@ } Filter = FuzzyMatcher( Recorder->CCSema->getPreprocessor().getCodeCompletionFilter()); - std::tie(QueryScopes, AllScopes) = - getQueryScopes(Recorder->CCContext, *Recorder->CCSema, Opts); + std::tie(QueryScopes, AllScopes) = getQueryScopes( + Recorder->CCContext, *Recorder->CCSema, HeuristicPrefix, Opts); if (!QueryScopes.empty()) ScopeProximity.emplace(QueryScopes); PreferredType = @@ -1563,10 +1559,15 @@ const PreambleData *Preamble, llvm::StringRef Contents, Position Pos, llvm::IntrusiveRefCntPtr VFS, CodeCompleteOptions Opts, SpeculativeFuzzyFind *SpecFuzzyFind) { + auto Offset = positionToOffset(Contents, Pos); + if (!Offset) { + elog("Code completion position was invalid {0}", Offset.takeError()); + return CodeCompleteResult(); + } return CodeCompleteFlow(FileName, Preamble ? Preamble->Includes : IncludeStructure(), SpecFuzzyFind, Opts) - .run({FileName, Command, Preamble, Contents, Pos, VFS}); + .run({FileName, Command, Preamble, Contents, *Offset, VFS}); } SignatureHelp signatureHelp(PathRef FileName, @@ -1575,6 +1576,11 @@ llvm::StringRef Contents, Position Pos, llvm::IntrusiveRefCntPtr VFS, const SymbolIndex *Index) { + auto Offset = positionToOffset(Contents, Pos); + if (!Offset) { + elog("Code completion position was invalid {0}", Offset.takeError()); + return SignatureHelp(); + } SignatureHelp Result; clang::CodeCompleteOptions Options; Options.IncludeGlobals = false; @@ -1584,7 +1590,8 @@ IncludeStructure PreambleInclusions; // Unused for signatureHelp semaCodeComplete( llvm::make_unique(Options, Index, Result), - Options, {FileName, Command, Preamble, Contents, Pos, std::move(VFS)}); + Options, + {FileName, Command, Preamble, Contents, *Offset, std::move(VFS)}); return Result; } Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -2374,19 +2374,19 @@ UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ")))); } -// Regression test: clang parser gets confused here and doesn't report the ns:: -// prefix - naive behavior is to insert it again. -// However we can recognize this from the source code. -// Test disabled until we can make it pass. -TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) { +// Clang parser gets confused here and doesn't report the ns:: prefix. +// Naive behavior is to insert it again. We examine the source and recover. +TEST(CompletionTest, NamespaceDoubleInsertion) { clangd::CodeCompleteOptions Opts = {}; auto Results = completions(R"cpp( + namespace foo { namespace ns {} #define M(X) < X M(ns::ABC^ + } )cpp", - {cls("ns::ABCDE")}, Opts); + {cls("foo::ns::ABCDE")}, Opts); EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE")))); }