fix Issue 20461 - [dip1000] Passing stack allocated string to assert …

[WalterBright]

…compiles

Added an extra parameter to checkParamArgumentEscape() to make it work.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	20461	critical	[dip1000] Passing stack allocated string to assert compiles

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

To target stable perform these two steps:

1. Rebase your branch to upstream/stable:

git rebase --onto upstream/stable upstream/master

2. Change the base branch of your PR to stable

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#10947"

[Geod24]

Why would be assert be defined as escaping its argument ? assert can abort, writeln and crash, etc... Yes it can also throw, but I thought you weren't so in favor of this ?

[WalterBright]

Because it does (unfortunately). See explanation here:

Blocked by dlang/druntime#2999

Frankly what happens when assert() gets called is a ridiculous sequence of Rube Goldberg trampolines, but fixing that is beyond the scope of this patch.

[WalterBright]

Blocked by dlang/phobos#7426

[Geod24]

Fair enough. Personally not too thrilled about it (especially in the assert(0) case), as I might been using this pattern now and then. However, it should be a deprecation not to break code (as it already did in druntime and Phobos).

[WalterBright]

However, it should be a deprecation not to break code (as it already did in druntime and Phobos).

The problem is (as pointed out in bugzilla) these are unnoticed critical errors - the message will be garbage. That code does not work, and in good conscience we should not issue deprecations for it, only errors.

[Geod24]

Right, I thought it might work with assert(0), but it's only with checkaction=C, so objection withdrawn.

[Geod24]

Can you rebase ?

[WalterBright]

rebased

[12345swordy]

@thewilsonator this needs manually merge it seems

[Geod24]

In this case the CI just needed to be retriggered, the upstream bug has been fixed.