dexonsmith created this revision.
dexonsmith added reviewers: arphaman, Bigcheese, rsmith.
Herald added a subscriber: ributzka.
This changes ContentCache::Buffer to use `std::unique_ptr<MemoryBuffer>`
instead of the PointerIntPair. It drops the (mostly unused) `DoNotFree`
bit in favour of creating a new, non-owning MemoryBuffer instance.
https://reviews.llvm.org/D67030
Files:
clang/include/clang/Basic/SourceManager.h
clang/lib/Basic/SourceManager.cpp
clang/lib/Frontend/CompilerInstance.cpp
lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
Index: lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
===================================================================
--- lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
+++ lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
@@ -151,7 +151,7 @@
clang::SourceManager SM(diags, file_mgr);
auto buf = llvm::MemoryBuffer::getMemBuffer(full_source);
- FileID FID = SM.createFileID(clang::SourceManager::Unowned, buf.get());
+ FileID FID = SM.createFileID(buf->getMemBufferRef());
// Let's just enable the latest ObjC and C++ which should get most tokens
// right.
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -336,8 +336,12 @@
// Override the contents of the "from" file with the contents of
// the "to" file.
- SourceMgr.overrideFileContents(FromFile, RB.second,
- InitOpts.RetainRemappedFileBuffers);
+ if (InitOpts.RetainRemappedFileBuffers)
+ SourceMgr.overrideFileContents(FromFile, *RB.second);
+ else
+ SourceMgr.overrideFileContents(
+ FromFile, std::unique_ptr<llvm::MemoryBuffer>(
+ const_cast<llvm::MemoryBuffer *>(RB.second)));
}
// Remap files in the source manager (with other files).
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -49,28 +49,22 @@
// 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.
@@ -78,23 +72,8 @@
/// 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) {
- IsBufferInvalid = false;
-
- 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();
}
llvm::Optional<llvm::MemoryBufferRef>
@@ -102,10 +81,10 @@
SourceLocation Loc) const {
// Lazily create the Buffer for ContentCaches that wrap files. If we already
// computed it, just return what we have.
- if (isBufferInvalid())
+ if (IsBufferInvalid)
return None;
- if (auto *B = Buffer.getPointer())
- return B->getMemBufferRef();
+ if (Buffer)
+ return Buffer->getMemBufferRef();
if (!ContentsEntry)
return None;
@@ -147,7 +126,7 @@
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).
@@ -166,7 +145,7 @@
// 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 = llvm::StringSwitch<const char *>(BufStr)
.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
"UTF-32 (BE)")
@@ -189,7 +168,7 @@
return None;
}
- return Buffer.getPointer()->getMemBufferRef();
+ return Buffer->getMemBufferRef();
}
unsigned LineTableInfo::getLineTableFilenameID(StringRef Name) {
@@ -364,7 +343,7 @@
Clone->BufferOverridden = Cache->BufferOverridden;
Clone->IsFileVolatile = Cache->IsFileVolatile;
Clone->IsTransient = Cache->IsTransient;
- Clone->replaceBuffer(Cache->getRawBuffer(), /*DoNotFree*/true);
+ Clone->setUnownedBuffer(Cache->getRawBuffer());
return Clone;
};
@@ -419,14 +398,13 @@
/// Create a new ContentCache for the specified memory buffer.
/// This does no caching.
-const ContentCache *
-SourceManager::createMemBufferContentCache(const llvm::MemoryBuffer *Buffer,
- bool DoNotFree) {
+const ContentCache *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();
MemBufferInfos.push_back(Entry);
- Entry->replaceBuffer(Buffer, DoNotFree);
+ Entry->setBuffer(std::move(Buffer));
return Entry;
}
@@ -608,14 +586,14 @@
return IR->getBuffer(Diag, getFileManager());
}
-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);
}
Index: clang/include/clang/Basic/SourceManager.h
===================================================================
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -92,17 +92,8 @@
///
/// 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;
+ /// The actual buffer containing the characters from the input file.
+ mutable std::unique_ptr<llvm::MemoryBuffer> Buffer;
public:
/// Reference to the file entry representing this ContentCache.
@@ -152,17 +143,15 @@
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)
: ContentCache(RHS.OrigEntry, 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;
@@ -170,8 +159,6 @@
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
@@ -222,20 +209,19 @@
/// Get the underlying buffer, returning NULL if the buffer is not
/// yet available.
- const llvm::MemoryBuffer *getRawBuffer() const {
- return Buffer.getPointer();
- }
-
- /// Replace the existing buffer (which will be deleted)
- /// with the given buffer.
- void replaceBuffer(const llvm::MemoryBuffer *B, bool DoNotFree = false);
+ const llvm::MemoryBuffer *getRawBuffer() const { return Buffer.get(); }
- /// Determine whether the buffer itself is invalid.
- bool isBufferInvalid() const { return IsBufferInvalid; }
+ /// 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, possibly, to nullptr.
+ void setUnownedBuffer(const llvm::MemoryBuffer *B) {
+ assert(!Buffer && "Expected to be called right after construction");
+ if (B)
+ setBuffer(llvm::MemoryBuffer::getMemBuffer(B->getMemBufferRef()));
}
};
@@ -869,9 +855,8 @@
int LoadedID = 0, unsigned LoadedOffset = 0,
SourceLocation IncludeLoc = SourceLocation()) {
StringRef Name = Buffer->getBufferIdentifier();
- return createFileID(
- createMemBufferContentCache(Buffer.release(), /*DoNotFree*/ false),
- Name, IncludeLoc, FileCharacter, LoadedID, LoadedOffset);
+ return createFileID(createMemBufferContentCache(std::move(Buffer)), Name,
+ IncludeLoc, FileCharacter, LoadedID, LoadedOffset);
}
/// Create a new FileID that represents the specified memory buffer.
@@ -934,14 +919,14 @@
///
/// \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);
+ std::unique_ptr<llvm::MemoryBuffer> Buffer);
+
+ /// Override a buffer without taking ownership.
void overrideFileContents(const FileEntry *SourceFile,
- std::unique_ptr<llvm::MemoryBuffer> Buffer) {
- overrideFileContents(SourceFile, Buffer.release(), /*DoNotFree*/ false);
+ const llvm::MemoryBuffer &Buffer) {
+ overrideFileContents(
+ SourceFile, llvm::MemoryBuffer::getMemBuffer(Buffer.getMemBufferRef()));
}
/// Override the given source file with another one.
@@ -1863,7 +1848,7 @@
/// Create a new ContentCache for the specified memory buffer.
const SrcMgr::ContentCache *
- createMemBufferContentCache(const llvm::MemoryBuffer *Buf, bool DoNotFree);
+ createMemBufferContentCache(std::unique_ptr<llvm::MemoryBuffer> Buf);
FileID getFileIDSlow(unsigned SLocOffset) const;
FileID getFileIDLocal(unsigned SLocOffset) const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
