Index: clang-tools-extra/trunk/clangd/CodeComplete.h =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.h +++ clang-tools-extra/trunk/clangd/CodeComplete.h @@ -254,10 +254,21 @@ // completion. bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx); -/// Retrives a speculative code completion filter text before the cursor. -/// Exposed for testing only. -llvm::Expected -speculateCompletionFilter(llvm::StringRef Content, Position Pos); +// Text immediately before the completion point that should be completed. +// This is heuristically derived from the source code, and is used when: +// - semantic analysis fails +// - semantic analysis may be slow, and we speculatively query the index +struct CompletionPrefix { + // The unqualified partial name. + // If there is none, begin() == end() == completion position. + llvm::StringRef Name; + // The spelled scope qualifier, such as Foo::. + // If there is none, begin() == end() == Name.begin(). + llvm::StringRef Qualifier; +}; +// Heuristically parses before Offset to determine what should be completed. +CompletionPrefix guessCompletionPrefix(llvm::StringRef Content, + unsigned Offset); } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -37,6 +37,7 @@ #include "index/Symbol.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/Basic/CharInfo.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Format/Format.h" @@ -1110,14 +1111,12 @@ speculativeFuzzyFindRequestForCompletion(FuzzyFindRequest CachedReq, PathRef File, llvm::StringRef Content, Position Pos) { - auto Filter = speculateCompletionFilter(Content, Pos); - if (!Filter) { - elog("Failed to speculate filter text for code completion at Pos " - "{0}:{1}: {2}", - Pos.line, Pos.character, Filter.takeError()); - return None; + auto Offset = positionToOffset(Content, Pos); + if (!Offset) { + elog("No speculative filter: bad offset {0} in {1}", Pos, File); + return llvm::None; } - CachedReq.Query = *Filter; + CachedReq.Query = guessCompletionPrefix(Content, *Offset).Name; return CachedReq; } @@ -1537,29 +1536,26 @@ return Result; } -llvm::Expected -speculateCompletionFilter(llvm::StringRef Content, Position Pos) { - auto Offset = positionToOffset(Content, Pos); - if (!Offset) - return llvm::make_error( - "Failed to convert position to offset in content.", - llvm::inconvertibleErrorCode()); - if (*Offset == 0) - return ""; +CompletionPrefix +guessCompletionPrefix(llvm::StringRef Content, unsigned Offset) { + assert(Offset <= Content.size()); + StringRef Rest = Content.take_front(Offset); + CompletionPrefix Result; + + // Consume the unqualified name. We only handle ASCII characters. + // isIdentifierBody will let us match "0invalid", but we don't mind. + while (!Rest.empty() && isIdentifierBody(Rest.back())) + Rest = Rest.drop_back(); + Result.Name = Content.slice(Rest.size(), Offset); + + // Consume qualifiers. + while (Rest.consume_back("::") && !Rest.endswith(":")) // reject :::: + while (!Rest.empty() && isIdentifierBody(Rest.back())) + Rest = Rest.drop_back(); + Result.Qualifier = + Content.slice(Rest.size(), Result.Name.begin() - Content.begin()); - // Start from the character before the cursor. - int St = *Offset - 1; - // FIXME(ioeric): consider UTF characters? - auto IsValidIdentifierChar = [](char C) { - return ((C >= 'a' && C <= 'z') || (C >= 'A' && C <= 'Z') || - (C >= '0' && C <= '9') || (C == '_')); - }; - size_t Len = 0; - for (; (St >= 0) && IsValidIdentifierChar(Content[St]); --St, ++Len) { - } - if (Len > 0) - St++; // Shift to the first valid character. - return Content.substr(St, Len); + return Result; } CodeCompleteResult Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -32,7 +32,6 @@ using ::llvm::Failed; using ::testing::AllOf; using ::testing::Contains; -using ::testing::Each; using ::testing::ElementsAre; using ::testing::Field; using ::testing::HasSubstr; @@ -1915,28 +1914,37 @@ )cpp"); } -TEST(SpeculateCompletionFilter, Filters) { - Annotations F(R"cpp($bof^ - $bol^ - ab$ab^ - x.ab$dot^ - x.$dotempty^ - x::ab$scoped^ - x::$scopedempty^ +TEST(GuessCompletionPrefix, Filters) { + for (llvm::StringRef Case : { + "[[scope::]][[ident]]^", + "[[]][[]]^", + "\n[[]][[]]^", + "[[]][[ab]]^", + "x.[[]][[ab]]^", + "x.[[]][[]]^", + "[[x::]][[ab]]^", + "[[x::]][[]]^", + "[[::x::]][[ab]]^", + "some text [[scope::more::]][[identif]]^ier", + "some text [[scope::]][[mor]]^e::identifier", + "weird case foo::[[::bar::]][[baz]]^", + }) { + Annotations F(Case); + auto Offset = cantFail(positionToOffset(F.code(), F.point())); + auto ToStringRef = [&](Range R) { + return F.code().slice(cantFail(positionToOffset(F.code(), R.start)), + cantFail(positionToOffset(F.code(), R.end))); + }; + auto WantQualifier = ToStringRef(F.ranges()[0]), + WantName = ToStringRef(F.ranges()[1]); - )cpp"); - auto speculate = [&](StringRef PointName) { - auto Filter = speculateCompletionFilter(F.code(), F.point(PointName)); - assert(Filter); - return *Filter; - }; - EXPECT_EQ(speculate("bof"), ""); - EXPECT_EQ(speculate("bol"), ""); - EXPECT_EQ(speculate("ab"), "ab"); - EXPECT_EQ(speculate("dot"), "ab"); - EXPECT_EQ(speculate("dotempty"), ""); - EXPECT_EQ(speculate("scoped"), "ab"); - EXPECT_EQ(speculate("scopedempty"), ""); + auto Prefix = guessCompletionPrefix(F.code(), Offset); + // Even when components are empty, check their offsets are correct. + EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case; + EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case; + EXPECT_EQ(WantName, Prefix.Name) << Case; + EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case; + } } TEST(CompletionTest, EnableSpeculativeIndexRequest) { @@ -2366,6 +2374,23 @@ 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) { + clangd::CodeCompleteOptions Opts = {}; + + auto Results = completions(R"cpp( + namespace ns {} + #define M(X) < X + M(ns::ABC^ + )cpp", + {cls("ns::ABCDE")}, Opts); + EXPECT_THAT(Results.Completions, + UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE")))); +} + } // namespace } // namespace clangd } // namespace clang