[clang-tools-extra] r329571 - [clangd] Adapt index interfaces to D45014, and fix the old bugs.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 9 07:28:52 PDT 2018
Author: sammccall
Date: Mon Apr 9 07:28:52 2018
New Revision: 329571
URL: http://llvm.org/viewvc/llvm-project?rev=329571&view=rev
Log:
[clangd] Adapt index interfaces to D45014, and fix the old bugs.
Summary:
Fix bugs:
- don't count occurrences of decls where we don't spell the name
- findDefinitions at MACRO(^X) goes to the definition of MACRO
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits
Differential Revision: https://reviews.llvm.org/D45356
Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=329571&r1=329570&r2=329571&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Apr 9 07:28:52 2018
@@ -79,10 +79,10 @@ public:
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
- ArrayRef<index::SymbolRelation> Relations, FileID FID,
- unsigned Offset,
+ ArrayRef<index::SymbolRelation> Relations,
+ SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
- if (isSearchedLocation(FID, Offset)) {
+ if (Loc == SearchedLocation) {
// Find and add definition declarations (for GoToDefinition).
// We don't use parameter `D`, as Parameter `D` is the canonical
// declaration, which is the first declaration of a redeclarable
@@ -98,18 +98,12 @@ public:
}
private:
- bool isSearchedLocation(FileID FID, unsigned Offset) const {
- const SourceManager &SourceMgr = AST.getSourceManager();
- return SourceMgr.getFileOffset(SearchedLocation) == Offset &&
- SourceMgr.getFileID(SearchedLocation) == FID;
- }
-
void finish() override {
// Also handle possible macro at the searched location.
Token Result;
auto &Mgr = AST.getSourceManager();
- if (!Lexer::getRawToken(SearchedLocation, Result, Mgr, AST.getLangOpts(),
- false)) {
+ if (!Lexer::getRawToken(Mgr.getSpellingLoc(SearchedLocation), Result, Mgr,
+ AST.getLangOpts(), false)) {
if (Result.is(tok::raw_identifier)) {
PP.LookUpIdentifierInfo(Result);
}
@@ -127,16 +121,7 @@ private:
MacroInfo *MacroInf = MacroDef.getMacroInfo();
if (MacroInf) {
MacroInfos.push_back(MacroDecl{IdentifierInfo->getName(), MacroInf});
- // Clear all collected delcarations if this is a macro search.
- //
- // In theory, there should be no declarataions being collected when we
- // search a source location that refers to a macro.
- // The occurrence location returned by `handleDeclOccurence` is
- // limited (FID, Offset are from expansion location), we will collect
- // all declarations inside the macro.
- //
- // FIXME: Avoid adding decls from inside macros in handlDeclOccurence.
- Decls.clear();
+ assert(Decls.empty());
}
}
}
@@ -252,21 +237,19 @@ public:
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
- ArrayRef<index::SymbolRelation> Relations, FileID FID,
- unsigned Offset,
+ ArrayRef<index::SymbolRelation> Relations,
+ SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
const SourceManager &SourceMgr = AST.getSourceManager();
- if (SourceMgr.getMainFileID() != FID ||
+ SourceLocation HighlightStartLoc = SourceMgr.getFileLoc(Loc);
+ if (SourceMgr.getMainFileID() != SourceMgr.getFileID(HighlightStartLoc) ||
std::find(Decls.begin(), Decls.end(), D) == Decls.end()) {
return true;
}
SourceLocation End;
const LangOptions &LangOpts = AST.getLangOpts();
- SourceLocation StartOfFileLoc = SourceMgr.getLocForStartOfFile(FID);
- SourceLocation HightlightStartLoc = StartOfFileLoc.getLocWithOffset(Offset);
- End =
- Lexer::getLocForEndOfToken(HightlightStartLoc, 0, SourceMgr, LangOpts);
- SourceRange SR(HightlightStartLoc, End);
+ End = Lexer::getLocForEndOfToken(HighlightStartLoc, 0, SourceMgr, LangOpts);
+ SourceRange SR(HighlightStartLoc, End);
DocumentHighlightKind Kind = DocumentHighlightKind::Text;
if (static_cast<index::SymbolRoleSet>(index::SymbolRole::Write) & Roles)
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=329571&r1=329570&r2=329571&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Apr 9 07:28:52 2018
@@ -225,7 +225,7 @@ void SymbolCollector::initialize(ASTCont
// Always return true to continue indexing.
bool SymbolCollector::handleDeclOccurence(
const Decl *D, index::SymbolRoleSet Roles,
- ArrayRef<index::SymbolRelation> Relations, FileID FID, unsigned Offset,
+ ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) {
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
assert(CompletionAllocator && CompletionTUInfo);
@@ -235,9 +235,10 @@ bool SymbolCollector::handleDeclOccurenc
// Mark D as referenced if this is a reference coming from the main file.
// D may not be an interesting symbol, but it's cheaper to check at the end.
+ auto &SM = ASTCtx->getSourceManager();
if (Opts.CountReferences &&
(Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
- ASTCtx->getSourceManager().getMainFileID() == FID)
+ SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
ReferencedDecls.insert(ND);
// Don't continue indexing if this is a mere reference.
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=329571&r1=329570&r2=329571&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Mon Apr 9 07:28:52 2018
@@ -59,8 +59,8 @@ public:
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
- ArrayRef<index::SymbolRelation> Relations, FileID FID,
- unsigned Offset,
+ ArrayRef<index::SymbolRelation> Relations,
+ SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
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=329571&r1=329570&r2=329571&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon Apr 9 07:28:52 2018
@@ -250,6 +250,7 @@ TEST_F(SymbolCollectorTest, References)
class Y;
class Z {}; // not used anywhere
Y* y = nullptr; // used in header doesn't count
+ #define GLOBAL_Z(name) Z name;
)";
const std::string Main = R"(
W* w = nullptr;
@@ -258,6 +259,7 @@ TEST_F(SymbolCollectorTest, References)
class V;
V* v = nullptr; // Used, but not eligible for indexing.
class Y{}; // definition doesn't count as a reference
+ GLOBAL_Z(z); // Not a reference to Z, we don't spell the type.
)";
CollectorOpts.CountReferences = true;
runSymbolCollector(Header, Main);
Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=329571&r1=329570&r2=329571&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Mon Apr 9 07:28:52 2018
@@ -222,6 +222,18 @@ TEST(GoToDefinition, All) {
}
)cpp",
+ R"cpp(// Macro argument
+ int [[i]];
+ #define ADDRESSOF(X) &X;
+ int *j = ADDRESSOF(^i);
+ )cpp",
+
+ R"cpp(// Symbol concatenated inside macro (not supported)
+ int *pi;
+ #define POINTER(X) p # X;
+ int i = *POINTER(^i);
+ )cpp",
+
R"cpp(// Forward class declaration
class Foo;
class [[Foo]] {};
@@ -249,8 +261,11 @@ TEST(GoToDefinition, All) {
for (const char *Test : Tests) {
Annotations T(Test);
auto AST = build(T.code());
+ std::vector<Matcher<Location>> ExpectedLocations;
+ for (const auto &R : T.ranges())
+ ExpectedLocations.push_back(RangeIs(R));
EXPECT_THAT(findDefinitions(AST, T.point()),
- ElementsAre(RangeIs(T.range())))
+ ElementsAreArray(ExpectedLocations))
<< Test;
}
}
More information about the cfe-commits
mailing list