On Saturday, 3 October 2020 at 12:09:34 UTC, Mathias LANG wrote:
> On Saturday, 3 October 2020 at 05:02:36 UTC, Andrei Alexandrescu wrote:
>> [...]
>>
>> Wait a SECOND! Are we really in the market of developing and deploying language features that come unglued at the slightest and subtlest misuse? We most certainly shouldn't.
>
> I agree. What I don't agree with is that this aliasing is a "slight and subtle misuse". I went through the 64 projects and their dependencies that are on Buildkite, and didn't see a hint of this pattern emerging. Nor did it in any other code I've surveyed in the almost 5 months the PR was open.

The codebases you look at are too small. If you look at the design rationale for Ada SPARK (which does not allow aliasing) you will see that aliasing is considered to be a problem that leads to very serious bugs in running systems.

This is well established irrespective of language.

On Saturday, 3 October 2020 at 11:04:48 UTC, Timon Gehr wrote:
> No. UB means demons may fly out of your nose. It's not that. You just get one of two behaviors, one is pass-by-reference, the other is pass-by-value.

UB just means that it is left out of the language.
UB does not mean that implementors cannot specify what will happen.

That is a complete misunderstanding of the term.

The fact that Clang exploits UB to achieve higher performance in the optimizer is a deliberate choice they made. It is not a consequence of UB in the language spec per se. It is a consequence of deliberate optimization efforts.

On Saturday, 3 October 2020 at 08:51:26 UTC, Daniel N wrote:
> You can freely mix any number of 'in' and 'out' parameters and keep the optimized behavior.
>
> As soon as you add any 'ref' parameter 'in' will pass by value.

I don't know what would be the best mix. :-) But it is interesting to think about various options that could work.

There are many ways to make it a little better, but very difficult to come up with something that feels right.

For instance it is possible to use the type system to prove that in some cases aliasing is not possible simply because the types of objects that are reachable from parameter 1 are all different from the types reachable from parameter 2. Then you know that there can be no aliasing between the parameters. Unfortunately that excludes way too many valid cases where this does not hold.

And well, you also need to consider aliasing with globals that are accessed within the function.

On 10/3/20 8:09 AM, Mathias LANG wrote:
> On Saturday, 3 October 2020 at 05:02:36 UTC, Andrei Alexandrescu wrote:
>> [...]
>>
>> Wait a SECOND! Are we really in the market of developing and deploying language features that come unglued at the slightest and subtlest misuse? We most certainly shouldn't.
> 
> I agree. What I don't agree with is that this aliasing is a "slight and subtle misuse". I went through the 64 projects and their dependencies that are on Buildkite, and didn't see a hint of this pattern emerging. Nor did it in any other code I've surveyed in the almost 5 months the PR was open.

First off, thanks for engaging. Having taken a look at existing projects is a wise thing to do. There are a few counterpoints here:

* Proving a negative is inherently difficult.

* It's difficult to look at millions of lines of code, much of which is templated (and so can be subject to problems in the future), and make an assessment. (Clearly it's very impressive that you did.) That is not a guarantee and does not cover future uses, however.

* The fallacy of numbers comes to mind. You wouldn't want to introduce, for example, some subtle concurrency problem on account on not having seen it in existing projects.

* This has been discussed in C++ circles a number of times, and aliasing has always been a concern. If /C++/ deemed that too dangerous... <insert broadside>. A much more explicit solution has been implemented in https://www.boost.org/doc/libs/1_66_0/libs/utility/call_traits.htm.

>> I sincerely congratulated Mathias and the other participants for working on this. It's an important topic. Knowing that all those involved are very good at what they do, and without having looked closely, I was sure they got something really nice going that avoids this absolutely blatant semantic grenade. And now I see this is exactly it - we got a -preview of a grenade. How is this possible? How can we sleep at night now?
>>
>> Again: take a step back and reconsider, why did this pass muster?
> 
> Maybe because the people that want to use this actually know this is nowhere near as big of an issue as some people try to make it look.

Sorry if it looks like I'm pushing an agenda here - I most certainly am not. I'm acting on the perception that this looks distinctly like one of the mistakes of the past that now we're wondering how they made it and how to undo them. At the very minimum a lot more scrutiny is necessary.

