[PATCH] unique_ptrify llvm::getLazyBitcodeModule's MemoryBuffer parameter

[David Blaikie]

On Wed, Aug 13, 2014 at 9:03 AM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> I am somewhat OK with this approach. I think the one item I feel
> should be done differently is llvm::parseBitcodeFile. It can take just
> a stringref and build a fake membuf to pass down.  Maybe we should do
> that first (attached patch)?

I'm curious - why pass by StringRef if a MemoryBuffer is needed after
anyway? If getLazyBitcodeModuleImpl doesn't need the MemoryBuffer's
name - why not pass by StringRef alone? (well, ownership, yeah... -
but it seems you called out this particular usage for
MemoryBuffer->StringRef->MemoryBuffer and I'm just confused as to why)

Does getLazyBitcodeModuleImpl not need the MemoryBuffer's name?

- David

>
> On 12 August 2014 17:51, David Blaikie <dblaikie at gmail.com> wrote:
>> Hi lhames, rafael,
>>
>> This turned out to be a bit tricky as there are some callers doing
>> strange things with ownership of the this MemoryBuffer.
>>
>> A couple of callers (llvm::getBitcodeTargetTriple,
>> llvm::parseBitcodeFile) don't take ownership of the MemoryBuffer, but
>> still want to construct a Module (but they either ensure it's fully
>> materialized, or otherwise destroy it before the function is over, being
>> sure to take the MemoryBuffer back from the Module so it is not
>> destroyed). Some of these callers were adjusted to use ScopeGuards to
>> ensure the release of a fake local unique_ptr, others were able to be
>> adjusted to pass ownership transiently, then take it back again later.
>>
>> Other ideas are welcome - one possible option would be to create new
>> shallow MemoryBuffers in these cases. Another would be to make these
>> data structures consistently non-owning, though that sounds difficult
>> for other users. I'd like to throw a few ideas around here because I
>> think this pattern shows up a few times in LLVM - where an API has
>> multiple users, some of whom want to give ownership, some of whom don't.
>> I'd like to figure out a good/consistent/simple answer for when we come
>> across this again.
>>
>> http://reviews.llvm.org/D4876
>>
>> Files:
>>   include/llvm/ADT/ScopeGuard.h
>>   include/llvm/Bitcode/ReaderWriter.h
>>   include/llvm/IR/GVMaterializer.h
>>   include/llvm/IR/Module.h
>>   lib/Bitcode/Reader/BitReader.cpp
>>   lib/Bitcode/Reader/BitcodeReader.cpp
>>   lib/Bitcode/Reader/BitcodeReader.h
>>   lib/IR/Module.cpp
>>   lib/IRReader/IRReader.cpp
>>   lib/LTO/LTOModule.cpp
>>   lib/Object/IRObjectFile.cpp
>>   unittests/Bitcode/BitReaderTest.cpp
>>   unittests/ExecutionEngine/JIT/JITTest.cpp