Index: clangd/index/CanonicalIncludes.h =================================================================== --- clangd/index/CanonicalIncludes.h +++ clangd/index/CanonicalIncludes.h @@ -48,9 +48,10 @@ void addSymbolMapping(llvm::StringRef QualifiedName, llvm::StringRef CanonicalPath); - /// Returns the canonical include for symbol with \p QualifiedName, which is - /// declared in \p Header - llvm::StringRef mapHeader(llvm::StringRef Header, + /// Returns the canonical include for symbol with \p QualifiedName. + /// \p Headers is the include stack: Headers.front() is the file declaring the + /// symbol, and Headers.back() is the main file. + llvm::StringRef mapHeader(llvm::ArrayRef Headers, llvm::StringRef QualifiedName) const; private: Index: clangd/index/CanonicalIncludes.cpp =================================================================== --- clangd/index/CanonicalIncludes.cpp +++ clangd/index/CanonicalIncludes.cpp @@ -8,6 +8,8 @@ //===----------------------------------------------------------------------===// #include "CanonicalIncludes.h" +#include "../Headers.h" +#include "clang/Driver/Types.h" #include "llvm/Support/Regex.h" namespace clang { @@ -33,12 +35,33 @@ } llvm::StringRef -CanonicalIncludes::mapHeader(llvm::StringRef Header, +CanonicalIncludes::mapHeader(llvm::ArrayRef Headers, llvm::StringRef QualifiedName) const { + assert(!Headers.empty()); auto SE = SymbolMapping.find(QualifiedName); if (SE != SymbolMapping.end()) return SE->second; std::lock_guard Lock(RegexMutex); + // Find the first header such that the extension is not '.inc', and isn't a + // recognized non-header file + auto I = + std::find_if(Headers.begin(), Headers.end(), [](llvm::StringRef Include) { + // Skip .inc file whose including header file should + // be #included instead. + return !Include.endswith(".inc"); + }); + if (I == Headers.end()) + return Headers[0]; // Fallback to the declaring header. + StringRef Header = *I; + // If Header is not expected be included (e.g. .cc file), we fall back to + // the declaring header. + StringRef Ext = llvm::sys::path::extension(Header).trim('.'); + // Include-able headers must have precompile type. Treat files with + // non-recognized extenstions (TY_INVALID) as headers. + auto ExtType = driver::types::lookupTypeForExtension(Ext); + if ((ExtType != driver::types::TY_INVALID) && + !driver::types::onlyPrecompileType(ExtType)) + return Headers[0]; for (auto &Entry : RegexHeaderMappingTable) { #ifndef NDEBUG std::string Dummy; @@ -65,9 +88,8 @@ // FIXME(ioeric): resolve the header and store actual file path. For now, // we simply assume the written header is suitable to be #included. Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()), - (Text.startswith("<") || Text.startswith("\"")) - ? Text.str() - : ("\"" + Text + "\"").str()); + isLiteralInclude(Text) ? Text.str() + : ("\"" + Text + "\"").str()); return false; } Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -200,23 +200,31 @@ /// Gets a canonical include (URI of the header or
or "header") for /// header of \p Loc. /// Returns None if fails to get include header for \p Loc. -/// FIXME: we should handle .inc files whose symbols are expected be exported by -/// their containing headers. llvm::Optional getIncludeHeader(llvm::StringRef QName, const SourceManager &SM, SourceLocation Loc, const SymbolCollector::Options &Opts) { - llvm::StringRef FilePath = SM.getFilename(Loc); - if (FilePath.empty()) + std::vector Headers; + // Collect the #include stack. + while (true) { + if (!Loc.isValid()) + break; + auto FilePath = SM.getFilename(Loc); + if (FilePath.empty()) + break; + Headers.push_back(FilePath); + if (SM.isInMainFile(Loc)) + break; + Loc = SM.getIncludeLoc(SM.getFileID(Loc)); + } + if (Headers.empty()) return llvm::None; + llvm::StringRef Header = Headers[0]; if (Opts.Includes) { - llvm::StringRef Mapped = Opts.Includes->mapHeader(FilePath, QName); - if (Mapped != FilePath) - return (Mapped.startswith("<") || Mapped.startswith("\"")) - ? Mapped.str() - : ("\"" + Mapped + "\"").str(); + Header = Opts.Includes->mapHeader(Headers, QName); + if (Header.startswith("<") || Header.startswith("\"")) + return Header.str(); } - - return toURI(SM, SM.getFilename(Loc), Opts); + return toURI(SM, Header, Opts); } // Return the symbol location of the given declaration `D`. Index: unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -131,9 +131,9 @@ auto Factory = llvm::make_unique( CollectorOpts, PragmaHandler.get()); - std::vector Args = {"symbol_collector", "-fsyntax-only", - "-std=c++11", "-include", - TestHeaderName, TestFileName}; + std::vector Args = { + "symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11", + "-include", TestHeaderName, TestFileName}; Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end()); tooling::ToolInvocation Invocation( Args, @@ -666,6 +666,70 @@ IncludeHeader("\"the/good/header.h\"")))); } +TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) { + CollectorOpts.CollectIncludePath = true; + CanonicalIncludes Includes; + Includes.addMapping(TestHeaderName, ""); + CollectorOpts.Includes = &Includes; + auto IncFile = testPath("test.inc"); + auto IncURI = URI::createFile(IncFile).toString(); + InMemoryFileSystem->addFile(IncFile, 0, + llvm::MemoryBuffer::getMemBuffer("class X {};")); + runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, + UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI), + IncludeHeader("")), + AllOf(QName("Y"), DeclURI(TestHeaderURI), + IncludeHeader("")))); +} + +TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) { + CollectorOpts.CollectIncludePath = true; + CanonicalIncludes Includes; + CollectorOpts.Includes = &Includes; + TestFileName = testPath("main.h"); + TestFileURI = URI::createFile(TestFileName).toString(); + auto IncFile = testPath("test.inc"); + auto IncURI = URI::createFile(IncFile).toString(); + InMemoryFileSystem->addFile(IncFile, 0, + llvm::MemoryBuffer::getMemBuffer("class X {};")); + runSymbolCollector("", /*Main=*/"#include \"test.inc\"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI), + IncludeHeader(TestFileURI)))); +} + +TEST_F(SymbolCollectorTest, MainFileIsHeaderWithoutExtensionWhenSkipIncFile) { + CollectorOpts.CollectIncludePath = true; + CanonicalIncludes Includes; + CollectorOpts.Includes = &Includes; + TestFileName = testPath("no_ext_main"); + TestFileURI = URI::createFile(TestFileName).toString(); + auto IncFile = testPath("test.inc"); + auto IncURI = URI::createFile(IncFile).toString(); + InMemoryFileSystem->addFile(IncFile, 0, + llvm::MemoryBuffer::getMemBuffer("class X {};")); + runSymbolCollector("", /*Main=*/"#include \"test.inc\"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI), + IncludeHeader(TestFileURI)))); +} + +TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) { + CollectorOpts.CollectIncludePath = true; + CanonicalIncludes Includes; + CollectorOpts.Includes = &Includes; + auto IncFile = testPath("test.inc"); + auto IncURI = URI::createFile(IncFile).toString(); + InMemoryFileSystem->addFile(IncFile, 0, + llvm::MemoryBuffer::getMemBuffer("class X {};")); + runSymbolCollector("", /*Main=*/"#include \"test.inc\"", + /*ExtraArgs=*/{"-I", testRoot()}); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI), + IncludeHeader(IncURI)))); +} + TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) { CollectorOpts.CollectIncludePath = true; Annotations Header(R"(