> Perhaps you don't know this, but the very first implementation, the one I had when I opened the PR on April 3rd actually always used `ref`. It had quite a few issues. @Kinke suggested an alternative, and that alternative brought many benefits with it, for a very minor downside, which can easily be mitigated: if your function's semantic really depend on mutation through an alias propagating (or not) to an `in` parameter, then you can use `__traits(isRef, paramname)` to check it.
> Later on, on July 31st, I brought the topic to the forum, which pressed on another suggestion @Kinke has made, using the full function type instead of just the parameter type. At the time, you also made mention of this issue, and it was considered.

Thank you for the context.

> I wonder, did you try to adapt one of your projects to `in` ? Not merely throwing the switch, but also adding a couple `in` here and there, were it makes sense.

I have no doubt it will work in many instances. What I'm worry about is that when it doesn't, the user has very little indication and tooling to help with the most puzzling (and platform-dependant!) behavior.

> Not that this `-preview` is the only one that has ever been usable from release day (perhaps `markdown` was as well, but it only affected documentation generation). All the other previews would fail on your project because druntime and Phobos were never adapted after the switch was merged. `-preview=in` not only works with druntime/phobos, but many libraries from Buildkite were also adapted to work with or without it (https://github.com/dlang/dmd/pull/11632). I was expecting this would increase user engagement, allow to gather feedback, use cases, and lead to a healthy discussion, just like it allowed to refine the implementation. But I wasn't expecting FUD to be spread over a `-preview` before people even tried it.

Again, indeed sorry if it looks I'm trying to spread FUD and thanks for engaging. All I'm doing is to try to be sensible.

On Saturday, 3 October 2020 at 11:04:48 UTC, Timon Gehr wrote:
> There's a difference between "the behavior may be either A or B" and "the behavior may be anything you like"...

And just to be clear on this: the example Steven gave showed code that should be defined to be outside of the language. It is not code that anyone would want to pass the compilation stage if the compiler could be made smart enough to detect it.

The moment you say the undesirable behaviour is within the language, you have effectively made the example valid and thus an improved compiler cannot reject it. If it is within the language an improved compiler would be a breaking change.

The value-implementation of reference passing is different, because such implementation specific behaviour may be desirable! But the code Steven provided was not desirable to allow as a "valid text" in the language.

(A language spec can provide details of how the runtime should behave when the compiler fails to detect whether code is within the language or not. Then you just label "undefined behaviour" as "illegal constructs", but it is essence the same thing).

On Saturday, 3 October 2020 at 12:09:34 UTC, Mathias LANG wrote:
> On Saturday, 3 October 2020 at 05:02:36 UTC, Andrei Alexandrescu wrote:
>> [...]
>
> I agree. What I don't agree with is that this aliasing is a "slight and subtle misuse". I went through the 64 projects and their dependencies that are on Buildkite, and didn't see a hint of this pattern emerging. Nor did it in any other code I've surveyed in the almost 5 months the PR was open.
>
> [...]

You have obviously done your homework.

I just want to say one thing to the audience:

Isn't this exactly why -preview exists?

So we can try out features and report any problems?

I think the process is working as intended, and we should all be thankful for all attempts to improve D.

Thanks

On Saturday, 3 October 2020 at 12:09:34 UTC, Mathias LANG wrote:
> Perhaps you don't know this, but the very first implementation, the one I had when I opened the PR on April 3rd actually always used `ref`. It had quite a few issues. @Kinke suggested an alternative, and that alternative brought many benefits with it, for a very minor downside, which can easily be mitigated: if your function's semantic really depend on mutation through an alias propagating (or not) to an `in` parameter, then you can use `__traits(isRef, paramname)` to check it.

The idea is that `in` is explicit, and users of a function with `in` params should think of such params as `const T& __restrict__` in C++ terms, which should clarify the potential aliasing problem. Whether the thing is optimized to pass-by-value then shouldn't make any observable difference for the caller.

> Later on, on July 31st, I brought the topic to the forum, which pressed on another suggestion @Kinke has made, using the full function type instead of just the parameter type.

I don't recall that, are you sure it wasn't someone else? I'd rather have it based on the parameter type alone.

On 10/3/20 8:09 AM, Mathias LANG wrote:
> On Saturday, 3 October 2020 at 05:02:36 UTC, Andrei Alexandrescu wrote:
>> [...]
>>
>> Wait a SECOND! Are we really in the market of developing and deploying language features that come unglued at the slightest and subtlest misuse? We most certainly shouldn't.
> 
> I agree. What I don't agree with is that this aliasing is a "slight and subtle misuse". I went through the 64 projects and their dependencies that are on Buildkite, and didn't see a hint of this pattern emerging. Nor did it in any other code I've surveyed in the almost 5 months the PR was open.

First, I want to say, I think `in` being adjusted is a good initiative. And I think there are some ways to get back to sanity here.

But the problem *IS* that it is *very uncommon* that you will run into issues. An issue that is common is a problem in it's own right. An issue that is subtle and very uncommon (so uncommon that you haven't found a case of it yet) is a different kind of problem. I don't believe that less frequent means we have a better situation, I think it's worse.

