-
-
Notifications
You must be signed in to change notification settings - Fork 606
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 20956 - [DIP1000] @safe defeated by closure capturing ref p… #14364
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request, @WalterBright! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
|
DG forwardDg(ref int c) | ||
{ | ||
return () {assert(c == 42);}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the compiler infer the closure to be scope
in this case ?
Regardless if it does/can infer it currently, we should not have this error for scope
delegate, should we ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ref
parameters are implicitly pointers, and are automatically not allowed to escape the function. No need for scope
.
The trouble here is the delegate escapes the function, thereby escaping the reference to c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, we're returning the delegate
. Can we add a test that not escaping the delegate (the delegate being scope
) does not trigger error ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error is triggered when the delegate is tested to see if a dynamic closure is being created. So the tests for @nogc already do this.
Blocked by dlang/phobos#8530 |
1756948
to
0bbc580
Compare
I guess the test suite is going to drip-drip-drip the phobos failings one at a time. |
requires refactoring of other code to implement suggestion
Blocked by bugs in Buildkite projects. |
1b1c31d
to
36b0a22
Compare
36b0a22
to
e853c99
Compare
…arameter
Very similar to fix for #14363