Add -preview=in to make in mean scope const as originally planned

[atilaneves]

Reboot of #10506

[dlang-bot]

Thanks for your pull request and interest in making D better, @atilaneves! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#10769"

[Geod24]

I don't think that's the right fix. It means there is a discrepancy between header / JSON generation when this preview switch is activated.

Before:

alias Dg = void delegate(in char[]) @safe;

void stringify(scope Dg sink) @safe {
    sink("oops");
}

void main() @safe {
    auto x = bar();
}

const(char)[] bar() @safe {
    string str;
    const(char)[] arg;
    stringify((chars) { arg ~= chars; });
    return null;
}

Turns into:

// D import file generated from 'atila.d'
alias Dg = void delegate(in char[]) @safe;
@safe void stringify(scope Dg sink);
@safe void main();
@safe const(char)[] bar();

With this switch on:

// D import file generated from 'atila.d'
alias Dg = void delegate(scope const char[]) @safe;
@safe void stringify(scope Dg sink);
@safe void main();
@safe const(char)[] bar();

[atilaneves]

What's the problem in practice?

[Geod24]

Sorry it took this long to answer...

The problem is that in is used in multiple places in DMD. It is its own storage class, which is weird, but the way it is currently.

Just substituting it in the parse phase is not enough. grep for STC.in_, you'll see it used for calling , you'll see it added to variadic arguments for D-style variadic functions (

dmd/src/dmd/expressionsem.d

Line 2332 in 91adbef

auto arg = new Parameter(STC.in_, (*arguments)[nparams + i].type, null, null, null);

), used for array iteration (

dmd/src/dmd/statementsem.d

Line 1811 in 91adbef

params.push(new Parameter(STC.in_, tn.arrayOf(), null, null, null));

), etc...

I did a few tests and since those are mostly druntime calls, and scope is ignored, most seems harmless (although they could interfere with escape analysis), but those kinds of disparities will come back to bite us at some point.

[Geod24]

And I shall again express my distaste for the double preview switch, as -preview=in is useless on its own and in was always intended to be scope const, the reason it was originally done this way was to avoid large breakage, but that was before DIP1000 was put behind a -preview switch (actually, -safe at the time). And it's not uncommon to add additional restrictions to DIP1000 when bugs are found.

[atilaneves]

@Geod24 I ended up submitting this with a preview switch because that way nothing breaks. I went back to my original idea of making in mean scope const with DIP1000 but, as you can see in the current status, it breaks a lot of things.

[wilzbach]

I ended up submitting this with a preview switch because that way nothing breaks.

The problem with those is that they just keep growing and we haven't activated a single one in the last two (three?) years. I guess at some point D3 (or 202X) will be equal to a set of preview switches - though I guess that's the only option available :/

[WalterBright]

One way is to have it on for the PR, and then fix everything that it breaks in all the tests. Once that's done, then put it behind a preview switch.

I did something similar for #10812 except instead of a preview switch it printed deprecation messages. I don't see a way to do deprecations for this PR, so preview should be it.

[12345swordy]

Lets merge this for now, and start working on breakage that may cause from this.

[Geod24]

Something that was overlooked: A changelog entry.

[atilaneves]

Something that was overlooked: A changelog entry.

#11003