Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Issue 22023 - adding to escaped argument of a variadic defeats @… #13710

Merged
merged 1 commit into from
Feb 23, 2022

Conversation

WalterBright
Copy link
Member

…safe

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
22023 major adding `return` to escaped argument of a variadic defeats @safe

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

@WalterBright
Copy link
Member Author

Fixing this uncovered a related issue:

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

@dkorpel
Copy link
Contributor

dkorpel commented Feb 23, 2022

Shouldn't this start as a deprecation?

@Geod24
Copy link
Member

Geod24 commented Feb 23, 2022

Aren't typesafe variadic inherently scope then ? They don't seem to be marked as such.

@WalterBright
Copy link
Member Author

Aren't typesafe variadic inherently scope then ?

Only arrays and classes. See https://issues.dlang.org/show_bug.cgi?id=22818

@WalterBright
Copy link
Member Author

Shouldn't this start as a deprecation?

Why would fixing a sure-fire crash require a deprecation?

@dkorpel
Copy link
Contributor

dkorpel commented Feb 23, 2022

Why would fixing a sure-fire crash require a deprecation?

It's not a sure-fire crash, someone might add return to a variadic array mistakenly thinking it would apply to the array elements. Or they did return a pointer to the array, but it was not stomped by new stack allocations before use.

Personally I don't mind making this an error directly, I only ask because you recently mentioned that people expect no breakage when not using preview switches.

@WalterBright
Copy link
Member Author

@dkorpel you could apply that reasoning to every bug fix. This is why we leaven our rules with good judgement, and do not apply them absolutely.

@WalterBright WalterBright merged commit 871eaeb into dlang:master Feb 23, 2022
@WalterBright WalterBright deleted the fix22023 branch February 23, 2022 22:26
@RazvanN7
Copy link
Contributor

This PR has introduced a regression: https://issues.dlang.org/show_bug.cgi?id=23244 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants