Skip to content

Commit

Permalink
[clangd] Treat class constructor as in the same scope as the class in…
Browse files Browse the repository at this point in the history
… ranking.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits

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

llvm-svn: 336318
  • Loading branch information
Eric Liu committed Jul 5, 2018
1 parent 592718f commit 8944f0e
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 7 deletions.
5 changes: 5 additions & 0 deletions clang-tools-extra/clangd/Quality.cpp
Expand Up @@ -11,10 +11,12 @@
#include "URI.h"
#include "index/Index.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclVisitor.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Sema/CodeCompleteConsumer.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
Expand Down Expand Up @@ -195,6 +197,9 @@ ComputeScope(const NamedDecl *D) {
if (auto *R = dyn_cast_or_null<RecordDecl>(D))
if (R->isInjectedClassName())
DC = DC->getParent();
// Class constructor should have the same scope as the class.
if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(D))
DC = DC->getParent();
bool InClass = false;
for (; !DC->isFileContext(); DC = DC->getParent()) {
if (DC->isFunctionOrMethod())
Expand Down
28 changes: 28 additions & 0 deletions clang-tools-extra/unittests/clangd/QualityTests.cpp
Expand Up @@ -21,6 +21,9 @@
#include "Quality.h"
#include "TestFS.h"
#include "TestTU.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "llvm/Support/Casting.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

Expand Down Expand Up @@ -199,6 +202,31 @@ TEST(QualityTests, SortText) {
EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
}

TEST(QualityTests, NoBoostForClassConstructor) {
auto Header = TestTU::withHeaderCode(R"cpp(
class Foo {
public:
Foo(int);
};
)cpp");
auto Symbols = Header.headerSymbols();
auto AST = Header.build();

const NamedDecl *Foo = &findDecl(AST, "Foo");
SymbolRelevanceSignals Cls;
Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));

const NamedDecl *CtorDecl = &findAnyDecl(AST, [](const NamedDecl &ND) {
return (ND.getQualifiedNameAsString() == "Foo::Foo") &&
llvm::isa<CXXConstructorDecl>(&ND);
});
SymbolRelevanceSignals Ctor;
Ctor.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));

EXPECT_EQ(Cls.Scope, SymbolRelevanceSignals::GlobalScope);
EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
}

} // namespace
} // namespace clangd
} // namespace clang
23 changes: 16 additions & 7 deletions clang-tools-extra/unittests/clangd/TestTU.cpp
Expand Up @@ -93,26 +93,35 @@ const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
return LookupDecl(*Scope, Components.back());
}

const NamedDecl &findAnyDecl(ParsedAST &AST, llvm::StringRef Name) {
const NamedDecl &findAnyDecl(ParsedAST &AST,
std::function<bool(const NamedDecl &)> Callback) {
struct Visitor : RecursiveASTVisitor<Visitor> {
llvm::StringRef Name;
decltype(Callback) CB;
llvm::SmallVector<const NamedDecl *, 1> Decls;
bool VisitNamedDecl(const NamedDecl *ND) {
if (auto *ID = ND->getIdentifier())
if (ID->getName() == Name)
Decls.push_back(ND);
if (CB(*ND))
Decls.push_back(ND);
return true;
}
} Visitor;
Visitor.Name = Name;
Visitor.CB = Callback;
for (Decl *D : AST.getLocalTopLevelDecls())
Visitor.TraverseDecl(D);
if (Visitor.Decls.size() != 1) {
ADD_FAILURE() << Visitor.Decls.size() << " symbols named " << Name;
ADD_FAILURE() << Visitor.Decls.size() << " symbols matched.";
assert(Visitor.Decls.size() == 1);
}
return *Visitor.Decls.front();
}

const NamedDecl &findAnyDecl(ParsedAST &AST, llvm::StringRef Name) {
return findAnyDecl(AST, [Name](const NamedDecl &ND) {
if (auto *ID = ND.getIdentifier())
if (ID->getName() == Name)
return true;
return false;
});
}

} // namespace clangd
} // namespace clang
3 changes: 3 additions & 0 deletions clang-tools-extra/unittests/clangd/TestTU.h
Expand Up @@ -56,6 +56,9 @@ struct TestTU {
const Symbol &findSymbol(const SymbolSlab &, llvm::StringRef QName);
// Look up an AST symbol by qualified name, which must be unique and top-level.
const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName);
// Look up a main-file AST symbol that satisfies \p Filter.
const NamedDecl &findAnyDecl(ParsedAST &AST,
std::function<bool(const NamedDecl &)> Filter);
// Look up a main-file AST symbol by unqualified name, which must be unique.
const NamedDecl &findAnyDecl(ParsedAST &AST, llvm::StringRef Name);

Expand Down

0 comments on commit 8944f0e

Please sign in to comment.