Fix Issue 16678 - [REG] Fix for issue 16193 creates major breakage

[mihails-strasuns]

https://issues.dlang.org/show_bug.cgi?id=16678

Fix for the original issue probably needs to be redone in more convoluted fashion to prevent this kind of breakage.

[dlang-bot]

Fix	Bugzilla	Description
✓	16193	opApply() doesn't heap allocate closure
✓	16678	[REG] Fix for issue 16193 creates major breakage

[mihails-strasuns]

FYI @MartinNowak @WalterBright

[WalterBright]

This safety fix was enabled only when -transition=safe is thrown. Why is it a regression?

[WalterBright]

Please edit the subject to be:

Fix Issue 16678 - [REG] Fix for issue 16193 creates major breakage

This is the standard form for any bugzilla PRs.

[mihails-strasuns]

This safety fix was enabled only when -transition=safe is thrown

It is not, I have changed that in #6183 (which you have reviewed and merged ;)). With released DMD only thing it does it to print message like sample.d(20): To enforce @safe compiler allocates a closure unless the opApply() uses 'scope'. And this commit removes any traces of -transition=safe from the released DMD completely.

Let me repeat previously discussed points:

• -transition switches must not introduce semantic changes
• -transition switches must not be used to hide bug fixes
• we will need to modify your return scope implementation to NOT use -transition=safe before it can be merged to master, that is critical requirement

Getting back to this regression, my original reasoning of adjusting your fix was that closure allocation does not break builds and those concerned about performance can hunt it with -transition=safe help. However I have missed the fact that turning delegate into closure can actually break code if there are structs with destructors captured.

This means that your original bug fix has to be redone against master in a more subtle way (as I feared).

[WalterBright]

What do you suggest? Making it a deprecation?

As for merging problems, that is an inevitable result of making a feature branch.

[mihails-strasuns]

What do you suggest? Making it a deprecation?

This PR reverts it completely so that we can finally ship working release and stop scarying away users. After that we can discuss how/if deprecation can be implemented in a re-implementation PR targeting master branch.

As for merging problems, that is an inevitable result of making a feature branch.

I haven't mentioned any merging problems at all (there are none).

[WalterBright]

Since this is only being merged into stable, and not master, I agree.