[llvm] r253446 - [OperandBundles] Tighten OperandBundleDef's interface; NFC

[Sanjoy Das via llvm-commits]

David Blaikie wrote:
 >     -template <typename InputTy> struct OperandBundleDefT {
 >     +template <typename InputTy> class OperandBundleDefT {
 >         std::string Tag;
 >         std::vector<InputTy> Inputs;
 >
 >     -  OperandBundleDefT() {}
 >     -  explicit OperandBundleDefT(StringRef Tag, const
 >     std::vector<InputTy> &Inputs)
 >     +public:
 >     +  explicit OperandBundleDefT(StringRef Tag, std::vector<InputTy>
 >     &&Inputs)
 >
 >
 > As a general rule, you shouldn't pass by rvalue reference - instead pass
 > by value (that way you don't force callers to provide an rvalue when
 > they may not want to - but if they do provide one, it's efficiently
 > moved, etc) - similar below
 >
 >             : Tag(Tag), Inputs(Inputs) {}
 >
 >
 > Without the std::move here ^ ("Inputs(std::move(Inputs))") this will be
 > a copy where a move was probably intended (similarly below)

Just to be clear, you're suggesting changing this to:

  struct Foo {
   explicit Foo(std::vector<V> vect) member(std::move(vect)) {}
  }

If so, then that sounds like a good idea.

 >     -  explicit OperandBundleDefT(StringRef Tag, std::vector<InputTy>
 >     &&Inputs)
 >     +  explicit OperandBundleDefT(std::string &&Tag,
 >     std::vector<InputTy> &&Inputs)
 >             : Tag(Tag), Inputs(Inputs) {}
 >
 >         explicit OperandBundleDefT(const OperandBundleUse &OBU) {
 >           Tag = OBU.getTagName();
 >           Inputs.insert(Inputs.end(), OBU.Inputs.begin(), 
OBU.Inputs.end());
 >         }
 >     +
 >     +  ArrayRef<InputTy> getInputs() const { return Inputs; }
 >
 >
 > Probably just name this ^ "inputs()" to be consistent with other range
 > accessors?

Yeah, that sounds like a good idea -- will make it inconsistent with
args(), arg_size(), arg_begin(), arg_end() etc.

-- Sanjoy