Index: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h =================================================================== --- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h +++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h @@ -50,9 +50,9 @@ llvm::StringRef CanonicalPath); /// 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, + /// \p Header is the file the declaration was reachable from. + /// Header itself will be returned if there is no relevant mapping. + llvm::StringRef mapHeader(llvm::StringRef Header, llvm::StringRef QualifiedName) const; private: Index: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp +++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp @@ -37,31 +37,12 @@ } llvm::StringRef -CanonicalIncludes::mapHeader(llvm::ArrayRef Headers, +CanonicalIncludes::mapHeader(llvm::StringRef Header, llvm::StringRef QualifiedName) const { - assert(!Headers.empty()); + assert(!Header.empty()); auto SE = SymbolMapping.find(QualifiedName); if (SE != SymbolMapping.end()) return SE->second; - // Find the first header such that the extension is not '.inc', and isn't a - // recognized non-header file - auto I = llvm::find_if(Headers, [](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. - llvm::StringRef Header = *I; - // If Header is not expected be included (e.g. .cc file), we fall back to - // the declaring header. - llvm::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]; auto MapIt = FullPathMapping.find(Header); if (MapIt != FullPathMapping.end()) Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp @@ -25,6 +25,7 @@ #include "clang/Basic/Specifiers.h" #include "clang/Index/IndexSymbol.h" #include "clang/Index/USRGeneration.h" +#include "clang/Lex/Preprocessor.h" #include "llvm/Support/Casting.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" @@ -127,34 +128,46 @@ } } -/// 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. +bool isSelfContainedHeader(FileID FID, const SourceManager &SM, + const Preprocessor &PP) { + const FileEntry *FE = SM.getFileEntryForID(FID); + if (!FE) + return false; + return PP.getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE); +} + +/// Gets a canonical include (URI of the header or
or "header") for +/// header of \p FID (which should usually be the *expansion* file). +/// Returns None if includes should not be inserted for this file. llvm::Optional getIncludeHeader(llvm::StringRef QName, const SourceManager &SM, - SourceLocation Loc, const SymbolCollector::Options &Opts) { - 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 None; - llvm::StringRef Header = Headers[0]; + const Preprocessor &PP, FileID FID, + const SymbolCollector::Options &Opts) { + const FileEntry *FE = SM.getFileEntryForID(FID); + if (!FE || FE->getName().empty()) + return llvm::None; + llvm::StringRef Filename = FE->getName(); + // If a file is mapped by canonical headers, use that mapping, regardless + // of whether it's an otherwise-good header (header guards etc). if (Opts.Includes) { - Header = Opts.Includes->mapHeader(Headers, QName); - if (Header.startswith("<") || Header.startswith("\"")) - return Header.str(); + llvm::StringRef Canonical = Opts.Includes->mapHeader(Filename, QName); + // If we had a mapping, always use it. + if (Canonical.startswith("<") || Canonical.startswith("\"")) + return Canonical.str(); + if (Canonical != Filename) + return toURI(SM, Canonical, Opts); + } + if (!isSelfContainedHeader(FID, SM, PP)) { + // A .inc or .def file is often included into a real header to define + // symbols (e.g. LLVM tablegen files). + if (Filename.endswith(".inc") || Filename.endswith(".def")) + return getIncludeHeader(QName, SM, PP, + SM.getFileID(SM.getIncludeLoc(FID)), Opts); + // Conservatively refuse to insert #includes to files without guards. + return llvm::None; } - return toURI(SM, Header, Opts); + // Standard case: just insert the file itself. + return toURI(SM, Filename, Opts); } // Return the symbol range of the token at \p TokLoc. @@ -439,8 +452,9 @@ std::string Include; if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) { - if (auto Header = getIncludeHeader(Name->getName(), SM, - SM.getExpansionLoc(DefLoc), Opts)) + if (auto Header = + getIncludeHeader(Name->getName(), SM, *PP, + SM.getDecomposedExpansionLoc(DefLoc).first, Opts)) Include = std::move(*Header); } S.Signature = Signature; @@ -588,7 +602,8 @@ // Use the expansion location to get the #include header since this is // where the symbol is exposed. if (auto Header = getIncludeHeader( - QName, SM, SM.getExpansionLoc(ND.getLocation()), Opts)) + QName, SM, *PP, + SM.getDecomposedExpansionLoc(ND.getLocation()).first, Opts)) Include = std::move(*Header); } if (!Include.empty()) Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -684,6 +684,7 @@ ClangdServer Server(CDB, FS, DiagConsumer, Opts); FS.Files[testPath("foo_header.h")] = R"cpp( + #pragma once struct Foo { // Member doc int foo(); Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp @@ -212,33 +212,11 @@ } TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) { - FileIndex Index; - const std::string Header = R"cpp( - class Foo {}; - )cpp"; - auto MainFile = testPath("foo.cpp"); - auto HeaderFile = testPath("algorithm"); - std::vector Cmd = {"clang", "-xc++", MainFile.c_str(), - "-include", HeaderFile.c_str()}; - // Preparse ParseInputs. - ParseInputs PI; - PI.CompileCommand.Directory = testRoot(); - PI.CompileCommand.Filename = MainFile; - PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()}; - PI.Contents = ""; - PI.FS = buildTestFS({{MainFile, ""}, {HeaderFile, Header}}); + TestTU TU; + TU.HeaderCode = "class Foo{};"; + TU.HeaderFilename = "algorithm"; - // Prepare preamble. - auto CI = buildCompilerInvocation(PI); - auto PreambleData = - buildPreamble(MainFile, *buildCompilerInvocation(PI), - /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI, - /*StoreInMemory=*/true, - [&](ASTContext &Ctx, std::shared_ptr PP, - const CanonicalIncludes &Includes) { - Index.updatePreamble(MainFile, Ctx, PP, Includes); - }); - auto Symbols = runFuzzyFind(Index, ""); + auto Symbols = runFuzzyFind(*TU.index(), ""); EXPECT_THAT(Symbols, ElementsAre(_)); EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader, ""); @@ -369,50 +347,23 @@ } TEST(FileIndexTest, ReferencesInMainFileWithPreamble) { - const std::string Header = R"cpp( - class Foo {}; - )cpp"; + TestTU TU; + TU.HeaderCode = "class Foo{};"; Annotations Main(R"cpp( #include "foo.h" void f() { [[Foo]] foo; } )cpp"); - auto MainFile = testPath("foo.cpp"); - auto HeaderFile = testPath("foo.h"); - std::vector Cmd = {"clang", "-xc++", MainFile.c_str()}; - // Preparse ParseInputs. - ParseInputs PI; - PI.CompileCommand.Directory = testRoot(); - PI.CompileCommand.Filename = MainFile; - PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()}; - PI.Contents = Main.code(); - PI.FS = buildTestFS({{MainFile, Main.code()}, {HeaderFile, Header}}); - - // Prepare preamble. - auto CI = buildCompilerInvocation(PI); - auto PreambleData = - buildPreamble(MainFile, *buildCompilerInvocation(PI), - /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI, - /*StoreInMemory=*/true, - [&](ASTContext &Ctx, std::shared_ptr PP, - const CanonicalIncludes &) {}); - // Build AST for main file with preamble. - auto AST = - ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData, - llvm::MemoryBuffer::getMemBufferCopy(Main.code()), PI.FS, - /*Index=*/nullptr, ParseOptions()); - ASSERT_TRUE(AST); + TU.Code = Main.code(); + auto AST = TU.build(); FileIndex Index; - Index.updateMain(MainFile, *AST); - - auto Foo = findSymbol(TestTU::withHeaderCode(Header).headerSymbols(), "Foo"); - RefsRequest Request; - Request.IDs.insert(Foo.ID); + Index.updateMain(testPath(TU.Filename), AST); // Expect to see references in main file, references in headers are excluded // because we only index main AST. - EXPECT_THAT(getRefs(Index, Foo.ID), RefsAre({RefRange(Main.range())})); + EXPECT_THAT(getRefs(Index, findSymbol(TU.headerSymbols(), "Foo").ID), + RefsAre({RefRange(Main.range())})); } } // namespace Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp @@ -20,6 +20,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" +#include "gmock/gmock-more-matchers.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -33,7 +34,7 @@ using testing::_; using testing::AllOf; using testing::Contains; -using testing::Eq; +using testing::ElementsAre; using testing::Field; using testing::IsEmpty; using testing::Not; @@ -58,6 +59,7 @@ return StringRef(arg.CanonicalDeclaration.FileURI) == P; } MATCHER_P(DefURI, P, "") { return StringRef(arg.Definition.FileURI) == P; } +MATCHER(IncludeHeader, "") { return !arg.IncludeHeaders.empty(); } MATCHER_P(IncludeHeader, P, "") { return (arg.IncludeHeaders.size() == 1) && (arg.IncludeHeaders.begin()->IncludeHeader == P); @@ -249,6 +251,8 @@ TestFileURI = URI::create(TestFileName).toString(); } + // Note that unlike TestTU, no automatic header guard is added. + // HeaderCode should start with #pragma once to be treated as modular. bool runSymbolCollector(llvm::StringRef HeaderCode, llvm::StringRef MainCode, const std::vector &ExtraArgs = {}) { llvm::IntrusiveRefCntPtr Files( @@ -920,7 +924,7 @@ TEST_F(SymbolCollectorTest, IncludeHeaderSameAsFileURI) { CollectorOpts.CollectIncludePath = true; - runSymbolCollector("class Foo {};", /*Main=*/""); + runSymbolCollector("#pragma once\nclass Foo {};", /*Main=*/""); EXPECT_THAT(Symbols, UnorderedElementsAre( AllOf(QName("Foo"), DeclURI(TestHeaderURI)))); EXPECT_THAT(Symbols.begin()->IncludeHeaders, @@ -1020,53 +1024,54 @@ TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) { CollectorOpts.CollectIncludePath = true; - CanonicalIncludes Includes; - CollectorOpts.Includes = &Includes; - TestFileName = testPath("main.h"); + // To make this case as hard as possible, we won't tell clang main is a + // header. No extension, no -x c++-header. + TestFileName = testPath("no_ext_main"); TestFileURI = URI::create(TestFileName).toString(); auto IncFile = testPath("test.inc"); auto IncURI = URI::create(IncFile).toString(); InMemoryFileSystem->addFile(IncFile, 0, llvm::MemoryBuffer::getMemBuffer("class X {};")); - runSymbolCollector("", /*Main=*/"#include \"test.inc\"", + runSymbolCollector("", R"cpp( + // Can't use #pragma once in a main file clang doesn't think is a header. + #ifndef MAIN_H_ + #define MAIN_H_ + #include "test.inc" + #endif + )cpp", /*ExtraArgs=*/{"-I", testRoot()}); EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI), IncludeHeader(TestFileURI)))); } -TEST_F(SymbolCollectorTest, MainFileIsHeaderWithoutExtensionWhenSkipIncFile) { +TEST_F(SymbolCollectorTest, IncFileInNonHeader) { CollectorOpts.CollectIncludePath = true; - CanonicalIncludes Includes; - CollectorOpts.Includes = &Includes; - TestFileName = testPath("no_ext_main"); + TestFileName = testPath("main.cc"); TestFileURI = URI::create(TestFileName).toString(); auto IncFile = testPath("test.inc"); auto IncURI = URI::create(IncFile).toString(); InMemoryFileSystem->addFile(IncFile, 0, llvm::MemoryBuffer::getMemBuffer("class X {};")); - runSymbolCollector("", /*Main=*/"#include \"test.inc\"", + runSymbolCollector("", R"cpp( + #include "test.inc" + )cpp", /*ExtraArgs=*/{"-I", testRoot()}); EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI), - IncludeHeader(TestFileURI)))); + Not(IncludeHeader())))); } -TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) { - CollectorOpts.CollectIncludePath = true; - CanonicalIncludes Includes; - CollectorOpts.Includes = &Includes; - auto IncFile = testPath("test.inc"); - auto IncURI = URI::create(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, NonModularHeader) { + auto TU = TestTU::withHeaderCode("int x();"); + EXPECT_THAT(TU.headerSymbols(), ElementsAre(IncludeHeader())); + + TU.ImplicitHeaderGuard = false; + EXPECT_THAT(TU.headerSymbols(), ElementsAre(Not(IncludeHeader()))); } TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) { CollectorOpts.CollectIncludePath = true; Annotations Header(R"( + #pragma once // Forward declarations of TagDecls. class C; struct S; @@ -1100,7 +1105,8 @@ TEST_F(SymbolCollectorTest, ClassForwardDeclarationIsCanonical) { CollectorOpts.CollectIncludePath = true; - runSymbolCollector(/*Header=*/"class X;", /*Main=*/"class X {};"); + runSymbolCollector(/*Header=*/"#pragma once\nclass X;", + /*Main=*/"class X {};"); EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf( QName("X"), DeclURI(TestHeaderURI), IncludeHeader(TestHeaderURI), DefURI(TestFileURI)))); @@ -1167,6 +1173,7 @@ TEST_F(SymbolCollectorTest, CollectMacros) { CollectorOpts.CollectIncludePath = true; Annotations Header(R"( + #pragma once #define X 1 #define $mac[[MAC]](x) int x #define $used[[USED]](y) float y; Index: clang-tools-extra/trunk/unittests/clangd/TestTU.h =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TestTU.h +++ clang-tools-extra/trunk/unittests/clangd/TestTU.h @@ -52,6 +52,9 @@ // Index to use when building AST. const SymbolIndex *ExternalIndex = nullptr; + // Simulate a header guard of the header (using an #import directive). + bool ImplicitHeaderGuard = true; + ParsedAST build() const; SymbolSlab headerSymbols() const; std::unique_ptr index() const; Index: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp +++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp @@ -19,13 +19,19 @@ ParsedAST TestTU::build() const { std::string FullFilename = testPath(Filename), - FullHeaderName = testPath(HeaderFilename); + FullHeaderName = testPath(HeaderFilename), + ImportThunk = testPath("import_thunk.h"); std::vector Cmd = {"clang", FullFilename.c_str()}; + // We want to implicitly include HeaderFilename without messing up offsets. + // -include achieves this, but sometimes we want #import (to simulate a header + // guard without messing up offsets). In this case, use an intermediate file. + std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n"; // FIXME: this shouldn't need to be conditional, but it breaks a // GoToDefinition test for some reason (getMacroArgExpandedLocation fails). if (!HeaderCode.empty()) { Cmd.push_back("-include"); - Cmd.push_back(FullHeaderName.c_str()); + Cmd.push_back(ImplicitHeaderGuard ? ImportThunk.c_str() + : FullHeaderName.c_str()); } Cmd.insert(Cmd.end(), ExtraArgs.begin(), ExtraArgs.end()); ParseInputs Inputs; @@ -33,7 +39,9 @@ Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()}; Inputs.CompileCommand.Directory = testRoot(); Inputs.Contents = Code; - Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}}); + Inputs.FS = buildTestFS({{FullFilename, Code}, + {FullHeaderName, HeaderCode}, + {ImportThunk, ThunkContents}}); Inputs.Opts = ParseOptions(); Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks; Inputs.Index = ExternalIndex;