omtcyfz created this revision.
omtcyfz added reviewers: ioeric, sammccall, ilya-biryukov.
omtcyfz added a project: clang-tools-extra.
Herald added subscribers: jkorous, MaskRay.
Having `using qualified::name;` for some symbol is an important signal for
clangd code completion as the user is more likely to use such symbol. This
patch helps to uprank the relevant symbols by saving UsingShadowDecl in the new
field of CodeCompletionResult and checking whether the corresponding
UsingShadowDecl is located in the main file later in ClangD code completion
routine. While the relative importance of such signal is a subject to change in
the future, this patch simply bumps DeclProximity score to the value of 1.0
which should be enough for now.
The patch was tested using
$ ninja check-clang check-clang-tools
No unexpected failures were noticed after running the relevant testsets.
https://reviews.llvm.org/D49012
Files:
clang-tools-extra/clangd/Quality.cpp
clang-tools-extra/unittests/clangd/QualityTests.cpp
clang-tools-extra/unittests/clangd/TestTU.cpp
clang/include/clang/Sema/CodeCompleteConsumer.h
clang/lib/Sema/SemaCodeComplete.cpp
Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -859,12 +859,12 @@
}
// Look through using declarations.
- if (const UsingShadowDecl *Using =
- dyn_cast<UsingShadowDecl>(R.Declaration)) {
- MaybeAddResult(Result(Using->getTargetDecl(),
- getBasePriority(Using->getTargetDecl()),
- R.Qualifier),
- CurContext);
+ if (const UsingShadowDecl *Using = dyn_cast<UsingShadowDecl>(R.Declaration)) {
+ CodeCompletionResult Result(Using->getTargetDecl(),
+ getBasePriority(Using->getTargetDecl()),
+ R.Qualifier);
+ Result.setShadowDecl(Using);
+ MaybeAddResult(Result, CurContext);
return;
}
@@ -977,10 +977,11 @@
// Look through using declarations.
if (const UsingShadowDecl *Using = dyn_cast<UsingShadowDecl>(R.Declaration)) {
- AddResult(Result(Using->getTargetDecl(),
- getBasePriority(Using->getTargetDecl()),
- R.Qualifier),
- CurContext, Hiding);
+ CodeCompletionResult Result(Using->getTargetDecl(),
+ getBasePriority(Using->getTargetDecl()),
+ R.Qualifier);
+ Result.setShadowDecl(Using);
+ AddResult(Result, CurContext, Hiding);
return;
}
@@ -1004,10 +1005,10 @@
if (AsNestedNameSpecifier) {
R.StartsNestedNameSpecifier = true;
R.Priority = CCP_NestedNameSpecifier;
- }
- else if (Filter == &ResultBuilder::IsMember && !R.Qualifier && InBaseClass &&
- isa<CXXRecordDecl>(R.Declaration->getDeclContext()
- ->getRedeclContext()))
+ } else if (Filter == &ResultBuilder::IsMember && !R.Qualifier &&
+ InBaseClass &&
+ isa<CXXRecordDecl>(
+ R.Declaration->getDeclContext()->getRedeclContext()))
R.QualifierIsInformative = true;
// If this result is supposed to have an informative qualifier, add one.
Index: clang/include/clang/Sema/CodeCompleteConsumer.h
===================================================================
--- clang/include/clang/Sema/CodeCompleteConsumer.h
+++ clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -45,8 +45,9 @@
class NamedDecl;
class NestedNameSpecifier;
class Preprocessor;
-class Sema;
class RawComment;
+class Sema;
+class UsingShadowDecl;
/// Default priority values for code-completion results based
/// on their kind.
@@ -836,18 +837,25 @@
/// informative rather than required.
NestedNameSpecifier *Qualifier = nullptr;
+ /// If this Decl was unshadowed by using declaration, this would would store
+ /// a pointer to the UsingShadowDecl which was used in the unshadowing
+ /// process. This information can be used to uprank CodeCompletionResults
+ /// which have corresponding `using decl::qualified::name;` nearby.
+ const UsingShadowDecl *ShadowDecl = nullptr;
+
/// Build a result that refers to a declaration.
CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority,
NestedNameSpecifier *Qualifier = nullptr,
bool QualifierIsInformative = false,
bool Accessible = true,
- std::vector<FixItHint> FixIts = std::vector<FixItHint>())
+ std::vector<FixItHint> FixIts = std::vector<FixItHint>(),
+ const UsingShadowDecl *ShadowDecl = nullptr)
: Declaration(Declaration), Priority(Priority), Kind(RK_Declaration),
FixIts(std::move(FixIts)), Hidden(false),
QualifierIsInformative(QualifierIsInformative),
StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
- DeclaringEntity(false), Qualifier(Qualifier) {
- //FIXME: Add assert to check FixIts range requirements.
+ DeclaringEntity(false), Qualifier(Qualifier), ShadowDecl(ShadowDecl) {
+ // FIXME: Add assert to check FixIts range requirements.
computeCursorKindAndAvailability(Accessible);
}
@@ -900,6 +908,10 @@
return Keyword;
}
+ void setShadowDecl(const UsingShadowDecl *Shadow) {
+ this->ShadowDecl = Shadow;
+ }
+
/// Create a new code-completion string that describes how to insert
/// this result into a program.
///
Index: clang-tools-extra/unittests/clangd/TestTU.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/TestTU.cpp
+++ clang-tools-extra/unittests/clangd/TestTU.cpp
@@ -72,6 +72,7 @@
return *Result;
}
+// Finds Decl in AST matching fully qualified name.
const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
llvm::SmallVector<llvm::StringRef, 4> Components;
QName.split(Components, "::");
@@ -114,6 +115,7 @@
return *Visitor.Decls.front();
}
+// Finds any Decl having given unqualified name.
const NamedDecl &findAnyDecl(ParsedAST &AST, llvm::StringRef Name) {
return findAnyDecl(AST, [Name](const NamedDecl &ND) {
if (auto *ID = ND.getIdentifier())
Index: clang-tools-extra/unittests/clangd/QualityTests.cpp
===================================================================
--- clang-tools-extra/unittests/clangd/QualityTests.cpp
+++ clang-tools-extra/unittests/clangd/QualityTests.cpp
@@ -79,8 +79,18 @@
Test.HeaderCode = R"cpp(
int header();
int header_main();
+
+ namespace foo {
+ class Bar {};
+ } // namespace foo
+
+ namespace bar {
+ class Foo {};
+ } // namespace bar
)cpp";
Test.Code = R"cpp(
+ using foo::Bar;
+
int ::header_main() {}
int main();
@@ -109,7 +119,36 @@
Relevance = {};
Relevance.merge(CodeCompletionResult(&findDecl(AST, "header_main"), 42));
EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
- << "Current file and header";
+ << "Current file and definition in header";
+
+ Relevance = {};
+ const auto SymbolName = "Bar";
+ const NamedDecl *FoundDecl;
+ UsingShadowDecl *Shadow;
+ auto findUsingDecl = [&SymbolName, &Shadow,
+ &FoundDecl](const NamedDecl &ND) -> bool {
+ if (const UsingDecl *Using = dyn_cast<UsingDecl>(&ND)) {
+ if (UsingShadowDecl *ShadowDecl = *Using->shadow_begin()) {
+ if (ShadowDecl->getTargetDecl()->getName() == SymbolName) {
+ Shadow = ShadowDecl;
+ FoundDecl = Shadow->getTargetDecl();
+ return true;
+ }
+ }
+ }
+ return false;
+ };
+ findAnyDecl(AST, findUsingDecl);
+ CodeCompletionResult Result(FoundDecl, 42);
+ Result.setShadowDecl(Shadow);
+ Relevance.merge(Result);
+ EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
+ << "Uprank declaration which has using directive in current directory";
+
+ Relevance = {};
+ Relevance.merge(CodeCompletionResult(&findDecl(AST, "bar::Foo"), 42));
+ EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 0.6f)
+ << "Only definition in header";
Relevance = {};
Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "X"), 42));
Index: clang-tools-extra/clangd/Quality.cpp
===================================================================
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -41,6 +41,17 @@
return false;
}
+static bool hasUsingDeclInMainFile(const CodeCompletionResult &R) {
+ const auto &Context = R.Declaration->getASTContext();
+ const auto &SourceMgr = Context.getSourceManager();
+ if (R.ShadowDecl) {
+ const auto Loc = SourceMgr.getSpellingLoc(R.ShadowDecl->getLocation());
+ if (SourceMgr.isWrittenInMainFile(Loc))
+ return true;
+ }
+ return false;
+}
+
static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) {
class Switch
: public ConstDeclVisitor<Switch, SymbolQualitySignals::SymbolCategory> {
@@ -231,8 +242,10 @@
// We boost things that have decls in the main file. We give a fixed score
// for all other declarations in sema as they are already included in the
// translation unit.
- float DeclProximity =
- hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.6;
+ float DeclProximity = (hasDeclInMainFile(*SemaCCResult.Declaration) ||
+ hasUsingDeclInMainFile(SemaCCResult))
+ ? 1.0
+ : 0.6;
SemaProximityScore = std::max(DeclProximity, SemaProximityScore);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
