[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