[PATCH] D125994: [NFC] Define move and copy constructors and copy assignment operators for MDOperand.

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added inline comments.


================
Comment at: llvm/include/llvm/IR/Metadata.h:782
+  }
+  MDOperand(const MDOperand &Op) {
+    MD = Op.MD;
----------------
wolfgangp wrote:
> dexonsmith wrote:
> > wolfgangp wrote:
> > > dexonsmith wrote:
> > > > I don’t think we need this and it seems better to error here. I think we should keep the copy constructor/operator deleted and make this move-only.
> > > In that case I'm a bit stuck because the implementation of SmallVector uses std::fill_n() and std::uninitialized_fill_n() which assign and copy the value type and want the copy constructor (at least on gcc-9). Do you have any suggestions of should we roll our own hung-off storage class after all? 
> > I suspect that's because the move constructor/operator had the wrong signature (an incorrect `const`). The following seems to work for me locally:
> > ```
> > lang=c++
> >   struct MoveOnly {
> >     MoveOnly() = default;
> >     MoveOnly(MoveOnly &&) = default;
> >     MoveOnly &operator=(MoveOnly &&) = default;
> >     MoveOnly(const MoveOnly &) = delete;
> >     MoveOnly &operator=(const MoveOnly &) = delete;
> >   };
> >   SmallVector<MoveOnly> X;
> >   X.resize(10);
> >   X.push_back(MoveOnly());
> > ```
> > 
> Ah, I had the equivalent of
> ```
> SmallVector<MoveOnly> X(NumOps);
> ```
> which doesn't compile for me. Anyway that's the workaround.
Huh! We should probably fix that to make it work at some point! Curious that one works and not the other. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125994/new/

https://reviews.llvm.org/D125994