r216712 - Improve unique_ptr-y ownership in ASTUnit::ComputePreamble
David Blaikie
dblaikie at gmail.com
Thu Aug 28 23:34:53 PDT 2014
Author: dblaikie
Date: Fri Aug 29 01:34:53 2014
New Revision: 216712
URL: http://llvm.org/viewvc/llvm-project?rev=216712&view=rev
Log:
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...
Modified:
cfe/trunk/include/clang/Frontend/ASTUnit.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp
Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=216712&r1=216711&r2=216712&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Fri Aug 29 01:34:53 2014
@@ -424,10 +424,23 @@ private:
void CleanTemporaryFiles();
bool Parse(std::unique_ptr<llvm::MemoryBuffer> OverrideMainBuffer);
-
- std::pair<llvm::MemoryBuffer *, std::pair<unsigned, bool> >
- ComputePreamble(CompilerInvocation &Invocation,
- unsigned MaxLines, bool &CreatedBuffer);
+
+ struct ComputedPreamble {
+ llvm::MemoryBuffer *Buffer;
+ std::unique_ptr<llvm::MemoryBuffer> Owner;
+ unsigned Size;
+ bool PreambleEndsAtStartOfLine;
+ ComputedPreamble(llvm::MemoryBuffer *Buffer,
+ std::unique_ptr<llvm::MemoryBuffer> 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<llvm::MemoryBuffer> getMainBufferWithPrecompiledPreamble(
const CompilerInvocation &PreambleInvocationIn, bool AllowRebuild = true,
Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=216712&r1=216711&r2=216712&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Fri Aug 29 01:34:53 2014
@@ -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<llvm::MemoryBuffer *, std::pair<unsigned, bool> >
-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<llvm::MemoryBuffer> 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(CompilerInvocat
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(CompilerInvocat
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<llvm::MemoryBuffer *>(RB.second);
}
}
@@ -1233,17 +1222,18 @@ ASTUnit::ComputePreamble(CompilerInvocat
}
// 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::getMainBufferWithPrecompiledPre
PreprocessorOptions &PreprocessorOpts
= PreambleInvocation->getPreprocessorOpts();
- bool CreatedPreambleBuffer = false;
- std::pair<llvm::MemoryBuffer *, std::pair<unsigned, bool> > NewPreamble
- = ComputePreamble(*PreambleInvocation, MaxLines, CreatedPreambleBuffer);
-
- // If ComputePreamble() Take ownership of the preamble buffer.
- std::unique_ptr<llvm::MemoryBuffer> OwnedPreambleBuffer;
- if (CreatedPreambleBuffer)
- OwnedPreambleBuffer.reset(NewPreamble.first);
+ ComputedPreamble NewPreamble = ComputePreamble(*PreambleInvocation, MaxLines);
- 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::getMainBufferWithPrecompiledPre
// 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::getMainBufferWithPrecompiledPre
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::getMainBufferWithPrecompiledPre
// 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::getMainBufferWithPrecompiledPre
PreambleTopLevelHashValue = CurrentTopLevelHashValue;
}
- return llvm::MemoryBuffer::getMemBufferCopy(NewPreamble.first->getBuffer(),
+ return llvm::MemoryBuffer::getMemBufferCopy(NewPreamble.Buffer->getBuffer(),
MainFilename);
}
Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=216712&r1=216711&r2=216712&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Fri Aug 29 01:34:53 2014
@@ -683,9 +683,8 @@ void PrintPreambleAction::ExecuteAction(
}
CompilerInstance &CI = getCompilerInstance();
- std::unique_ptr<llvm::MemoryBuffer> Buffer
- = CI.getFileManager().getBufferForFile(getCurrentFile());
- if (Buffer) {
+ if (std::unique_ptr<llvm::MemoryBuffer> Buffer =
+ CI.getFileManager().getBufferForFile(getCurrentFile())) {
unsigned Preamble =
Lexer::ComputePreamble(Buffer->getBuffer(), CI.getLangOpts()).first;
llvm::outs().write(Buffer->getBufferStart(), Preamble);
More information about the cfe-commits
mailing list