benlangmuir created this revision.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When including a header, store the filename per include (per FileInfo)
rather than storing it once in the ContentCache and mutating it. This
fixes the spelling of includes for -E and --show-includes when working
with symlinks/hardlinks for headers included multiple times.
Logically, the filename and FileEntryRef belong to the FileInfo, but to
avoid increasing the size of SLocEntry we indirect them in a new struct
NamedContentCache.
TODO
- Evaluate the performance cost of indirecting ContentCache an extra level.
- Audit uses of FileInfos to see if we need to do extra for for NamedFileInfos.
- Consider making the FileEntryRef changes here the default -- it doesn't make
sense to me that FileEntryRef == or DenseMap would match FileEntry pointer
semantics instead of filename semantics.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137304
Files:
clang/include/clang/Basic/FileEntry.h
clang/include/clang/Basic/SourceManager.h
clang/lib/Basic/SourceManager.cpp
clang/lib/Serialization/ASTReader.cpp
clang/test/Frontend/show-includes-symlink.c
clang/test/Preprocessor/header-symlink.c
Index: clang/test/Preprocessor/header-symlink.c
===================================================================
--- /dev/null
+++ clang/test/Preprocessor/header-symlink.c
@@ -0,0 +1,18 @@
+// REQUIRES: shell
+// RUN: rm -rf %t && mkdir %t
+// RUN: touch %t/A.h
+// RUN: ln -s A.h %t/B.h
+// RUN: %clang_cc1 -I%t -E %s | FileCheck %s
+
+int A_table[] = {
+ #include "A.h"
+// CHECK: # 1 "{{.*}}A.h" 1
+};
+int B_table[] = {
+ #include "B.h"
+// CHECK: # 1 "{{.*}}B.h" 1
+};
+int A_table[] = {
+ #include "A.h"
+// CHECK: # 1 "{{.*}}A.h" 1
+};
\ No newline at end of file
Index: clang/test/Frontend/show-includes-symlink.c
===================================================================
--- /dev/null
+++ clang/test/Frontend/show-includes-symlink.c
@@ -0,0 +1,10 @@
+// REQUIRES: shell
+// RUN: rm -rf %t && mkdir %t
+// RUN: touch %t/A.h
+// RUN: ln -s A.h %t/B.h
+// RUN: %clang_cc1 -I%t --show-includes -E -o /dev/null %s 2>&1 | FileCheck %s
+
+#include "A.h"
+// CHECK: Note: including file: {{.*}}A.h
+#include "B.h"
+// CHECK: Note: including file: {{.*}}B.h
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1550,11 +1550,11 @@
NumFileDecls));
}
- const SrcMgr::ContentCache &ContentCache =
+ const SrcMgr::NamedContentCache &ContentCache =
SourceMgr.getOrCreateContentCache(*File, isSystem(FileCharacter));
- if (OverriddenBuffer && !ContentCache.BufferOverridden &&
- ContentCache.ContentsEntry == FileInfo.getOrigEntry() &&
- !ContentCache.getBufferIfLoaded()) {
+ if (OverriddenBuffer && !ContentCache.Cache->BufferOverridden &&
+ ContentCache.Cache->ContentsEntry == FileInfo.getOrigEntry() &&
+ !ContentCache.Cache->getBufferIfLoaded()) {
auto Buffer = ReadBuffer(SLocEntryCursor, File->getName());
if (!Buffer)
return true;
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -360,7 +360,6 @@
ContentCache ContentCache::createUnownedCopy(const ContentCache &Other) {
ContentCache Clone;
- Clone.OrigEntry = Other.OrigEntry;
Clone.ContentsEntry = Other.ContentsEntry;
Clone.BufferOverridden = Other.BufferOverridden;
Clone.IsFileVolatile = Other.IsFileVolatile;
@@ -391,47 +390,70 @@
}
}
-ContentCache &SourceManager::getOrCreateContentCache(FileEntryRef FileEnt,
- bool isSystemFile) {
+NamedContentCache &
+SourceManager::getOrCreateContentCache(FileEntryRef SourceFile,
+ bool isSystemFile) {
+ NamedContentCache *&NamedEntry = NamedFileInfos[SourceFile];
+ if (NamedEntry)
+ return *NamedEntry;
+
+ NamedEntry = ContentCacheAlloc.Allocate<NamedContentCache>();
+ NamedEntry->OrigEntry = SourceFile;
+ NamedEntry->Filename = SourceFile.getName();
+
+ if (OverriddenFilesInfo && !OverridenFilesKeepOriginalName) {
+ auto overI = OverriddenFilesInfo->OverriddenFiles.find(SourceFile);
+ if (overI != OverriddenFilesInfo->OverriddenFiles.end()) {
+ NamedEntry->OrigEntry = overI->second;
+ NamedEntry->Filename = overI->second.getName();
+ }
+ }
+
// Do we already have information about this file?
- ContentCache *&Entry = FileInfos[FileEnt];
- if (Entry)
- return *Entry;
+ ContentCache *&Entry = FileInfos[&SourceFile.getFileEntry()];
+ if (Entry) {
+ NamedEntry->Cache = Entry;
+ return *NamedEntry;
+ }
// Nope, create a new Cache entry.
+
Entry = ContentCacheAlloc.Allocate<ContentCache>();
+ NamedEntry->Cache = Entry;
if (OverriddenFilesInfo) {
// If the file contents are overridden with contents from another file,
// pass that file to ContentCache.
- auto overI = OverriddenFilesInfo->OverriddenFiles.find(FileEnt);
+ auto overI = OverriddenFilesInfo->OverriddenFiles.find(SourceFile);
if (overI == OverriddenFilesInfo->OverriddenFiles.end())
- new (Entry) ContentCache(FileEnt);
+ new (Entry) ContentCache(&SourceFile.getFileEntry());
else
- new (Entry) ContentCache(OverridenFilesKeepOriginalName ? FileEnt
- : overI->second,
- overI->second);
+ new (Entry) ContentCache(&overI->second.getFileEntry());
} else {
- new (Entry) ContentCache(FileEnt);
+ new (Entry) ContentCache(&SourceFile.getFileEntry());
}
Entry->IsFileVolatile = UserFilesAreVolatile && !isSystemFile;
Entry->IsTransient = FilesAreTransient;
- Entry->BufferOverridden |= FileEnt.isNamedPipe();
+ Entry->BufferOverridden |= SourceFile.isNamedPipe();
- return *Entry;
+ return *NamedEntry;
}
/// Create a new ContentCache for the specified memory buffer.
/// This does no caching.
-ContentCache &SourceManager::createMemBufferContentCache(
+NamedContentCache &SourceManager::createMemBufferContentCache(
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
// Add a new ContentCache to the MemBufferInfos list and return it.
- ContentCache *Entry = ContentCacheAlloc.Allocate<ContentCache>();
- new (Entry) ContentCache();
+ NamedContentCache *NamedEntry =
+ new (ContentCacheAlloc.Allocate<NamedContentCache>()) NamedContentCache();
+ NamedEntry->Filename = Buffer->getBufferIdentifier();
+ ContentCache *Entry =
+ new (ContentCacheAlloc.Allocate<ContentCache>()) ContentCache();
MemBufferInfos.push_back(Entry);
Entry->setBuffer(std::move(Buffer));
- return *Entry;
+ NamedEntry->Cache = Entry;
+ return *NamedEntry;
}
const SrcMgr::SLocEntry &SourceManager::loadSLocEntry(unsigned Index,
@@ -446,7 +468,7 @@
if (!FakeSLocEntryForRecovery)
FakeSLocEntryForRecovery = std::make_unique<SLocEntry>(SLocEntry::get(
0, FileInfo::get(SourceLocation(), getFakeContentCacheForRecovery(),
- SrcMgr::C_User, "")));
+ SrcMgr::C_User)));
return *FakeSLocEntryForRecovery;
}
}
@@ -482,10 +504,13 @@
/// As part of recovering from missing or changed content, produce a
/// fake content cache.
-SrcMgr::ContentCache &SourceManager::getFakeContentCacheForRecovery() const {
+SrcMgr::NamedContentCache &
+SourceManager::getFakeContentCacheForRecovery() const {
if (!FakeContentCacheForRecovery) {
- FakeContentCacheForRecovery = std::make_unique<SrcMgr::ContentCache>();
- FakeContentCacheForRecovery->setUnownedBuffer(getFakeBufferForRecovery());
+ FakeContentCacheForRecovery = std::make_unique<SrcMgr::NamedContentCache>();
+ FakeContentCacheForRecovery->Cache = new SrcMgr::ContentCache();
+ FakeContentCacheForRecovery->Cache->setUnownedBuffer(
+ getFakeBufferForRecovery());
}
return *FakeContentCacheForRecovery;
}
@@ -549,16 +574,16 @@
SrcMgr::CharacteristicKind FileCharacter,
int LoadedID,
SourceLocation::UIntTy LoadedOffset) {
- SrcMgr::ContentCache &IR = getOrCreateContentCache(SourceFile,
- isSystem(FileCharacter));
+ SrcMgr::NamedContentCache &IR =
+ getOrCreateContentCache(SourceFile, isSystem(FileCharacter));
// 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 (IR.Cache->ContentsEntry->isNamedPipe())
+ (void)IR.Cache->getBufferOrNone(Diag, getFileManager(), SourceLocation());
- return createFileIDImpl(IR, SourceFile.getName(), IncludePos, FileCharacter,
- LoadedID, LoadedOffset);
+ return createFileIDImpl(IR, IncludePos, FileCharacter, LoadedID,
+ LoadedOffset);
}
/// Create a new FileID that represents the specified memory buffer.
@@ -570,8 +595,7 @@
int LoadedID,
SourceLocation::UIntTy LoadedOffset,
SourceLocation IncludeLoc) {
- StringRef Name = Buffer->getBufferIdentifier();
- return createFileIDImpl(createMemBufferContentCache(std::move(Buffer)), Name,
+ return createFileIDImpl(createMemBufferContentCache(std::move(Buffer)),
IncludeLoc, FileCharacter, LoadedID, LoadedOffset);
}
@@ -601,7 +625,7 @@
/// createFileID - Create a new FileID for the specified ContentCache and
/// include position. This works regardless of whether the ContentCache
/// corresponds to a file or some other input source.
-FileID SourceManager::createFileIDImpl(ContentCache &File, StringRef Filename,
+FileID SourceManager::createFileIDImpl(NamedContentCache &File,
SourceLocation IncludePos,
SrcMgr::CharacteristicKind FileCharacter,
int LoadedID,
@@ -612,19 +636,18 @@
assert(Index < LoadedSLocEntryTable.size() && "FileID out of range");
assert(!SLocEntryLoaded[Index] && "FileID already loaded");
LoadedSLocEntryTable[Index] = SLocEntry::get(
- LoadedOffset, FileInfo::get(IncludePos, File, FileCharacter, Filename));
+ LoadedOffset, FileInfo::get(IncludePos, File, FileCharacter));
SLocEntryLoaded[Index] = true;
return FileID::get(LoadedID);
}
- unsigned FileSize = File.getSize();
+ unsigned FileSize = File.Cache->getSize();
if (!(NextLocalOffset + FileSize + 1 > NextLocalOffset &&
NextLocalOffset + FileSize + 1 <= CurrentLoadedOffset)) {
Diag.Report(IncludePos, diag::err_include_too_large);
return FileID();
}
- LocalSLocEntryTable.push_back(
- SLocEntry::get(NextLocalOffset,
- FileInfo::get(IncludePos, File, FileCharacter, Filename)));
+ LocalSLocEntryTable.push_back(SLocEntry::get(
+ NextLocalOffset, FileInfo::get(IncludePos, File, FileCharacter)));
// We do a +1 here because we want a SourceLocation that means "the end of the
// file", e.g. for the "no newline at the end of the file" diagnostic.
NextLocalOffset += FileSize + 1;
@@ -686,16 +709,17 @@
llvm::Optional<llvm::MemoryBufferRef>
SourceManager::getMemoryBufferForFileOrNone(const FileEntry *File) {
- SrcMgr::ContentCache &IR = getOrCreateContentCache(File->getLastRef());
- return IR.getBufferOrNone(Diag, getFileManager(), SourceLocation());
+ SrcMgr::NamedContentCache &IR = getOrCreateContentCache(File->getLastRef());
+ return IR.Cache->getBufferOrNone(Diag, getFileManager(), SourceLocation());
}
void SourceManager::overrideFileContents(
const FileEntry *SourceFile, std::unique_ptr<llvm::MemoryBuffer> Buffer) {
- SrcMgr::ContentCache &IR = getOrCreateContentCache(SourceFile->getLastRef());
+ SrcMgr::NamedContentCache &IR =
+ getOrCreateContentCache(SourceFile->getLastRef());
- IR.setBuffer(std::move(Buffer));
- IR.BufferOverridden = true;
+ IR.Cache->setBuffer(std::move(Buffer));
+ IR.Cache->BufferOverridden = true;
getOverriddenFilesInfo().OverriddenFilesWithBuffer.insert(SourceFile);
}
@@ -729,7 +753,7 @@
}
void SourceManager::setFileIsTransient(const FileEntry *File) {
- getOrCreateContentCache(File->getLastRef()).IsTransient = true;
+ getOrCreateContentCache(File->getLastRef()).Cache->IsTransient = true;
}
Optional<StringRef>
Index: clang/include/clang/Basic/SourceManager.h
===================================================================
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -126,29 +126,11 @@
/// One instance of this struct is kept for every file loaded or used.
///
/// This object owns the MemoryBuffer object.
-class alignas(8) ContentCache {
+class ContentCache {
/// The actual buffer containing the characters from the input
/// file.
mutable std::unique_ptr<llvm::MemoryBuffer> Buffer;
- friend class FileInfo;
-
- /// Reference to the file entry representing this ContentCache.
- ///
- /// This reference does not own the FileEntry object.
- ///
- /// It is possible for this to be NULL if the ContentCache encapsulates
- /// an imaginary text buffer.
- ///
- /// FIXME: Make non-optional using a virtual file as needed, remove \c
- /// Filename and use \c OrigEntry.getNameAsRequested() instead.
- OptionalFileEntryRefDegradesToFileEntryPtr OrigEntry;
-
- /// The filename that is used to access OrigEntry.
- ///
- /// FIXME: Remove this once OrigEntry is a FileEntryRef with a stable name.
- StringRef Filename;
-
public:
/// References the file which the contents were actually loaded from.
///
@@ -181,13 +163,14 @@
mutable unsigned IsBufferInvalid : 1;
ContentCache()
- : OrigEntry(None), ContentsEntry(nullptr), BufferOverridden(false),
- IsFileVolatile(false), IsTransient(false), IsBufferInvalid(false) {}
+ : ContentsEntry(nullptr), BufferOverridden(false), IsFileVolatile(false),
+ IsTransient(false), IsBufferInvalid(false) {}
- ContentCache(FileEntryRef Ent) : ContentCache(Ent, Ent) {}
+ // FIXME: remove
+ ContentCache(FileEntryRef Ent) = delete;
- ContentCache(FileEntryRef Ent, const FileEntry *contentEnt)
- : OrigEntry(Ent), ContentsEntry(contentEnt), BufferOverridden(false),
+ ContentCache(const FileEntry *contentEnt)
+ : ContentsEntry(contentEnt), BufferOverridden(false),
IsFileVolatile(false), IsTransient(false), IsBufferInvalid(false) {}
/// Create a copy of \p Other with an unowned reference to its buffer as if
@@ -262,10 +245,33 @@
static const char *getInvalidBOM(StringRef BufStr);
};
-// Assert that the \c ContentCache objects will always be 8-byte aligned so
+/// A \c ContentCache and the \c FileEntryRef that represents its name, needed
+/// by \c FileInfo. This is stored out of line to keep \c SLocEntry small.
+struct alignas(8) NamedContentCache {
+ /// The content cache entry.
+ ContentCache *Cache;
+
+ /// Reference to the file entry representing \c ContentCache.
+ ///
+ /// This reference does not own the FileEntry object.
+ ///
+ /// It is possible for this to be NULL if the ContentCache encapsulates
+ /// an imaginary text buffer.
+ ///
+ /// FIXME: Make non-optional using a virtual file as needed, remove \c
+ /// Filename and use \c OrigEntry.getNameAsRequested() instead.
+ OptionalFileEntryRefDegradesToFileEntryPtr OrigEntry;
+
+ /// The filename that is used to access OrigEntry.
+ ///
+ /// FIXME: Remove this once OrigEntry is a FileEntryRef with a stable name.
+ StringRef Filename;
+};
+
+// Assert that the \c NamedContentCache objects will always be 8-byte aligned so
// that we can pack 3 bits of integer into pointers to such objects.
-static_assert(alignof(ContentCache) >= 8,
- "ContentCache must be 8-byte aligned.");
+static_assert(alignof(NamedContentCache) >= 8,
+ "NamedContentCache must be 8-byte aligned.");
/// Information about a FileID, basically just the logical file
/// that it represents and include stack information.
@@ -299,21 +305,20 @@
/// Whether this FileInfo has any \#line directives.
unsigned HasLineDirectives : 1;
- /// The content cache and the characteristic of the file.
- llvm::PointerIntPair<const ContentCache *, 3, CharacteristicKind>
+ /// The name, content cache and the characteristic of the file.
+ llvm::PointerIntPair<const NamedContentCache *, 3, CharacteristicKind>
ContentAndKind;
public:
/// Return a FileInfo object.
- static FileInfo get(SourceLocation IL, ContentCache &Con,
- CharacteristicKind FileCharacter, StringRef Filename) {
+ static FileInfo get(SourceLocation IL, NamedContentCache &Con,
+ CharacteristicKind FileCharacter) {
FileInfo X;
X.IncludeLoc = IL;
X.NumCreatedFIDs = 0;
X.HasLineDirectives = false;
X.ContentAndKind.setPointer(&Con);
X.ContentAndKind.setInt(FileCharacter);
- Con.Filename = Filename;
return X;
}
@@ -321,10 +326,14 @@
return IncludeLoc;
}
- const ContentCache &getContentCache() const {
+ const NamedContentCache &getNamedContentCache() const {
return *ContentAndKind.getPointer();
}
+ const ContentCache &getContentCache() const {
+ return *getNamedContentCache().Cache;
+ }
+
/// Return whether this is a system header or not.
CharacteristicKind getFileCharacteristic() const {
return ContentAndKind.getInt();
@@ -339,10 +348,10 @@
/// Returns the name of the file that was used when the file was loaded from
/// the underlying file system.
- StringRef getName() const { return getContentCache().Filename; }
+ StringRef getName() const { return getNamedContentCache().Filename; }
OptionalFileEntryRefDegradesToFileEntryPtr getOrigEntry() const {
- return getContentCache().OrigEntry;
+ return getNamedContentCache().OrigEntry;
}
};
@@ -640,6 +649,10 @@
mutable llvm::BumpPtrAllocator ContentCacheAlloc;
+ llvm::DenseMap<FileEntryRef, SrcMgr::NamedContentCache *,
+ FileEntryRefSameRefDenseMapInfo>
+ NamedFileInfos;
+
/// Memoized information about all of the files tracked by this
/// SourceManager.
///
@@ -777,7 +790,8 @@
// Cache for the "fake" buffer used for error-recovery purposes.
mutable std::unique_ptr<llvm::MemoryBuffer> FakeBufferForRecovery;
- mutable std::unique_ptr<SrcMgr::ContentCache> FakeContentCacheForRecovery;
+ mutable std::unique_ptr<SrcMgr::NamedContentCache>
+ FakeContentCacheForRecovery;
mutable std::unique_ptr<SrcMgr::SLocEntry> FakeSLocEntryForRecovery;
@@ -1789,7 +1803,7 @@
friend class ASTWriter;
llvm::MemoryBufferRef getFakeBufferForRecovery() const;
- SrcMgr::ContentCache &getFakeContentCacheForRecovery() const;
+ SrcMgr::NamedContentCache &getFakeContentCacheForRecovery() const;
const SrcMgr::SLocEntry &loadSLocEntry(unsigned Index, bool *Invalid) const;
@@ -1862,16 +1876,16 @@
///
/// This works regardless of whether the ContentCache corresponds to a
/// file or some other input source.
- FileID createFileIDImpl(SrcMgr::ContentCache &File, StringRef Filename,
+ FileID createFileIDImpl(SrcMgr::NamedContentCache &File,
SourceLocation IncludePos,
SrcMgr::CharacteristicKind DirCharacter, int LoadedID,
SourceLocation::UIntTy LoadedOffset);
- SrcMgr::ContentCache &getOrCreateContentCache(FileEntryRef SourceFile,
- bool isSystemFile = false);
+ SrcMgr::NamedContentCache &getOrCreateContentCache(FileEntryRef SourceFile,
+ bool isSystemFile = false);
/// Create a new ContentCache for the specified memory buffer.
- SrcMgr::ContentCache &
+ SrcMgr::NamedContentCache &
createMemBufferContentCache(std::unique_ptr<llvm::MemoryBuffer> Buf);
FileID getFileIDSlow(SourceLocation::UIntTy SLocOffset) const;
Index: clang/include/clang/Basic/FileEntry.h
===================================================================
--- clang/include/clang/Basic/FileEntry.h
+++ clang/include/clang/Basic/FileEntry.h
@@ -185,6 +185,7 @@
bool hasOptionalValue() const { return ME; }
friend struct llvm::DenseMapInfo<FileEntryRef>;
+ friend struct FileEntryRefSameRefDenseMapInfo;
struct dense_map_empty_tag {};
struct dense_map_tombstone_tag {};
@@ -274,6 +275,26 @@
namespace clang {
+/// Specialization of \c DenseMapInfo for \c FileEntryRef that treats different
+/// paths to the same underlying file independently, as with \c isSameRef.
+struct FileEntryRefSameRefDenseMapInfo {
+ static inline FileEntryRef getEmptyKey() {
+ return FileEntryRef(FileEntryRef::dense_map_empty_tag());
+ }
+
+ static inline clang::FileEntryRef getTombstoneKey() {
+ return clang::FileEntryRef(clang::FileEntryRef::dense_map_tombstone_tag());
+ }
+
+ static unsigned getHashValue(clang::FileEntryRef Val) {
+ return hash_value(Val.ME);
+ }
+
+ static bool isEqual(clang::FileEntryRef LHS, clang::FileEntryRef RHS) {
+ return LHS.isSameRef(RHS);
+ }
+};
+
/// Wrapper around Optional<FileEntryRef> that degrades to 'const FileEntry*',
/// facilitating incremental patches to propagate FileEntryRef.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
