On Wednesday, 30 January 2019 at 18:29:37 UTC, Manu wrote:
> On Wed, Jan 30, 2019 at 9:20 AM Neia Neutuladh via Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
>> The result of a CastExpression is an rvalue. An implicit cast is a compiler-inserted CastExpression. Therefore all lvalues with a potential implicit cast are rvalues.
>
> But there's no existing language rule that attempts to perform an implicit cast where an lvalue is supplied to a ref arg...?
> Why is the cast being attempted? 'p' is an lvalue, and whatever that does should remain exactly as is (ie, emits a compile error).
>
> We could perhaps allow this for `const` args, but that feels like separate follow-up work to me, and substantially lesser value. This DIP doesn't want to change anything about lvalues.

It appears to say it does:

fun(my_short); // implicit type conversions (ie, short->int promotion)

You should clarify that ;)

On Thursday, 31 January 2019 at 02:10:05 UTC, Manu wrote:
> On Wed, Jan 30, 2019 at 1:05 PM Andrei Alexandrescu via
>> fun(my_short); // implicit type conversions (ie, short->int promotion)
>> ========
>
> Oh I see.
>
>> fun(short(10)); // implicit type conversions (ie, short->int promotion)
>
> I did not intend for this DIP to apply to anything other than rvalues.
> I can totally see how that's not clear. `my_short` should be an rvalue
> of some form, like the rest.
> Is that the only such line?

I think so.

>> Presumably my_short is a variable of type short. Is that correct?
>
> It is not. It should be an rvalue like everything else. Perhaps it's an enum... but I should write `short(10)`, that would be clear.

It would.

>> * DIP 1016 proposes a hole in the language one could drive a truck through.
>
> I still can't see a truck-sized hole.
>
>> * The problem goes undetected in community review.
>
> I don't know how I could have influenced this outcome.
>
>> * Its own author seems to not have an understanding of what the DIP proposes.
>
> More classy comments. I can't get enough of the way you belittle people.
>
> I made a 1-word error, where I should have written `short(10)` to be clear.
> 1-word error feels amendment-worthy, and not a call for "let's start
> over from scratch".

You should just PR it back to review with that fix and a note about how it lowers to statements (incl. an example of lambdification for if/while/for/switch statements (see https://forum.dlang.org/post/qysmnatmjquuhylaqumm@forum.dlang.org ))

On Wed, Jan 30, 2019 at 6:40 PM Nicholas Wilson via Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
>
> On Wednesday, 30 January 2019 at 18:29:37 UTC, Manu wrote:
> > On Wed, Jan 30, 2019 at 9:20 AM Neia Neutuladh via Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
> >> The result of a CastExpression is an rvalue. An implicit cast is a compiler-inserted CastExpression. Therefore all lvalues with a potential implicit cast are rvalues.
> >
> > But there's no existing language rule that attempts to perform an implicit cast where an lvalue is supplied to a ref arg...? Why is the cast being attempted? 'p' is an lvalue, and whatever that does should remain exactly as is (ie, emits a compile error).
> >
> > We could perhaps allow this for `const` args, but that feels like separate follow-up work to me, and substantially lesser value. This DIP doesn't want to change anything about lvalues.
>
> It appears to say it does:
>
> fun(my_short); // implicit type conversions (ie, short->int
> promotion)
>
> You should clarify that ;)

Yes, as said above, read `short(10)`. I can understand the confusion
that it may look like a variable when taken out of context; but listed
beneath the heading immediately above which says:
"This inconvenience extends broadly to every manner of **rvalue**
passed to functions"
It didn't occur to me the reader might interpret the clearly stated
list of cases of rvalues passed to functions to include arguments that
are not rvalues.
The name was just chosen to indicate the argument is a short, perhaps
an enum, or any expression that is a short... I could have used
`short(10)`, but apparently I didn't think of it at the time.

Is this the basis for the claims of "a hole you could drive a truck through"? Again, a request for clarification, and a couldn't-possibly-be-more-trivial revision may resolve this.

