get started on scope diagnostics

[WalterBright]

No description provided.

[WalterBright]

Blocked by dlang/phobos#4549

[wilzbach]

Small language nitpick, "may" sounds a bit too optional for me. Shouldn't we use sth like "can't"?

(Disclaimer: I am not a native speaker.)

[WalterBright]

"may not" does not mean "optional". I also prefer to avoid contractions in code because grepping for them is a pain because of the quote.

[PetarKirov]

"may" can be used to indicate both optionality or regulation, but when negated it is always as strong as "must not", so it sounds good to me.
In the long term we should probably adopt RFC 2119 for the specification and compiler messages, but that would be a huge effort.

[WalterBright]

Blocked by dlang/phobos#4556

[PetarKirov]

In general I support this change, but as noted in the Phobos PR, this could potentially affect a lot of code that uses functions that take scope delegates. Is it possible to mitigate this by making something like return dg_t work, i.e. ensure that the delegate would not allocate it's captured variables on the heap and cannot be escaped, with exception that it can be returned?

[WalterBright]

In the future there will be return scope which will do what you wish.

[andralex]

Auto-merge toggled on

[PetarKirov]

In the future there will be return scope which will do what you wish

Great, that's exactly what I want. I just didn't know that we would be able to use both at the same time.

[MartinNowak]

This PR caused a regression.
Issue 16582 – [REG2.072.0-b1] ParamterDefaultValueTuple fails to compile for scope paramters
Another breaking change w/o notice or clear workaround.
Why do we make valid code an error before introducing a mean to clarify the intention (return scope) 🙄?

[andralex]

@MartinNowak shall we undo this?

[WalterBright]

It is fixed by #5972

[MartinNowak]

It is fixed by #5972

But we can't drag that into the release now that the beta is almost done.
This is what I mean when I say that incremental PRs merged into master create lots of problems.

@MartinNowak shall we undo this?

It's non-trivial (but doable) to undo after 3 month and lots of follow-up refactorings.
Not sure how much code is affected by this, but I'd favor to back this off until return scope works.
Made a hacky workaround for the phobos bug.

[WalterBright]

But we can't drag that into the release now that the beta is almost done.

Sure, but I suggested merging it in a month ago for just that reason.

This is what I mean when I say that incremental PRs merged into master create lots of problems.

Not merging things creates another set of problems. For example, there's not a single review yet of the return scope PR. Nobody wants to review large PRs. And at least with this regression, the culprit is small rather than large.

I'd favor to back this off until return scope works.

A practical alternative is to disable the error unless -transition=safe.

Made a hacky workaround for the phobos bug.

Thank you.

[MartinNowak]

Sure, but I suggested merging it in a month ago for just that reason.

Well you still submitted them in a different order :).

Not merging things creates another set of problems. For example, there's not a single review yet of the return scope PR. Nobody wants to review large PRs.

Hence feature branches, early review, no risk in master.

And at least with this regression, the culprit is small rather than large.

The one we see, but not necessarily the effect of it. The phobos bug was triggered by some meta code in vibe.d or so, which would affect plenty of users...
We have little knowledge about other code, and w/o a clear workaround estimating the impact to be small remains a motivated guess.

[MartinNowak]

A practical alternative is to disable the error unless -transition=safe.

See #6183 for why this isn't the correct approach.

I'll make a pass through the project tester, trying to find any other code that runs into problems with this.
I don't expect to find any, so we might get away with it. Still not the way to do such changes.

[mihails-strasuns]

Is in expanded right now into scope const as has been stated in docs? If yes, this can cause a huge amount of breakage.

[mihails-strasuns]

For example, there's not a single review yet of the return scope PR. Nobody wants to review large PRs.

Acceptance tests > reviews. Focus on featuring clear practical examples of new feature as part of PR unittests and there will be much less reluctance to merge because it becomes obvious what that huge PR actually does.

It would help a lot if each time I was sending you e-mail with snippets regarding DIP1000, you would have added them as unit-tests instead of explanations via mail.

[MartinNowak]

Is in expanded right now into scope const as has been stated in docs?

No, it's not, not even sure I can find docs for that.

[mihails-strasuns]

https://dlang.org/spec/function.html#parameters

[MartinNowak]

Thanks, dmd doesn't seem to apply scope to in parameters atm. This might actually be an oversight of this PR.

const(int)* test(in int* p)
{
    return p;
}