Skip to content

Commit

Permalink
ContentCache: Simplify by always owning the MemoryBuffer
Browse files Browse the repository at this point in the history 
This changes `ContentCache::Buffer` to use
`std::unique_ptr<MemoryBuffer>` instead of the `PointerIntPair`. It
drops the (mostly unused) `DoNotFree` bit, instead creating a (new)
non-owning `MemoryBuffer` instance when passed a `MemoryBufferRef`.

Differential Revision: https://reviews.llvm.org/D67030
  • Loading branch information
@dexonsmith
dexonsmith committed Oct 21, 2020
1 parent 134ffa8 commit 2963145
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 78 deletions.
65 changes: 31 additions & 34 deletions clang/include/clang/Basic/SourceManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,9 @@ namespace SrcMgr {
///
/// This object owns the MemoryBuffer object.
class alignas(8) ContentCache {
enum CCFlags {
/// Whether the buffer should not be freed on destruction.
DoNotFreeFlag = 0x02
};

/// The actual buffer containing the characters from the input
/// file.
///
/// This is owned by the ContentCache object. The bits indicate
/// whether the buffer is invalid.
mutable llvm::PointerIntPair<const llvm::MemoryBuffer *, 2> Buffer;
mutable std::unique_ptr<llvm::MemoryBuffer> Buffer;

public:
/// Reference to the file entry representing this ContentCache.
Expand Down Expand Up @@ -153,30 +145,26 @@ namespace SrcMgr {
ContentCache(const FileEntry *Ent = nullptr) : ContentCache(Ent, Ent) {}

ContentCache(const FileEntry *Ent, const FileEntry *contentEnt)
: Buffer(nullptr, false), OrigEntry(Ent), ContentsEntry(contentEnt),
BufferOverridden(false), IsFileVolatile(false), IsTransient(false),
IsBufferInvalid(false) {}
: OrigEntry(Ent), ContentsEntry(contentEnt), BufferOverridden(false),
IsFileVolatile(false), IsTransient(false), IsBufferInvalid(false) {}

/// The copy ctor does not allow copies where source object has either
/// a non-NULL Buffer or SourceLineCache. Ownership of allocated memory
/// is not transferred, so this is a logical error.
ContentCache(const ContentCache &RHS)
: Buffer(nullptr, false), BufferOverridden(false),
IsFileVolatile(false), IsTransient(false), IsBufferInvalid(false) {
: BufferOverridden(false), IsFileVolatile(false), IsTransient(false),
IsBufferInvalid(false) {
OrigEntry = RHS.OrigEntry;
ContentsEntry = RHS.ContentsEntry;

assert(RHS.Buffer.getPointer() == nullptr &&
RHS.SourceLineCache == nullptr &&
assert(!RHS.Buffer && RHS.SourceLineCache == nullptr &&
"Passed ContentCache object cannot own a buffer.");

NumLines = RHS.NumLines;
}

ContentCache &operator=(const ContentCache& RHS) = delete;

~ContentCache();

/// Returns the memory buffer for the associated content.
///
/// \param Diag Object through which diagnostics will be emitted if the
Expand Down Expand Up @@ -208,17 +196,21 @@ namespace SrcMgr {

/// Get the underlying buffer, returning NULL if the buffer is not
/// yet available.
const llvm::MemoryBuffer *getRawBuffer() const {
return Buffer.getPointer();
}
const llvm::MemoryBuffer *getRawBuffer() const { return Buffer.get(); }

/// Replace the existing buffer (which will be deleted)
/// with the given buffer.
void replaceBuffer(const llvm::MemoryBuffer *B, bool DoNotFree = false);
/// Set the buffer.
void setBuffer(std::unique_ptr<llvm::MemoryBuffer> B) {
IsBufferInvalid = false;
Buffer = std::move(B);
}

/// Determine whether the buffer should be freed.
bool shouldFreeBuffer() const {
return (Buffer.getInt() & DoNotFreeFlag) == 0;
/// Set the buffer to one that's not owned (or to nullptr).
///
/// \pre Buffer cannot already be set.
void setUnownedBuffer(const llvm::MemoryBuffer *B) {
assert(!Buffer && "Expected to be called right after construction");
if (B)
setBuffer(llvm::MemoryBuffer::getMemBuffer(B->getMemBufferRef()));
}

// If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
Expand Down Expand Up @@ -905,16 +897,21 @@ class SourceManager : public RefCountedBase<SourceManager> {
///
/// \param Buffer the memory buffer whose contents will be used as the
/// data in the given source file.
///
/// \param DoNotFree If true, then the buffer will not be freed when the
/// source manager is destroyed.
void overrideFileContents(const FileEntry *SourceFile,
llvm::MemoryBuffer *Buffer, bool DoNotFree);
void overrideFileContents(const FileEntry *SourceFile,
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
overrideFileContents(SourceFile, Buffer.release(), /*DoNotFree*/ false);
const llvm::MemoryBufferRef &Buffer) {
overrideFileContents(SourceFile, llvm::MemoryBuffer::getMemBuffer(Buffer));
}

/// Override the contents of the given source file by providing an
/// already-allocated buffer.
///
/// \param SourceFile the source file whose contents will be overridden.
///
/// \param Buffer the memory buffer whose contents will be used as the
/// data in the given source file.
void overrideFileContents(const FileEntry *SourceFile,
std::unique_ptr<llvm::MemoryBuffer> Buffer);

/// Override the given source file with another one.
///
/// \param SourceFile the source file which will be overridden.
Expand Down
60 changes: 20 additions & 40 deletions clang/lib/Basic/SourceManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,50 +49,31 @@ using llvm::MemoryBuffer;
// SourceManager Helper Classes
//===----------------------------------------------------------------------===//

ContentCache::~ContentCache() {
if (shouldFreeBuffer())
delete Buffer.getPointer();
}

/// getSizeBytesMapped - Returns the number of bytes actually mapped for this
/// ContentCache. This can be 0 if the MemBuffer was not actually expanded.
unsigned ContentCache::getSizeBytesMapped() const {
return Buffer.getPointer() ? Buffer.getPointer()->getBufferSize() : 0;
return Buffer ? Buffer->getBufferSize() : 0;
}

/// Returns the kind of memory used to back the memory buffer for
/// this content cache. This is used for performance analysis.
llvm::MemoryBuffer::BufferKind ContentCache::getMemoryBufferKind() const {
assert(Buffer.getPointer());
assert(Buffer);

// Should be unreachable, but keep for sanity.
if (!Buffer.getPointer())
if (!Buffer)
return llvm::MemoryBuffer::MemoryBuffer_Malloc;

const llvm::MemoryBuffer *buf = Buffer.getPointer();
return buf->getBufferKind();
return Buffer->getBufferKind();
}

/// getSize - Returns the size of the content encapsulated by this ContentCache.
/// This can be the size of the source file or the size of an arbitrary
/// scratch buffer. If the ContentCache encapsulates a source file, that
/// file is not lazily brought in from disk to satisfy this query.
unsigned ContentCache::getSize() const {
return Buffer.getPointer() ? (unsigned) Buffer.getPointer()->getBufferSize()
: (unsigned) ContentsEntry->getSize();
}

void ContentCache::replaceBuffer(const llvm::MemoryBuffer *B, bool DoNotFree) {
if (B && B == Buffer.getPointer()) {
assert(0 && "Replacing with the same buffer");
Buffer.setInt(DoNotFree? DoNotFreeFlag : 0);
return;
}

if (shouldFreeBuffer())
delete Buffer.getPointer();
Buffer.setPointer(B);
Buffer.setInt((B && DoNotFree) ? DoNotFreeFlag : 0);
return Buffer ? (unsigned)Buffer->getBufferSize()
: (unsigned)ContentsEntry->getSize();
}

const char *ContentCache::getInvalidBOM(StringRef BufStr) {
Expand Down Expand Up @@ -125,8 +106,8 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
// computed it, just return what we have.
if (IsBufferInvalid)
return None;
if (auto *B = Buffer.getPointer())
return B->getMemBufferRef();
if (Buffer)
return Buffer->getMemBufferRef();
if (!ContentsEntry)
return None;

Expand Down Expand Up @@ -168,7 +149,7 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
return None;
}

Buffer.setPointer(BufferOrError->release());
Buffer = std::move(*BufferOrError);

// Check that the file's size is the same as in the file entry (which may
// have come from a stat cache).
Expand All @@ -187,7 +168,7 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
// If the buffer is valid, check to see if it has a UTF Byte Order Mark
// (BOM). We only support UTF-8 with and without a BOM right now. See
// http://en.wikipedia.org/wiki/Byte_order_mark for more information.
StringRef BufStr = Buffer.getPointer()->getBuffer();
StringRef BufStr = Buffer->getBuffer();
const char *InvalidBOM = getInvalidBOM(BufStr);

if (InvalidBOM) {
Expand All @@ -197,7 +178,7 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
return None;
}

return Buffer.getPointer()->getMemBufferRef();
return Buffer->getMemBufferRef();
}

unsigned LineTableInfo::getLineTableFilenameID(StringRef Name) {
Expand Down Expand Up @@ -380,7 +361,7 @@ void SourceManager::initializeForReplay(const SourceManager &Old) {
Clone->BufferOverridden = Cache->BufferOverridden;
Clone->IsFileVolatile = Cache->IsFileVolatile;
Clone->IsTransient = Cache->IsTransient;
Clone->replaceBuffer(Cache->getRawBuffer(), /*DoNotFree*/true);
Clone->setUnownedBuffer(Cache->getRawBuffer());
return Clone;
};

Expand Down Expand Up @@ -441,7 +422,7 @@ const ContentCache *SourceManager::createMemBufferContentCache(
ContentCache *Entry = ContentCacheAlloc.Allocate<ContentCache>();
new (Entry) ContentCache();
MemBufferInfos.push_back(Entry);
Entry->replaceBuffer(Buffer.release(), /*DoNotFree=*/false);
Entry->setBuffer(std::move(Buffer));
return Entry;
}

Expand Down Expand Up @@ -493,8 +474,7 @@ const SrcMgr::ContentCache *
SourceManager::getFakeContentCacheForRecovery() const {
if (!FakeContentCacheForRecovery) {
FakeContentCacheForRecovery = std::make_unique<SrcMgr::ContentCache>();
FakeContentCacheForRecovery->replaceBuffer(getFakeBufferForRecovery(),
/*DoNotFree=*/true);
FakeContentCacheForRecovery->setUnownedBuffer(getFakeBufferForRecovery());
}
return FakeContentCacheForRecovery.get();
}
Expand Down Expand Up @@ -700,14 +680,14 @@ SourceManager::getMemoryBufferForFileOrNone(const FileEntry *File) {
return IR->getBufferOrNone(Diag, getFileManager(), SourceLocation());
}

void SourceManager::overrideFileContents(const FileEntry *SourceFile,
llvm::MemoryBuffer *Buffer,
bool DoNotFree) {
const SrcMgr::ContentCache *IR = getOrCreateContentCache(SourceFile);
void SourceManager::overrideFileContents(
const FileEntry *SourceFile, std::unique_ptr<llvm::MemoryBuffer> Buffer) {
auto *IR =
const_cast<SrcMgr::ContentCache *>(getOrCreateContentCache(SourceFile));
assert(IR && "getOrCreateContentCache() cannot return NULL");

const_cast<SrcMgr::ContentCache *>(IR)->replaceBuffer(Buffer, DoNotFree);
const_cast<SrcMgr::ContentCache *>(IR)->BufferOverridden = true;
IR->setBuffer(std::move(Buffer));
IR->BufferOverridden = true;

getOverriddenFilesInfo().OverriddenFilesWithBuffer.insert(SourceFile);
}
Expand Down
14 changes: 10 additions & 4 deletions clang/lib/Frontend/CompilerInstance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,16 @@ static void InitializeFileRemapping(DiagnosticsEngine &Diags,
continue;
}

// Override the contents of the "from" file with the contents of
// the "to" file.
SourceMgr.overrideFileContents(FromFile, RB.second,
InitOpts.RetainRemappedFileBuffers);
// Override the contents of the "from" file with the contents of the
// "to" file. If the caller owns the buffers, then pass a MemoryBufferRef;
// otherwise, pass as a std::unique_ptr<MemoryBuffer> to transfer ownership
// to the SourceManager.
if (InitOpts.RetainRemappedFileBuffers)
SourceMgr.overrideFileContents(FromFile, RB.second->getMemBufferRef());
else
SourceMgr.overrideFileContents(
FromFile, std::unique_ptr<llvm::MemoryBuffer>(
const_cast<llvm::MemoryBuffer *>(RB.second)));
}

// Remap files in the source manager (with other files).
Expand Down

0 comments on commit 2963145

Please sign in to comment.