DmitryPolukhin created this revision.
DmitryPolukhin added reviewers: dexonsmith, bruno, rsmith.
DmitryPolukhin added a project: clang.
Herald added subscribers: usaxena95, ormris, kadircet, arphaman.
DmitryPolukhin requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
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.
Test Plan: check-clang
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D103142
Files:
clang/include/clang/Lex/HeaderMap.h
clang/lib/Lex/HeaderMap.cpp
clang/lib/Lex/HeaderSearch.cpp
clang/unittests/Lex/HeaderMapTest.cpp
clang/unittests/Lex/HeaderMapTestUtils.h
clang/unittests/Lex/HeaderSearchTest.cpp
Index: clang/unittests/Lex/HeaderSearchTest.cpp
===================================================================
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ 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 @@
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 @@
std::shared_ptr<TargetOptions> TargetOpts;
IntrusiveRefCntPtr<TargetInfo> Target;
HeaderSearch Search;
+ std::unique_ptr<HeaderMap> HMap;
};
TEST_F(HeaderSearchTest, NoSearchDir) {
@@ -136,5 +153,31 @@
"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
Index: clang/unittests/Lex/HeaderMapTestUtils.h
===================================================================
--- /dev/null
+++ clang/unittests/Lex/HeaderMapTestUtils.h
@@ -0,0 +1,100 @@
+//===- unittests/Lex/HeaderMapTestUtils.h - HeaderMap utils -------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_UNITTESTS_LEX_HEADERMAPTESTUTILS_H
+#define LLVM_CLANG_UNITTESTS_LEX_HEADERMAPTESTUTILS_H
+
+#include "clang/Basic/CharInfo.h"
+#include "clang/Lex/HeaderMap.h"
+#include "clang/Lex/HeaderMapTypes.h"
+#include "llvm/Support/SwapByteOrder.h"
+#include <cassert>
+
+namespace clang {
+namespace test {
+
+// Lay out a header file for testing.
+template <unsigned NumBuckets, unsigned NumBytes> struct HMapFileMock {
+ HMapHeader Header;
+ HMapBucket Buckets[NumBuckets];
+ unsigned char Bytes[NumBytes];
+
+ void init() {
+ memset(this, 0, sizeof(HMapFileMock));
+ Header.Magic = HMAP_HeaderMagicNumber;
+ Header.Version = HMAP_HeaderVersion;
+ Header.NumBuckets = NumBuckets;
+ Header.StringsOffset = sizeof(Header) + sizeof(Buckets);
+ }
+
+ void swapBytes() {
+ using llvm::sys::getSwappedBytes;
+ Header.Magic = getSwappedBytes(Header.Magic);
+ Header.Version = getSwappedBytes(Header.Version);
+ Header.NumBuckets = getSwappedBytes(Header.NumBuckets);
+ Header.StringsOffset = getSwappedBytes(Header.StringsOffset);
+ }
+
+ std::unique_ptr<llvm::MemoryBuffer> getBuffer() {
+ return llvm::MemoryBuffer::getMemBuffer(
+ StringRef(reinterpret_cast<char *>(this), sizeof(HMapFileMock)),
+ "header",
+ /* RequresNullTerminator */ false);
+ }
+};
+
+template <class FileTy> struct HMapFileMockMaker {
+ FileTy &File;
+ unsigned SI = 1;
+ unsigned BI = 0;
+ HMapFileMockMaker(FileTy &File) : File(File) {}
+
+ unsigned addString(StringRef S) {
+ assert(SI + S.size() + 1 <= sizeof(File.Bytes));
+ std::copy(S.begin(), S.end(), File.Bytes + SI);
+ auto OldSI = SI;
+ SI += S.size() + 1;
+ return OldSI;
+ }
+
+ void addBucket(StringRef Str, unsigned Key, unsigned Prefix,
+ unsigned Suffix) {
+ addBucket(getHash(Str), Key, Prefix, Suffix);
+ }
+
+ void addBucket(unsigned Hash, unsigned Key, unsigned Prefix,
+ unsigned Suffix) {
+ assert(!(File.Header.NumBuckets & (File.Header.NumBuckets - 1)));
+ unsigned I = Hash & (File.Header.NumBuckets - 1);
+ do {
+ if (!File.Buckets[I].Key) {
+ File.Buckets[I].Key = Key;
+ File.Buckets[I].Prefix = Prefix;
+ File.Buckets[I].Suffix = Suffix;
+ ++File.Header.NumEntries;
+ return;
+ }
+ ++I;
+ I &= File.Header.NumBuckets - 1;
+ } while (I != (Hash & (File.Header.NumBuckets - 1)));
+ llvm_unreachable("no empty buckets");
+ }
+
+ // The header map hash function.
+ static unsigned getHash(StringRef Str) {
+ unsigned Result = 0;
+ for (char C : Str)
+ Result += toLowercase(C) * 13;
+ return Result;
+ }
+};
+
+} // namespace test
+} // namespace clang
+
+#endif
Index: clang/unittests/Lex/HeaderMapTest.cpp
===================================================================
--- clang/unittests/Lex/HeaderMapTest.cpp
+++ clang/unittests/Lex/HeaderMapTest.cpp
@@ -6,89 +6,17 @@
//
//===--------------------------------------------------------------===//
-#include "clang/Basic/CharInfo.h"
-#include "clang/Lex/HeaderMap.h"
-#include "clang/Lex/HeaderMapTypes.h"
+#include "HeaderMapTestUtils.h"
#include "llvm/ADT/SmallString.h"
-#include "llvm/Support/SwapByteOrder.h"
#include "gtest/gtest.h"
-#include <cassert>
#include <type_traits>
using namespace clang;
using namespace llvm;
+using namespace clang::test;
namespace {
-// Lay out a header file for testing.
-template <unsigned NumBuckets, unsigned NumBytes> struct MapFile {
- HMapHeader Header;
- HMapBucket Buckets[NumBuckets];
- unsigned char Bytes[NumBytes];
-
- void init() {
- memset(this, 0, sizeof(MapFile));
- Header.Magic = HMAP_HeaderMagicNumber;
- Header.Version = HMAP_HeaderVersion;
- Header.NumBuckets = NumBuckets;
- Header.StringsOffset = sizeof(Header) + sizeof(Buckets);
- }
-
- void swapBytes() {
- using llvm::sys::getSwappedBytes;
- Header.Magic = getSwappedBytes(Header.Magic);
- Header.Version = getSwappedBytes(Header.Version);
- Header.NumBuckets = getSwappedBytes(Header.NumBuckets);
- Header.StringsOffset = getSwappedBytes(Header.StringsOffset);
- }
-
- std::unique_ptr<const MemoryBuffer> getBuffer() const {
- return MemoryBuffer::getMemBuffer(
- StringRef(reinterpret_cast<const char *>(this), sizeof(MapFile)),
- "header",
- /* RequresNullTerminator */ false);
- }
-};
-
-// The header map hash function.
-static inline unsigned getHash(StringRef Str) {
- unsigned Result = 0;
- for (char C : Str)
- Result += toLowercase(C) * 13;
- return Result;
-}
-
-template <class FileTy> struct FileMaker {
- FileTy &File;
- unsigned SI = 1;
- unsigned BI = 0;
- FileMaker(FileTy &File) : File(File) {}
-
- unsigned addString(StringRef S) {
- assert(SI + S.size() + 1 <= sizeof(File.Bytes));
- std::copy(S.begin(), S.end(), File.Bytes + SI);
- auto OldSI = SI;
- SI += S.size() + 1;
- return OldSI;
- }
- void addBucket(unsigned Hash, unsigned Key, unsigned Prefix, unsigned Suffix) {
- assert(!(File.Header.NumBuckets & (File.Header.NumBuckets - 1)));
- unsigned I = Hash & (File.Header.NumBuckets - 1);
- do {
- if (!File.Buckets[I].Key) {
- File.Buckets[I].Key = Key;
- File.Buckets[I].Prefix = Prefix;
- File.Buckets[I].Suffix = Suffix;
- ++File.Header.NumEntries;
- return;
- }
- ++I;
- I &= File.Header.NumBuckets - 1;
- } while (I != (Hash & (File.Header.NumBuckets - 1)));
- llvm_unreachable("no empty buckets");
- }
-};
-
TEST(HeaderMapTest, checkHeaderEmpty) {
bool NeedsSwap;
ASSERT_FALSE(HeaderMapImpl::checkHeader(
@@ -98,7 +26,7 @@
}
TEST(HeaderMapTest, checkHeaderMagic) {
- MapFile<1, 1> File;
+ HMapFileMock<1, 1> File;
File.init();
File.Header.Magic = 0;
bool NeedsSwap;
@@ -106,7 +34,7 @@
}
TEST(HeaderMapTest, checkHeaderReserved) {
- MapFile<1, 1> File;
+ HMapFileMock<1, 1> File;
File.init();
File.Header.Reserved = 1;
bool NeedsSwap;
@@ -114,7 +42,7 @@
}
TEST(HeaderMapTest, checkHeaderVersion) {
- MapFile<1, 1> File;
+ HMapFileMock<1, 1> File;
File.init();
++File.Header.Version;
bool NeedsSwap;
@@ -122,7 +50,7 @@
}
TEST(HeaderMapTest, checkHeaderValidButEmpty) {
- MapFile<1, 1> File;
+ HMapFileMock<1, 1> File;
File.init();
bool NeedsSwap;
ASSERT_TRUE(HeaderMapImpl::checkHeader(*File.getBuffer(), NeedsSwap));
@@ -134,7 +62,7 @@
}
TEST(HeaderMapTest, checkHeader3Buckets) {
- MapFile<3, 1> File;
+ HMapFileMock<3, 1> File;
ASSERT_EQ(3 * sizeof(HMapBucket), sizeof(File.Buckets));
File.init();
@@ -144,7 +72,7 @@
TEST(HeaderMapTest, checkHeader0Buckets) {
// Create with 1 bucket to avoid 0-sized arrays.
- MapFile<1, 1> File;
+ HMapFileMock<1, 1> File;
File.init();
File.Header.NumBuckets = 0;
bool NeedsSwap;
@@ -152,7 +80,7 @@
}
TEST(HeaderMapTest, checkHeaderNotEnoughBuckets) {
- MapFile<1, 1> File;
+ HMapFileMock<1, 1> File;
File.init();
File.Header.NumBuckets = 8;
bool NeedsSwap;
@@ -160,15 +88,15 @@
}
TEST(HeaderMapTest, lookupFilename) {
- typedef MapFile<2, 7> FileTy;
+ typedef HMapFileMock<2, 7> FileTy;
FileTy File;
File.init();
- FileMaker<FileTy> Maker(File);
+ HMapFileMockMaker<FileTy> Maker(File);
auto a = Maker.addString("a");
auto b = Maker.addString("b");
auto c = Maker.addString("c");
- Maker.addBucket(getHash("a"), a, b, c);
+ Maker.addBucket("a", a, b, c);
bool NeedsSwap;
ASSERT_TRUE(HeaderMapImpl::checkHeader(*File.getBuffer(), NeedsSwap));
@@ -185,7 +113,8 @@
};
TEST(HeaderMapTest, lookupFilenameTruncatedSuffix) {
- typedef MapFile<2, 64 - sizeof(HMapHeader) - 2 * sizeof(HMapBucket)> FileTy;
+ typedef HMapFileMock<2, 64 - sizeof(HMapHeader) - 2 * sizeof(HMapBucket)>
+ FileTy;
static_assert(std::is_standard_layout<FileTy>::value,
"Expected standard layout");
static_assert(sizeof(FileTy) == 64, "check the math");
@@ -194,11 +123,11 @@
auto &Padding = P.Padding;
File.init();
- FileMaker<FileTy> Maker(File);
+ HMapFileMockMaker<FileTy> Maker(File);
auto a = Maker.addString("a");
auto b = Maker.addString("b");
auto c = Maker.addString("c");
- Maker.addBucket(getHash("a"), a, b, c);
+ Maker.addBucket("a", a, b, c);
// Add 'x' characters to cause an overflow into Padding.
ASSERT_EQ('c', File.Bytes[5]);
@@ -220,7 +149,8 @@
}
TEST(HeaderMapTest, lookupFilenameTruncatedPrefix) {
- typedef MapFile<2, 64 - sizeof(HMapHeader) - 2 * sizeof(HMapBucket)> FileTy;
+ typedef HMapFileMock<2, 64 - sizeof(HMapHeader) - 2 * sizeof(HMapBucket)>
+ FileTy;
static_assert(std::is_standard_layout<FileTy>::value,
"Expected standard layout");
static_assert(sizeof(FileTy) == 64, "check the math");
@@ -229,11 +159,11 @@
auto &Padding = P.Padding;
File.init();
- FileMaker<FileTy> Maker(File);
+ HMapFileMockMaker<FileTy> Maker(File);
auto a = Maker.addString("a");
auto c = Maker.addString("c");
auto b = Maker.addString("b"); // Store the prefix last.
- Maker.addBucket(getHash("a"), a, b, c);
+ Maker.addBucket("a", a, b, c);
// Add 'x' characters to cause an overflow into Padding.
ASSERT_EQ('b', File.Bytes[5]);
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1834,7 +1834,7 @@
};
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,18 @@
if (!BestPrefixLength && CheckDir(path::parent_path(MainFile)) && IsSystem)
*IsSystem = false;
-
- return path::convert_to_slash(File.drop_front(BestPrefixLength));
+ // Try resolving resulting filaname 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()) {
+ StringRef SpelledFilename =
+ SearchDirs[I].getHeaderMap()->reverseLookupFilename(Filename);
+ if (!SpelledFilename.empty()) {
+ Filename = SpelledFilename;
+ break;
+ }
+ }
+ }
+ return path::convert_to_slash(Filename);
}
Index: clang/lib/Lex/HeaderMap.cpp
===================================================================
--- clang/lib/Lex/HeaderMap.cpp
+++ clang/lib/Lex/HeaderMap.cpp
@@ -240,3 +240,26 @@
return StringRef(DestPath.begin(), DestPath.size());
}
}
+
+StringRef HeaderMapImpl::reverseLookupFilename(StringRef DestPath) const {
+ if (ReverseMap.empty()) {
+ const HMapHeader &Hdr = getHeader();
+ unsigned NumBuckets = getEndianAdjustedWord(Hdr.NumBuckets);
+ 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 (Key && Prefix && Suffix) {
+ SmallVector<char, 1024> Buf;
+ Buf.append(Prefix->begin(), Prefix->end());
+ Buf.append(Suffix->begin(), Suffix->end());
+ ReverseMap[StringRef(Buf.begin(), Buf.size())] = *Key;
+ }
+ }
+ }
+ return ReverseMap.lookup(DestPath);
+}
Index: clang/include/clang/Lex/HeaderMap.h
===================================================================
--- clang/include/clang/Lex/HeaderMap.h
+++ 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 @@
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 @@
/// 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;
@@ -82,6 +87,7 @@
using HeaderMapImpl::lookupFilename;
using HeaderMapImpl::getFileName;
using HeaderMapImpl::dump;
+ using HeaderMapImpl::reverseLookupFilename;
};
} // end namespace clang.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
