diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -354,9 +354,9 @@ return headerResponsible(ID, SM, Includes); }, [&SM, &CanonIncludes](FileID ID) -> Optional { - const FileEntry *Entry = SM.getFileEntryForID(ID); + Optional Entry = SM.getFileEntryRefForID(ID); if (Entry) { - auto PublicHeader = CanonIncludes.mapHeader(Entry->getName()); + auto PublicHeader = CanonIncludes.mapHeader(*Entry); if (!PublicHeader.empty()) { return PublicHeader; } diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.h b/clang-tools-extra/clangd/index/CanonicalIncludes.h --- a/clang-tools-extra/clangd/index/CanonicalIncludes.h +++ b/clang-tools-extra/clangd/index/CanonicalIncludes.h @@ -19,9 +19,11 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_CANONICALINCLUDES_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_CANONICALINCLUDES_H +#include "clang/Basic/FileEntry.h" #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/FileSystem/UniqueID.h" #include #include #include @@ -34,14 +36,14 @@ /// Only const methods (i.e. mapHeader) in this class are thread safe. class CanonicalIncludes { public: - /// Adds a string-to-string mapping from \p Path to \p CanonicalPath. - void addMapping(llvm::StringRef Path, llvm::StringRef CanonicalPath); + /// Adds a file-to-string mapping from \p ID to \p CanonicalPath. + void addMapping(FileEntryRef Header, llvm::StringRef CanonicalPath); /// Returns the overridden include for symbol with \p QualifiedName, or "". llvm::StringRef mapSymbol(llvm::StringRef QualifiedName) const; /// Returns the overridden include for for files in \p Header, or "". - llvm::StringRef mapHeader(llvm::StringRef Header) const; + llvm::StringRef mapHeader(FileEntryRef Header) const; /// Adds mapping for system headers and some special symbols (e.g. STL symbols /// in need to be mapped individually). Approximately, the following @@ -54,8 +56,8 @@ void addSystemHeadersMapping(const LangOptions &Language); private: - /// A map from full include path to a canonical path. - llvm::StringMap FullPathMapping; + /// A map from the private header to a canonical include path. + llvm::DenseMap FullPathMapping; /// A map from a suffix (one or components of a path) to a canonical path. /// Used only for mapping standard headers. const llvm::StringMap *StdSuffixHeaderMapping = nullptr; diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp --- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp +++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp @@ -8,7 +8,9 @@ #include "CanonicalIncludes.h" #include "Headers.h" +#include "clang/Basic/FileEntry.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/FileSystem/UniqueID.h" #include "llvm/Support/Path.h" #include @@ -18,18 +20,17 @@ const char IWYUPragma[] = "// IWYU pragma: private, include "; } // namespace -void CanonicalIncludes::addMapping(llvm::StringRef Path, +void CanonicalIncludes::addMapping(FileEntryRef Header, llvm::StringRef CanonicalPath) { - FullPathMapping[Path] = std::string(CanonicalPath); + FullPathMapping[Header.getUniqueID()] = std::string(CanonicalPath); } /// The maximum number of path components in a key from StdSuffixHeaderMapping. /// Used to minimize the number of lookups in suffix path mappings. constexpr int MaxSuffixComponents = 3; -llvm::StringRef CanonicalIncludes::mapHeader(llvm::StringRef Header) const { - assert(!Header.empty()); - auto MapIt = FullPathMapping.find(Header); +llvm::StringRef CanonicalIncludes::mapHeader(FileEntryRef Header) const { + auto MapIt = FullPathMapping.find(Header.getUniqueID()); if (MapIt != FullPathMapping.end()) return MapIt->second; @@ -39,10 +40,11 @@ int Components = 1; // FIXME: check that this works on Windows and add tests. - for (auto It = llvm::sys::path::rbegin(Header), - End = llvm::sys::path::rend(Header); + auto Filename = Header.getName(); + for (auto It = llvm::sys::path::rbegin(Filename), + End = llvm::sys::path::rend(Filename); It != End && Components <= MaxSuffixComponents; ++It, ++Components) { - auto SubPath = Header.substr(It->data() - Header.begin()); + auto SubPath = Filename.substr(It->data() - Filename.begin()); auto MappingIt = StdSuffixHeaderMapping->find(SubPath); if (MappingIt != StdSuffixHeaderMapping->end()) return MappingIt->second; @@ -66,12 +68,12 @@ PP.getSourceManager(), PP.getLangOpts()); if (!Text.consume_front(IWYUPragma)) return false; + auto &SM = PP.getSourceManager(); // We always insert using the spelling from the pragma. - if (auto *FE = PP.getSourceManager().getFileEntryForID( - PP.getSourceManager().getFileID(Range.getBegin()))) - Includes->addMapping(FE->getName(), isLiteralInclude(Text) - ? Text.str() - : ("\"" + Text + "\"").str()); + if (auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))) + Includes->addMapping( + FE->getLastRef(), + isLiteralInclude(Text) ? Text.str() : ("\"" + Text + "\"").str()); return false; } diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -381,7 +381,8 @@ // If a file is mapped by canonical headers, use that mapping, regardless // of whether it's an otherwise-good header (header guards etc). if (Includes) { - llvm::StringRef Canonical = Includes->mapHeader(Filename); + llvm::StringRef Canonical = + Includes->mapHeader(*SM.getFileEntryRefForID(FID)); if (!Canonical.empty()) { // If we had a mapping, always use it. if (Canonical.startswith("<") || Canonical.startswith("\"")) diff --git a/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp b/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp --- a/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp +++ b/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp @@ -6,14 +6,30 @@ // //===----------------------------------------------------------------------===// +#include "TestFS.h" #include "index/CanonicalIncludes.h" +#include "clang/Basic/FileEntry.h" +#include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/VirtualFileSystem.h" +#include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" namespace clang { namespace clangd { namespace { +FileEntryRef addFile(llvm::vfs::InMemoryFileSystem &FS, FileManager &FM, + llvm::StringRef Filename) { + FS.addFile(Filename, 0, llvm::MemoryBuffer::getMemBuffer("")); + auto File = FM.getFileRef(Filename); + EXPECT_THAT_EXPECTED(File, llvm::Succeeded()); + return *File; +} + TEST(CanonicalIncludesTest, CStandardLibrary) { CanonicalIncludes CI; auto Language = LangOptions(); @@ -40,29 +56,52 @@ // iosfwd declares some symbols it doesn't own. EXPECT_EQ("", CI.mapSymbol("std::ostream")); // And (for now) we assume it owns the others. - EXPECT_EQ("", CI.mapHeader("iosfwd")); + auto InMemFS = llvm::makeIntrusiveRefCnt(); + FileManager Files(FileSystemOptions(), InMemFS); + auto File = addFile(*InMemFS, Files, testPath("iosfwd")); + EXPECT_EQ("", CI.mapHeader(File)); } TEST(CanonicalIncludesTest, PathMapping) { + auto InMemFS = llvm::makeIntrusiveRefCnt(); + FileManager Files(FileSystemOptions(), InMemFS); + std::string BarPath = testPath("foo/bar"); + auto Bar = addFile(*InMemFS, Files, BarPath); + auto Other = addFile(*InMemFS, Files, testPath("foo/baz")); // As used for IWYU pragmas. CanonicalIncludes CI; - CI.addMapping("foo/bar", ""); + CI.addMapping(Bar, ""); - EXPECT_EQ("", CI.mapHeader("foo/bar")); - EXPECT_EQ("", CI.mapHeader("bar/bar")); + // We added a mapping for baz. + EXPECT_EQ("", CI.mapHeader(Bar)); + // Other file doesn't have a mapping. + EXPECT_EQ("", CI.mapHeader(Other)); + + // Add hard link to "foo/bar" and check that it is also mapped to , hence + // does not depend on the header name. + std::string HardLinkPath = testPath("hard/link"); + InMemFS->addHardLink(HardLinkPath, BarPath); + auto HardLinkFile = Files.getFileRef(HardLinkPath); + ASSERT_THAT_EXPECTED(HardLinkFile, llvm::Succeeded()); + EXPECT_EQ("", CI.mapHeader(*HardLinkFile)); } TEST(CanonicalIncludesTest, Precedence) { + auto InMemFS = llvm::makeIntrusiveRefCnt(); + FileManager Files(FileSystemOptions(), InMemFS); + auto File = addFile(*InMemFS, Files, testPath("some/path")); + CanonicalIncludes CI; - CI.addMapping("some/path", ""); + CI.addMapping(File, ""); LangOptions Language; Language.CPlusPlus = true; CI.addSystemHeadersMapping(Language); // We added a mapping from some/path to . - ASSERT_EQ("", CI.mapHeader("some/path")); + ASSERT_EQ("", CI.mapHeader(File)); // We should have a path from 'bits/stl_vector.h' to ''. - ASSERT_EQ("", CI.mapHeader("bits/stl_vector.h")); + auto STLVectorFile = addFile(*InMemFS, Files, testPath("bits/stl_vector.h")); + ASSERT_EQ("", CI.mapHeader(STLVectorFile)); } } // namespace diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -20,6 +20,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" +#include "llvm/Testing/Support/Error.h" #include "gmock/gmock-matchers.h" #include "gmock/gmock-more-matchers.h" #include "gmock/gmock.h" @@ -1578,15 +1579,22 @@ } TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) { - CollectorOpts.CollectIncludePath = true; - CanonicalIncludes Includes; - Includes.addMapping(TestHeaderName, ""); - CollectorOpts.Includes = &Includes; auto IncFile = testPath("test.inc"); auto IncURI = URI::create(IncFile).toString(); InMemoryFileSystem->addFile(IncFile, 0, llvm::MemoryBuffer::getMemBuffer("class X {};")); - runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"", + llvm::IntrusiveRefCntPtr Files( + new FileManager(FileSystemOptions(), InMemoryFileSystem)); + std::string HeaderCode = "#include \"test.inc\"\nclass Y {};"; + InMemoryFileSystem->addFile(TestHeaderName, 0, + llvm::MemoryBuffer::getMemBuffer(HeaderCode)); + auto File = Files->getFileRef(TestHeaderName); + ASSERT_THAT_EXPECTED(File, llvm::Succeeded()); + CanonicalIncludes Includes; + Includes.addMapping(*File, ""); + CollectorOpts.CollectIncludePath = true; + CollectorOpts.Includes = &Includes; + runSymbolCollector(HeaderCode, /*Main=*/"", /*ExtraArgs=*/{"-I", testRoot()}); EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(qName("X"), declURI(IncURI),