Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp =================================================================== --- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -305,22 +305,8 @@ static llvm::Optional getBuffer(const SourceManager &SM, FileID File, bool AllowIO) { - // This is similar to the implementation of SourceManager::getBufferData(), - // but uses ContentCache::getRawBuffer() rather than getBuffer() if - // AllowIO=false, to avoid triggering file I/O if the file contents aren't - // already mapped. - bool CharDataInvalid = false; - const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(File, &CharDataInvalid); - if (CharDataInvalid || !Entry.isFile()) - return llvm::None; - const SrcMgr::ContentCache *Cache = Entry.getFile().getContentCache(); - const llvm::MemoryBuffer *Buffer = - AllowIO ? Cache->getBuffer(SM.getDiagnostics(), SM.getFileManager(), - SourceLocation(), &CharDataInvalid) - : Cache->getRawBuffer(); - if (!Buffer || CharDataInvalid) - return llvm::None; - return Buffer->getBuffer(); + return AllowIO ? SM.getBufferDataOrNone(File) + : SM.getBufferDataIfLoaded(File); } static bool LineIsMarkedWithNOLINT(const SourceManager &SM, SourceLocation Loc, Index: clang/include/clang/Basic/SourceManager.h =================================================================== --- clang/include/clang/Basic/SourceManager.h +++ clang/include/clang/Basic/SourceManager.h @@ -184,13 +184,21 @@ /// /// \param Loc If specified, is the location that invalid file diagnostics /// will be emitted at. + llvm::Optional + getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM, + SourceLocation Loc = SourceLocation()) const; + + private: + /// Returns pointer to memory buffer. /// - /// \param Invalid If non-NULL, will be set \c true if an error occurred. - const llvm::MemoryBuffer *getBuffer(DiagnosticsEngine &Diag, - FileManager &FM, - SourceLocation Loc = SourceLocation(), - bool *Invalid = nullptr) const; + /// TODO: SourceManager needs access to this for now, but once that's done + /// we should remove this API. + const llvm::MemoryBuffer *getBufferPointer(DiagnosticsEngine &Diag, + FileManager &FM, + SourceLocation Loc) const; + friend class clang::SourceManager; + public: /// Returns the size of the content encapsulated by this /// ContentCache. /// @@ -962,6 +970,24 @@ /// /// If there is an error opening this buffer the first time, this /// manufactures a temporary buffer and returns a non-empty error string. + llvm::Optional + getBufferOrNone(FileID FID, SourceLocation Loc = SourceLocation()) { + bool MyInvalid = false; + const SrcMgr::SLocEntry &Entry = getSLocEntry(FID, &MyInvalid); + if (MyInvalid || !Entry.isFile()) + return None; + + return Entry.getFile().getContentCache()->getBufferOrNone( + Diag, getFileManager(), Loc); + } + + /// Return the buffer for the specified FileID. + /// + /// If there is an error opening this buffer the first time, this + /// manufactures a temporary buffer and returns a non-empty error string. + /// + /// TODO: Update users of Invalid to call getBufferOrNone and change return + /// type to MemoryBufferRef. const llvm::MemoryBuffer *getBuffer(FileID FID, SourceLocation Loc, bool *Invalid = nullptr) const { bool MyInvalid = false; @@ -973,8 +999,11 @@ return getFakeBufferForRecovery(); } - return Entry.getFile().getContentCache()->getBuffer(Diag, getFileManager(), - Loc, Invalid); + auto *B = Entry.getFile().getContentCache()->getBufferPointer( + Diag, getFileManager(), Loc); + if (Invalid) + *Invalid = !B; + return B ? B : getFakeBufferForRecovery(); } const llvm::MemoryBuffer *getBuffer(FileID FID, @@ -1014,6 +1043,18 @@ /// \param Invalid If non-NULL, will be set true if an error occurred. StringRef getBufferData(FileID FID, bool *Invalid = nullptr) const; + /// Return a StringRef to the source buffer data for the + /// specified FileID, returning None if invalid. + /// + /// \param FID The file ID whose contents will be returned. + llvm::Optional getBufferDataOrNone(FileID FID) const; + + /// Return a StringRef to the source buffer data for the + /// specified FileID, returning None if it's not yet loaded. + /// + /// \param FID The file ID whose contents will be returned. + llvm::Optional getBufferDataIfLoaded(FileID FID) const; + /// Get the number of FileIDs (files and macros) that were created /// during preprocessing of \p FID, including it. unsigned getNumCreatedFIDsForFileID(FileID FID) const { Index: clang/lib/AST/ASTImporter.cpp =================================================================== --- clang/lib/AST/ASTImporter.cpp +++ clang/lib/AST/ASTImporter.cpp @@ -8683,12 +8683,10 @@ if (ToID.isInvalid() || IsBuiltin) { // FIXME: We want to re-use the existing MemoryBuffer! - bool Invalid = true; - const llvm::MemoryBuffer *FromBuf = - Cache->getBuffer(FromContext.getDiagnostics(), - FromSM.getFileManager(), SourceLocation{}, &Invalid); - if (!FromBuf || Invalid) - // FIXME: Use a new error kind? + llvm::Optional FromBuf = + Cache->getBufferOrNone(FromContext.getDiagnostics(), + FromSM.getFileManager(), SourceLocation{}); + if (!FromBuf) return llvm::make_error(ImportError::Unknown); std::unique_ptr ToBuf = Index: clang/lib/Basic/SourceManager.cpp =================================================================== --- clang/lib/Basic/SourceManager.cpp +++ clang/lib/Basic/SourceManager.cpp @@ -118,18 +118,25 @@ return InvalidBOM; } -const llvm::MemoryBuffer *ContentCache::getBuffer(DiagnosticsEngine &Diag, - FileManager &FM, - SourceLocation Loc, - bool *Invalid) const { +llvm::Optional +ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM, + SourceLocation Loc) const { + if (auto *B = getBufferPointer(Diag, FM, Loc)) + return B->getMemBufferRef(); + return None; +} + +const llvm::MemoryBuffer * +ContentCache::getBufferPointer(DiagnosticsEngine &Diag, FileManager &FM, + SourceLocation Loc) const { // Lazily create the Buffer for ContentCaches that wrap files. If we already // computed it, just return what we have. - if (Buffer.getPointer() || !ContentsEntry) { - if (Invalid) - *Invalid = isBufferInvalid(); - - return Buffer.getPointer(); - } + if (isBufferInvalid()) + return nullptr; + if (auto *B = Buffer.getPointer()) + return B; + if (!ContentsEntry) + return nullptr; // Check that the file's size fits in an 'unsigned' (with room for a // past-the-end value). This is deeply regrettable, but various parts of @@ -138,13 +145,6 @@ // miserably on large source files. if ((uint64_t)ContentsEntry->getSize() >= std::numeric_limits::max()) { - // We can't make a memory buffer of the required size, so just make a small - // one. We should never hit a situation where we've already parsed to a - // later offset of the file, so it shouldn't matter that the buffer is - // smaller than the file. - Buffer.setPointer( - llvm::MemoryBuffer::getMemBuffer("", ContentsEntry->getName()) - .release()); if (Diag.isDiagnosticInFlight()) Diag.SetDelayedDiagnostic(diag::err_file_too_large, ContentsEntry->getName()); @@ -153,8 +153,7 @@ << ContentsEntry->getName(); Buffer.setInt(Buffer.getInt() | InvalidFlag); - if (Invalid) *Invalid = true; - return Buffer.getPointer(); + return nullptr; } auto BufferOrError = FM.getBufferForFile(ContentsEntry, IsFileVolatile); @@ -164,20 +163,7 @@ // exists. Most likely, we were using a stat cache with an invalid entry but // the file could also have been removed during processing. Since we can't // really deal with this situation, just create an empty buffer. - // - // FIXME: This is definitely not ideal, but our immediate clients can't - // currently handle returning a null entry here. Ideally we should detect - // that we are in an inconsistent situation and error out as quickly as - // possible. if (!BufferOrError) { - StringRef FillStr("<<>>\n"); - auto BackupBuffer = llvm::WritableMemoryBuffer::getNewUninitMemBuffer( - ContentsEntry->getSize(), ""); - char *Ptr = BackupBuffer->getBufferStart(); - for (unsigned i = 0, e = ContentsEntry->getSize(); i != e; ++i) - Ptr[i] = FillStr[i % FillStr.size()]; - Buffer.setPointer(BackupBuffer.release()); - if (Diag.isDiagnosticInFlight()) Diag.SetDelayedDiagnostic(diag::err_cannot_open_file, ContentsEntry->getName(), @@ -187,9 +173,7 @@ << ContentsEntry->getName() << BufferOrError.getError().message(); Buffer.setInt(Buffer.getInt() | InvalidFlag); - - if (Invalid) *Invalid = true; - return Buffer.getPointer(); + return nullptr; } Buffer.setPointer(BufferOrError->release()); @@ -205,8 +189,7 @@ << ContentsEntry->getName(); Buffer.setInt(Buffer.getInt() | InvalidFlag); - if (Invalid) *Invalid = true; - return Buffer.getPointer(); + return nullptr; } // If the buffer is valid, check to see if it has a UTF Byte Order Mark @@ -219,11 +202,9 @@ Diag.Report(Loc, diag::err_unsupported_bom) << InvalidBOM << ContentsEntry->getName(); Buffer.setInt(Buffer.getInt() | InvalidFlag); + return nullptr; } - if (Invalid) - *Invalid = isBufferInvalid(); - return Buffer.getPointer(); } @@ -727,7 +708,10 @@ SourceManager::getMemoryBufferForFile(const FileEntry *File, bool *Invalid) { const SrcMgr::ContentCache *IR = getOrCreateContentCache(File); assert(IR && "getOrCreateContentCache() cannot return NULL"); - return IR->getBuffer(Diag, getFileManager(), SourceLocation(), Invalid); + auto *B = IR->getBufferPointer(Diag, getFileManager(), SourceLocation()); + if (Invalid) + *Invalid = !B; + return B ? B : getFakeBufferForRecovery(); } void SourceManager::overrideFileContents(const FileEntry *SourceFile, @@ -786,23 +770,35 @@ } StringRef SourceManager::getBufferData(FileID FID, bool *Invalid) const { + auto B = getBufferDataOrNone(FID); + if (Invalid) + *Invalid = !B; + return B ? *B : "<<<<>>>>"; +} + +llvm::Optional +SourceManager::getBufferDataIfLoaded(FileID FID) const { bool MyInvalid = false; const SLocEntry &SLoc = getSLocEntry(FID, &MyInvalid); - if (!SLoc.isFile() || MyInvalid) { - if (Invalid) - *Invalid = true; - return "<<<<>>>>"; - } + if (!SLoc.isFile() || MyInvalid) + return None; - const llvm::MemoryBuffer *Buf = SLoc.getFile().getContentCache()->getBuffer( - Diag, getFileManager(), SourceLocation(), &MyInvalid); - if (Invalid) - *Invalid = MyInvalid; + if (const llvm::MemoryBuffer *Buf = + SLoc.getFile().getContentCache()->getRawBuffer()) + return Buf->getBuffer(); + return None; +} - if (MyInvalid) - return "<<<<>>>>"; +llvm::Optional SourceManager::getBufferDataOrNone(FileID FID) const { + bool MyInvalid = false; + const SLocEntry &SLoc = getSLocEntry(FID, &MyInvalid); + if (!SLoc.isFile() || MyInvalid) + return None; - return Buf->getBuffer(); + if (auto B = SLoc.getFile().getContentCache()->getBufferOrNone( + Diag, getFileManager(), SourceLocation())) + return B->getBuffer(); + return None; } //===----------------------------------------------------------------------===// @@ -1219,12 +1215,13 @@ return "<<<>>>"; } - const llvm::MemoryBuffer *Buffer = - Entry.getFile().getContentCache()->getBuffer( - Diag, getFileManager(), SourceLocation(), &CharDataInvalid); + llvm::Optional Buffer = + Entry.getFile().getContentCache()->getBufferOrNone(Diag, getFileManager(), + SourceLocation()); if (Invalid) - *Invalid = CharDataInvalid; - return Buffer->getBufferStart() + (CharDataInvalid? 0 : LocInfo.second); + *Invalid = !Buffer; + return Buffer ? Buffer->getBufferStart() + LocInfo.second + : "<<<>>>"; } /// getColumnNumber - Return the column # for the specified file position. @@ -1317,8 +1314,9 @@ llvm::BumpPtrAllocator &Alloc, const SourceManager &SM, bool &Invalid) { // Note that calling 'getBuffer()' may lazily page in the file. - const MemoryBuffer *Buffer = - FI->getBuffer(Diag, SM.getFileManager(), SourceLocation(), &Invalid); + llvm::Optional Buffer = + FI->getBufferOrNone(Diag, SM.getFileManager(), SourceLocation()); + Invalid = !Buffer; if (Invalid) return; @@ -1543,8 +1541,8 @@ StringRef Filename; if (C->OrigEntry) Filename = C->OrigEntry->getName(); - else - Filename = C->getBuffer(Diag, getFileManager())->getBufferIdentifier(); + else if (auto Buffer = C->getBufferOrNone(Diag, getFileManager())) + Filename = Buffer->getBufferIdentifier(); unsigned LineNo = getLineNumber(LocInfo.first, LocInfo.second, &Invalid); if (Invalid) @@ -1739,14 +1737,18 @@ return SourceLocation(); } + llvm::Optional Buffer = + Content->getBufferOrNone(Diag, getFileManager()); + if (!Buffer) + return SourceLocation(); + if (Line > Content->NumLines) { - unsigned Size = Content->getBuffer(Diag, getFileManager())->getBufferSize(); + unsigned Size = Buffer->getBufferSize(); if (Size > 0) --Size; return FileLoc.getLocWithOffset(Size); } - const llvm::MemoryBuffer *Buffer = Content->getBuffer(Diag, getFileManager()); unsigned FilePos = Content->SourceLineCache[Line - 1]; const char *Buf = Buffer->getBufferStart() + FilePos; unsigned BufLength = Buffer->getBufferSize() - FilePos; Index: clang/lib/Serialization/ASTWriter.cpp =================================================================== --- clang/lib/Serialization/ASTWriter.cpp +++ clang/lib/Serialization/ASTWriter.cpp @@ -2002,9 +2002,9 @@ // We add one to the size so that we capture the trailing NULL // that is required by llvm::MemoryBuffer::getMemBuffer (on // the reader side). - const llvm::MemoryBuffer *Buffer = - Content->getBuffer(PP.getDiagnostics(), PP.getFileManager()); - StringRef Name = Buffer->getBufferIdentifier(); + llvm::Optional Buffer = + Content->getBufferOrNone(PP.getDiagnostics(), PP.getFileManager()); + StringRef Name = Buffer ? Buffer->getBufferIdentifier() : ""; Stream.EmitRecordWithBlob(SLocBufferAbbrv, Record, StringRef(Name.data(), Name.size() + 1)); EmitBlob = true; @@ -2016,8 +2016,10 @@ if (EmitBlob) { // Include the implicit terminating null character in the on-disk buffer // if we're writing it uncompressed. - const llvm::MemoryBuffer *Buffer = - Content->getBuffer(PP.getDiagnostics(), PP.getFileManager()); + llvm::Optional Buffer = + Content->getBufferOrNone(PP.getDiagnostics(), PP.getFileManager()); + if (!Buffer) + Buffer = llvm::MemoryBufferRef("<<>>", ""); StringRef Blob(Buffer->getBufferStart(), Buffer->getBufferSize() + 1); emitBlob(Stream, Blob, SLocBufferBlobCompressedAbbrv, SLocBufferBlobAbbrv);