[clang-tools-extra] r358074 - [clangd] Refactor speculateCompletionFilter and also extract scope.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 10 04:50:40 PDT 2019
Author: sammccall
Date: Wed Apr 10 04:50:40 2019
New Revision: 358074
URL: http://llvm.org/viewvc/llvm-project?rev=358074&view=rev
Log:
[clangd] Refactor speculateCompletionFilter and also extract scope.
Summary:
Intent is to use the heuristically-parsed scope in cases where we get bogus
results from sema, such as in complex macro expansions.
Added a motivating testcase we currently get wrong.
Name changed because we (already) use this for things other than speculation.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60500
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358074&r1=358073&r2=358074&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 04:50:40 2019
@@ -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 @@ llvm::Optional<FuzzyFindRequest>
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 @@ clang::CodeCompleteOptions CodeCompleteO
return Result;
}
-llvm::Expected<llvm::StringRef>
-speculateCompletionFilter(llvm::StringRef Content, Position Pos) {
- auto Offset = positionToOffset(Content, Pos);
- if (!Offset)
- return llvm::make_error<llvm::StringError>(
- "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
Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=358074&r1=358073&r2=358074&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Wed Apr 10 04:50:40 2019
@@ -254,10 +254,21 @@ SignatureHelp signatureHelp(PathRef File
// completion.
bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx);
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected<llvm::StringRef>
-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
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=358074&r1=358073&r2=358074&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Apr 10 04:50:40 2019
@@ -32,7 +32,6 @@ namespace {
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 @@ TEST(CompletionTest, OverridesNonIdentNa
)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 @@ TEST(CompletionTest, NestedScopeIsUnreso
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
More information about the cfe-commits
mailing list