kbobyrev updated this revision to Diff 420423.
kbobyrev added a comment.
Switch to stable file UniqueIDs.
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/IncludeCleaner.cpp
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
@@ -986,12 +986,12 @@
llvm::StringRef Main;
llvm::StringRef TargetSymbolName;
} TestCases[] = {
- {
- R"cpp(
+ {
+ R"cpp(
struct Foo;
#define MACRO Foo
)cpp",
- R"cpp(
+ R"cpp(
struct $spelled[[Foo]] {
$spelled[[Foo]]();
~$spelled[[Foo]]();
@@ -999,24 +999,24 @@
$spelled[[Foo]] Variable1;
$implicit[[MACRO]] Variable2;
)cpp",
- "Foo",
- },
- {
- R"cpp(
+ "Foo",
+ },
+ {
+ R"cpp(
class Foo {
public:
Foo() = default;
};
)cpp",
- R"cpp(
+ R"cpp(
void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();}
)cpp",
- "Foo::Foo" /// constructor.
- },
+ "Foo::Foo" /// constructor.
+ },
};
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.RefsInHeaders = false;
- for (const auto& T : TestCases) {
+ for (const auto &T : TestCases) {
Annotations Header(T.Header);
Annotations Main(T.Main);
// Reset the file system.
@@ -1578,15 +1578,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->getFile(TestHeaderName);
+ ASSERT_TRUE(File);
+ 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,27 @@
//
//===----------------------------------------------------------------------===//
+#include "TestFS.h"
#include "index/CanonicalIncludes.h"
+#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include "gtest/gtest.h"
namespace clang {
namespace clangd {
namespace {
+const FileEntry *
+addFile(llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> &FS,
+ llvm::IntrusiveRefCntPtr<FileManager> &FM, llvm::StringRef Filename) {
+ FS->addFile(Filename, 0, llvm::MemoryBuffer::getMemBuffer(""));
+ auto File = FM->getFile(Filename);
+ return *File;
+}
+
TEST(CanonicalIncludesTest, CStandardLibrary) {
CanonicalIncludes CI;
auto Language = LangOptions();
@@ -40,29 +53,47 @@
// 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>();
+ llvm::IntrusiveRefCntPtr<FileManager> Files(
+ new FileManager(FileSystemOptions(), InMemFS));
+ const auto *File = addFile(InMemFS, Files, testPath("iosfwd"));
+ EXPECT_EQ("<iosfwd>", CI.mapHeader(File));
}
TEST(CanonicalIncludesTest, PathMapping) {
+ auto InMemFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ llvm::IntrusiveRefCntPtr<FileManager> Files(
+ new FileManager(FileSystemOptions(), InMemFS));
+ const auto *Bar = addFile(InMemFS, Files, testPath("foo/bar"));
+ const 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));
}
TEST(CanonicalIncludesTest, Precedence) {
+ auto InMemFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+ llvm::IntrusiveRefCntPtr<FileManager> Files(
+ new FileManager(FileSystemOptions(), InMemFS));
+ const 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"));
+ const 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,7 @@
// 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(FE);
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
@@ -22,6 +22,7 @@
#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 +35,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 string-to-string mapping from \p ID to \p CanonicalPath.
+ void addMapping(const FileEntry *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(const FileEntry *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 +55,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
@@ -9,6 +9,7 @@
#include "CanonicalIncludes.h"
#include "Headers.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FileSystem/UniqueID.h"
#include "llvm/Support/Path.h"
#include <algorithm>
@@ -18,18 +19,17 @@
const char IWYUPragma[] = "// IWYU pragma: private, include ";
} // namespace
-void CanonicalIncludes::addMapping(llvm::StringRef Path,
+void CanonicalIncludes::addMapping(const FileEntry *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(const FileEntry *Header) const {
+ auto MapIt = FullPathMapping.find(Header->getUniqueID());
if (MapIt != FullPathMapping.end())
return MapIt->second;
@@ -39,10 +39,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;
@@ -68,7 +69,8 @@
return false;
// 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()),
+ auto &SM = PP.getSourceManager();
+ Includes->addMapping(SM.getFileEntryForID(SM.getFileID(Range.getBegin())),
isLiteralInclude(Text) ? Text.str()
: ("\"" + Text + "\"").str());
return false;
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -356,7 +356,7 @@
[&SM, &CanonIncludes](FileID ID) -> Optional<StringRef> {
const FileEntry *Entry = SM.getFileEntryForID(ID);
if (Entry) {
- auto PublicHeader = CanonIncludes.mapHeader(Entry->getName());
+ auto PublicHeader = CanonIncludes.mapHeader(Entry);
if (!PublicHeader.empty()) {
return PublicHeader;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