On Wed, Jan 30, 2019 at 7:05 PM Nicholas Wilson via Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
>
> On Thursday, 31 January 2019 at 02:10:05 UTC, Manu wrote:
> > On Wed, Jan 30, 2019 at 1:05 PM Andrei Alexandrescu via
> >> fun(my_short); // implicit type conversions (ie, short->int
> >> promotion)
> >> ========
> >
> > Oh I see.
> >
> >> fun(short(10)); // implicit type conversions (ie, short->int
> >> promotion)
> >
> > I did not intend for this DIP to apply to anything other than
> > rvalues.
> > I can totally see how that's not clear. `my_short` should be an
> > rvalue
> > of some form, like the rest.
> > Is that the only such line?
>
> I think so.
>
> >> Presumably my_short is a variable of type short. Is that correct?
> >
> > It is not. It should be an rvalue like everything else. Perhaps it's an enum... but I should write `short(10)`, that would be clear.
>
> It would.
>
> >> * DIP 1016 proposes a hole in the language one could drive a truck through.
> >
> > I still can't see a truck-sized hole.
> >
> >> * The problem goes undetected in community review.
> >
> > I don't know how I could have influenced this outcome.
> >
> >> * Its own author seems to not have an understanding of what the DIP proposes.
> >
> > More classy comments. I can't get enough of the way you belittle people.
> >
> > I made a 1-word error, where I should have written `short(10)`
> > to be clear.
> > 1-word error feels amendment-worthy, and not a call for "let's
> > start
> > over from scratch".
>
> You should just PR it back to review

I can't do that, it's been rejected, with mostly incorrect rejection text affixed to the bottom.

> with that fix and a note
> about how it lowers to statements (incl. an example of
> lambdification for if/while/for/switch statements (see
> https://forum.dlang.org/post/qysmnatmjquuhylaqumm@forum.dlang.org
> ))

I'm pretty sure that's not necessary. I haven't understood why this
noise about expressions. This DIP applies to statements.
I can't see how there's any problem with the lowering if the statement
is a control statement?

if (ref_fun(10)) { ... }
==>
{
  int __tmp = 10;
  if (ref_fun(__tmp)) { ... }
}

What's the trouble?

On 1/30/19 10:05 PM, Manu wrote:
> On Wed, Jan 30, 2019 at 6:40 PM Nicholas Wilson via
> Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
>> You should clarify that ;)
> 
> Yes, as said above, read `short(10)`. I can understand the confusion
> that it may look like a variable when taken out of context; but listed
> beneath the heading immediately above which says:
> "This inconvenience extends broadly to every manner of **rvalue**
> passed to functions"
> It didn't occur to me the reader might interpret the clearly stated
> list of cases of rvalues passed to functions to include arguments that
> are not rvalues.
> The name was just chosen to indicate the argument is a short, perhaps
> an enum, or any expression that is a short... I could have used
> `short(10)`, but apparently I didn't think of it at the time.
> 
> Is this the basis for the claims of "a hole you could drive a truck
> through"? Again, a request for clarification, and a
> couldn't-possibly-be-more-trivial revision may resolve this.
> 

I think changing it to `short(10)` helps the argument that you didn't intend it to mean conversions from lvalues, but I'd recommend still spelling out that they are forbidden.

Leaving the reader to infer intent is not as good as clarifying intent directly. The whole rvalue vs. lvalue thing is confusing to me, because I assumed an lvalue converted to a different type changes it to an rvalue. I think of it like an implicit function that returns that new value.

-Steve

On Wed, Jan 30, 2019 at 7:35 PM Steven Schveighoffer via Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
>
> On 1/30/19 10:05 PM, Manu wrote:
> > On Wed, Jan 30, 2019 at 6:40 PM Nicholas Wilson via Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
> >> You should clarify that ;)
> >
> > Yes, as said above, read `short(10)`. I can understand the confusion
> > that it may look like a variable when taken out of context; but listed
> > beneath the heading immediately above which says:
> > "This inconvenience extends broadly to every manner of **rvalue**
> > passed to functions"
> > It didn't occur to me the reader might interpret the clearly stated
> > list of cases of rvalues passed to functions to include arguments that
> > are not rvalues.
> > The name was just chosen to indicate the argument is a short, perhaps
> > an enum, or any expression that is a short... I could have used
> > `short(10)`, but apparently I didn't think of it at the time.
> >
> > Is this the basis for the claims of "a hole you could drive a truck through"? Again, a request for clarification, and a couldn't-possibly-be-more-trivial revision may resolve this.
> >
>
> I think changing it to `short(10)` helps the argument that you didn't intend it to mean conversions from lvalues, but I'd recommend still spelling out that they are forbidden.