> Also because some of the criticism is based on the rather loose definition of the promotion to value at the moment. That definition is intentionally loose, not because it needs to be, but because the feature is in `-preview` and the rules need to take into account all platforms that D support, something that cannot be done in DMD alone.

If we can make this solidified, then I think we can be OK with it.

One option is to always pass by reference.

Another option is to tag an `in` value as "I'm OK if this is passed by reference". like `in ref` or something (that still binds to rvalues). Then an optimization can say "actually, I'm going to pass this by value", and nobody cares. This is different in that you have to *declare* you're ok with a reference, not the other way around.

What I want is for the system to behave in a reliable way, so I can write code that works regardless of optimizations.

> Perhaps you don't know this, but the very first implementation, the one I had when I opened the PR on April 3rd actually always used `ref`. It had quite a few issues.

What were those issues?

> @Kinke suggested an alternative, and that alternative brought many benefits with it, for a very minor downside, which can easily be mitigated: if your function's semantic really depend on mutation through an alias propagating (or not) to an `in` parameter, then you can use `__traits(isRef, paramname)` to check it.

But this isn't what happens. What happens is, I write code that assumes some `in` parameter won't change, and it actually doesn't. It works great, because the backend decides to pass by value. Then one day, the backend decides, "you know what, I found that passing this size data by reference is more efficient". Now, because I didn't *proactively* put in some __traits(isRef, paramname) check, my code breaks. And only in one call on one user's code base. And the result is memory corruption or some other subtle off-by-one thing that is difficult to trace. Maybe it results in a thread deadlock. I know because I've ran into stuff like this, and it takes weeks to debug.

If all `in` parameters have to be checked with

static assert(!__traits(isRef, paramname))

and I have to review all functions with `in` parameters that aren't ref with this relationship in mind (is it possible for the data to change while I'm using the in parameter), then in soon becomes a feature that is rejected at code review (it's too complex to be worth it).

> Not that this `-preview` is the only one that has ever been usable from release day (perhaps `markdown` was as well, but it only affected documentation generation). All the other previews would fail on your project because druntime and Phobos were never adapted after the switch was merged. `-preview=in` not only works with druntime/phobos, but many libraries from Buildkite were also adapted to work with or without it (https://github.com/dlang/dmd/pull/11632). I was expecting this would increase user engagement, allow to gather feedback, use cases, and lead to a healthy discussion, just like it allowed to refine the implementation. But I wasn't expecting FUD to be spread over a `-preview` before people even tried it.

This is a bit ridiculous. If I was trying to sell you a gun, which in 0.001% of cases explodes when you pull the trigger, and I say "Yeah that's true, but that almost never happens. You aren't even trying it!!!" does that make you feel better?

I don't want to try it, have it work, and then later on down the road explode in my face.

The term FUD has connotations that the problems we are talking about aren't real. They rely on the person's ignorance of the actual issues, or on things that aren't provable. This is not that.

-Steve

On Saturday, 3 October 2020 at 13:05:43 UTC, Andrei Alexandrescu wrote:
> On 10/3/20 8:09 AM, Mathias LANG wrote:
>> On Saturday, 3 October 2020 at 05:02:36 UTC, Andrei Alexandrescu wrote:
 But I wasn't
>> expecting FUD to be spread over a `-preview` before people even tried it.
>
> Again, indeed sorry if it looks I'm trying to spread FUD and thanks for engaging. All I'm doing is to try to be sensible.

It looked like FUD to me as well.
Thanks for clarifying that it, "most certainly" was not.

On Saturday, 3 October 2020 at 14:10:34 UTC, Steven Schveighoffer wrote:
>
> I don't want to try it, have it work, and then later on down the road explode in my face.
>
> The term FUD has connotations that the problems we are talking about aren't real. They rely on the person's ignorance of the actual issues, or on things that aren't provable. This is not that.

While I don't like -preview=in because it complicates the ABI somewhat.
I have to agree that it is just a preview for now.
If it doesn't prove itself, we can pretend it never existed.
We could even have a revert switch once, it has proven itself.
(I do think that should be a common practice for new feature additions from now on, i.e. allow me to turn them off and get back to an older version of D with a new compiler.)