Revert "Merge pull request #6172 from WalterBright/fix16589"

[mihails-strasuns]

Requirement for merge of stable into master (#6287)

Reason for reverting is that trying to remove flag protection has shown it results in some false
triggers in dmd test suite, for example:

// Expected: @safe destructor 'diag7050c.B.~this' cannot call @system destructor 'diag7050c.A.~this'
// With 6172: cannot take address of parameter this in @safe function ~this
struct A
{
    ~this(){}
}

@safe struct B
{
    A a;
}

@safe void f()
{
    auto x = B.init;
}

[WalterBright]

Why is it a false trigger? It is an @System destructor being called in @safe code.

[mihails-strasuns]

It hides original (useful) error message with cannot take address of parameter this in @safe function ~this which is extremely confusing.

[mihails-strasuns]

(confusing both because no actual address is taken in the code and because it is too low level compared with original error)

[MartinNowak]

Well

[MartinNowak]

Is this the only issue you found as a reason for reverting?

[mihails-strasuns]

Is this the only issue you found as a reason for reverting?

The only one I have noticed when running DMD own test suite. I haven't checked further.

Updated with left-over merge markup removed.

[mihails-strasuns]

Green

[MartinNowak]

It smells like we could find a better (and slightly less confusing) solution than reverting it.

trying to remove flag protection

Can't really parse/understand that. We're all short of time, but remember, a bit more time spent on better expressing what you want to say, easily saves 10x or more time on understanding and pointless discussions. Already tried 3 times this week to shortly get the problem, but only today found enough time (30min.) to puzzle the pieces together.

The issue we want to solve is the following. The check to be reverted by this PR currently gets enabled by -transition=safe on master, and produces an incorrect (or confusing?) error message. On stable -transition=safe was correctly changed to deprecations, so merging the branch back will now enable the false deprecation by default, hence it needs to be fixed prior to that.
I'd say disabling the failing tests for now and merging back stable would be the best move to untangle the two tasks.

Could you please try to fix the incorrect error/warning @WalterBright, I filed an issue so we can better track it. Issue 16949 – [Reg 2.073] confusing @safe error message for fields with unsafe destructors

-transition= vs. deprecations

I guess the first step necessary is to get Walter to fully understand the argument against using -transition=safe for deprecations/errors. So far it seems it's still being used on the scope branch.
Since @Dicebot just changed it on stable without clearing the argument, new conflicts will emerge from the misunderstanding.

As argued before, the -transition switch should not be used to enable deprecations or errors. @WalterBright just created a new parallel transition processes aside from the established deprecate-then-error one. As with the -dip25 switch nobody will use them, and we later need to go through deprecate-then-error anyhow. Also hiding some parts behind a switch are a way to merge unfinished stuff into master, leading to messy releases such as with all the scope/safe related changes atm.
As @Dicebot argued the only use-case for the -transition switch should be semantic changes of code that has been valid before and after a change, a good example being the conservative allocation of closures for non-scope foreach delegates.

before change	after change	transition process
valid	invalid	deprecate-then-error
invalid	valid	none
invalid	invalid	none
valid	valid	-transition=feature to find semantic changes

If disputable, we should discuss this in more detail on D.internals and/or in a call.

[mihails-strasuns]

I have started a http://forum.dlang.org/post/tiasmffqbwmqnejckemz@forum.dlang.org thread - that addresses both of mentioned issues (reverting and flag choices). I assumed the matter is obvious after many previous explanations but it all has likely been lost in the noise.

[MartinNowak]

I assumed the matter is obvious after many previous explanations but it all has likely been lost in the noise.

My experience is that things have to be told multiple (~10x) times, and at least once need to be stated/clarified explicitly. Assumptions about what people pick up from the amount of email, github, and newsgroup information are often not correct and a common source for misunderstandings.

[mihails-strasuns]

Closing as hopeless