diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -94,17 +94,9 @@ /// /// 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 Buffer; + mutable std::unique_ptr Buffer; public: /// Reference to the file entry representing this ContentCache. @@ -153,21 +145,19 @@ 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; @@ -175,8 +165,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 @@ -208,17 +196,21 @@ /// 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 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 @@ -905,16 +897,21 @@ /// /// \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 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 Buffer); + /// Override the given source file with another one. /// /// \param SourceFile the source file which will be overridden. diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp --- a/clang/lib/Basic/SourceManager.cpp +++ b/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,21 +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) { - 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) { @@ -125,8 +106,8 @@ // 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; @@ -168,7 +149,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). @@ -187,7 +168,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 = getInvalidBOM(BufStr); if (InvalidBOM) { @@ -197,7 +178,7 @@ return None; } - return Buffer.getPointer()->getMemBufferRef(); + return Buffer->getMemBufferRef(); } unsigned LineTableInfo::getLineTableFilenameID(StringRef Name) { @@ -380,7 +361,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; }; @@ -441,7 +422,7 @@ ContentCache *Entry = ContentCacheAlloc.Allocate(); new (Entry) ContentCache(); MemBufferInfos.push_back(Entry); - Entry->replaceBuffer(Buffer.release(), /*DoNotFree=*/false); + Entry->setBuffer(std::move(Buffer)); return Entry; } @@ -493,8 +474,7 @@ SourceManager::getFakeContentCacheForRecovery() const { if (!FakeContentCacheForRecovery) { FakeContentCacheForRecovery = std::make_unique(); - FakeContentCacheForRecovery->replaceBuffer(getFakeBufferForRecovery(), - /*DoNotFree=*/true); + FakeContentCacheForRecovery->setUnownedBuffer(getFakeBufferForRecovery()); } return FakeContentCacheForRecovery.get(); } @@ -700,14 +680,14 @@ 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 Buffer) { + auto *IR = + const_cast(getOrCreateContentCache(SourceFile)); assert(IR && "getOrCreateContentCache() cannot return NULL"); - const_cast(IR)->replaceBuffer(Buffer, DoNotFree); - const_cast(IR)->BufferOverridden = true; + IR->setBuffer(std::move(Buffer)); + IR->BufferOverridden = true; getOverriddenFilesInfo().OverriddenFilesWithBuffer.insert(SourceFile); } diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -346,10 +346,16 @@ 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 to transfer ownership + // to the SourceManager. + if (InitOpts.RetainRemappedFileBuffers) + SourceMgr.overrideFileContents(FromFile, RB.second->getMemBufferRef()); + else + SourceMgr.overrideFileContents( + FromFile, std::unique_ptr( + const_cast(RB.second))); } // Remap files in the source manager (with other files).