[clang-tools-extra] r323867 - [clangd] Better handling symbols defined in macros.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 31 04:56:52 PST 2018
Author: hokein
Date: Wed Jan 31 04:56:51 2018
New Revision: 323867
URL: http://llvm.org/viewvc/llvm-project?rev=323867&view=rev
Log:
[clangd] Better handling symbols defined in macros.
Summary:
For symbols defined inside macros:
* use expansion location, if the symbol is formed via macro concatenation.
* use spelling location, otherwise.
This will fix some symbols that have ill-format location (especial invalid filepath).
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D42575
Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
clang-tools-extra/trunk/unittests/clangd/Annotations.h
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
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=323867&r1=323866&r2=323867&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Jan 31 04:56:51 2018
@@ -112,6 +112,37 @@ bool shouldFilterDecl(const NamedDecl *N
return false;
}
+// Return the symbol location of the given declaration `D`.
+//
+// For symbols defined inside macros:
+// * use expansion location, if the symbol is formed via macro concatenation.
+// * use spelling location, otherwise.
+SymbolLocation GetSymbolLocation(const NamedDecl *D, SourceManager &SM,
+ StringRef FallbackDir,
+ std::string &FilePathStorage) {
+ SymbolLocation Location;
+
+ SourceLocation Loc = SM.getSpellingLoc(D->getLocation());
+ if (D->getLocation().isMacroID()) {
+ // The symbol is formed via macro concatenation, the spelling location will
+ // be "<scratch space>", which is not interesting to us, use the expansion
+ // location instead.
+ if (llvm::StringRef(Loc.printToString(SM)).startswith("<scratch")) {
+ FilePathStorage = makeAbsolutePath(
+ SM, SM.getFilename(SM.getExpansionLoc(D->getLocation())),
+ FallbackDir);
+ return {FilePathStorage,
+ SM.getFileOffset(SM.getExpansionRange(D->getLocStart()).first),
+ SM.getFileOffset(SM.getExpansionRange(D->getLocEnd()).second)};
+ }
+ }
+
+ FilePathStorage = makeAbsolutePath(SM, SM.getFilename(Loc), FallbackDir);
+ return {FilePathStorage,
+ SM.getFileOffset(SM.getSpellingLoc(D->getLocStart())),
+ SM.getFileOffset(SM.getSpellingLoc(D->getLocEnd()))};
+}
+
} // namespace
SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {}
@@ -149,17 +180,14 @@ bool SymbolCollector::handleDeclOccurenc
return true;
auto &SM = ND->getASTContext().getSourceManager();
- std::string FilePath = makeAbsolutePath(
- SM, SM.getFilename(D->getLocation()), Opts.FallbackDir);
- SymbolLocation Location = {FilePath, SM.getFileOffset(D->getLocStart()),
- SM.getFileOffset(D->getLocEnd())};
std::string QName = ND->getQualifiedNameAsString();
Symbol S;
S.ID = std::move(ID);
std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
S.SymInfo = index::getSymbolInfo(D);
- S.CanonicalDeclaration = Location;
+ std::string FilePath;
+ S.CanonicalDeclaration = GetSymbolLocation(ND, SM, Opts.FallbackDir, FilePath);
// Add completion info.
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.cpp?rev=323867&r1=323866&r2=323867&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/Annotations.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/Annotations.cpp Wed Jan 31 04:56:51 2018
@@ -83,5 +83,11 @@ std::vector<Range> Annotations::ranges(l
return {R.begin(), R.end()};
}
+std::pair<std::size_t, std::size_t>
+Annotations::offsetRange(llvm::StringRef Name) const {
+ auto R = range(Name);
+ return {positionToOffset(Code, R.start), positionToOffset(Code, R.end)};
+}
+
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.h?rev=323867&r1=323866&r2=323867&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/Annotations.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/Annotations.h Wed Jan 31 04:56:51 2018
@@ -58,6 +58,10 @@ public:
// Returns the location of all ranges marked by [[ ]] (or $name[[ ]]).
std::vector<Range> ranges(llvm::StringRef Name = "") const;
+ // The same to `range` method, but returns range in offsets [start, end).
+ std::pair<std::size_t, std::size_t>
+ offsetRange(llvm::StringRef Name = "") const;
+
private:
std::string Code;
llvm::StringMap<llvm::SmallVector<Position, 1>> Points;
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=323867&r1=323866&r2=323867&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Wed Jan 31 04:56:51 2018
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
+#include "Annotations.h"
#include "TestFS.h"
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
@@ -46,11 +47,22 @@ MATCHER_P(Snippet, S, "") {
}
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
MATCHER_P(CPath, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
+MATCHER_P(LocationOffsets, Offsets, "") {
+ // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
+ // end).
+ return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
+ arg.CanonicalDeclaration.EndOffset == Offsets.second - 1;
+}
+//MATCHER_P(FilePath, P, "") {
+ //return arg.CanonicalDeclaration.FilePath.contains(P);
+//}
namespace clang {
namespace clangd {
namespace {
+const char TestHeaderName[] = "symbols.h";
+const char TestFileName[] = "symbol.cc";
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
SymbolIndexActionFactory(SymbolCollector::Options COpts)
@@ -79,21 +91,20 @@ public:
llvm::IntrusiveRefCntPtr<FileManager> Files(
new FileManager(FileSystemOptions(), InMemoryFileSystem));
- const std::string FileName = "symbol.cc";
- const std::string HeaderName = "symbols.h";
auto Factory = llvm::make_unique<SymbolIndexActionFactory>(CollectorOpts);
tooling::ToolInvocation Invocation(
- {"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
+ {"symbol_collector", "-fsyntax-only", "-std=c++11", TestFileName},
Factory->create(), Files.get(),
std::make_shared<PCHContainerOperations>());
- InMemoryFileSystem->addFile(HeaderName, 0,
+ InMemoryFileSystem->addFile(TestHeaderName, 0,
llvm::MemoryBuffer::getMemBuffer(HeaderCode));
- std::string Content = "#include\"" + std::string(HeaderName) + "\"";
- Content += "\n" + MainCode.str();
- InMemoryFileSystem->addFile(FileName, 0,
+ std::string Content = MainCode;
+ if (!HeaderCode.empty())
+ Content = "#include\"" + std::string(TestHeaderName) + "\"\n" + Content;
+ InMemoryFileSystem->addFile(TestFileName, 0,
llvm::MemoryBuffer::getMemBuffer(Content));
Invocation.run();
Symbols = Factory->Collector->takeSymbols();
@@ -206,6 +217,57 @@ TEST_F(SymbolCollectorTest, IgnoreNamele
UnorderedElementsAre(QName("Foo")));
}
+TEST_F(SymbolCollectorTest, SymbolFormedFromMacro) {
+ CollectorOpts.IndexMainFiles = false;
+
+ Annotations Header(R"(
+ #define FF(name) \
+ class name##_Test {};
+
+ $expansion[[FF(abc)]];
+
+ #define FF2() \
+ $spelling[[class Test {}]];
+
+ FF2();
+ )");
+
+ runSymbolCollector(Header.code(), /*Main=*/"");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("abc_Test"),
+ LocationOffsets(Header.offsetRange("expansion")),
+ CPath(TestHeaderName)),
+ AllOf(QName("Test"),
+ LocationOffsets(Header.offsetRange("spelling")),
+ CPath(TestHeaderName))));
+}
+
+TEST_F(SymbolCollectorTest, SymbolFormedFromMacroInMainFile) {
+ CollectorOpts.IndexMainFiles = true;
+
+ Annotations Main(R"(
+ #define FF(name) \
+ class name##_Test {};
+
+ $expansion[[FF(abc)]];
+
+ #define FF2() \
+ $spelling[[class Test {}]];
+
+ FF2();
+ )");
+ runSymbolCollector(/*Header=*/"", Main.code());
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("abc_Test"),
+ LocationOffsets(Main.offsetRange("expansion")),
+ CPath(TestFileName)),
+ AllOf(QName("Test"),
+ LocationOffsets(Main.offsetRange("spelling")),
+ CPath(TestFileName))));
+}
+
TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
CollectorOpts.IndexMainFiles = false;
const std::string Header = R"(
More information about the cfe-commits
mailing list