I mean, the heading of the DIP is "ref T accepts r-values", the whole abstract talks about nothing but rvalues, the header of the confusing block couldn't say 'rvalues' more clearly... I didn't consider that it was possible to confuse this as anything other than an rvalue DIP... but yes, I can certainly spell it out.

> Leaving the reader to infer intent is not as good as clarifying intent
> directly. The whole rvalue vs. lvalue thing is confusing to me, because
> I assumed an lvalue converted to a different type changes it to an
> rvalue. I think of it like an implicit function that returns that new value.

Obviously all of this is true, but I didn't think of it that way;
didn't realise there was a point of confusion, and nobody during the
community reviews appeared to raise confusion either.
I'll obviously revise it, except that it's rejected and moved to the
rejected folder.

For reference, the key point that justifies its mention in the first
place is a little further down:
"It is important that T be defined as the parameter type, and not auto
(ie, the argument type), because it will allow implicit conversions to
occur naturally, with identical behavior as when the parameter is not
ref."
It was important to consider mis-matching types (implicit
conversions), because there is detail in the rules that allows them to
work properly and make the call uniform with the same function if it
passed by-val.

On 1/30/19 9:10 PM, Manu wrote:
>> * Its own author seems to not have an understanding of what the DIP
>> proposes.
> 
> More classy comments. I can't get enough of the way you belittle people.

You're right. I have deleted this post a few seconds after having sent it on account of that remark, but somehow it got resuscitated. Please accept my apologies.

On 1/30/19 10:05 PM, Manu wrote:
> On Wed, Jan 30, 2019 at 6:40 PM Nicholas Wilson via
> Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
>>
>> On Wednesday, 30 January 2019 at 18:29:37 UTC, Manu wrote:
>>> On Wed, Jan 30, 2019 at 9:20 AM Neia Neutuladh via
>>> Digitalmars-d-announce <digitalmars-d-announce@puremagic.com>
>>> wrote:
>>>> The result of a CastExpression is an rvalue. An implicit cast
>>>> is a compiler-inserted CastExpression. Therefore all lvalues
>>>> with a potential implicit cast are rvalues.
>>>
>>> But there's no existing language rule that attempts to perform
>>> an implicit cast where an lvalue is supplied to a ref arg...?
>>> Why is the cast being attempted? 'p' is an lvalue, and whatever
>>> that does should remain exactly as is (ie, emits a compile
>>> error).
>>>
>>> We could perhaps allow this for `const` args, but that feels
>>> like separate follow-up work to me, and substantially lesser
>>> value. This DIP doesn't want to change anything about lvalues.
>>
>> It appears to say it does:
>>
>> fun(my_short); // implicit type conversions (ie, short->int
>> promotion)
>>
>> You should clarify that ;)
> 
> Yes, as said above, read `short(10)`. I can understand the confusion
> that it may look like a variable when taken out of context; but listed
> beneath the heading immediately above which says:
> "This inconvenience extends broadly to every manner of **rvalue**
> passed to functions"
> It didn't occur to me the reader might interpret the clearly stated
> list of cases of rvalues passed to functions to include arguments that
> are not rvalues.
> The name was just chosen to indicate the argument is a short, perhaps
> an enum, or any expression that is a short... I could have used
> `short(10)`, but apparently I didn't think of it at the time.
> 
> Is this the basis for the claims of "a hole you could drive a truck
> through"?

Affirmative.

With the restriction that the expression passed into the function must be an rvalue to start with, by Walter's and my understanding, the proposed semantics would work and be helpful.

> Again, a request for clarification, and a
> couldn't-possibly-be-more-trivial revision may resolve this.

Negative.

It must be clear that the reason of this misunderstanding is squarely due to the DIP itself. It has multiple problems of informality and vague language that have worked together to cause said misunderstanding. (It is great it's just that, a misunderstanding; I have been worried people would believe such an awful semantics was considered just fine. That explains but does not justify my use of unkind language.)

The DIP must convince the reader, and in a way the reader does not "owe" the DIP. For good reason, they call the research theme chosen by a doctoral candidate a "charge"; the root of "dissertation" is Latin for "debate"; and the final doctoral examination is plainly called a "defense". The whole thing is structured like a criminal investigation :o). Of course we don't want to be as harsh as academics could get, but we don't want to transform DIP acceptance into a farmers market bargaining process.

