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 21880 - [REG 2.095] scope variable assigned to non-scope parameter calling function #12493

Merged
merged 1 commit into from
May 3, 2021

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented May 1, 2021

Propagate scope from the parameter to the temporary that is being created for it.

However... it does seem suspect that a temporary is being created for a type that doesn't need it, just because another parameter needs destruction.

The inverse of this fix would be to constrain needsPrefix to not be true for trivial/pod types, or scope parameters.

@ibuclaw ibuclaw requested a review from Geod24 May 1, 2021 18:24
@dlang-bot
Copy link
Contributor

dlang-bot commented May 1, 2021

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
21880 regression [REG 2.095] scope variable assigned to non-scope parameter calling function

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 "stable + dmd#12493"

@kinke
Copy link
Contributor

kinke commented May 1, 2021

However... it does seem suspect that a temporary is being created for a type that doesn't need it, just because another parameter needs destruction.

It's about strict left-to-right evaluation order of arguments. foo(makeTemporaryWithDtor(), makePODWithSideEffects(), mightThrow(), 123) => makePODWithSideEffects() must be evaluated as 2nd expression.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 2, 2021

However... it does seem suspect that a temporary is being created for a type that doesn't need it, just because another parameter needs destruction.

It's about strict left-to-right evaluation order of arguments. foo(makeTemporaryWithDtor(), makePODWithSideEffects(), mightThrow(), 123) => makePODWithSideEffects() must be evaluated as 2nd expression.

An argument with side effects is justifiable, a pointer or string less so.

@ibuclaw
Copy link
Member Author

ibuclaw commented May 2, 2021

Oops, I committed on top of stable, but for some reason this PR is targeting master.

@ibuclaw ibuclaw changed the base branch from master to stable May 2, 2021 07:50
@Geod24 Geod24 merged commit 4ec6b7e into dlang:stable May 3, 2021
@ibuclaw ibuclaw deleted the issue21880 branch May 3, 2021 06:59
@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 26, 2021

This caused a regression. :-(
https://issues.dlang.org/show_bug.cgi?id=22084

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.

4 participants