[clang-tools-extra] r346488 - [clangd] Make TestTU build with preamble, and fix the fallout.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 9 04:56:50 PST 2018
Author: sammccall
Date: Fri Nov 9 04:56:49 2018
New Revision: 346488
URL: http://llvm.org/viewvc/llvm-project?rev=346488&view=rev
Log:
[clangd] Make TestTU build with preamble, and fix the fallout.
Our testing didn't reflect reality: live clangd almost always uses a
preamble, and sometimes the preamble behaves differently.
This patch fixes a common test helper to be more realistic.
Preamble doesn't preserve information about which tokens come from the
command-line (this gets inlined into a source file). So remove logic
that attempts to treat symbols with such names differently.
A SymbolCollectorTest tries to verify that locals in headers are not
indexed, with preamble enabled this is only meaningful for locals of
auto-typed functions (otherwise the bodies aren't parsed).
Tests were relying on the fact that the findAnyDecl helper actually did expose
symbols from headers. Resolve by making all these functions consistently
able to find symbols in headers/preambles.
Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.h
Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=346488&r1=346487&r2=346488&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Fri Nov 9 04:56:49 2018
@@ -20,11 +20,9 @@ namespace clang {
namespace clangd {
// Returns true if the complete name of decl \p D is spelled in the source code.
-// This is not the case for
-// * symbols formed via macro concatenation, the spelling location will
-// be "<scratch space>"
-// * symbols controlled and defined by a compile command-line option
-// `-DName=foo`, the spelling location will be "<command line>".
+// This is not the case for symbols formed via macro concatenation.
+// (We used to attempt to treat names spelled on the command-line this way too,
+// but the preamble doesn't preserve the required information).
bool isSpelledInSourceCode(const Decl *D) {
const auto &SM = D->getASTContext().getSourceManager();
auto Loc = D->getLocation();
@@ -32,8 +30,7 @@ bool isSpelledInSourceCode(const Decl *D
// macros, we should use the location where the whole definition occurs.
if (Loc.isMacroID()) {
std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
- if (StringRef(PrintLoc).startswith("<scratch") ||
- StringRef(PrintLoc).startswith("<command line>"))
+ if (StringRef(PrintLoc).startswith("<scratch"))
return false;
}
return true;
Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=346488&r1=346487&r2=346488&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Fri Nov 9 04:56:49 2018
@@ -50,10 +50,7 @@ TEST(QualityTests, SymbolQualitySignalEx
#define DECL_NAME(x, y) x##_##y##_Decl
#define DECL(x, y) class DECL_NAME(x, y) {};
DECL(X, Y); // X_Y_Decl
-
- class MAC {};
)cpp");
- Header.ExtraArgs = {"-DMAC=mac_name"};
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
@@ -69,11 +66,6 @@ TEST(QualityTests, SymbolQualitySignalEx
Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
EXPECT_TRUE(Quality.ImplementationDetail);
- Quality.ImplementationDetail = false;
- Quality.merge(
- CodeCompletionResult(&findDecl(AST, "mac_name"), /*Priority=*/42));
- EXPECT_TRUE(Quality.ImplementationDetail);
-
Symbol F = findSymbol(Symbols, "_f");
F.References = 24; // TestTU doesn't count references, so fake it.
Quality = {};
@@ -148,17 +140,13 @@ TEST(QualityTests, SymbolRelevanceSignal
auto constructShadowDeclCompletionResult = [&](const std::string DeclName) {
auto *Shadow =
- *dyn_cast<UsingDecl>(
- &findAnyDecl(AST,
- [&](const NamedDecl &ND) {
- if (const UsingDecl *Using =
- dyn_cast<UsingDecl>(&ND))
- if (Using->shadow_size() &&
- Using->getQualifiedNameAsString() == DeclName)
- return true;
- return false;
- }))
- ->shadow_begin();
+ *dyn_cast<UsingDecl>(&findDecl(AST, [&](const NamedDecl &ND) {
+ if (const UsingDecl *Using = dyn_cast<UsingDecl>(&ND))
+ if (Using->shadow_size() &&
+ Using->getQualifiedNameAsString() == DeclName)
+ return true;
+ return false;
+ }))->shadow_begin();
CodeCompletionResult Result(Shadow->getTargetDecl(), 42);
Result.ShadowDecl = Shadow;
return Result;
@@ -173,13 +161,13 @@ TEST(QualityTests, SymbolRelevanceSignal
<< "Using declaration in main file";
Relevance = {};
- Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "X"), 42));
+ Relevance.merge(CodeCompletionResult(&findUnqualifiedDecl(AST, "X"), 42));
EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FileScope);
Relevance = {};
- Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "y"), 42));
+ Relevance.merge(CodeCompletionResult(&findUnqualifiedDecl(AST, "y"), 42));
EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::ClassScope);
Relevance = {};
- Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "z"), 42));
+ Relevance.merge(CodeCompletionResult(&findUnqualifiedDecl(AST, "z"), 42));
EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FunctionScope);
// The injected class name is treated as the outer class name.
Relevance = {};
@@ -188,7 +176,7 @@ TEST(QualityTests, SymbolRelevanceSignal
Relevance = {};
EXPECT_FALSE(Relevance.InBaseClass);
- auto BaseMember = CodeCompletionResult(&findAnyDecl(AST, "y"), 42);
+ auto BaseMember = CodeCompletionResult(&findUnqualifiedDecl(AST, "y"), 42);
BaseMember.InBaseClass = true;
Relevance.merge(BaseMember);
EXPECT_TRUE(Relevance.InBaseClass);
@@ -345,7 +333,7 @@ TEST(QualityTests, NoBoostForClassConstr
SymbolRelevanceSignals Cls;
Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));
- const NamedDecl *CtorDecl = &findAnyDecl(AST, [](const NamedDecl &ND) {
+ const NamedDecl *CtorDecl = &findDecl(AST, [](const NamedDecl &ND) {
return (ND.getQualifiedNameAsString() == "Foo::Foo") &&
isa<CXXConstructorDecl>(&ND);
});
@@ -407,7 +395,7 @@ TEST(QualityTests, ConstructorQuality) {
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
- const NamedDecl *CtorDecl = &findAnyDecl(AST, [](const NamedDecl &ND) {
+ const NamedDecl *CtorDecl = &findDecl(AST, [](const NamedDecl &ND) {
return (ND.getQualifiedNameAsString() == "Foo::Foo") &&
isa<CXXConstructorDecl>(&ND);
});
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=346488&r1=346487&r2=346488&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Fri Nov 9 04:56:49 2018
@@ -113,7 +113,7 @@ public:
bool shouldCollect(StringRef Name, bool Qualified = true) {
assert(AST.hasValue());
return SymbolCollector::shouldCollectSymbol(
- Qualified ? findDecl(*AST, Name) : findAnyDecl(*AST, Name),
+ Qualified ? findDecl(*AST, Name) : findUnqualifiedDecl(*AST, Name),
AST->getASTContext(), SymbolCollector::Options());
}
@@ -127,8 +127,8 @@ protected:
TEST_F(ShouldCollectSymbolTest, ShouldCollectSymbol) {
build(R"(
namespace nx {
- class X{}
- void f() { int Local; }
+ class X{};
+ auto f() { int Local; } // auto ensures function body is parsed.
struct { int x } var;
namespace { class InAnonymous {}; }
}
@@ -628,22 +628,6 @@ TEST_F(SymbolCollectorTest, SymbolFormed
DeclURI(TestHeaderURI))));
}
-TEST_F(SymbolCollectorTest, SymbolFormedByCLI) {
- Annotations Header(R"(
- #ifdef NAME
- class $expansion[[NAME]] {};
- #endif
- )");
-
- runSymbolCollector(Header.code(), /*Main=*/"",
- /*ExtraArgs=*/{"-DNAME=name"});
- EXPECT_THAT(Symbols,
- UnorderedElementsAre(AllOf(
- QName("name"),
- DeclRange(Header.range("expansion")),
- DeclURI(TestHeaderURI))));
-}
-
TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
const std::string Header = R"(
class Foo {};
Modified: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestTU.cpp?rev=346488&r1=346487&r2=346488&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp Fri Nov 9 04:56:49 2018
@@ -31,11 +31,20 @@ ParsedAST TestTU::build() const {
Cmd.push_back(FullHeaderName.c_str());
}
Cmd.insert(Cmd.end(), ExtraArgs.begin(), ExtraArgs.end());
- auto AST = ParsedAST::build(
- createInvocationFromCommandLine(Cmd), nullptr,
- MemoryBuffer::getMemBufferCopy(Code),
- std::make_shared<PCHContainerOperations>(),
- buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}}));
+ ParseInputs Inputs;
+ Inputs.CompileCommand.Filename = FullFilename;
+ Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+ Inputs.CompileCommand.Directory = testRoot();
+ Inputs.Contents = Code;
+ Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}});
+ auto PCHs = std::make_shared<PCHContainerOperations>();
+ auto Preamble =
+ buildPreamble(FullFilename, *createInvocationFromCommandLine(Cmd),
+ /*OldPreamble=*/nullptr,
+ /*OldCommand=*/Inputs.CompileCommand, Inputs, PCHs,
+ /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+ auto AST = buildAST(FullFilename, createInvocationFromCommandLine(Cmd),
+ Inputs, Preamble, PCHs);
if (!AST.hasValue()) {
ADD_FAILURE() << "Failed to build code:\n" << Code;
llvm_unreachable("Failed to build TestTU!");
@@ -99,8 +108,8 @@ const NamedDecl &findDecl(ParsedAST &AST
return LookupDecl(*Scope, Components.back());
}
-const NamedDecl &findAnyDecl(ParsedAST &AST,
- std::function<bool(const NamedDecl &)> Callback) {
+const NamedDecl &findDecl(ParsedAST &AST,
+ std::function<bool(const NamedDecl &)> Callback) {
struct Visitor : RecursiveASTVisitor<Visitor> {
decltype(Callback) CB;
SmallVector<const NamedDecl *, 1> Decls;
@@ -111,8 +120,7 @@ const NamedDecl &findAnyDecl(ParsedAST &
}
} Visitor;
Visitor.CB = Callback;
- for (Decl *D : AST.getLocalTopLevelDecls())
- Visitor.TraverseDecl(D);
+ Visitor.TraverseDecl(AST.getASTContext().getTranslationUnitDecl());
if (Visitor.Decls.size() != 1) {
ADD_FAILURE() << Visitor.Decls.size() << " symbols matched.";
assert(Visitor.Decls.size() == 1);
@@ -120,8 +128,8 @@ const NamedDecl &findAnyDecl(ParsedAST &
return *Visitor.Decls.front();
}
-const NamedDecl &findAnyDecl(ParsedAST &AST, StringRef Name) {
- return findAnyDecl(AST, [Name](const NamedDecl &ND) {
+const NamedDecl &findUnqualifiedDecl(ParsedAST &AST, StringRef Name) {
+ return findDecl(AST, [Name](const NamedDecl &ND) {
if (auto *ID = ND.getIdentifier())
if (ID->getName() == Name)
return true;
Modified: clang-tools-extra/trunk/unittests/clangd/TestTU.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestTU.h?rev=346488&r1=346487&r2=346488&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TestTU.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestTU.h Fri Nov 9 04:56:49 2018
@@ -58,11 +58,11 @@ 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);
+// Look up an AST symbol that satisfies \p Filter.
+const NamedDecl &findDecl(ParsedAST &AST,
+ std::function<bool(const NamedDecl &)> Filter);
+// Look up an AST symbol by unqualified name, which must be unique.
+const NamedDecl &findUnqualifiedDecl(ParsedAST &AST, llvm::StringRef Name);
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list