hokein updated this revision to Diff 130618.
hokein marked 4 inline comments as done.
hokein added a comment.
- Rebase the patch to latest
- Update the patch based on the offline discussion
Updating D42073: [clangd] Query all visible scopes based on all visible
using-namespace declarations
====================================================================================================
and containing namespace for global qualified code completion.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42073
Files:
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -58,6 +58,7 @@
using ::testing::ElementsAre;
using ::testing::Not;
using ::testing::UnorderedElementsAre;
+using ::testing::Field;
class IgnoreDiagnostics : public DiagnosticsConsumer {
void
@@ -676,6 +677,123 @@
EXPECT_EQ(1, Results.activeParameter);
}
+class IndexRequestCollector : public SymbolIndex {
+public:
+ bool
+ fuzzyFind(const Context &Ctx, const FuzzyFindRequest &Req,
+ llvm::function_ref<void(const Symbol &)> Callback) const override {
+ Requests.push_back(Req);
+ return false;
+ }
+
+ const std::vector<FuzzyFindRequest> allRequests() const { return Requests; }
+
+private:
+ mutable std::vector<FuzzyFindRequest> Requests;
+};
+
+TEST(CompletionTest, UnqualifiedIdQuery) {
+ clangd::CodeCompleteOptions Opts;
+ IndexRequestCollector Requests;
+ Opts.Index = &Requests;
+
+ auto Results = completions(R"cpp(
+ namespace std {}
+ using namespace std;
+ namespace ns {
+ void f() {
+ vec^
+ }
+ }
+ )cpp", {}, Opts);
+
+ EXPECT_THAT(Requests.allRequests(),
+ ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ UnorderedElementsAre("", "ns", "std"))));
+}
+
+TEST(CompletionTest, ResolvedQualifiedIdQuery) {
+ clangd::CodeCompleteOptions Opts;
+ IndexRequestCollector Requests;
+ Opts.Index = &Requests;
+
+ auto Results = completions(R"cpp(
+ namespace ns1 {}
+ namespace ns2 {} // ignore
+ namespace ns3 { namespace nns3 {} }
+ namespace foo {
+ using namespace ns1;
+ using namespace ns3::nns3;
+ }
+ namespace ns {
+ void f() {
+ foo::^
+ }
+ }
+ )cpp", {}, Opts);
+
+ EXPECT_THAT(
+ Requests.allRequests(),
+ ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ UnorderedElementsAre("foo", "ns1", "ns3::nns3"))));
+}
+
+TEST(CompletionTest, UnresolvedQualifierIdQuery) {
+ clangd::CodeCompleteOptions Opts;
+ IndexRequestCollector Requests;
+ Opts.Index = &Requests;
+
+ auto Results = completions(R"cpp(
+ namespace a {}
+ using namespace a;
+ namespace ns {
+ void f() {
+ bar::^
+ }
+ } // namespace ns
+ )cpp", {}, Opts);
+
+ EXPECT_THAT(Requests.allRequests(),
+ ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ UnorderedElementsAre("bar"))));
+}
+
+TEST(CompletionTest, EmptyQualifiedQuery) {
+ clangd::CodeCompleteOptions Opts;
+ IndexRequestCollector Requests;
+ Opts.Index = &Requests;
+
+ auto Results = completions(R"cpp(
+ namespace ns {
+ void f() {
+ ^
+ }
+ } // namespace ns
+ )cpp", {}, Opts);
+
+ EXPECT_THAT(Requests.allRequests(),
+ ElementsAre(Field(&FuzzyFindRequest::Scopes,
+ UnorderedElementsAre("", "ns"))));
+}
+
+TEST(CompletionTest, GlobalQualifiedQuery) {
+ clangd::CodeCompleteOptions Opts;
+ IndexRequestCollector Requests;
+ Opts.Index = &Requests;
+
+ auto Results = completions(R"cpp(
+ namespace ns {
+ void f() {
+ ::^
+ }
+ } // namespace ns
+ )cpp", {}, Opts);
+
+ EXPECT_THAT(
+ Requests.allRequests(),
+ ElementsAre(Field(&FuzzyFindRequest::Scopes, UnorderedElementsAre(""))));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -309,20 +309,52 @@
llvm_unreachable("unknown CodeCompletionResult kind");
}
-/// \brief Information about the scope specifier in the qualified-id code
-/// completion (e.g. "ns::ab?").
-struct SpecifiedScope {
- /// The scope specifier as written. For example, for completion "ns::ab?", the
- /// written scope specifier is "ns".
- std::string Written;
- // If this scope specifier is recognized in Sema (e.g. as a namespace
- // context), this will be set to the fully qualfied name of the corresponding
- // context.
- std::string Resolved;
-
- llvm::StringRef forIndex() {
- llvm::StringRef Chosen = Resolved.empty() ? Written : Resolved;
- return Chosen.trim(':');
+StringRef normalizeScope(StringRef Q) {
+ return Q.trim(':');
+}
+
+/// \brief Scopes being queried in indexes for the qualified-id code completion
+/// (e.g. "ns::ab^").
+//
+// FIXME: we might want to make Sema code completion smarter on handling
+// unresolved qualified-id completion.
+//
+// E.g. foo::ns1::ns2::
+// ^ ^ unresolved qualifier
+// ^~~ resolved qualifier
+// Instead of getting the whole unresolved qualifier "foo::ns1::ns2::", a better
+// way is to return an unresolved qualifier "ns1::ns2" with all scopes that are
+// accessible in "foo".
+struct QualifiedScopes {
+ // All scopes that are accessible in the completion scope qualifier.
+ //
+ // If the completion scope qualifier is:
+ // * resolved by Sema, it contains fully qualified names of all accessible
+ // scopes (namespace) in the resolved qualifier;
+ // * unresolved by Sema, global namespace "::" is the only accessible scope.
+ //
+ // See examples:
+ // "::vec" => {"::"}
+ // "using namespace std; ::vec^" => {"::", "std"}
+ // "namespace ns {using namespace std;} ns::^" => {"ns", "std"}
+ // "std::vec^" => {"::"} // unresolved "std::"
+ std::vector<std::string> AccessibleScopes;
+ // The scope qualifier that is not resolved in Sema, it is the user-written
+ // qualifiers.
+ llvm::Optional<std::string> UnresolvedQualifier;
+
+ std::vector<std::string> forIndex() {
+ std::vector<std::string> Results;
+ for (llvm::StringRef VS : AccessibleScopes) {
+ std::string QueryScope =
+ (normalizeScope(VS) +
+ (UnresolvedQualifier
+ ? "::" + normalizeScope(*UnresolvedQualifier)
+ : ""))
+ .str();
+ Results.push_back(normalizeScope(QueryScope));
+ }
+ return Results;
}
};
@@ -625,22 +657,27 @@
return true;
}
-SpecifiedScope getSpecifiedScope(Sema &S, const CXXScopeSpec &SS) {
- SpecifiedScope Info;
- auto &SM = S.getSourceManager();
- auto SpecifierRange = SS.getRange();
- Info.Written = Lexer::getSourceText(
- CharSourceRange::getCharRange(SpecifierRange), SM, clang::LangOptions());
- if (SS.isValid()) {
- DeclContext *DC = S.computeDeclContext(SS);
- if (auto *NS = llvm::dyn_cast<NamespaceDecl>(DC)) {
- Info.Resolved = NS->getQualifiedNameAsString();
- } else if (llvm::dyn_cast<TranslationUnitDecl>(DC) != nullptr) {
- Info.Resolved = "::";
- // Sema does not include the suffix "::" in the range of SS, so we add
- // it back here.
- Info.Written = "::";
+QualifiedScopes getQualifiedScopes(Sema &S, const CXXScopeSpec &SS,
+ const CodeCompletionContext &CCContext) {
+ QualifiedScopes Info;
+ if (SS.isValid()) { // Resolved qualifier.
+ // FIXME: Disable Sema typo correction dureing code completion.
+ // The resolved qualifier might not perfectly match the written qualifier.
+ // e.g. "namespace clang { clangd::^ }", we will get "clang" declaration
+ // for completion "clangd::".
+ //
+ // Add all namespaces that are accessible from `DC` (including `DC` itself).
+ for (const auto* Ctx : CCContext.getVisitedContexts()) {
+ if (isa<TranslationUnitDecl>(Ctx))
+ Info.AccessibleScopes.push_back("::"); // global namespace
+ else if (const auto* VNS = dyn_cast<NamespaceDecl>(Ctx))
+ Info.AccessibleScopes.push_back(VNS->getQualifiedNameAsString());
}
+ } else { // Unresolved qualifier.
+ Info.AccessibleScopes.push_back("::"); // global namespace
+ Info.UnresolvedQualifier =
+ Lexer::getSourceText(CharSourceRange::getCharRange(
+ SS.getRange()), S.getSourceManager(), clang::LangOptions());
}
return Info;
}
@@ -805,15 +842,33 @@
FuzzyFindRequest Req;
Req.Query = Filter->pattern();
// If the user typed a scope, e.g. a::b::xxx(), restrict to that scope.
- // FIXME(ioeric): add scopes based on using directives and enclosing ns.
- if (auto SS = Recorder.CCContext.getCXXScopeSpecifier())
- Req.Scopes = {getSpecifiedScope(*Recorder.CCSema, **SS).forIndex()};
- else
- // Unless the user typed a ns qualifier, complete in global scope only.
- // FIXME: once we know what namespaces are in scope (D42073), use those.
+ if (auto SS = Recorder.CCContext.getCXXScopeSpecifier()) {
+ Req.Scopes =
+ getQualifiedScopes(*Recorder.CCSema, **SS, Recorder.CCContext)
+ .forIndex();
+ } else {
+ // Unqualified code completion. Use the containing namespace and all
+ // accessible namespaces to query symbols from index, for examples:
+ // "vec^" => {"::"}
+ // "using namespace std; vec^" => {"::", "std"}
+ // "using namespace std; namespace ns { vec^ }" => {"ns", "std", "::"}
+ //
// FIXME: once we can insert namespace qualifiers and use the in-scope
// namespaces for scoring, search in all namespaces.
- Req.Scopes = {""};
+ // FIXME: Capture scopes and use for scoring, for example,
+ // "using namespace std; namespace foo {v^}" =>
+ // foo::value > std::vector > boost::variant
+ for (auto* Context : Recorder.CCContext.getVisitedContexts()) {
+ if (isa<TranslationUnitDecl>(Context))
+ Req.Scopes.push_back(normalizeScope("::")); // global namespace
+ else if (const auto*NS = dyn_cast<NamespaceDecl>(Context))
+ Req.Scopes.push_back(normalizeScope(NS->getQualifiedNameAsString()));
+ }
+ }
+ log(Ctx, "Query scopes: [");
+ for (auto &R : Req.Scopes)
+ log(Ctx, R);
+ log(Ctx, "]");
// Run the query against the index.
Incomplete |= !Opts.Index->fuzzyFind(
Ctx, Req, [&](const Symbol &Sym) { ResultsBuilder.insert(Sym); });
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
