Skip to content

Commit

Permalink
[clangd] Initial supoprt for cross-namespace global code completion.
Browse files Browse the repository at this point in the history
Summary:
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.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: kbobyrev, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D52364

llvm-svn: 343248
  • Loading branch information
Eric Liu committed Sep 27, 2018
1 parent ee7fe93 commit 670c147
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 15 deletions.
51 changes: 37 additions & 14 deletions clang-tools-extra/clangd/CodeComplete.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -330,6 +331,7 @@ struct ScoredBundleGreater {
struct CodeCompletionBuilder {
CodeCompletionBuilder(ASTContext &ASTCtx, const CompletionCandidate &C,
CodeCompletionString *SemaCCS,
llvm::ArrayRef<std::string> QueryScopes,
const IncludeInserter &Includes, StringRef FileName,
CodeCompletionContext::Kind ContextKind,
const CodeCompleteOptions &Opts)
Expand Down Expand Up @@ -374,6 +376,18 @@ struct CodeCompletionBuilder {
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);
}

Expand Down Expand Up @@ -604,9 +618,11 @@ struct SpecifiedScope {
}
};

// Get all scopes that will be queried in indexes.
std::vector<std::string> getQueryScopes(CodeCompletionContext &CCContext,
const SourceManager &SM) {
// Get all scopes that will be queried in indexes and whether symbols from
// any scope is allowed.
std::pair<std::vector<std::string>, bool>
getQueryScopes(CodeCompletionContext &CCContext, const SourceManager &SM,
const CodeCompleteOptions &Opts) {
auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
SpecifiedScope Info;
for (auto *Context : CCContext.getVisitedContexts()) {
Expand All @@ -627,13 +643,15 @@ std::vector<std::string> getQueryScopes(CodeCompletionContext &CCContext,
// 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.
Expand All @@ -651,7 +669,7 @@ std::vector<std::string> getQueryScopes(CodeCompletionContext &CCContext,
if (!Info.UnresolvedQualifier->empty())
*Info.UnresolvedQualifier += "::";

return Info.scopesForIndexQuery();
return {Info.scopesForIndexQuery(), false};
}

// Should we perform index-based completion in a context of the specified kind?
Expand Down Expand Up @@ -1262,8 +1280,10 @@ class CodeCompleteFlow {
CompletionRecorder *Recorder = nullptr;
int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
bool Incomplete = false; // Would more be available with a higher limit?
llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
std::vector<std::string> QueryScopes; // Initialized once Sema runs.
llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
std::vector<std::string> QueryScopes; // Initialized once Sema runs.
// Whether to query symbols from any scope. Initialized once Sema runs.
bool AllScopes = false;
// Include-insertion and proximity scoring rely on the include structure.
// This is available after Sema has run.
llvm::Optional<IncludeInserter> Inserter; // Available during runWithSema.
Expand Down Expand Up @@ -1339,9 +1359,9 @@ class CodeCompleteFlow {
Inserter.reset(); // Make sure this doesn't out-live Clang.
SPAN_ATTACH(Tracer, "sema_completion_kind",
getCompletionKindString(Recorder->CCContext.getKind()));
log("Code complete: sema context {0}, query scopes [{1}]",
log("Code complete: sema context {0}, query scopes [{1}] (AnyScope={2})",
getCompletionKindString(Recorder->CCContext.getKind()),
llvm::join(QueryScopes.begin(), QueryScopes.end(), ","));
llvm::join(QueryScopes.begin(), QueryScopes.end(), ","), AllScopes);
});

Recorder = RecorderOwner.get();
Expand Down Expand Up @@ -1387,8 +1407,8 @@ class CodeCompleteFlow {
}
Filter = FuzzyMatcher(
Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
QueryScopes = getQueryScopes(Recorder->CCContext,
Recorder->CCSema->getSourceManager());
std::tie(QueryScopes, AllScopes) = getQueryScopes(
Recorder->CCContext, Recorder->CCSema->getSourceManager(), Opts);
// Sema provides the needed context to query the index.
// FIXME: in addition to querying for extra/overlapping symbols, we should
// explicitly request symbols corresponding to Sema results.
Expand Down Expand Up @@ -1428,6 +1448,7 @@ class CodeCompleteFlow {
Req.Query = Filter->pattern();
Req.RestrictForCodeCompletion = true;
Req.Scopes = QueryScopes;
Req.AnyScope = AllScopes;
// FIXME: we should send multiple weighted paths here.
Req.ProximityPaths.push_back(FileName);
vlog("Code complete: fuzzyFind({0:2})", toJSON(Req));
Expand Down Expand Up @@ -1538,6 +1559,8 @@ class CodeCompleteFlow {
Relevance.Context = Recorder->CCContext.getKind();
Relevance.Query = SymbolRelevanceSignals::CodeComplete;
Relevance.FileProximityMatch = FileProximity.getPointer();
// FIXME: incorparate scope proximity into relevance score.

auto &First = Bundle.front();
if (auto FuzzyScore = fuzzyScore(First))
Relevance.NameMatch = *FuzzyScore;
Expand Down Expand Up @@ -1587,8 +1610,8 @@ class CodeCompleteFlow {
: nullptr;
if (!Builder)
Builder.emplace(Recorder->CCSema->getASTContext(), Item, SemaCCS,
*Inserter, FileName, Recorder->CCContext.getKind(),
Opts);
QueryScopes, *Inserter, FileName,
Recorder->CCContext.getKind(), Opts);
else
Builder->add(Item, SemaCCS);
}
Expand Down
7 changes: 7 additions & 0 deletions clang-tools-extra/clangd/CodeComplete.h
Expand Up @@ -101,6 +101,13 @@ struct CodeCompleteOptions {
/// Whether to generate snippets for function arguments on code-completion.
/// Needs snippets to be enabled as well.
bool EnableFunctionArgSnippets = true;

/// Whether to include index symbols that are not defined in the scopes
/// visible from the code completion point. This applies in contexts without
/// explicit scope qualifiers.
///
/// Such completions can insert scope qualifiers.
bool AllScopes = false;
};

// Semi-structured representation of a code-complete suggestion for our C++ API.
Expand Down
6 changes: 6 additions & 0 deletions clang-tools-extra/clangd/index/Index.h
Expand Up @@ -428,7 +428,13 @@ struct FuzzyFindRequest {
/// namespace xyz::abc.
///
/// The global scope is "", a top level scope is "foo::", etc.
/// FIXME: drop the special case for empty list, which is the same as
/// `AnyScope = true`.
/// FIXME: support scope proximity.
std::vector<std::string> Scopes;
/// If set to true, allow symbols from any scope. Scopes explicitly listed
/// above will be ranked higher.
bool AnyScope = false;
/// \brief The number of top candidates to return. The index may choose to
/// return more than this, e.g. if it doesn't know which candidates are best.
llvm::Optional<uint32_t> Limit;
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clangd/index/MemIndex.cpp
Expand Up @@ -39,7 +39,8 @@ bool MemIndex::fuzzyFind(
const Symbol *Sym = Pair.second;

// Exact match against all possible scopes.
if (!Req.Scopes.empty() && !llvm::is_contained(Req.Scopes, Sym->Scope))
if (!Req.AnyScope && !Req.Scopes.empty() &&
!llvm::is_contained(Req.Scopes, Sym->Scope))
continue;
if (Req.RestrictForCodeCompletion &&
!(Sym->Flags & Symbol::IndexedForCodeCompletion))
Expand Down
6 changes: 6 additions & 0 deletions clang-tools-extra/clangd/index/dex/Dex.cpp
Expand Up @@ -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 <algorithm>
#include <queue>
Expand Down Expand Up @@ -166,6 +168,10 @@ bool Dex::fuzzyFind(const FuzzyFindRequest &Req,
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 Iterators.
if (!ScopeIterators.empty())
TopLevelChildren.push_back(createOr(move(ScopeIterators)));
Expand Down
10 changes: 10 additions & 0 deletions clang-tools-extra/clangd/tool/ClangdMain.cpp
Expand Up @@ -136,6 +136,15 @@ static llvm::cl::opt<bool> EnableIndex(
"enabled separatedly."),
llvm::cl::init(true), llvm::cl::Hidden);

static llvm::cl::opt<bool> 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<bool>
ShowOrigins("debug-origin",
llvm::cl::desc("Show origins of completion items"),
Expand Down Expand Up @@ -304,6 +313,7 @@ int main(int argc, char *argv[]) {
}
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
CCOpts.AllScopes = AllScopesCompletion;

// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
Expand Down
51 changes: 51 additions & 0 deletions clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
Expand Up @@ -2093,6 +2093,57 @@ TEST(CompletionTest, IncludedCompletionKinds) {
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
Expand Down
11 changes: 11 additions & 0 deletions clang-tools-extra/unittests/clangd/DexTests.cpp
Expand Up @@ -523,6 +523,17 @@ TEST(DexTest, NoMatchNestedScopes) {
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;
Expand Down

0 comments on commit 670c147

Please sign in to comment.