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"( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits