This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3de4d5e6dd66: [clangd] Use stable keys for CanonicalIncludes
mappings (authored by kbobyrev).
Changed prior to commit:
https://reviews.llvm.org/D123031?vs=420500&id=420503#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123031/new/
https://reviews.llvm.org/D123031
Files:
clang-tools-extra/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/clangd/index/CanonicalIncludes.h
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ 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, "<canonical>");
- 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<FileManager> 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, "<canonical>");
+ CollectorOpts.CollectIncludePath = true;
+ CollectorOpts.Includes = &Includes;
+ runSymbolCollector(HeaderCode, /*Main=*/"",
/*ExtraArgs=*/{"-I", testRoot()});
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(qName("X"), declURI(IncURI),
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ 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("<ostream>", CI.mapSymbol("std::ostream"));
// And (for now) we assume it owns the others.
- EXPECT_EQ("<iosfwd>", CI.mapHeader("iosfwd"));
+ auto InMemFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ FileManager Files(FileSystemOptions(), InMemFS);
+ auto File = addFile(*InMemFS, Files, testPath("iosfwd"));
+ EXPECT_EQ("<iosfwd>", CI.mapHeader(File));
}
TEST(CanonicalIncludesTest, PathMapping) {
+ auto InMemFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ 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", "<baz>");
+ CI.addMapping(Bar, "<baz>");
- EXPECT_EQ("<baz>", CI.mapHeader("foo/bar"));
- EXPECT_EQ("", CI.mapHeader("bar/bar"));
+ // We added a mapping for baz.
+ EXPECT_EQ("<baz>", 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 <baz>, 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("<baz>", CI.mapHeader(*HardLinkFile));
}
TEST(CanonicalIncludesTest, Precedence) {
+ auto InMemFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ FileManager Files(FileSystemOptions(), InMemFS);
+ auto File = addFile(*InMemFS, Files, testPath("some/path"));
+
CanonicalIncludes CI;
- CI.addMapping("some/path", "<path>");
+ CI.addMapping(File, "<path>");
LangOptions Language;
Language.CPlusPlus = true;
CI.addSystemHeadersMapping(Language);
// We added a mapping from some/path to <path>.
- ASSERT_EQ("<path>", CI.mapHeader("some/path"));
+ ASSERT_EQ("<path>", CI.mapHeader(File));
// We should have a path from 'bits/stl_vector.h' to '<vector>'.
- ASSERT_EQ("<vector>", CI.mapHeader("bits/stl_vector.h"));
+ auto STLVectorFile = addFile(*InMemFS, Files, testPath("bits/stl_vector.h"));
+ ASSERT_EQ("<vector>", CI.mapHeader(STLVectorFile));
}
} // namespace
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ 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("\""))
Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===================================================================
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ 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 <mutex>
#include <string>
#include <vector>
@@ -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 <iosfwd> 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<std::string> FullPathMapping;
+ /// A map from the private header to a canonical include path.
+ llvm::DenseMap<llvm::sys::fs::UniqueID, std::string> 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<llvm::StringRef> *StdSuffixHeaderMapping = nullptr;
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===================================================================
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ 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 <algorithm>
@@ -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;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
