hokein created this revision.
hokein added a reviewer: ioeric.
Herald added subscribers: jkorous-apple, ilya-biryukov, klimek.
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).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42575
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/Annotations.cpp
unittests/clangd/Annotations.h
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
+#include "Annotations.h"
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
#include "clang/Basic/FileManager.h"
@@ -44,11 +45,22 @@
return arg.CompletionSnippetInsertText == S;
}
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+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)
@@ -77,21 +89,19 @@
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) + "\"";
+ std::string Content = "#include\"" + std::string(TestHeaderName) + "\"";
Content += "\n" + MainCode.str();
- InMemoryFileSystem->addFile(FileName, 0,
+ InMemoryFileSystem->addFile(TestFileName, 0,
llvm::MemoryBuffer::getMemBuffer(Content));
Invocation.run();
Symbols = Factory->Collector->takeSymbols();
@@ -196,6 +206,31 @@
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")),
+ FilePath(TestHeaderName)),
+ AllOf(QName("Test"),
+ LocationOffsets(Header.offsetRange("spelling")),
+ FilePath(TestHeaderName))));
+}
+
TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
CollectorOpts.IndexMainFiles = false;
const std::string Header = R"(
Index: unittests/clangd/Annotations.h
===================================================================
--- unittests/clangd/Annotations.h
+++ unittests/clangd/Annotations.h
@@ -58,6 +58,10 @@
// Returns the location of all ranges marked by [[ ]] (or $name[[ ]]).
std::vector<Range> ranges(llvm::StringRef Name = "") const;
+ // 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;
Index: unittests/clangd/Annotations.cpp
===================================================================
--- unittests/clangd/Annotations.cpp
+++ unittests/clangd/Annotations.cpp
@@ -83,5 +83,11 @@
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
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -105,6 +105,43 @@
return false;
}
+std::string PrintableLoc(SourceLocation Loc, SourceManager &SM) {
+ if (Loc.isInvalid())
+ return "Invalid location";
+ std::string Buffer;
+ llvm::raw_string_ostream ostream(Buffer);
+ Loc.print(ostream, SM);
+ ostream.flush();
+ return Buffer;
+}
+
+SymbolLocation GetSymbolLocation(const NamedDecl* D, SourceManager& SM,
+ 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(PrintableLoc(Loc, SM)).startswith("<scratch")) {
+ FilePathStorage = makeAbsolutePath(
+ SM, SM.getFilename(SM.getExpansionLoc(D->getLocation())));
+ Location = {
+ FilePathStorage,
+ SM.getFileOffset(SM.getExpansionRange(D->getLocStart()).first),
+ SM.getFileOffset(SM.getExpansionRange(D->getLocEnd()).second)};
+ return Location;
+ }
+ }
+
+ FilePathStorage = makeAbsolutePath(SM, SM.getFilename(Loc));
+ Location = {FilePathStorage,
+ SM.getFileOffset(SM.getSpellingLoc(D->getLocStart())),
+ SM.getFileOffset(SM.getSpellingLoc(D->getLocEnd()))};
+ return Location;
+}
+
} // namespace
SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {}
@@ -142,17 +179,14 @@
return true;
auto &SM = ND->getASTContext().getSourceManager();
- std::string FilePath =
- makeAbsolutePath(SM, SM.getFilename(D->getLocation()));
- 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, FilePath);
// Add completion info.
assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
