https://github.com/keith updated https://github.com/llvm/llvm-project/pull/189475
>From b4d2a800eea862d2fc86d74acfb82036b4003bba Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Mon, 30 Mar 2026 12:13:06 -0700 Subject: [PATCH 1/8] [clang] Fix incorrect filenames for hardlinks Previously hardlinked files would be discovered based on their inode, leading to incorrect filepaths printed in some cases. Fixes https://github.com/llvm/llvm-project/issues/26953 Fixes https://github.com/llvm/llvm-project/issues/58726 Related https://reviews.llvm.org/D137304 --- clang/include/clang/Basic/SourceManager.h | 6 ++ clang/lib/Basic/SourceManager.cpp | 58 ++++++++++----- .../Preprocessor/hardlink-include-names.c | 42 +++++++++++ clang/unittests/Basic/SourceManagerTest.cpp | 72 +++++++++++++++++++ 4 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 clang/test/Preprocessor/hardlink-include-names.c diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index 4217b8683da1e..1939d1aa4915e 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -714,6 +714,12 @@ class SourceManager : public RefCountedBase<SourceManager> { /// as they do not refer to a file. std::vector<SrcMgr::ContentCache*> MemBufferInfos; + /// Per-FileID content caches for aliased file references. + /// + /// These caches preserve the spelling used for a particular include while + /// sharing file contents with the canonical cache in \c FileInfos. + std::vector<SrcMgr::ContentCache *> FileIDContentCaches; + /// The table of SLocEntries that are local to this module. /// /// Positive FileIDs are indexes into this table. Entry 0 indicates an invalid diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index b6cc6ec9365f5..bb4f4c6d87645 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -47,6 +47,21 @@ using llvm::MemoryBuffer; #define DEBUG_TYPE "source-manager" +static SrcMgr::ContentCache * +cloneContentCache(llvm::BumpPtrAllocator &Alloc, + const ContentCache &Other) { + auto *Clone = new (Alloc.Allocate<ContentCache>()) ContentCache; + Clone->OrigEntry = Other.OrigEntry; + Clone->ContentsEntry = Other.ContentsEntry; + Clone->Filename = Other.Filename; + Clone->BufferOverridden = Other.BufferOverridden; + Clone->IsFileVolatile = Other.IsFileVolatile; + Clone->IsTransient = Other.IsTransient; + Clone->IsBufferInvalid = Other.IsBufferInvalid; + Clone->setUnownedBuffer(Other.getBufferIfLoaded()); + return Clone; +} + // Reaching a limit of 2^31 results in a hard error. This metric allows to track // if particular invocation of the compiler is close to it. STATISTIC(MaxUsedSLocBytes, "Maximum number of bytes used by source locations " @@ -324,9 +339,23 @@ SourceManager::~SourceManager() { ContentCacheAlloc.Deallocate(I->second); } } + for (unsigned i = 0, e = FileIDContentCaches.size(); i != e; ++i) { + if (FileIDContentCaches[i]) { + FileIDContentCaches[i]->~ContentCache(); + ContentCacheAlloc.Deallocate(FileIDContentCaches[i]); + } + } } void SourceManager::clearIDTables() { + for (unsigned i = 0, e = FileIDContentCaches.size(); i != e; ++i) { + if (FileIDContentCaches[i]) { + FileIDContentCaches[i]->~ContentCache(); + ContentCacheAlloc.Deallocate(FileIDContentCaches[i]); + } + } + FileIDContentCaches.clear(); + MainFileID = FileID(); LocalSLocEntryTable.clear(); LocalLocOffsetTable.clear(); @@ -361,17 +390,6 @@ bool SourceManager::isMainFile(const FileEntry &SourceFile) { void SourceManager::initializeForReplay(const SourceManager &Old) { assert(MainFileID.isInvalid() && "expected uninitialized SourceManager"); - auto CloneContentCache = [&](const ContentCache *Cache) -> ContentCache * { - auto *Clone = new (ContentCacheAlloc.Allocate<ContentCache>()) ContentCache; - Clone->OrigEntry = Cache->OrigEntry; - Clone->ContentsEntry = Cache->ContentsEntry; - Clone->BufferOverridden = Cache->BufferOverridden; - Clone->IsFileVolatile = Cache->IsFileVolatile; - Clone->IsTransient = Cache->IsTransient; - Clone->setUnownedBuffer(Cache->getBufferIfLoaded()); - return Clone; - }; - // Ensure all SLocEntries are loaded from the external source. for (unsigned I = 0, N = Old.LoadedSLocEntryTable.size(); I != N; ++I) if (!Old.SLocEntryLoaded[I]) @@ -382,7 +400,7 @@ void SourceManager::initializeForReplay(const SourceManager &Old) { SrcMgr::ContentCache *&Slot = FileInfos[FileInfo.first]; if (Slot) continue; - Slot = CloneContentCache(FileInfo.second); + Slot = cloneContentCache(ContentCacheAlloc, *FileInfo.second); } } @@ -542,14 +560,21 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile, SourceLocation::UIntTy LoadedOffset) { SrcMgr::ContentCache &IR = getOrCreateContentCache(SourceFile, isSystem(FileCharacter)); + SrcMgr::ContentCache *Cache = &IR; + + if (IR.OrigEntry && !IR.OrigEntry->isSameRef(SourceFile)) { + Cache = cloneContentCache(ContentCacheAlloc, IR); + Cache->OrigEntry = SourceFile; + FileIDContentCaches.push_back(Cache); + } // If this is a named pipe, immediately load the buffer to ensure subsequent // calls to ContentCache::getSize() are accurate. - if (IR.ContentsEntry->isNamedPipe()) - (void)IR.getBufferOrNone(Diag, getFileManager(), SourceLocation()); + if (Cache->ContentsEntry->isNamedPipe()) + (void)Cache->getBufferOrNone(Diag, getFileManager(), SourceLocation()); - return createFileIDImpl(IR, SourceFile.getName(), IncludePos, FileCharacter, - LoadedID, LoadedOffset); + return createFileIDImpl(*Cache, SourceFile.getName(), IncludePos, + FileCharacter, LoadedID, LoadedOffset); } /// Create a new FileID that represents the specified memory buffer. @@ -2310,6 +2335,7 @@ SourceManager::MemoryBufferSizes SourceManager::getMemoryBufferSizes() const { size_t SourceManager::getDataStructureSizes() const { size_t size = llvm::capacity_in_bytes(MemBufferInfos) + + llvm::capacity_in_bytes(FileIDContentCaches) + llvm::capacity_in_bytes(LocalSLocEntryTable) + llvm::capacity_in_bytes(LoadedSLocEntryTable) + llvm::capacity_in_bytes(SLocEntryLoaded) + diff --git a/clang/test/Preprocessor/hardlink-include-names.c b/clang/test/Preprocessor/hardlink-include-names.c new file mode 100644 index 0000000000000..675ba30438ce9 --- /dev/null +++ b/clang/test/Preprocessor/hardlink-include-names.c @@ -0,0 +1,42 @@ +// Test that symlinked and hardlinked files are reported with their correct +// filenames +// +// REQUIRES: symlinks +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: ln %t/foo.inc %t/bar.inc +// RUN: ln -s %t/foo.inc %t/baz.inc + +// Test 1: -E should show both filenames. +// RUN: %clang_cc1 -E %t/main.c -o - | FileCheck --check-prefix=PP %s +// PP: # 1 "{{.*(/|\\\\)}}foo.inc" 1 +// PP: # 1 "{{.*(/|\\\\)}}bar.inc" 1 +// PP: # 1 "{{.*(/|\\\\)}}baz.inc" 1 + +// Test 2: .d should list both filenames. +// RUN: %clang_cc1 -dependency-file %t/deps.d -MT main.o %t/main.c -fsyntax-only +// RUN: FileCheck --check-prefix=DEPS -input-file=%t/deps.d %s +// DEPS: foo.inc +// DEPS: bar.inc +// DEPS: baz.inc + +// Test 3: --show-includes should list both filenames. +// RUN: %clang_cc1 --show-includes -o /dev/null %t/main.c | \ +// RUN: FileCheck --check-prefix=SHOW %s +// SHOW: Note: including file: {{.*}}foo.inc +// SHOW: Note: including file: {{.*}}bar.inc +// SHOW: Note: including file: {{.*}}baz.inc + +//--- main.c +const char *a = +#include "foo.inc" +; +const char *b = +#include "bar.inc" +; +const char *c = +#include "baz.inc" +; + +//--- foo.inc +"contents" diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp index 7c8aae5c5834f..c4d1763eec2ef 100644 --- a/clang/unittests/Basic/SourceManagerTest.cpp +++ b/clang/unittests/Basic/SourceManagerTest.cpp @@ -10,6 +10,7 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" +#include "clang/Basic/FileSystemStatCache.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" @@ -22,6 +23,7 @@ #include "llvm/Config/llvm-config.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Process.h" +#include "llvm/Support/VirtualFileSystem.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <cstddef> @@ -38,6 +40,41 @@ class SourceManagerTestHelper { namespace { +#ifndef _WIN32 + +class FakeStatCache : public FileSystemStatCache { + llvm::StringMap<llvm::vfs::Status, llvm::BumpPtrAllocator> StatCalls; + +public: + void InjectFile(StringRef Path, ino_t INode) { + StatCalls[Path] = llvm::vfs::Status( + Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + /*User=*/0, /*Group=*/0, /*Size=*/0, + llvm::sys::fs::file_type::regular_file, + llvm::sys::fs::perms::all_all); + } + + void InjectDirectory(StringRef Path, ino_t INode) { + StatCalls[Path] = llvm::vfs::Status( + Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + /*User=*/0, /*Group=*/0, /*Size=*/0, + llvm::sys::fs::file_type::directory_file, + llvm::sys::fs::perms::all_all); + } + + std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, + bool isFile, std::unique_ptr<llvm::vfs::File> *F, + llvm::vfs::FileSystem &FS) override { + auto It = StatCalls.find(Path); + if (It == StatCalls.end()) + return std::make_error_code(std::errc::no_such_file_or_directory); + Status = It->second; + return std::error_code(); + } +}; + +#endif + // The test fixture. class SourceManagerTest : public ::testing::Test { protected: @@ -417,6 +454,41 @@ TEST_F(SourceManagerTest, getLineNumber) { ASSERT_NO_FATAL_FAILURE(SourceMgr.getLineNumber(mainFileID, 1, nullptr)); } +#ifndef _WIN32 + +TEST_F(SourceManagerTest, aliasedFilesKeepRequestedNamesPerFileID) { + auto StatCache = std::make_unique<FakeStatCache>(); + StatCache->InjectDirectory("dir", 40); + StatCache->InjectFile("dir/foo.h", 41); + StatCache->InjectFile("dir/bar.h", 41); + FileMgr.setStatCache(std::move(StatCache)); + + auto FooOrErr = FileMgr.getFileRef("dir/foo.h"); + auto BarOrErr = FileMgr.getFileRef("dir/bar.h"); + ASSERT_TRUE(static_cast<bool>(FooOrErr)); + ASSERT_TRUE(static_cast<bool>(BarOrErr)); + + FileEntryRef Foo = *FooOrErr; + FileEntryRef Bar = *BarOrErr; + EXPECT_FALSE(Foo.isSameRef(Bar)); + EXPECT_EQ(Foo, Bar); + + SourceMgr.overrideFileContents(Foo, llvm::MemoryBuffer::getMemBuffer("x\n")); + + FileID FooID = SourceMgr.createFileID(Foo, SourceLocation(), SrcMgr::C_User); + FileID BarID = SourceMgr.createFileID(Bar, SourceLocation(), SrcMgr::C_User); + + SourceLocation FooLoc = SourceMgr.getLocForStartOfFile(FooID); + SourceLocation BarLoc = SourceMgr.getLocForStartOfFile(BarID); + + EXPECT_EQ("dir/foo.h", SourceMgr.getFilename(FooLoc)); + EXPECT_EQ("dir/bar.h", SourceMgr.getFilename(BarLoc)); + EXPECT_STREQ("dir/foo.h", SourceMgr.getPresumedLoc(FooLoc).getFilename()); + EXPECT_STREQ("dir/bar.h", SourceMgr.getPresumedLoc(BarLoc).getFilename()); +} + +#endif + struct FakeExternalSLocEntrySource : ExternalSLocEntrySource { bool ReadSLocEntry(int ID) override { return {}; } int getSLocEntryID(SourceLocation::UIntTy SLocOffset) override { return 0; } >From afcc676e1a9a20e2c6a006a87296b5a1f58c2e8e Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Mon, 30 Mar 2026 13:48:28 -0700 Subject: [PATCH 2/8] format --- clang/lib/Basic/SourceManager.cpp | 5 ++--- clang/unittests/Basic/SourceManagerTest.cpp | 13 ++++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index bb4f4c6d87645..b27a07dc677b6 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -47,9 +47,8 @@ using llvm::MemoryBuffer; #define DEBUG_TYPE "source-manager" -static SrcMgr::ContentCache * -cloneContentCache(llvm::BumpPtrAllocator &Alloc, - const ContentCache &Other) { +static SrcMgr::ContentCache *cloneContentCache(llvm::BumpPtrAllocator &Alloc, + const ContentCache &Other) { auto *Clone = new (Alloc.Allocate<ContentCache>()) ContentCache; Clone->OrigEntry = Other.OrigEntry; Clone->ContentsEntry = Other.ContentsEntry; diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp index c4d1763eec2ef..2594d35a19960 100644 --- a/clang/unittests/Basic/SourceManagerTest.cpp +++ b/clang/unittests/Basic/SourceManagerTest.cpp @@ -50,16 +50,15 @@ class FakeStatCache : public FileSystemStatCache { StatCalls[Path] = llvm::vfs::Status( Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, /*User=*/0, /*Group=*/0, /*Size=*/0, - llvm::sys::fs::file_type::regular_file, - llvm::sys::fs::perms::all_all); + llvm::sys::fs::file_type::regular_file, llvm::sys::fs::perms::all_all); } void InjectDirectory(StringRef Path, ino_t INode) { - StatCalls[Path] = llvm::vfs::Status( - Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, - /*User=*/0, /*Group=*/0, /*Size=*/0, - llvm::sys::fs::file_type::directory_file, - llvm::sys::fs::perms::all_all); + StatCalls[Path] = + llvm::vfs::Status(Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + /*User=*/0, /*Group=*/0, /*Size=*/0, + llvm::sys::fs::file_type::directory_file, + llvm::sys::fs::perms::all_all); } std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, >From fb75178945560277477f74aa53bd2b35d838cbd1 Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Mon, 30 Mar 2026 15:41:14 -0700 Subject: [PATCH 3/8] Fix path comparison for unimportant differences Tests revealed some places where foo\\bar and foo/bar were treated differently because of my change --- clang/lib/Basic/SourceManager.cpp | 37 +++++- clang/unittests/Basic/SourceManagerTest.cpp | 121 +++++++++++++++++--- 2 files changed, 135 insertions(+), 23 deletions(-) diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index b27a07dc677b6..4b04ea7f2ae58 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -30,6 +30,7 @@ #include "llvm/Support/Endian.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" #include <algorithm> #include <cassert> @@ -61,6 +62,26 @@ static SrcMgr::ContentCache *cloneContentCache(llvm::BumpPtrAllocator &Alloc, return Clone; } +static SmallString<128> normalizeFilePathForComparison(const FileManager &FM, + StringRef Name) { + SmallString<128> Path(Name); + FM.makeAbsolutePath(Path, /*Canonicalize=*/true); + llvm::sys::path::native(Path); + return Path; +} + +static bool referencesDistinctFilePaths(const FileManager &FM, FileEntryRef LHS, + FileEntryRef RHS) { + SmallString<128> LHSPath = normalizeFilePathForComparison(FM, LHS.getName()); + SmallString<128> RHSPath = normalizeFilePathForComparison(FM, RHS.getName()); + StringRef LHSRef(LHSPath); + StringRef RHSRef(RHSPath); + + if (llvm::sys::path::is_style_windows(llvm::sys::path::Style::native)) + return !LHSRef.equals_insensitive(RHSRef); + return LHSRef != RHSRef; +} + // Reaching a limit of 2^31 results in a hard error. This metric allows to track // if particular invocation of the compiler is close to it. STATISTIC(MaxUsedSLocBytes, "Maximum number of bytes used by source locations " @@ -560,11 +581,17 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile, SrcMgr::ContentCache &IR = getOrCreateContentCache(SourceFile, isSystem(FileCharacter)); SrcMgr::ContentCache *Cache = &IR; + StringRef Filename = SourceFile.getName(); if (IR.OrigEntry && !IR.OrigEntry->isSameRef(SourceFile)) { - Cache = cloneContentCache(ContentCacheAlloc, IR); - Cache->OrigEntry = SourceFile; - FileIDContentCaches.push_back(Cache); + if (referencesDistinctFilePaths(getFileManager(), *IR.OrigEntry, + SourceFile)) { + Cache = cloneContentCache(ContentCacheAlloc, IR); + Cache->OrigEntry = SourceFile; + FileIDContentCaches.push_back(Cache); + } else { + Filename = IR.OrigEntry->getName(); + } } // If this is a named pipe, immediately load the buffer to ensure subsequent @@ -572,8 +599,8 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile, if (Cache->ContentsEntry->isNamedPipe()) (void)Cache->getBufferOrNone(Diag, getFileManager(), SourceLocation()); - return createFileIDImpl(*Cache, SourceFile.getName(), IncludePos, - FileCharacter, LoadedID, LoadedOffset); + return createFileIDImpl(*Cache, Filename, IncludePos, FileCharacter, + LoadedID, LoadedOffset); } /// Create a new FileID that represents the specified memory buffer. diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp index 2594d35a19960..3f69dd5394bcd 100644 --- a/clang/unittests/Basic/SourceManagerTest.cpp +++ b/clang/unittests/Basic/SourceManagerTest.cpp @@ -40,22 +40,20 @@ class SourceManagerTestHelper { namespace { -#ifndef _WIN32 - class FakeStatCache : public FileSystemStatCache { llvm::StringMap<llvm::vfs::Status, llvm::BumpPtrAllocator> StatCalls; public: - void InjectFile(StringRef Path, ino_t INode) { + void InjectFile(StringRef Path, uint64_t File) { StatCalls[Path] = llvm::vfs::Status( - Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + Path, llvm::sys::fs::UniqueID(1, File), /*MTime=*/{}, /*User=*/0, /*Group=*/0, /*Size=*/0, llvm::sys::fs::file_type::regular_file, llvm::sys::fs::perms::all_all); } - void InjectDirectory(StringRef Path, ino_t INode) { + void InjectDirectory(StringRef Path, uint64_t File) { StatCalls[Path] = - llvm::vfs::Status(Path, llvm::sys::fs::UniqueID(1, INode), /*MTime=*/{}, + llvm::vfs::Status(Path, llvm::sys::fs::UniqueID(1, File), /*MTime=*/{}, /*User=*/0, /*Group=*/0, /*Size=*/0, llvm::sys::fs::file_type::directory_file, llvm::sys::fs::perms::all_all); @@ -72,8 +70,6 @@ class FakeStatCache : public FileSystemStatCache { } }; -#endif - // The test fixture. class SourceManagerTest : public ::testing::Test { protected: @@ -453,17 +449,25 @@ TEST_F(SourceManagerTest, getLineNumber) { ASSERT_NO_FATAL_FAILURE(SourceMgr.getLineNumber(mainFileID, 1, nullptr)); } -#ifndef _WIN32 - TEST_F(SourceManagerTest, aliasedFilesKeepRequestedNamesPerFileID) { +#ifdef _WIN32 + constexpr StringRef DirPath = "dir"; + constexpr StringRef FooPath = "dir\\foo.h"; + constexpr StringRef BarPath = "dir\\bar.h"; +#else + constexpr StringRef DirPath = "dir"; + constexpr StringRef FooPath = "dir/foo.h"; + constexpr StringRef BarPath = "dir/bar.h"; +#endif + auto StatCache = std::make_unique<FakeStatCache>(); - StatCache->InjectDirectory("dir", 40); - StatCache->InjectFile("dir/foo.h", 41); - StatCache->InjectFile("dir/bar.h", 41); + StatCache->InjectDirectory(DirPath, 40); + StatCache->InjectFile(FooPath, 41); + StatCache->InjectFile(BarPath, 41); FileMgr.setStatCache(std::move(StatCache)); - auto FooOrErr = FileMgr.getFileRef("dir/foo.h"); - auto BarOrErr = FileMgr.getFileRef("dir/bar.h"); + auto FooOrErr = FileMgr.getFileRef(FooPath); + auto BarOrErr = FileMgr.getFileRef(BarPath); ASSERT_TRUE(static_cast<bool>(FooOrErr)); ASSERT_TRUE(static_cast<bool>(BarOrErr)); @@ -480,12 +484,93 @@ TEST_F(SourceManagerTest, aliasedFilesKeepRequestedNamesPerFileID) { SourceLocation FooLoc = SourceMgr.getLocForStartOfFile(FooID); SourceLocation BarLoc = SourceMgr.getLocForStartOfFile(BarID); + EXPECT_EQ(FooPath, SourceMgr.getFilename(FooLoc)); + EXPECT_EQ(BarPath, SourceMgr.getFilename(BarLoc)); + EXPECT_STREQ(FooPath.data(), SourceMgr.getPresumedLoc(FooLoc).getFilename()); + EXPECT_STREQ(BarPath.data(), SourceMgr.getPresumedLoc(BarLoc).getFilename()); +} + +TEST_F(SourceManagerTest, equivalentPathSpellingsReuseOriginalFileName) { +#ifdef _WIN32 + constexpr StringRef DirPath = "dir"; + constexpr StringRef DotDirPath = ".\\dir"; + constexpr StringRef FooPath = "dir\\foo.h"; + constexpr StringRef DotFooPath = ".\\dir\\foo.h"; +#else + constexpr StringRef DirPath = "dir"; + constexpr StringRef DotDirPath = "./dir"; + constexpr StringRef FooPath = "dir/foo.h"; + constexpr StringRef DotFooPath = "./dir/foo.h"; +#endif + + auto StatCache = std::make_unique<FakeStatCache>(); + StatCache->InjectDirectory(DirPath, 40); + StatCache->InjectDirectory(DotDirPath, 40); + StatCache->InjectFile(FooPath, 41); + StatCache->InjectFile(DotFooPath, 41); + FileMgr.setStatCache(std::move(StatCache)); + + auto FooOrErr = FileMgr.getFileRef(FooPath); + auto DotFooOrErr = FileMgr.getFileRef(DotFooPath); + ASSERT_TRUE(static_cast<bool>(FooOrErr)); + ASSERT_TRUE(static_cast<bool>(DotFooOrErr)); + + FileEntryRef Foo = *FooOrErr; + FileEntryRef DotFoo = *DotFooOrErr; + EXPECT_FALSE(Foo.isSameRef(DotFoo)); + EXPECT_EQ(Foo, DotFoo); + + SourceMgr.overrideFileContents(Foo, llvm::MemoryBuffer::getMemBuffer("x\n")); + + FileID FooID = SourceMgr.createFileID(Foo, SourceLocation(), SrcMgr::C_User); + FileID DotFooID = + SourceMgr.createFileID(DotFoo, SourceLocation(), SrcMgr::C_User); + + SourceLocation FooLoc = SourceMgr.getLocForStartOfFile(FooID); + SourceLocation DotFooLoc = SourceMgr.getLocForStartOfFile(DotFooID); + + EXPECT_EQ(FooPath, SourceMgr.getFilename(FooLoc)); + EXPECT_EQ(FooPath, SourceMgr.getFilename(DotFooLoc)); + EXPECT_STREQ(FooPath.data(), SourceMgr.getPresumedLoc(FooLoc).getFilename()); + EXPECT_STREQ(FooPath.data(), + SourceMgr.getPresumedLoc(DotFooLoc).getFilename()); + EXPECT_EQ(FooPath, *SourceMgr.getNonBuiltinFilenameForID(DotFooID)); +} + +#ifdef _WIN32 +TEST_F(SourceManagerTest, windowsEquivalentPathSpellingsReuseOriginalFileName) { + auto StatCache = std::make_unique<FakeStatCache>(); + StatCache->InjectDirectory("dir", 40); + StatCache->InjectDirectory(".\\DIR", 40); + StatCache->InjectFile("dir/foo.h", 41); + StatCache->InjectFile(".\\DIR\\foo.h", 41); + FileMgr.setStatCache(std::move(StatCache)); + + auto FooOrErr = FileMgr.getFileRef("dir/foo.h"); + auto WinFooOrErr = FileMgr.getFileRef(".\\DIR\\foo.h"); + ASSERT_TRUE(static_cast<bool>(FooOrErr)); + ASSERT_TRUE(static_cast<bool>(WinFooOrErr)); + + FileEntryRef Foo = *FooOrErr; + FileEntryRef WinFoo = *WinFooOrErr; + EXPECT_FALSE(Foo.isSameRef(WinFoo)); + EXPECT_EQ(Foo, WinFoo); + + SourceMgr.overrideFileContents(Foo, llvm::MemoryBuffer::getMemBuffer("x\n")); + + FileID FooID = SourceMgr.createFileID(Foo, SourceLocation(), SrcMgr::C_User); + FileID WinFooID = + SourceMgr.createFileID(WinFoo, SourceLocation(), SrcMgr::C_User); + + SourceLocation FooLoc = SourceMgr.getLocForStartOfFile(FooID); + SourceLocation WinFooLoc = SourceMgr.getLocForStartOfFile(WinFooID); + EXPECT_EQ("dir/foo.h", SourceMgr.getFilename(FooLoc)); - EXPECT_EQ("dir/bar.h", SourceMgr.getFilename(BarLoc)); + EXPECT_EQ("dir/foo.h", SourceMgr.getFilename(WinFooLoc)); EXPECT_STREQ("dir/foo.h", SourceMgr.getPresumedLoc(FooLoc).getFilename()); - EXPECT_STREQ("dir/bar.h", SourceMgr.getPresumedLoc(BarLoc).getFilename()); + EXPECT_STREQ("dir/foo.h", SourceMgr.getPresumedLoc(WinFooLoc).getFilename()); + EXPECT_EQ("dir/foo.h", *SourceMgr.getNonBuiltinFilenameForID(WinFooID)); } - #endif struct FakeExternalSLocEntrySource : ExternalSLocEntrySource { >From 07cf770ae7c4d4e7ee48609bce15374ba99e1baf Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Mon, 30 Mar 2026 16:22:35 -0700 Subject: [PATCH 4/8] lint --- clang/lib/Basic/SourceManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index 4b04ea7f2ae58..0c4dc65b0d85e 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -599,8 +599,8 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile, if (Cache->ContentsEntry->isNamedPipe()) (void)Cache->getBufferOrNone(Diag, getFileManager(), SourceLocation()); - return createFileIDImpl(*Cache, Filename, IncludePos, FileCharacter, - LoadedID, LoadedOffset); + return createFileIDImpl(*Cache, Filename, IncludePos, FileCharacter, LoadedID, + LoadedOffset); } /// Create a new FileID that represents the specified memory buffer. >From fd8755cfdbaff9ebbb40514157dd7bcb106ce564 Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Thu, 4 Jun 2026 16:17:15 -0700 Subject: [PATCH 5/8] Update tests for stat cache removal, add new case --- clang/unittests/Basic/SourceManagerTest.cpp | 156 ++++++++++---------- 1 file changed, 80 insertions(+), 76 deletions(-) diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp index 3f69dd5394bcd..1e80e3bf9c299 100644 --- a/clang/unittests/Basic/SourceManagerTest.cpp +++ b/clang/unittests/Basic/SourceManagerTest.cpp @@ -10,7 +10,6 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" -#include "clang/Basic/FileSystemStatCache.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" @@ -40,48 +39,30 @@ class SourceManagerTestHelper { namespace { -class FakeStatCache : public FileSystemStatCache { - llvm::StringMap<llvm::vfs::Status, llvm::BumpPtrAllocator> StatCalls; - -public: - void InjectFile(StringRef Path, uint64_t File) { - StatCalls[Path] = llvm::vfs::Status( - Path, llvm::sys::fs::UniqueID(1, File), /*MTime=*/{}, - /*User=*/0, /*Group=*/0, /*Size=*/0, - llvm::sys::fs::file_type::regular_file, llvm::sys::fs::perms::all_all); - } - - void InjectDirectory(StringRef Path, uint64_t File) { - StatCalls[Path] = - llvm::vfs::Status(Path, llvm::sys::fs::UniqueID(1, File), /*MTime=*/{}, - /*User=*/0, /*Group=*/0, /*Size=*/0, - llvm::sys::fs::file_type::directory_file, - llvm::sys::fs::perms::all_all); - } - - std::error_code getStat(StringRef Path, llvm::vfs::Status &Status, - bool isFile, std::unique_ptr<llvm::vfs::File> *F, - llvm::vfs::FileSystem &FS) override { - auto It = StatCalls.find(Path); - if (It == StatCalls.end()) - return std::make_error_code(std::errc::no_such_file_or_directory); - Status = It->second; - return std::error_code(); - } -}; - // The test fixture. class SourceManagerTest : public ::testing::Test { protected: SourceManagerTest() - : FileMgr(FileMgrOpts), + : FileMgr(FileMgrOpts, FS), Diags(DiagnosticIDs::create(), DiagOpts, new IgnoringDiagConsumer()), SourceMgr(Diags, FileMgr), TargetOpts(new TargetOptions) { TargetOpts->Triple = "x86_64-apple-darwin11.1.0"; Target = TargetInfo::CreateTargetInfo(Diags, *TargetOpts); } + void AddFile(StringRef Path) { + ASSERT_TRUE(FS->addFile(Path, /*ModificationTime=*/0, + llvm::MemoryBuffer::getMemBuffer("x\n"))); + } + + void AddHardLink(StringRef NewLink, StringRef Target) { + ASSERT_TRUE(FS->addHardLink(NewLink, Target)); + } + FileSystemOptions FileMgrOpts; + IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS = + llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>( + /*UseNormalizedPaths=*/false); FileManager FileMgr; DiagnosticOptions DiagOpts; DiagnosticsEngine Diags; @@ -451,20 +432,15 @@ TEST_F(SourceManagerTest, getLineNumber) { TEST_F(SourceManagerTest, aliasedFilesKeepRequestedNamesPerFileID) { #ifdef _WIN32 - constexpr StringRef DirPath = "dir"; - constexpr StringRef FooPath = "dir\\foo.h"; - constexpr StringRef BarPath = "dir\\bar.h"; + constexpr StringRef FooPath = "C:\\dir\\foo.h"; + constexpr StringRef BarPath = "C:\\dir\\bar.h"; #else - constexpr StringRef DirPath = "dir"; - constexpr StringRef FooPath = "dir/foo.h"; - constexpr StringRef BarPath = "dir/bar.h"; + constexpr StringRef FooPath = "/dir/foo.h"; + constexpr StringRef BarPath = "/dir/bar.h"; #endif - auto StatCache = std::make_unique<FakeStatCache>(); - StatCache->InjectDirectory(DirPath, 40); - StatCache->InjectFile(FooPath, 41); - StatCache->InjectFile(BarPath, 41); - FileMgr.setStatCache(std::move(StatCache)); + AddFile(FooPath); + AddHardLink(BarPath, FooPath); auto FooOrErr = FileMgr.getFileRef(FooPath); auto BarOrErr = FileMgr.getFileRef(BarPath); @@ -490,25 +466,17 @@ TEST_F(SourceManagerTest, aliasedFilesKeepRequestedNamesPerFileID) { EXPECT_STREQ(BarPath.data(), SourceMgr.getPresumedLoc(BarLoc).getFilename()); } -TEST_F(SourceManagerTest, equivalentPathSpellingsReuseOriginalFileName) { +TEST_F(SourceManagerTest, dotPathSpellingsKeepRequestedNamesPerFileID) { #ifdef _WIN32 - constexpr StringRef DirPath = "dir"; - constexpr StringRef DotDirPath = ".\\dir"; - constexpr StringRef FooPath = "dir\\foo.h"; - constexpr StringRef DotFooPath = ".\\dir\\foo.h"; + constexpr StringRef FooPath = "C:\\dir\\foo.h"; + constexpr StringRef DotFooPath = "C:\\.\\dir\\foo.h"; #else - constexpr StringRef DirPath = "dir"; - constexpr StringRef DotDirPath = "./dir"; - constexpr StringRef FooPath = "dir/foo.h"; - constexpr StringRef DotFooPath = "./dir/foo.h"; + constexpr StringRef FooPath = "/dir/foo.h"; + constexpr StringRef DotFooPath = "/./dir/foo.h"; #endif - auto StatCache = std::make_unique<FakeStatCache>(); - StatCache->InjectDirectory(DirPath, 40); - StatCache->InjectDirectory(DotDirPath, 40); - StatCache->InjectFile(FooPath, 41); - StatCache->InjectFile(DotFooPath, 41); - FileMgr.setStatCache(std::move(StatCache)); + AddFile(FooPath); + AddHardLink(DotFooPath, FooPath); auto FooOrErr = FileMgr.getFileRef(FooPath); auto DotFooOrErr = FileMgr.getFileRef(DotFooPath); @@ -530,24 +498,59 @@ TEST_F(SourceManagerTest, equivalentPathSpellingsReuseOriginalFileName) { SourceLocation DotFooLoc = SourceMgr.getLocForStartOfFile(DotFooID); EXPECT_EQ(FooPath, SourceMgr.getFilename(FooLoc)); - EXPECT_EQ(FooPath, SourceMgr.getFilename(DotFooLoc)); + EXPECT_EQ(DotFooPath, SourceMgr.getFilename(DotFooLoc)); EXPECT_STREQ(FooPath.data(), SourceMgr.getPresumedLoc(FooLoc).getFilename()); - EXPECT_STREQ(FooPath.data(), + EXPECT_STREQ(DotFooPath.data(), SourceMgr.getPresumedLoc(DotFooLoc).getFilename()); - EXPECT_EQ(FooPath, *SourceMgr.getNonBuiltinFilenameForID(DotFooID)); + EXPECT_EQ(DotFooPath, *SourceMgr.getNonBuiltinFilenameForID(DotFooID)); +} + +TEST_F(SourceManagerTest, dotDotPathSpellingsKeepRequestedNamesPerFileID) { +#ifdef _WIN32 + constexpr StringRef BPath = "C:\\a\\b\\..\\c.h"; + constexpr StringRef XPath = "C:\\a\\x\\..\\c.h"; +#else + constexpr StringRef BPath = "/a/b/../c.h"; + constexpr StringRef XPath = "/a/x/../c.h"; +#endif + + AddFile(BPath); + AddHardLink(XPath, BPath); + + auto BOrErr = FileMgr.getFileRef(BPath); + auto XOrErr = FileMgr.getFileRef(XPath); + ASSERT_TRUE(static_cast<bool>(BOrErr)); + ASSERT_TRUE(static_cast<bool>(XOrErr)); + + FileEntryRef B = *BOrErr; + FileEntryRef X = *XOrErr; + EXPECT_FALSE(B.isSameRef(X)); + EXPECT_EQ(B, X); + + SourceMgr.overrideFileContents(B, llvm::MemoryBuffer::getMemBuffer("x\n")); + + FileID BID = SourceMgr.createFileID(B, SourceLocation(), SrcMgr::C_User); + FileID XID = SourceMgr.createFileID(X, SourceLocation(), SrcMgr::C_User); + + SourceLocation BLoc = SourceMgr.getLocForStartOfFile(BID); + SourceLocation XLoc = SourceMgr.getLocForStartOfFile(XID); + + EXPECT_EQ(BPath, SourceMgr.getFilename(BLoc)); + EXPECT_EQ(XPath, SourceMgr.getFilename(XLoc)); + EXPECT_STREQ(BPath.data(), SourceMgr.getPresumedLoc(BLoc).getFilename()); + EXPECT_STREQ(XPath.data(), SourceMgr.getPresumedLoc(XLoc).getFilename()); + EXPECT_EQ(XPath, *SourceMgr.getNonBuiltinFilenameForID(XID)); } #ifdef _WIN32 -TEST_F(SourceManagerTest, windowsEquivalentPathSpellingsReuseOriginalFileName) { - auto StatCache = std::make_unique<FakeStatCache>(); - StatCache->InjectDirectory("dir", 40); - StatCache->InjectDirectory(".\\DIR", 40); - StatCache->InjectFile("dir/foo.h", 41); - StatCache->InjectFile(".\\DIR\\foo.h", 41); - FileMgr.setStatCache(std::move(StatCache)); - - auto FooOrErr = FileMgr.getFileRef("dir/foo.h"); - auto WinFooOrErr = FileMgr.getFileRef(".\\DIR\\foo.h"); +TEST_F(SourceManagerTest, windowsSeparatorSpellingsReuseOriginalFileName) { + constexpr StringRef FooPath = "C:/dir/foo.h"; + constexpr StringRef WinFooPath = "C:\\dir\\foo.h"; + + AddFile(FooPath); + + auto FooOrErr = FileMgr.getFileRef(FooPath); + auto WinFooOrErr = FileMgr.getFileRef(WinFooPath); ASSERT_TRUE(static_cast<bool>(FooOrErr)); ASSERT_TRUE(static_cast<bool>(WinFooOrErr)); @@ -565,11 +568,12 @@ TEST_F(SourceManagerTest, windowsEquivalentPathSpellingsReuseOriginalFileName) { SourceLocation FooLoc = SourceMgr.getLocForStartOfFile(FooID); SourceLocation WinFooLoc = SourceMgr.getLocForStartOfFile(WinFooID); - EXPECT_EQ("dir/foo.h", SourceMgr.getFilename(FooLoc)); - EXPECT_EQ("dir/foo.h", SourceMgr.getFilename(WinFooLoc)); - EXPECT_STREQ("dir/foo.h", SourceMgr.getPresumedLoc(FooLoc).getFilename()); - EXPECT_STREQ("dir/foo.h", SourceMgr.getPresumedLoc(WinFooLoc).getFilename()); - EXPECT_EQ("dir/foo.h", *SourceMgr.getNonBuiltinFilenameForID(WinFooID)); + EXPECT_EQ(FooPath, SourceMgr.getFilename(FooLoc)); + EXPECT_EQ(FooPath, SourceMgr.getFilename(WinFooLoc)); + EXPECT_STREQ(FooPath.data(), SourceMgr.getPresumedLoc(FooLoc).getFilename()); + EXPECT_STREQ(FooPath.data(), + SourceMgr.getPresumedLoc(WinFooLoc).getFilename()); + EXPECT_EQ(FooPath, *SourceMgr.getNonBuiltinFilenameForID(WinFooID)); } #endif >From a0069ea2132e6ca5f3b4c8031967a5f00005777a Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Thu, 4 Jun 2026 17:21:33 -0700 Subject: [PATCH 6/8] Compare files less strictly --- clang/lib/Basic/SourceManager.cpp | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index 0c4dc65b0d85e..5679ef607cbdb 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -38,6 +38,7 @@ #include <cstdint> #include <memory> #include <optional> +#include <string> #include <tuple> #include <utility> #include <vector> @@ -62,23 +63,11 @@ static SrcMgr::ContentCache *cloneContentCache(llvm::BumpPtrAllocator &Alloc, return Clone; } -static SmallString<128> normalizeFilePathForComparison(const FileManager &FM, - StringRef Name) { - SmallString<128> Path(Name); - FM.makeAbsolutePath(Path, /*Canonicalize=*/true); - llvm::sys::path::native(Path); - return Path; -} - -static bool referencesDistinctFilePaths(const FileManager &FM, FileEntryRef LHS, - FileEntryRef RHS) { - SmallString<128> LHSPath = normalizeFilePathForComparison(FM, LHS.getName()); - SmallString<128> RHSPath = normalizeFilePathForComparison(FM, RHS.getName()); +static bool pathDiffersIgnoringWindowsSlashes(FileEntryRef LHS, FileEntryRef RHS) { + std::string LHSPath = llvm::sys::path::convert_to_slash(LHS.getName()); + std::string RHSPath = llvm::sys::path::convert_to_slash(RHS.getName()); StringRef LHSRef(LHSPath); StringRef RHSRef(RHSPath); - - if (llvm::sys::path::is_style_windows(llvm::sys::path::Style::native)) - return !LHSRef.equals_insensitive(RHSRef); return LHSRef != RHSRef; } @@ -584,8 +573,7 @@ FileID SourceManager::createFileID(FileEntryRef SourceFile, StringRef Filename = SourceFile.getName(); if (IR.OrigEntry && !IR.OrigEntry->isSameRef(SourceFile)) { - if (referencesDistinctFilePaths(getFileManager(), *IR.OrigEntry, - SourceFile)) { + if (pathDiffersIgnoringWindowsSlashes(*IR.OrigEntry, SourceFile)) { Cache = cloneContentCache(ContentCacheAlloc, IR); Cache->OrigEntry = SourceFile; FileIDContentCaches.push_back(Cache); >From 545bbdeb0307612c046b89f1d7be751bafe4b618 Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Thu, 4 Jun 2026 17:29:03 -0700 Subject: [PATCH 7/8] clang-format --- clang/lib/Basic/SourceManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index 5679ef607cbdb..d7815e4ffdf36 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -63,7 +63,8 @@ static SrcMgr::ContentCache *cloneContentCache(llvm::BumpPtrAllocator &Alloc, return Clone; } -static bool pathDiffersIgnoringWindowsSlashes(FileEntryRef LHS, FileEntryRef RHS) { +static bool pathDiffersIgnoringWindowsSlashes(FileEntryRef LHS, + FileEntryRef RHS) { std::string LHSPath = llvm::sys::path::convert_to_slash(LHS.getName()); std::string RHSPath = llvm::sys::path::convert_to_slash(RHS.getName()); StringRef LHSRef(LHSPath); >From 4cc2a7b4245a208b4bf2cacb266eb8df186a8d58 Mon Sep 17 00:00:00 2001 From: Keith Smiley <[email protected]> Date: Fri, 5 Jun 2026 09:48:35 -0700 Subject: [PATCH 8/8] Fix test on windows the FS doesn't normalize the path --- clang/unittests/Basic/SourceManagerTest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/unittests/Basic/SourceManagerTest.cpp b/clang/unittests/Basic/SourceManagerTest.cpp index 1e80e3bf9c299..09f78003b9812 100644 --- a/clang/unittests/Basic/SourceManagerTest.cpp +++ b/clang/unittests/Basic/SourceManagerTest.cpp @@ -548,6 +548,7 @@ TEST_F(SourceManagerTest, windowsSeparatorSpellingsReuseOriginalFileName) { constexpr StringRef WinFooPath = "C:\\dir\\foo.h"; AddFile(FooPath); + AddHardLink(WinFooPath, FooPath); auto FooOrErr = FileMgr.getFileRef(FooPath); auto WinFooOrErr = FileMgr.getFileRef(WinFooPath); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
