DIP 1016--ref T accepts r-values--Community Review Round 1

[Manu]

On Fri, 20 Jul 2018 at 11:05, Jonathan M Davis via Digitalmars-d
<digitalmars-d at puremagic.com> wrote:
>
> On Friday, July 20, 2018 17:25:25 Daniel N via Digitalmars-d wrote:
> > On Friday, 20 July 2018 at 17:02:04 UTC, Jonathan M Davis wrote:
> > > On Friday, July 20, 2018 16:42:54 aliak via Digitalmars-d wrote:
> > >> On Friday, 20 July 2018 at 13:21:11 UTC, Jonathan M Davis
> > >>
> > >> But as for a UDA, maybe @implicit from the copy constructor
> > >> DIP can be reused here?
> > >>
> > >> void f(@implicit ref A a) {}
> > >
> > > I don't know. Maybe. I'd certainly prefer @rvalue, since it
> > > would be clearer (and slightly shorter for that matter), and I
> > > don't really agree with copy constructors requiring @implicit
> > > anyway. But at the moment, I don't see a technical reason why
> > > the attribute couldn't be reused.
> >
> > I find the DIP addresses this in a cleaner way with opt-out.
> > 1) Avoids adding more UDA:s
> > 2) It's probably more common to wish to use this feature than the
> > opposite, i.e. by taking the opt-out route we can significantly
> > reduce @uda clutter.
> >
> > See DIP:
> > > void lval_only(int x) @disable;
> > > void lval_only(ref int x);
> > >
> > > int x = 10;
> > > lval_only(x);  // ok: choose by-ref
> > > lval_only(10); // error: literal matches by-val, which is
> > > @disabled
>
> It changes the semantics of existing code,

No *existing* code 'accidentally' dismissed results of ref args. It
doesn't actually change semantics of existing code. It changes
semantics of code you could potentially write in the future.

> and honestly, it seems ridiculous
> to me to have to declare a separate function to make ref work the way that
> it's always worked.

This is exactly what @disable is designed and good for.
This feels like an excellent and intuitive choice to me. I think this
is an improvement by itself, irrespective of this DIP.

> And while I'm sure that if this is implemented, some folks will rush out and
> start slapping ref all over the place (or @rvalue ref if it were actually
> done with a new attribute like it should be IMHO), the reality of the matter
> is that whether creating a temporary to pass rvalues by ref or simply having
> the function accept everything by value is better is not straightforward. In
> many cases, passing by value is more efficient - especially when lots of
> rvalues are involved and relatively few lvalues. Even C++ has changed its
> tune on this as far as best practices go. With C++98, it was considered best
> practice to use const& all over the place, whereas once they added move
> constructors, the situation became much more complicated, and as I
> understand it, it's now often considered best practice to pass by value
> unless you find that you need to do otherwise, because using const& tends to
> prevent moves and often results in extra copies being made.

This DIP isn't a discussion on efficiency. 'best practise' is a
conversation that will continue to be ongoing (and is highly context
sensitive). Fortunately, D has a more mature pre-disposition relative
to C++ on this matter, so I expect the explosion of ref you predict
will not be so explosive as you imagine making direct comparison to
C++.
The point this DIP makes is that, choice to use 'ref' is a decision
_for the API author_, and for reasons that context sensitive.

Whether ref should or shouldn't be used is not your prescription to
make, and while there may be discussions about efficiency, I think
there are many more cases where the choice has nothing to do with
efficiency.
In many cases, it's already prescribed by 3rd party, and you have no choice.

> So, while being
> able to pass rvalues by ref may be useful in many cases, using @rvalue ref
> all over the place is unlikely to be a good idea in general. So, I don't
> think that it's at all safe to say that allowing ref accept rvalues by ref
> is going to reduce the amount of UDA clutter in the average D program -
> especially when @rvalue would mean just adding an attribute, whereas what
> the DIP proposes involves having to declare an extra overload just to
> disable it.

I think disabling it is going to be the overwhelmingly niche case, but
it's good that you can still do it and be satisfied if that's what you
want to do.

Can you link me to a single line of your own code where you've used
this pattern?

> Either way, what this DIP proposes means that existing uses of ref will need
> to be changed - and in an extremely verbose way - in order to get back their
> current behavior.

Their currently behaviour is mostly wrong, and the exact thing I'm
trying to fix though.