[clang] 178ad93 - [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics
Dmitry Polukhin via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 3 01:38:42 PDT 2021
Author: Dmitry Polukhin
Date: 2021-06-03T01:37:55-07:00
New Revision: 178ad93e3f1f2381f05baea300873ee5998ac288
URL: https://github.com/llvm/llvm-project/commit/178ad93e3f1f2381f05baea300873ee5998ac288
DIFF: https://github.com/llvm/llvm-project/commit/178ad93e3f1f2381f05baea300873ee5998ac288.diff
LOG: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics
Summary:
suggestPathToFileForDiagnostics is actively used in clangd for converting
an absolute path to a header file to a header name as it should be spelled
in the sources. Current approach converts absolute path to relative path.
This diff implements missing logic that makes a reverse lookup from the
relative path to the key in the header map that should be used in the sources.
Prerequisite diff: https://reviews.llvm.org/D103229
Test Plan: check-clang
Reviewers: dexonsmith, bruno, rsmith
Subscribers: cfe-commits
Tasks:
Tags: #clang
Differential Revision: https://reviews.llvm.org/D103142
Added:
Modified:
clang/include/clang/Lex/HeaderMap.h
clang/lib/Lex/HeaderMap.cpp
clang/lib/Lex/HeaderSearch.cpp
clang/unittests/Lex/HeaderSearchTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Lex/HeaderMap.h b/clang/include/clang/Lex/HeaderMap.h
index accb061e51ba3..53108b00bd16d 100644
--- a/clang/include/clang/Lex/HeaderMap.h
+++ b/clang/include/clang/Lex/HeaderMap.h
@@ -16,6 +16,7 @@
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/MemoryBuffer.h"
#include <memory>
@@ -29,6 +30,7 @@ struct HMapHeader;
class HeaderMapImpl {
std::unique_ptr<const llvm::MemoryBuffer> FileBuffer;
bool NeedsBSwap;
+ mutable llvm::StringMap<StringRef> ReverseMap;
public:
HeaderMapImpl(std::unique_ptr<const llvm::MemoryBuffer> File, bool NeedsBSwap)
@@ -48,6 +50,9 @@ class HeaderMapImpl {
/// Print the contents of this headermap to stderr.
void dump() const;
+ /// Return key for specifed path.
+ StringRef reverseLookupFilename(StringRef DestPath) const;
+
private:
unsigned getEndianAdjustedWord(unsigned X) const;
const HMapHeader &getHeader() const;
@@ -79,9 +84,10 @@ class HeaderMap : private HeaderMapImpl {
/// ".." and a filename "../file.h" this would be "../../file.h".
Optional<FileEntryRef> LookupFile(StringRef Filename, FileManager &FM) const;
- using HeaderMapImpl::lookupFilename;
- using HeaderMapImpl::getFileName;
using HeaderMapImpl::dump;
+ using HeaderMapImpl::getFileName;
+ using HeaderMapImpl::lookupFilename;
+ using HeaderMapImpl::reverseLookupFilename;
};
} // end namespace clang.
diff --git a/clang/lib/Lex/HeaderMap.cpp b/clang/lib/Lex/HeaderMap.cpp
index d44ef29c05d13..4b60cfa7b52dd 100644
--- a/clang/lib/Lex/HeaderMap.cpp
+++ b/clang/lib/Lex/HeaderMap.cpp
@@ -240,3 +240,32 @@ StringRef HeaderMapImpl::lookupFilename(StringRef Filename,
return StringRef(DestPath.begin(), DestPath.size());
}
}
+
+StringRef HeaderMapImpl::reverseLookupFilename(StringRef DestPath) const {
+ if (!ReverseMap.empty())
+ return ReverseMap.lookup(DestPath);
+
+ const HMapHeader &Hdr = getHeader();
+ unsigned NumBuckets = getEndianAdjustedWord(Hdr.NumBuckets);
+ StringRef RetKey;
+ for (unsigned i = 0; i != NumBuckets; ++i) {
+ HMapBucket B = getBucket(i);
+ if (B.Key == HMAP_EmptyBucketKey)
+ continue;
+
+ Optional<StringRef> Key = getString(B.Key);
+ Optional<StringRef> Prefix = getString(B.Prefix);
+ Optional<StringRef> Suffix = getString(B.Suffix);
+ if (LLVM_LIKELY(Key && Prefix && Suffix)) {
+ SmallVector<char, 1024> Buf;
+ Buf.append(Prefix->begin(), Prefix->end());
+ Buf.append(Suffix->begin(), Suffix->end());
+ StringRef Value(Buf.begin(), Buf.size());
+ ReverseMap[Value] = *Key;
+
+ if (DestPath == Value)
+ RetKey = *Key;
+ }
+ }
+ return RetKey;
+}
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 99c92e91aad51..9970c3c99a27f 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1834,7 +1834,7 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
};
for (unsigned I = 0; I != SearchDirs.size(); ++I) {
- // FIXME: Support this search within frameworks and header maps.
+ // FIXME: Support this search within frameworks.
if (!SearchDirs[I].isNormalDir())
continue;
@@ -1848,6 +1848,19 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
if (!BestPrefixLength && CheckDir(path::parent_path(MainFile)) && IsSystem)
*IsSystem = false;
+ // Try resolving resulting filename via reverse search in header maps,
+ // key from header name is user prefered name for the include file.
+ StringRef Filename = File.drop_front(BestPrefixLength);
+ for (unsigned I = 0; I != SearchDirs.size(); ++I) {
+ if (!SearchDirs[I].isHeaderMap())
+ continue;
- return path::convert_to_slash(File.drop_front(BestPrefixLength));
+ StringRef SpelledFilename =
+ SearchDirs[I].getHeaderMap()->reverseLookupFilename(Filename);
+ if (!SpelledFilename.empty()) {
+ Filename = SpelledFilename;
+ break;
+ }
+ }
+ return path::convert_to_slash(Filename);
}
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp b/clang/unittests/Lex/HeaderSearchTest.cpp
index abd392477f907..4121a73a52c11 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Lex/HeaderSearch.h"
+#include "HeaderMapTestUtils.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
@@ -45,6 +46,21 @@ class HeaderSearchTest : public ::testing::Test {
Search.AddSearchPath(DL, /*isAngled=*/false);
}
+ void addHeaderMap(llvm::StringRef Filename,
+ std::unique_ptr<llvm::MemoryBuffer> Buf) {
+ VFS->addFile(Filename, 0, std::move(Buf), /*User=*/None, /*Group=*/None,
+ llvm::sys::fs::file_type::regular_file);
+ auto FE = FileMgr.getFile(Filename, true);
+ assert(FE);
+
+ // Test class supports only one HMap at a time.
+ assert(!HMap);
+ HMap = HeaderMap::Create(*FE, FileMgr);
+ auto DL =
+ DirectoryLookup(HMap.get(), SrcMgr::C_User, /*isFramework=*/false);
+ Search.AddSearchPath(DL, /*isAngled=*/false);
+ }
+
IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS;
FileSystemOptions FileMgrOpts;
FileManager FileMgr;
@@ -55,6 +71,7 @@ class HeaderSearchTest : public ::testing::Test {
std::shared_ptr<TargetOptions> TargetOpts;
IntrusiveRefCntPtr<TargetInfo> Target;
HeaderSearch Search;
+ std::unique_ptr<HeaderMap> HMap;
};
TEST_F(HeaderSearchTest, NoSearchDir) {
@@ -136,5 +153,31 @@ TEST_F(HeaderSearchTest, IncludeFromSameDirectory) {
"y/z/t.h");
}
+// Helper struct with null terminator character to make MemoryBuffer happy.
+template <class FileTy, class PaddingTy>
+struct NullTerminatedFile : public FileTy {
+ PaddingTy Padding = 0;
+};
+
+TEST_F(HeaderSearchTest, HeaderMapReverseLookup) {
+ typedef NullTerminatedFile<test::HMapFileMock<2, 32>, char> FileTy;
+ FileTy File;
+ File.init();
+
+ test::HMapFileMockMaker<FileTy> Maker(File);
+ auto a = Maker.addString("d.h");
+ auto b = Maker.addString("b/");
+ auto c = Maker.addString("c.h");
+ Maker.addBucket("d.h", a, b, c);
+
+ addHeaderMap("/x/y/z.hmap", File.getBuffer());
+ addSearchDir("/a");
+
+ EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c.h",
+ /*WorkingDir=*/"",
+ /*MainFile=*/""),
+ "d.h");
+}
+
} // namespace
} // namespace clang
More information about the cfe-commits
mailing list