From d6902a1403c46161db316c582a50acc0cb1dd60b Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Fri, 29 Aug 2014 06:34:53 +0000 Subject: [PATCH] Improve unique_ptr-y ownership in ASTUnit::ComputePreamble Rather than having a pair of pairs and a reference out parameter, build a structure with everything together and named. A raw pointer and a unique_ptr, rather than a raw pointer and a boolean, are used to communicate ownership transfer. It's possible one day we'll end up with a conditional pointer (probably represented by a raw pointer and a boolean) abstraction to use in places like this. Conditional ownership seems to be coming up more often than I'd hoped... llvm-svn: 216712 --- clang/include/clang/Frontend/ASTUnit.h | 21 +++++-- clang/lib/Frontend/ASTUnit.cpp | 76 ++++++++++---------------- clang/lib/Frontend/FrontendActions.cpp | 5 +- 3 files changed, 48 insertions(+), 54 deletions(-) diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h index 5d4adf2c81dc..f2e2102cd05a 100644 --- a/clang/include/clang/Frontend/ASTUnit.h +++ b/clang/include/clang/Frontend/ASTUnit.h @@ -424,10 +424,23 @@ private: void CleanTemporaryFiles(); bool Parse(std::unique_ptr OverrideMainBuffer); - - std::pair > - ComputePreamble(CompilerInvocation &Invocation, - unsigned MaxLines, bool &CreatedBuffer); + + struct ComputedPreamble { + llvm::MemoryBuffer *Buffer; + std::unique_ptr Owner; + unsigned Size; + bool PreambleEndsAtStartOfLine; + ComputedPreamble(llvm::MemoryBuffer *Buffer, + std::unique_ptr Owner, unsigned Size, + bool PreambleEndsAtStartOfLine) + : Buffer(Buffer), Owner(std::move(Owner)), Size(Size), + PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine) {} + ComputedPreamble(ComputedPreamble &&C) + : Buffer(C.Buffer), Owner(std::move(C.Owner)), Size(C.Size), + PreambleEndsAtStartOfLine(C.PreambleEndsAtStartOfLine) {} + }; + ComputedPreamble ComputePreamble(CompilerInvocation &Invocation, + unsigned MaxLines); std::unique_ptr getMainBufferWithPrecompiledPreamble( const CompilerInvocation &PreambleInvocationIn, bool AllowRebuild = true, diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 31266dca9280..38d9aeaaf50a 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1179,17 +1179,16 @@ static std::string GetPreamblePCHPath() { /// \brief Compute the preamble for the main file, providing the source buffer /// that corresponds to the main file along with a pair (bytes, start-of-line) /// that describes the preamble. -std::pair > -ASTUnit::ComputePreamble(CompilerInvocation &Invocation, - unsigned MaxLines, bool &CreatedBuffer) { +ASTUnit::ComputedPreamble +ASTUnit::ComputePreamble(CompilerInvocation &Invocation, unsigned MaxLines) { FrontendOptions &FrontendOpts = Invocation.getFrontendOpts(); PreprocessorOptions &PreprocessorOpts = Invocation.getPreprocessorOpts(); - CreatedBuffer = false; // Try to determine if the main file has been remapped, either from the // command line (to another file) or directly through the compiler invocation // (to a memory buffer). llvm::MemoryBuffer *Buffer = nullptr; + std::unique_ptr BufferOwner; std::string MainFilePath(FrontendOpts.Inputs[0].getFile()); llvm::sys::fs::UniqueID MainFileID; if (!llvm::sys::fs::getUniqueID(MainFilePath, MainFileID)) { @@ -1200,15 +1199,9 @@ ASTUnit::ComputePreamble(CompilerInvocation &Invocation, if (!llvm::sys::fs::getUniqueID(MPath, MID)) { if (MainFileID == MID) { // We found a remapping. Try to load the resulting, remapped source. - if (CreatedBuffer) { - delete Buffer; - CreatedBuffer = false; - } - - Buffer = getBufferForFile(RF.second).release(); - if (!Buffer) - return std::make_pair(nullptr, std::make_pair(0, true)); - CreatedBuffer = true; + BufferOwner = getBufferForFile(RF.second); + if (!BufferOwner) + return ComputedPreamble(nullptr, nullptr, 0, true); } } } @@ -1221,11 +1214,7 @@ ASTUnit::ComputePreamble(CompilerInvocation &Invocation, if (!llvm::sys::fs::getUniqueID(MPath, MID)) { if (MainFileID == MID) { // We found a remapping. - if (CreatedBuffer) { - delete Buffer; - CreatedBuffer = false; - } - + BufferOwner.reset(); Buffer = const_cast(RB.second); } } @@ -1233,17 +1222,18 @@ ASTUnit::ComputePreamble(CompilerInvocation &Invocation, } // If the main source file was not remapped, load it now. - if (!Buffer) { - Buffer = getBufferForFile(FrontendOpts.Inputs[0].getFile()).release(); - if (!Buffer) - return std::make_pair(nullptr, std::make_pair(0, true)); - - CreatedBuffer = true; + if (!Buffer && !BufferOwner) { + BufferOwner = getBufferForFile(FrontendOpts.Inputs[0].getFile()); + if (!BufferOwner) + return ComputedPreamble(nullptr, nullptr, 0, true); } - return std::make_pair( - Buffer, Lexer::ComputePreamble(Buffer->getBuffer(), - *Invocation.getLangOpts(), MaxLines)); + if (!Buffer) + Buffer = BufferOwner.get(); + auto Pre = Lexer::ComputePreamble(Buffer->getBuffer(), + *Invocation.getLangOpts(), MaxLines); + return ComputedPreamble(Buffer, std::move(BufferOwner), Pre.first, + Pre.second); } ASTUnit::PreambleFileHash @@ -1354,16 +1344,9 @@ ASTUnit::getMainBufferWithPrecompiledPreamble( PreprocessorOptions &PreprocessorOpts = PreambleInvocation->getPreprocessorOpts(); - bool CreatedPreambleBuffer = false; - std::pair > NewPreamble - = ComputePreamble(*PreambleInvocation, MaxLines, CreatedPreambleBuffer); + ComputedPreamble NewPreamble = ComputePreamble(*PreambleInvocation, MaxLines); - // If ComputePreamble() Take ownership of the preamble buffer. - std::unique_ptr OwnedPreambleBuffer; - if (CreatedPreambleBuffer) - OwnedPreambleBuffer.reset(NewPreamble.first); - - if (!NewPreamble.second.first) { + if (!NewPreamble.Size) { // We couldn't find a preamble in the main source. Clear out the current // preamble, if we have one. It's obviously no good any more. Preamble.clear(); @@ -1379,10 +1362,10 @@ ASTUnit::getMainBufferWithPrecompiledPreamble( // preamble now that we did before, and that there's enough space in // the main-file buffer within the precompiled preamble to fit the // new main file. - if (Preamble.size() == NewPreamble.second.first && - PreambleEndsAtStartOfLine == NewPreamble.second.second && - memcmp(Preamble.getBufferStart(), NewPreamble.first->getBufferStart(), - NewPreamble.second.first) == 0) { + if (Preamble.size() == NewPreamble.Size && + PreambleEndsAtStartOfLine == NewPreamble.PreambleEndsAtStartOfLine && + memcmp(Preamble.getBufferStart(), NewPreamble.Buffer->getBufferStart(), + NewPreamble.Size) == 0) { // The preamble has not changed. We may be able to re-use the precompiled // preamble. @@ -1452,7 +1435,7 @@ ASTUnit::getMainBufferWithPrecompiledPreamble( getDiagnostics().setNumWarnings(NumWarningsInPreamble); return llvm::MemoryBuffer::getMemBufferCopy( - NewPreamble.first->getBuffer(), FrontendOpts.Inputs[0].getFile()); + NewPreamble.Buffer->getBuffer(), FrontendOpts.Inputs[0].getFile()); } } @@ -1497,13 +1480,12 @@ ASTUnit::getMainBufferWithPrecompiledPreamble( // subsequent reparses. StringRef MainFilename = FrontendOpts.Inputs[0].getFile(); Preamble.assign(FileMgr->getFile(MainFilename), - NewPreamble.first->getBufferStart(), - NewPreamble.first->getBufferStart() - + NewPreamble.second.first); - PreambleEndsAtStartOfLine = NewPreamble.second.second; + NewPreamble.Buffer->getBufferStart(), + NewPreamble.Buffer->getBufferStart() + NewPreamble.Size); + PreambleEndsAtStartOfLine = NewPreamble.PreambleEndsAtStartOfLine; PreambleBuffer = llvm::MemoryBuffer::getMemBufferCopy( - NewPreamble.first->getBuffer().slice(0, Preamble.size()), MainFilename); + NewPreamble.Buffer->getBuffer().slice(0, Preamble.size()), MainFilename); // Remap the main source file to the preamble buffer. StringRef MainFilePath = FrontendOpts.Inputs[0].getFile(); @@ -1647,7 +1629,7 @@ ASTUnit::getMainBufferWithPrecompiledPreamble( PreambleTopLevelHashValue = CurrentTopLevelHashValue; } - return llvm::MemoryBuffer::getMemBufferCopy(NewPreamble.first->getBuffer(), + return llvm::MemoryBuffer::getMemBufferCopy(NewPreamble.Buffer->getBuffer(), MainFilename); } diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 0e2512762593..99cd4a2e6d66 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -683,9 +683,8 @@ void PrintPreambleAction::ExecuteAction() { } CompilerInstance &CI = getCompilerInstance(); - std::unique_ptr Buffer - = CI.getFileManager().getBufferForFile(getCurrentFile()); - if (Buffer) { + if (std::unique_ptr Buffer = + CI.getFileManager().getBufferForFile(getCurrentFile())) { unsigned Preamble = Lexer::ComputePreamble(Buffer->getBuffer(), CI.getLangOpts()).first; llvm::outs().write(Buffer->getBufferStart(), Preamble);