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

[David Blaikie via llvm-commits]

On Wed, Nov 18, 2015 at 11:06 AM, Sanjoy Das <sanjoy at playingwithpointers.com
> wrote:

>
>
> 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)) {}
>

Yep, just like that


>  }
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151118/33006ac7/attachment.html>