So the code with my_short was open to interpretation. Cool. In a thorough submission, however, there would have been many places that clear that up:

* Use of a distinct notation (non-code non-text font for metalanguage, i.e. general expressions);

* Description of the typechecking process, with examples of code that passes and code that fails;

* A clarification that lowering proceeds not against all expressions, but only against rvalues;

* Several places in text in which it is explained that rvalues resulted from implicit conversions are not eligible;

* etc. etc. etc.

So if we rejected the DIP, we didn't do so on account of one word that can be so easily changed; we did so on account of a DIP that as a whole failed to clarify what it purports to do (and equally importantly, to not do).

The purpose of us all is to move things forward, and in that spirit allow me to put forward a short list of matters that a revised proposal should do, at a minimum:

* Walter has posted about a few issues with various parts of the proposal. Those should be addressed.

* The "Reference" section does good to mention the issues, but the litany of forum discussions has no value besides "there have been repeated discussion in community forums of the topic", and refer to a list in an bibliography. Placing them in the "Reference" section suggests the reader that they need to read the forum debates in order to understand the DIP, which isn't and shouldn't be the case.

* An "Existing Work" section discussing C++ (and possibly Rust) is a must. Studious neglect of what other languages do and what problems they have does not serve us well. I think Walter could help with that.

* The "Rationale" section currently focuses only on issues caused by the current rule. It should have three parts:

- Open with a brief description of the current rule and why it is that way. Here we have the advantage that confusing conversions are disallowed.
- Then continue with "However, the binding rule also causes a variety of undue limitations whereby valid and useful code is rejected". Insert examples.
- Then continue with "If the rule got relaxed so as to only allow certain bindings but not all, we could allow the sensible cases illustrated above, yet still disallow the problematic cases."

Boom! The reader is already loving it.

* Subsection "Why are we here?" is not necessary. Its first part is folded in the "Rationale", and its second part is actually clearly an argument that is part of the rationale. IMHO the "pipeline" argument is not necessary, or can be moved in an "Examples of use" section toward the end.

* "Description" needs a complete rewrite. Here typing rules, lowering, exception safety, should be explained in ultimate detail using metalanguage (not just a few simplistic examples). It should be mechanically clear how an arbitrary expression containing function calls can be lowered into preexisting D code. Again I very much recommend lowering expressions to expressions instead of statements to statements, but anything clear and thorough will do.

* Also the description should clarify what happens in certain odd cases, such as alias this, class inheritance, properties that return lvalues vs. rvalues using the same syntax, and probably more. An important aspect of the proposal is "rvalues look different than lvalues, so the user won't get confused".

* "Temporary destruction" is unnecessary.

* "Function calls as arguments" us unnecessary if "Description" describes the feature completely.

* "Interaction with return ref" is probably unnecessary as a subsection but it would be nice to have as an explanatory example after the general lowering has been introduced.

* "Overload resolution" - see Walter's comments.

* Eliminate imprecise language "It has been noted that..." (by whom? when? reference?) If no reference, fine - spend a sentence to create that note!

* "Why not XXX?" should be massaged into a "Workarounds" section that follows the "Rationale" section, or is a subsection of it.

* "Key use cases" is good, and more examples should be only better.

Hope this helps! The outlook is definitely better after this misunderstanding has been cleared.


Andrei

