dmd
dmd copied to clipboard
dlang
fix Issue 14251 - synchronized (mtx) doesn't check attributes (pure, …
Thanks for your pull request, @WalterBright!
Bugzilla references
| Auto-close | Bugzilla | Severity | Description |
|---|---|---|---|
| ✓ | 14251 | normal | synchronized (mtx) doesn't check attributes (pure, const) |
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#6092"
@WalterBright, thanks for your PR! By analyzing the annotation information on this pull request, we identified @yebblies, @9rnsr and @Geod24 to be potential reviewers. @yebblies: The PR was automatically assigned to you, please reassign it if you were identified mistakenly.
(The DLang Bot is under development. If you experience any issues, please open an issue at its repo.)
Hmm, I'm not sure this is even a bug. See the bugzilla entry for discussion.
I don't think the code breakage (at least for pure) is the best way forward. We had a nicer solution to the problem by lowering synchronized statements to allow inference, see https://github.com/MartinNowak/dmd/commit/b80ed04d1c7fa60aaa8d252a521fd3199dc98234 and https://github.com/MartinNowak/druntime/commits/synchronizedLowering. This was implemented after we had to revert the synchronized nothrow requirement which didn't work w/ vibe.d. In vibe.d (and other event loops) synchronizing is asynchronous and every async yield point can throw pending exceptions. https://github.com/dlang/dmd/pull/4459
Leaving the attributes to inference, supporting direct lock/unlock calls instead of only Object's monitor, and also allow to synchronize on structs (w/ lock/unlock method) are enough arguments to make this switch. One problem where this got stuck is that the current synchronized implementation never checks the attributes. Changing this was breaking some code, though we can easily fix core.sync. Not sure if we could find a friendlier deprecation path for the attribute change.
Emperor's new groove: lower synchonized to a nice template and leave the rest to library code.
