March 29, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Olivier FAURE | On Thursday, 28 March 2019 at 23:40:45 UTC, Olivier FAURE wrote:
> On Thursday, 28 March 2019 at 20:11:26 UTC, Andrei Alexandrescu wrote:
>> The nonassignable requirement is a major improvement, but won't catch all potential misuses. I don't know how to do better.
>
> You could be more strict and consider that any call of the form
>
> applyDiscount(x.price)
>
The property disaster :
Struct X
{
Currency price;
}
Evolves Into
Struct X
{
Currency price() const {...}
Void price(currency x){...}
}
Applydiscount(x.price);
Compiles to nop.
Is this revalue ref work an opportunity to have the compiler to try and call the setter with the result ?
Apologies phone post.
|
March 29, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Wednesday, 27 March 2019 at 01:38:40 UTC, Andrei Alexandrescu wrote: > https://gist.github.com/andralex/e5405a5d773f07f73196c05f8339435a > > Thanks in advance for any feedback. - During discussions around DIP-1016, it was pointed out that the following code currently compiles, even though it really shouldn't: struct Point { private int _x, _y; ref int x() { return _x; } ref int y() { return _y; } } struct Rect { private Point _origin, _size; Point origin() { return _origin; } Point size() { return _size; } void origin(Point p) { _origin = p; } void size(Point p) { _size = p; } } Rect r; r.origin = Point(1, 2); r.size = Point(5, 5); doubleMyValue(r.size.x); assert(r.lengths.x == 10); // fail How would your proposal affect the above code? I would assume the code would fail to compile (at least I hope it does), in which case you need to include it in Breaking Changes. Either way, the proposal should probably include a description of how the `this` reference of struct methods currently interacts with rvalues, and how it will be affected by the proposal. - Other corner cases that were raised during discussion of DIP-1016: alias this, return ref, auto ref, and refs in foreach. The proposal needs to address those. - return ref in particular is interesting. How do you handle the following code? int getGraphSize(ref Node); Node makeNode(int x); Node makeNode(return ref Node child1, return ref Node child2); // Probably should compile getGraphSize(makeNode(makeNode(1), makeNode(2))); // Probably should not auto persistentNode = makeNode(makeNode(1), makeNode(2)); // Invalid write persitentNode.child1.x = 123; |
March 29, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Olivier FAURE | On 3/29/19 7:32 AM, Olivier FAURE wrote: > On Wednesday, 27 March 2019 at 01:38:40 UTC, Andrei Alexandrescu wrote: >> https://gist.github.com/andralex/e5405a5d773f07f73196c05f8339435a >> >> Thanks in advance for any feedback. > > - During discussions around DIP-1016, it was pointed out that the following code currently compiles, even though it really shouldn't: > > struct Point > { > private int _x, _y; > ref int x() { return _x; } > ref int y() { return _y; } > } > > struct Rect > { > private Point _origin, _size; > Point origin() { return _origin; } > Point size() { return _size; } > void origin(Point p) { _origin = p; } > void size(Point p) { _size = p; } > } > > Rect r; > r.origin = Point(1, 2); > r.size = Point(5, 5); > doubleMyValue(r.size.x); > assert(r.lengths.x == 10); // fail > > How would your proposal affect the above code? That code would still compile because the draft DIP does not require a DotExpression a.b.c to have lvalues at all levels (i.e. all of a, b, and c); as long as c is a ref, a and b don't matter. This is hardly a problem with the DIP though because this compiles and runs with the same uselessness: r.size.x *= 2; I could change the DIP to require lvalues throughout. Also, we should probably change the language to disallow other constructs as well. Essentially I think a.b.c should be an rvalue (even if it's ostensibly an lvalue) if any of a, b, or c is an rvalue. > Either way, the proposal should probably include a description of how the `this` reference of struct methods currently interacts with rvalues, and how it will be affected by the proposal. I have moved that part straight into the specification of lvalues and rvalues. "this" in struct and union types is an lvalue. In classes it's an rvalue. > - Other corner cases that were raised during discussion of DIP-1016: alias this, return ref, auto ref, and refs in foreach. The proposal needs to address those. > > - return ref in particular is interesting. How do you handle the following code? > > int getGraphSize(ref Node); > Node makeNode(int x); > Node makeNode(return ref Node child1, return ref Node child2); > > // Probably should compile > getGraphSize(makeNode(makeNode(1), makeNode(2))); > > // Probably should not > auto persistentNode = makeNode(makeNode(1), makeNode(2)); > // Invalid write > persitentNode.child1.x = 123; Did you mean makeNode to return by reference? Good list, thanks! Andrei |
March 29, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Friday, 29 March 2019 at 12:16:40 UTC, Andrei Alexandrescu wrote: > I could change the DIP to require lvalues throughout. Also, we should probably change the language to disallow other constructs as well. Essentially I think a.b.c should be an rvalue (even if it's ostensibly an lvalue) if any of a, b, or c is an rvalue. Yes please. > I have moved that part straight into the specification of lvalues and rvalues. "this" in struct and union types is an lvalue. In classes it's an rvalue. That's not what I meant. What I meant is getFoo(...).applyDiscount(...) should behave the same way whether applyDiscount is a method taking a Foo as its 'this' parameter, or a global function taking a Foo as its first parameter. >> int getGraphSize(ref Node); >> Node makeNode(int x); >> Node makeNode(return ref Node child1, return ref Node child2); >> >> // Probably should compile >> getGraphSize(makeNode(makeNode(1), makeNode(2))); >> >> // Probably should not >> auto persistentNode = makeNode(makeNode(1), makeNode(2)); >> // Invalid write >> persitentNode.child1.x = 123; > > Did you mean makeNode to return by reference? No, I meant something like that: struct Node { Node* child1; Node* child2; int x; } Node makeNode(return ref Node child1, return ref Node child2) @safe { Node newNode = { &child1, &child2 }; return newNode; } This currently compiles with -dip1000. Generally speaking, I think any function called with a rvalue as a 'return ref' argument should be considered as returning a rvalue. The "a.b.c is a rvalue if a, b, or c is a rvalue" rule could be considered a special case of that principle. |
March 29, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On 3/29/19 8:16 AM, Andrei Alexandrescu wrote:
> On 3/29/19 7:32 AM, Olivier FAURE wrote:
>> On Wednesday, 27 March 2019 at 01:38:40 UTC, Andrei Alexandrescu wrote:
>>> https://gist.github.com/andralex/e5405a5d773f07f73196c05f8339435a
>>>
>>> Thanks in advance for any feedback.
>>
>> - During discussions around DIP-1016, it was pointed out that the following code currently compiles, even though it really shouldn't:
>>
>> struct Point
>> {
>> private int _x, _y;
>> ref int x() { return _x; }
>> ref int y() { return _y; }
>> }
>>
>> struct Rect
>> {
>> private Point _origin, _size;
>> Point origin() { return _origin; }
>> Point size() { return _size; }
>> void origin(Point p) { _origin = p; }
>> void size(Point p) { _size = p; }
>> }
>>
>> Rect r;
>> r.origin = Point(1, 2);
>> r.size = Point(5, 5);
>> doubleMyValue(r.size.x);
>> assert(r.lengths.x == 10); // fail
>>
>> How would your proposal affect the above code?
>
> That code would still compile because the draft DIP does not require a DotExpression a.b.c to have lvalues at all levels (i.e. all of a, b, and c); as long as c is a ref, a and b don't matter.
>
> This is hardly a problem with the DIP though because this compiles and runs with the same uselessness:
>
> r.size.x *= 2;
>
> I could change the DIP to require lvalues throughout. Also, we should probably change the language to disallow other constructs as well. Essentially I think a.b.c should be an rvalue (even if it's ostensibly an lvalue) if any of a, b, or c is an rvalue.
No, please don't. Rvalues can return lvalues just fine:
struct S
{
int* x;
ref int getX() { return *x; }
}
Note that the point being made was not to point out a problem, but to point out that rvalue references have existed for years (ever since struct member functions were added), and have not caused catastrophic failure.
-Steve
|
March 31, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | `On Thu, Mar 28, 2019 at 1:15 PM Andrei Alexandrescu via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On 3/28/19 3:59 PM, bitwise wrote:
> > I agree that the nonassignability requirement fills a major pothole, but it doesn't address the issue of expressing your intent as the writer of a function with ref parameters. The caller may have trouble determining if a function will modify the argument passed to it. The issue could be mitigated by function/parameter naming convention, comments, documentation, or providing source code to the caller, but all of these solutions have the potential to be spotty or absent, so I believe something should be done to make the writer's intent clear to the caller.
>
> A salient point.
>
> Also made in the DIP as "In such cases, w.price is not assignable and calls such as applyDiscount(w.price) or w.price.applyDiscount will succeed but not perform any meaningful update. A maintainer expecting such calls to fail may be surprised. We consider this is an inevitable price to pay for the gained flexibility."
>
> I reckon that there's implied that some sort of @rvalue attribute would be a better solution, a la void fun(@rvalue ref T). I'm afraid this would be widely protested,
See, I would read `void fun(@rvalue ref T)` as a *completely* different thing.
If that were in the language, I would expect that to be an
rvalue-reference, and as such may absolutely only accept rvalues, and
not temporaries.
I also think there's room on the language for both things, but I always thought rvalue-references were a contentions non-starter, because D has implicit move semantics, and it never seemed to me like anyone wanted to explore explicit move semantics. This talk about move constructors seems to have opened that conversation.
|
March 31, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
On Sun, Mar 31, 2019 at 2:28 PM Manu <turkeyman@gmail.com> wrote:
>
> `On Thu, Mar 28, 2019 at 1:15 PM Andrei Alexandrescu via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> >
> > On 3/28/19 3:59 PM, bitwise wrote:
> > > I agree that the nonassignability requirement fills a major pothole, but it doesn't address the issue of expressing your intent as the writer of a function with ref parameters. The caller may have trouble determining if a function will modify the argument passed to it. The issue could be mitigated by function/parameter naming convention, comments, documentation, or providing source code to the caller, but all of these solutions have the potential to be spotty or absent, so I believe something should be done to make the writer's intent clear to the caller.
> >
> > A salient point.
> >
> > Also made in the DIP as "In such cases, w.price is not assignable and calls such as applyDiscount(w.price) or w.price.applyDiscount will succeed but not perform any meaningful update. A maintainer expecting such calls to fail may be surprised. We consider this is an inevitable price to pay for the gained flexibility."
> >
> > I reckon that there's implied that some sort of @rvalue attribute would be a better solution, a la void fun(@rvalue ref T). I'm afraid this would be widely protested,
>
> See, I would read `void fun(@rvalue ref T)` as a *completely* different thing.
> If that were in the language, I would expect that to be an
> rvalue-reference, and as such may absolutely only accept rvalues, and
> not temporaries.
~~, and not temporaries.~~ - sorry, tail from a re-written sentence. That should read "and not lvalue references".
|
April 01, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Rubn | On Thursday, 28 March 2019 at 20:17:25 UTC, Rubn wrote:
> I liked the idea someone else gave in regards to "out".
I recommended in another post that 'in' be used. Is that what you mean?
'out' would be the wrong keyword for this.
|
April 01, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to Nicholas Wilson | On Thursday, 28 March 2019 at 16:07:17 UTC, Nicholas Wilson wrote: > >> It seems like a waste to have a keyword that's just an alias for two others (in = const scope), so why not put 'in' to better use, like to qualify a ref parameter as accepting rvalues, or to outright replace 'ref' for rvalue-ref accepting parameters? > > Code breakage. In this case, I think it's worth it. Dlang's documentation has warned against using 'in' for YEARS: https://dlang.org/spec/function.html#param-storage Any code using 'in' right now deserves to break. (but actually, that may not be necessary) I think 'in' could actually be used without changing it's meaning at all. Technically, it is exactly what's needed: > struct S { } > > // void func(const scope ref S s) > void func(in ref S s) { > // .... >} If 's' is an rvalue, then the justification for each qualifier would be: const - Modifying 's' has no effect. Allowing it is misleading and should be an error. scope - Temporary arguments should not be allowed to escape, for memory safety. ref - Large objects like 4x4 matrices should be passed by ref for efficiency. So again, 'in' seems like the correct choice for qualifying 'ref' parameters as taking rvalues. As far as using const, I don't really think it's that bad. If I had to have my rvalues qualified with const, that would be fine. To be honest, I don't think I've written any C/C++ code that casts const away in years, and would consider it unjustifiable. Dealing with legacy code is the only reason I can think of that someone may legitimately need to cast const away, which is fine though, because if D's const is used with legacy code (Extern C or C++ code), you can cast it all you want (to the same extent you would in your C code) because you know that what's underneath can't possibly be immutable, as C/C++ have no such concept. So finally, I would suggest that rvalues only bind to 'in ref'. |
April 01, 2019 Re: Binding rvalues to ref parameters redux | ||||
---|---|---|---|---|
| ||||
Posted in reply to bitwise | On 01.04.19 18:43, bitwise wrote: > ... > As far as using const, I don't really think it's that bad. > Yes, it really is that bad. > If I had to have my rvalues qualified with const, that would be fine. To be honest, I don't think I've written any C/C++ code that casts const away in years, ... There is no D const in C++. |
Copyright © 1999-2021 by the D Language Foundation