[PATCH] Replace OwningPtr with std::unique_ptr.
David Blaikie
dblaikie at gmail.com
Wed Mar 12 10:27:03 PDT 2014
Mostly looks good, though lld's a bit out of my depth so I'll leave it to someone else (an owner or someone otherwise feeling sufficient authority in the area) to sign off, perhaps Rui.
================
Comment at: lib/Passes/RoundTripYAMLPass.cpp:48
@@ -46,4 +47,3 @@
- if (buff->getBufferSize() < MAX_YAML_FILE_SIZE) {
- std::unique_ptr<MemoryBuffer> mb(buff.take());
+ if (mb->getBufferSize() < MAX_YAML_FILE_SIZE) {
error_code ec = _context.registry().parseFile(mb, _yamlFile);
----------------
Any particular reason to change the name of the variable? The old one seemed about as good & less churn.
================
Comment at: lib/Core/InputGraph.cpp:143
@@ -141,3 +142,3 @@
- if (error_code ec = MemoryBuffer::getFileOrSTDIN(filePath, opmb))
+ if (error_code ec = MemoryBuffer::getFileOrSTDIN(filePath, mb))
return ec;
----------------
Could be worth checking, but I assume "MemoryBuffer::getFileOrSTDIN" leaves the pointer null if it fails - so you could just avoid the temporary 'mb' and pass '_buffer' here, maybe.. (but yeah, perfectly fine to leave this as-is - it's a strict improvement in any case)
================
Comment at: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp:282
@@ -281,3 +281,3 @@
}
- bin.take();
+ bin.release();
----------------
that's an interesting bit of memory (non)ownership... any idea what's going on here?
Oh, I see - this is an issue with unique_ptr going through dyn_cast. Ew.
It'd be great to improve dyn_cast to handle this sort of thing.
http://llvm-reviews.chandlerc.com/D3055
More information about the llvm-commits
mailing list