-preview=in might break code

[Steven Schveighoffer]

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