dmd
dmd copied to clipboard
dlang
Fix Issue 23964 - [REG2.102] inccorect error opAssign cannot be used …
Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:
- My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
- My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
- I have provided a detailed rationale explaining my changes
- New or modified functions have Ddoc comments (with
Params:andReturns:)
Please see CONTRIBUTING.md for more information.
If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.
Bugzilla references
| Auto-close | Bugzilla | Severity | Description |
|---|---|---|---|
| ✓ | 23964 | regression | [REG2.102] inccorect error opAssign cannot be used ... @disable |
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#15332"
I'm not an expert here, but it feels like a brittle fix. (do we still have a bug with shared? pure? or some other attribute instead of @system?)
I'm not an expert here, but it feels like a brittle fix. (do we still have a bug with shared? pure? or some other attribute instead of @system?)
@JohanEngelen this is not a brittle fix. The fundamental reason why your code is failing is because the compiler is trying to be smart when generating opAssign. It does not infer attributes, it simply looks at what functions the generated opAssign is going to call (opAssigns, postblits, destructors); it then does a conjunction of the attributes of those functions and obtains the attribute set. This is fine, because all other operations are going to be simple blit copies. However, once system variables were introduced this strategy was no longer bulletproof since a simple variable access can be unsafe. Other attributes are not affected. My fix is to simply take this into account and make the generated opAssign @system if there are any @system fields in the struct. This is the correct path, however, I am not happy with the patch because this is something that should be done during semantic1, however, that's not possible because system variables are marked as such during semantic2. I don't really understand why system variables are not marked during semantic1 (@dkorpel should have more insight on this), but that is besides the point.
I don't really understand why system variables are not marked during semantic1
Inferring @system from an initializer happens in semantic2, because then variable initializers are analyzed. semantic1 only looks at the type and name.
@dkorpel As far as I could tell, dsymbolsem.dsymbolSemantic on a VarDeclaration also analyzes the initializer.
This bit me again today. It would already help if better / more diagnostic can be given about why opAssign is disabled (possibly even harder to implement than the fix here)...
note: no deprecation message is seen, so the user has no clue why -de triggers the bug (actually, it might take many hours to discover that the bug disappears without -de)