[PATCH] D26884: Change setDiagnosticsOutputFile to take a unique_ptr from a raw pointer (NFC)

[Malcolm Parsons via llvm-commits]

malcolm.parsons added inline comments.


================
Comment at: llvm/include/llvm/IR/LLVMContext.h:197
   /// set, the handler is invoked for each diagnostic message.
-  void setDiagnosticsOutputFile(yaml::Output *F);
+  void setDiagnosticsOutputFile(std::unique_ptr<yaml::Output> F);
 
----------------
mehdi_amini wrote:
> malcolm.parsons wrote:
> > mehdi_amini wrote:
> > > malcolm.parsons wrote:
> > > > Pass by value if always moving doesn't apply to move-only types. Use && to avoid a temporary.
> > > Sorry I don't understand this comment?
> > Functions that accept a parameter by value do so to allow the caller to choose whether to supply an lvalue that is copied or an rvalue that is moved without having to write overloads for both.
> > For a move-only type there is no choice, so the function should accept an rvalue.
> > This might avoid the construction and destruction of a temporary.
> I see. It does not really sound idiomatic to me to have any unique_ptr passed by rvalue, you should try to submit a patch to the developer guide.
> 
> (I'm not really convinced by the "avoid the construction and destruction of a temporary" here, I don't expect the codegen to be any different)
There are a few places where unique_ptrs are passed by rvalue:
```
llvm/Transforms/Utils/MemorySSA.h
681:    Result(std::unique_ptr<MemorySSA> &&MSSA) : MSSA(std::move(MSSA)) {}

llvm/Support/TargetRegistry.h
77:                                 std::unique_ptr<MCRelocationInfo> &&RelInfo);
112:      TargetMachine &TM, std::unique_ptr<MCStreamer> &&Streamer);
157:      std::unique_ptr<MCRelocationInfo> &&RelInfo);
396:                               std::unique_ptr<MCStreamer> &&Streamer) const {
531:                     std::unique_ptr<MCRelocationInfo> &&RelInfo) const {
1125:                               std::unique_ptr<MCStreamer> &&Streamer) {

llvm/Bitcode/BitcodeReader.h
92:  getOwningLazyBitcodeModule(std::unique_ptr<MemoryBuffer> &&Buffer,
```
but there are lots of places it's passed by value.

There's usually no difference in codegen, but not always:
https://godbolt.org/g/8GoljO
https://godbolt.org/g/CNZzsS


Repository:
  rL LLVM

https://reviews.llvm.org/D26884