[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343248: [clangd] Initial supoprt for cross-namespace
global code completion. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52364?vs=167356=167357#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexTests.cpp
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -136,6 +136,15 @@
"enabled separatedly."),
llvm::cl::init(true), llvm::cl::Hidden);
+static llvm::cl::opt AllScopesCompletion(
+"all-scopes-completion",
+llvm::cl::desc(
+"If set to true, code completion will include index symbols that are "
+"not defined in the scopes (e.g. "
+"namespaces) visible from the code completion point. Such completions "
+"can insert scope qualifiers."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
static llvm::cl::opt
ShowOrigins("debug-origin",
llvm::cl::desc("Show origins of completion items"),
@@ -304,6 +313,7 @@
}
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+ CCOpts.AllScopes = AllScopesCompletion;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -44,6 +44,7 @@
#include "clang/Sema/CodeCompleteConsumer.h"
#include "clang/Sema/Sema.h"
#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Error.h"
@@ -330,6 +331,7 @@
struct CodeCompletionBuilder {
CodeCompletionBuilder(ASTContext , const CompletionCandidate ,
CodeCompletionString *SemaCCS,
+llvm::ArrayRef QueryScopes,
const IncludeInserter , StringRef FileName,
CodeCompletionContext::Kind ContextKind,
const CodeCompleteOptions )
@@ -374,6 +376,18 @@
Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind);
if (Completion.Name.empty())
Completion.Name = C.IndexResult->Name;
+ // If the completion was visible to Sema, no qualifier is needed. This
+ // avoids unneeded qualifiers in cases like with `using ns::X`.
+ if (Completion.RequiredQualifier.empty() && !C.SemaResult) {
+StringRef ShortestQualifier = C.IndexResult->Scope;
+for (StringRef Scope : QueryScopes) {
+ StringRef Qualifier = C.IndexResult->Scope;
+ if (Qualifier.consume_front(Scope) &&
+ Qualifier.size() < ShortestQualifier.size())
+ShortestQualifier = Qualifier;
+}
+Completion.RequiredQualifier = ShortestQualifier;
+ }
Completion.Deprecated |= (C.IndexResult->Flags & Symbol::Deprecated);
}
@@ -604,9 +618,11 @@
}
};
-// Get all scopes that will be queried in indexes.
-std::vector getQueryScopes(CodeCompletionContext ,
-const SourceManager ) {
+// Get all scopes that will be queried in indexes and whether symbols from
+// any scope is allowed.
+std::pair, bool>
+getQueryScopes(CodeCompletionContext , const SourceManager ,
+ const CodeCompleteOptions ) {
auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
@@ -627,13 +643,15 @@
// FIXME: Capture scopes and use for scoring, for example,
//"using namespace std; namespace foo {v^}" =>
//foo::value > std::vector > boost::variant
-return GetAllAccessibleScopes(CCContext).scopesForIndexQuery();
+auto Scopes = GetAllAccessibleScopes(CCContext).scopesForIndexQuery();
+// Allow AllScopes completion only for there is no explicit scope qualifier.
+return {Scopes, 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();
+return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
}
// Unresolved qualifier.
@@ -651,7 +669,7 @@
if (!Info.UnresolvedQualifier->empty())
*Info.UnresolvedQualifier += "::";
- return Info.scopesForIndexQuery();
+ return {Info.scopesForIndexQuery(), false};
}
// Should we perform
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
ioeric updated this revision to Diff 167356.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -523,6 +523,17 @@
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
+TEST(DexTest, WildcardScope) {
+ auto I =
+ Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), URISchemes);
+ FuzzyFindRequest Req;
+ Req.Query = "y";
+ Req.Scopes = {"a::"};
+ Req.AnyScope = true;
+ EXPECT_THAT(match(*I, Req),
+ UnorderedElementsAre("a::y1", "a::b::y2", "c::y3"));
+}
+
TEST(DexTest, IgnoreCases) {
auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), URISchemes);
FuzzyFindRequest Req;
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2093,6 +2093,57 @@
Has("bar.h\"", CompletionItemKind::File)));
}
+TEST(CompletionTest, NoAllScopesCompletionWhenQualified) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+void f() { na::Clangd^ }
+ )cpp",
+ {cls("na::ClangdA"), cls("nx::ClangdX"), cls("Clangd3")}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(
+ AllOf(Qualifier(""), Scope("na::"), Named("ClangdA";
+}
+
+TEST(CompletionTest, AllScopesCompletion) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+namespace na {
+void f() { Clangd^ }
+}
+ )cpp",
+ {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"),
+ cls("na::nb::Clangd4")},
+ Opts);
+ EXPECT_THAT(
+ Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier("nx::"), Named("Clangd1")),
+ AllOf(Qualifier("ny::"), Named("Clangd2")),
+ AllOf(Qualifier(""), Scope(""), Named("Clangd3")),
+ AllOf(Qualifier("nb::"), Named("Clangd4";
+}
+
+TEST(CompletionTest, NoQualifierIfShadowed) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(R"cpp(
+namespace nx { class Clangd1 {}; }
+using nx::Clangd1;
+void f() { Clangd^ }
+ )cpp",
+ {cls("nx::Clangd1"), cls("nx::Clangd2")}, Opts);
+ // Although Clangd1 is from another namespace, Sema tells us it's in-scope and
+ // needs no qualifier.
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier(""), Named("Clangd1")),
+ AllOf(Qualifier("nx::"), Named("Clangd2";
+}
} // namespace
} // namespace clangd
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -136,6 +136,15 @@
"enabled separatedly."),
llvm::cl::init(true), llvm::cl::Hidden);
+static llvm::cl::opt AllScopesCompletion(
+"all-scopes-completion",
+llvm::cl::desc(
+"If set to true, code completion will include index symbols that are "
+"not defined in the scopes (e.g. "
+"namespaces) visible from the code completion point. Such completions "
+"can insert scope qualifiers."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
static llvm::cl::opt
ShowOrigins("debug-origin",
llvm::cl::desc("Show origins of completion items"),
@@ -304,6 +313,7 @@
}
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+ CCOpts.AllScopes = AllScopesCompletion;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -13,6 +13,8 @@
#include "Logger.h"
#include "Quality.h"
#include "Trace.h"
+#include "index/Index.h"
+#include "index/dex/Iterator.h"
#include "llvm/ADT/StringSet.h"
#include
#include
@@ -166,6 +168,10 @@
if (It != InvertedIndex.end())
ScopeIterators.push_back(It->second.iterator());
}
+ if (Req.AnyScope)
+ScopeIterators.push_back(createBoost(createTrue(Symbols.size()),
+ ScopeIterators.empty() ? 1.0 : 0.2));
+
// Add OR iterator for scopes if there are any Scope
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/CodeComplete.cpp:1251 + // from any scope. + std::pair, bool> QueryScopes; // Include-insertion and proximity scoring rely on the include structure. consider splitting out AllScopes as a separate variable, and assign `std::tie(QueryScopes, AllScopes) = getQueryScopes(...)`, for readability Comment at: clangd/index/Index.h:432 + /// wildcard. + /// FIXME: support assigning different weight to each scope. std::vector Scopes; ioeric wrote: > sammccall wrote: > > May not want a heavyweight API with explicit weights... > > > > I think what we really **need** here is the ability to weight > > `clang::clangd::` > `clang::` > `` when you're in the scope of namespace > > clangd. > > > > We could just say something like "the primary scope should come first" and > > do some FileDistance-like penalizing of the others... > Changed the wording of `FIXME`. > > > I think what we really need here is the ability to weight clang::clangd:: > > > clang:: > `` when you're in the scope of namespace clangd. > It's unclear what this would mean for scopes from `using-namespace` > directives. But we can revisit when we get there. Yeah, my hypothesis is that it doesn't matter much, as long as the using-namespaces are ranked lower than the enclosing namespaces, and above "any" namespaces. Comment at: clangd/index/dex/Dex.cpp:171 } + if (Req.AnyScope) +ScopeIterators.push_back(createBoost(createTrue(Symbols.size()), 0.2)); kbobyrev wrote: > Probably also check `!ScopeIterators.empty()`: otherwise the latency might > increase for queries without any scope/any scope known to `Dex`. if there are no other scopes, the "naive" query tree will end up looking like and(..., or(boost(true)). The or() will already be optimized away today. I'd suggest you just remove the boost() here if req.scopes is empty (or better: make the boost 1) and we make "generic optimizations" take care of eliminating boosts with value 1, and dropping true when it's an argument to and() Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
kbobyrev added inline comments.
Comment at: clangd/index/dex/Dex.cpp:171
}
+ if (Req.AnyScope)
+ScopeIterators.push_back(createBoost(createTrue(Symbols.size()), 0.2));
Probably also check `!ScopeIterators.empty()`: otherwise the latency might
increase for queries without any scope/any scope known to `Dex`.
Comment at: unittests/clangd/DexTests.cpp:557
+TEST(DexTest, WildcardScope) {
+ auto I =
Probably also add test which ensures that wildcard symbols are downranked in
the presence of `Req.Scopes` (ProximityPathsBoosting test is probably the
closest in this sense)? Isn't too important, but nice-to-have in the presence
of `ScopesProximity` plans (judging from the `FIXME` in `FuzzyFindRequest`).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
ioeric added inline comments.
Comment at: clangd/index/Index.h:430
///
- /// The global scope is "", a top level scope is "foo::", etc.
+ /// The global scope is "", a top level scope is "foo::", etc. "*" is
+ /// wildcard.
sammccall wrote:
> I'm not a big fan of this magic value, it seems too easy to forget to handle.
> Especially since we have a lot of existing code that touches this interface,
> and we may forget to check some of it.
>
> I suggest making this a separate boolean field `AnyScope`, with a comment
> that scopes explicitly listed will be ranked higher.
> We can probably also remove the "If this is empty" special case from `Scopes`
> now too, as the old behavior can be achieved by setting `Scopes = {};
> AnyScope = true;`
sounds good.
> We can probably also remove the "If this is empty" special case from Scopes
> now too, as the old behavior can be achieved by setting Scopes = {}; AnyScope
> = true;
I think this is a good idea. Unfortunately, there seem to be many tests that
rely on the old behavior. I'll add a FIXME and do it in followup patch.
Comment at: clangd/index/Index.h:432
+ /// wildcard.
+ /// FIXME: support assigning different weight to each scope.
std::vector Scopes;
sammccall wrote:
> May not want a heavyweight API with explicit weights...
>
> I think what we really **need** here is the ability to weight
> `clang::clangd::` > `clang::` > `` when you're in the scope of namespace
> clangd.
>
> We could just say something like "the primary scope should come first" and do
> some FileDistance-like penalizing of the others...
Changed the wording of `FIXME`.
> I think what we really need here is the ability to weight clang::clangd:: >
> clang:: > `` when you're in the scope of namespace clangd.
It's unclear what this would mean for scopes from `using-namespace` directives.
But we can revisit when we get there.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
ioeric updated this revision to Diff 167135.
ioeric marked 14 inline comments as done.
ioeric added a comment.
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -554,6 +554,17 @@
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
+TEST(DexTest, WildcardScope) {
+ auto I =
+ Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), URISchemes);
+ FuzzyFindRequest Req;
+ Req.Query = "y";
+ Req.Scopes = {"a::"};
+ Req.AnyScope = true;
+ EXPECT_THAT(match(*I, Req),
+ UnorderedElementsAre("a::y1", "a::b::y2", "c::y3"));
+}
+
TEST(DexTest, IgnoreCases) {
auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), URISchemes);
FuzzyFindRequest Req;
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2040,6 +2040,58 @@
}
}
+TEST(CompletionTest, NoAllScopesCompletionWhenQualified) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+void f() { na::Clangd^ }
+ )cpp",
+ {cls("na::ClangdA"), cls("nx::ClangdX"), cls("Clangd3")}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(
+ AllOf(Qualifier(""), Scope("na::"), Named("ClangdA";
+}
+
+TEST(CompletionTest, AllScopesCompletion) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+namespace na {
+void f() { Clangd^ }
+}
+ )cpp",
+ {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"),
+ cls("na::nb::Clangd4")},
+ Opts);
+ EXPECT_THAT(
+ Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier("nx::"), Named("Clangd1")),
+ AllOf(Qualifier("ny::"), Named("Clangd2")),
+ AllOf(Qualifier(""), Scope(""), Named("Clangd3")),
+ AllOf(Qualifier("nb::"), Named("Clangd4";
+}
+
+TEST(CompletionTest, NoQualifierIfShadowed) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(R"cpp(
+namespace nx { class Clangd1 {}; }
+using nx::Clangd1;
+void f() { Clangd^ }
+ )cpp",
+ {cls("nx::Clangd1"), cls("nx::Clangd2")}, Opts);
+ // Although Clangd1 is from another namespace, Sema tells us it's in-scope and
+ // needs no qualifier.
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier(""), Named("Clangd1")),
+ AllOf(Qualifier("nx::"), Named("Clangd2";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -136,6 +136,15 @@
"enabled separatedly."),
llvm::cl::init(true), llvm::cl::Hidden);
+static llvm::cl::opt AllScopesCompletion(
+"all-scopes-completion",
+llvm::cl::desc(
+"If set to true, code completion will include index symbols that are "
+"not defined in the scopes (e.g. "
+"namespaces) visible from the code completion point. Such completions "
+"can insert scope qualifiers."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
static llvm::cl::opt
ShowOrigins("debug-origin",
llvm::cl::desc("Show origins of completion items"),
@@ -304,6 +313,7 @@
}
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+ CCOpts.AllScopes = AllScopesCompletion;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -12,6 +12,8 @@
#include "FuzzyMatch.h"
#include "Logger.h"
#include "Quality.h"
+#include "index/Index.h"
+#include "index/dex/Iterator.h"
#include "llvm/ADT/StringSet.h"
#include
#include
@@ -166,6 +168,9 @@
if (It != InvertedIndex.end())
ScopeIterators.push_back(It->second.iterator());
}
+ if (Req.AnyScope)
+ScopeIterators.push_back(createBoost(createTrue(Symbols.size()), 0.2));
+
// Add OR iterator for scopes if there are any Scope Iterators.
if (!ScopeIterators.empty())
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
sammccall added a comment.
This is really cool!
From reading the code this behaves exactly how I'd expect.
Ranking will be the real test.
Main comment is I'd like to tweak the interface on FuzzyFindRequest, rest is
basically nits.
Comment at: clangd/CodeComplete.cpp:367
if (C.IndexResult) {
- Completion.Origin |= C.IndexResult->Origin;
+ const auto = *C.IndexResult;
+ Completion.Origin |= Sym.Origin;
FWIW, I find "C.IndexResult" clearer than "Sym" here, the "index" part is
relevant
Comment at: clangd/CodeComplete.cpp:375
+Completion.Name = Sym.Name;
+ // To avoid inserting unnecessary qualifiers (e.g. no need for qualifier
+ // when symbol is accessed via using shadow like "using ns::X;"), only
Consider inverting this comment, I found it a bit hard to unpack:
```
// If the completion was visible to Sema, no qualifier is needed.
// This avoids unneeded qualifiers in cases like with `using ns::X`.
```
Comment at: clangd/CodeComplete.cpp:378
+ // insert qualifier if the symbol is not already available form Sema.
+ // FIXME(ioeric): find a better way to avoid inserting redundant
+ // qualifiers.
I'm not sure this FIXME is important enough to ever get fixed, consider
removing it.
Comment at: clangd/CodeComplete.h:105
+
+ /// Whether to include index symbols that are not defined in the scopes
+ /// visible from the code completion point (e.g. enclosing namespaces). Such
This comment should also mention that this applies in contexts without explicit
scope qualifiers.
Comment at: clangd/CodeComplete.h:106
+ /// Whether to include index symbols that are not defined in the scopes
+ /// visible from the code completion point (e.g. enclosing namespaces). Such
+ /// completions can insert scope qualifiers.
nit: I'd remove the parenthetical, it's a little ambiguous whether it applies
to the clause inside the "not" or outside it.
Comment at: clangd/CodeComplete.h:108
+ /// completions can insert scope qualifiers.
+ bool IncludeIndexSymbolsFromAllScopes = false;
};
what about just calling this `AllScopes`?
Comment at: clangd/index/Index.h:430
///
- /// The global scope is "", a top level scope is "foo::", etc.
+ /// The global scope is "", a top level scope is "foo::", etc. "*" is
+ /// wildcard.
I'm not a big fan of this magic value, it seems too easy to forget to handle.
Especially since we have a lot of existing code that touches this interface,
and we may forget to check some of it.
I suggest making this a separate boolean field `AnyScope`, with a comment that
scopes explicitly listed will be ranked higher.
We can probably also remove the "If this is empty" special case from `Scopes`
now too, as the old behavior can be achieved by setting `Scopes = {}; AnyScope
= true;`
Comment at: clangd/index/Index.h:432
+ /// wildcard.
+ /// FIXME: support assigning different weight to each scope.
std::vector Scopes;
May not want a heavyweight API with explicit weights...
I think what we really **need** here is the ability to weight `clang::clangd::`
> `clang::` > `` when you're in the scope of namespace clangd.
We could just say something like "the primary scope should come first" and do
some FileDistance-like penalizing of the others...
Comment at: clangd/index/dex/Dex.cpp:161
+if (Scope == "*") {
+ ScopeIterators.push_back(createTrue(Symbols.size()));
+ continue;
wrap in a `createBoost(..., 0.1)` or so?
Comment at: clangd/tool/ClangdMain.cpp:139
+static llvm::cl::opt CrossNamespaceCompletion(
+"cross-namespace-completion",
It's a little confusing to give these different internal vs external names - do
we have a strong reason not to call this "all-scopes-completion" or so?
Comment at: clangd/tool/ClangdMain.cpp:142
+llvm::cl::desc(
+"This is an experimental feature. If set to true, code completion will
"
+"include index symbols that are not defined in the scopes (e.g. "
I'm not sure whether putting "experimental" in every flag description is
worthwhile - maybe hidden is enough?
Comment at: unittests/clangd/CodeCompleteTests.cpp:2043
+TEST(CompletionTest, CrossNamespaceCompletion) {
+ clangd::CodeCompleteOptions Opts = {};
also verify that `na::Clangd^` only gets one result?
Comment at: unittests/clangd/CodeCompleteTests.cpp:2059
+ AllOf(Qualifier("ny::"), Named("Clangd2")),
+ AllOf(Qualifier(""), Named("Clangd3")),
+ AllOf(Qualifier("nb::"),
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
When no scope qualifier is specified, allow completing index symbols
from any scope and insert proper automatically. This is still experimental and
hidden behind a flag.
Things missing:
- Scope proximity based scoring.
- FuzzyFind supports weighted scopes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -565,6 +565,16 @@
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
+TEST(DexTest, WildcardScope) {
+ auto I =
+ Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), URISchemes);
+ FuzzyFindRequest Req;
+ Req.Query = "y";
+ Req.Scopes = {"a::", "*"};
+ EXPECT_THAT(match(*I, Req),
+ UnorderedElementsAre("a::y1", "a::b::y2", "c::y3"));
+}
+
TEST(DexTest, IgnoreCases) {
auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), URISchemes);
FuzzyFindRequest Req;
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2040,6 +2040,41 @@
}
}
+TEST(CompletionTest, CrossNamespaceCompletion) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.IncludeIndexSymbolsFromAllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+namespace na {
+void f() { Clangd^ }
+}
+ )cpp",
+ {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"),
+ cls("na::nb::Clangd4")},
+ Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier("nx::"), Named("Clangd1")),
+ AllOf(Qualifier("ny::"), Named("Clangd2")),
+ AllOf(Qualifier(""), Named("Clangd3")),
+ AllOf(Qualifier("nb::"), Named("Clangd4";
+}
+
+TEST(CompletionTest, NoQualifierIfShadowed) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.IncludeIndexSymbolsFromAllScopes = true;
+
+ auto Results = completions(R"cpp(
+namespace nx { class Clangd1 {}; }
+using nx::Clangd1;
+void f() { Clangd^ }
+ )cpp",
+ {cls("nx::Clangd1"), cls("nx::Clangd2")}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier(""), Named("Clangd1")),
+ AllOf(Qualifier("nx::"), Named("Clangd2";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -136,6 +136,15 @@
"enabled separatedly."),
llvm::cl::init(true), llvm::cl::Hidden);
+static llvm::cl::opt CrossNamespaceCompletion(
+"cross-namespace-completion",
+llvm::cl::desc(
+"This is an experimental feature. If set to true, code completion will "
+"include index symbols that are not defined in the scopes (e.g. "
+"namespaces) visible from the code completion point. Such completions "
+"can insert scope qualifiers."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
static llvm::cl::opt
ShowOrigins("debug-origin",
llvm::cl::desc("Show origins of completion items"),
@@ -304,6 +313,7 @@
}
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+ CCOpts.IncludeIndexSymbolsFromAllScopes = CrossNamespaceCompletion;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -12,6 +12,7 @@
#include "FuzzyMatch.h"
#include "Logger.h"
#include "Quality.h"
+#include "index/Index.h"
#include "llvm/ADT/StringSet.h"
#include
#include
@@ -156,6 +157,10 @@
// Generate scope tokens for search query.
std::vector> ScopeIterators;
for (const auto : Req.Scopes) {
+if (Scope == "*") {
+ ScopeIterators.push_back(createTrue(Symbols.size()));
+ continue;
+}
const auto It = InvertedIndex.find(Token(Token::Kind::Scope, Scope));
if (It != InvertedIndex.end())
ScopeIterators.push_back(It->second.iterator());
Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++