On 1/30/19 10:12 PM, Manu wrote:
> On Wed, Jan 30, 2019 at 7:05 PM Nicholas Wilson via
> Digitalmars-d-announce <digitalmars-d-announce@puremagic.com> wrote:
>>
>> On Thursday, 31 January 2019 at 02:10:05 UTC, Manu wrote:
>>> On Wed, Jan 30, 2019 at 1:05 PM Andrei Alexandrescu via
>>>> fun(my_short); // implicit type conversions (ie, short->int
>>>> promotion)
>>>> ========
>>>
>>> Oh I see.
>>>
>>>> fun(short(10)); // implicit type conversions (ie, short->int
>>>> promotion)
>>>
>>> I did not intend for this DIP to apply to anything other than
>>> rvalues.
>>> I can totally see how that's not clear. `my_short` should be an
>>> rvalue
>>> of some form, like the rest.
>>> Is that the only such line?
>>
>> I think so.
>>
>>>> Presumably my_short is a variable of type short. Is that
>>>> correct?
>>>
>>> It is not. It should be an rvalue like everything else. Perhaps
>>> it's an enum... but I should write `short(10)`, that would be
>>> clear.
>>
>> It would.
>>
>>>> * DIP 1016 proposes a hole in the language one could drive a
>>>> truck through.
>>>
>>> I still can't see a truck-sized hole.
>>>
>>>> * The problem goes undetected in community review.
>>>
>>> I don't know how I could have influenced this outcome.
>>>
>>>> * Its own author seems to not have an understanding of what
>>>> the DIP proposes.
>>>
>>> More classy comments. I can't get enough of the way you
>>> belittle people.
>>>
>>> I made a 1-word error, where I should have written `short(10)`
>>> to be clear.
>>> 1-word error feels amendment-worthy, and not a call for "let's
>>> start
>>> over from scratch".
>>
>> You should just PR it back to review
> 
> I can't do that, it's been rejected, with mostly incorrect rejection
> text affixed to the bottom.
> 
>> with that fix and a note
>> about how it lowers to statements (incl. an example of
>> lambdification for if/while/for/switch statements (see
>> https://forum.dlang.org/post/qysmnatmjquuhylaqumm@forum.dlang.org
>> ))
> 
> I'm pretty sure that's not necessary. I haven't understood why this
> noise about expressions. This DIP applies to statements.
> I can't see how there's any problem with the lowering if the statement
> is a control statement?
> 
> if (ref_fun(10)) { ... }
> ==>
> {
>    int __tmp = 10;
>    if (ref_fun(__tmp)) { ... }
> }
> 
> What's the trouble?

The trouble is major.

Replace "if" with "while":

while (ref_fun(10)) { ... }
==>
{
  int __tmp = 10;
  while (ref_fun(__tmp)) { ... }
}

That means ref_fun is called with the same lvalue multiple times. In all likelihood this is not what you want!

A possible retort is: "Of course, while would not be lowered that way, but a slightly different way!" etc. The point is, ALL OF THAT must be in the DIP, not assumed obvious or clarified in informal discusson outside the DIP.

Again: please be thorough, state your assumptions, cover all cases.

On 1/30/2019 5:55 PM, Manu wrote:
>> lets replace 10 with a short variable named: S
> "a short variable named: S" is an lvalue, so why would the rewrite be
> attempted? S must be an rvalue for any rewrite to occur. We're talking
> about rvalues here.

This illustrates why this should be compared with C++. Consider this C++ code:

  const int& foo(const int& x) { return x; }

  const int& test() {
    short s;
    return foo(s);
  }

It compiles with clang++. The code generated for test() is:

        push    RBP
        mov     RBP,RSP
        sub     RSP,010h
        lea     RDI,-8[RBP]
        movsx   EAX,word ptr -2[RBP]
        mov     -8[RBP],EAX
        call    foo
        add     RSP,010h
        pop     RBP
        ret

See what it is doing? It's converting s to an int, putting the int into a temporary, then passing a reference to that temporary to foo(). So when you ask why would a person think that this would happen with the DIP, if they know C++, they would assume similar behavior. This is why the DIP needs to specifically say this is not the proposed behavior. It is why a comparison to C++ behavior is essential. It is a lot easier to understand the DIP if people can apply their existing understanding, with a few modifications, to the D behavior. It's also necessary to compare with C++ to see if the DIP missed something important, and to justify any other behavioral differences.

The interesting question is, since C++ supports this behavior, what about the truck-sized hole? The answer is in the declaration of foo(const int&). The const is the reason. The referenced value cannot be modified. The realloc example is blocked by the compiler.

But the DIP says const ref is not required. Therefore, copying an lvalue to a temporary cannot be allowed, therefore implicit conversion of lvalues cannot happen.

Then we're faced with the question of if implicit conversion of lvalues is not allowed, should implicit conversion of rvalues be allowed? I'm not so sure it should be. For one thing, a lot of people are confused about lvalues vs rvalues, and would find the difference in behavior puzzling. For another, it can complicate overloading rules. I'd say allowing the conversions needs a strong rationale.