fix Issue 21324 - @live not detecting overwrite of Owner without disp… #11881
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#11881" |
WalterBright
force-pushed
the
fix21324
branch
2 times, most recently
from
145da4c
to
add25ec
Compare
| @@ -1976,7 +1976,12 @@ void checkObErrors(ref ObState obstate) | |||
| else if (isReadonlyPtr(v)) | |||
| pvs.state = PtrState.Readonly; | |||
| else | |||
| { | |||
| if (pvs.state == PtrState.Owner && v.type.hasPointersToMutableFields()) | |||
| v.error(e.loc, "assigning to Owner without disposing of owned value"); | |||
There was a problem hiding this comment.
This error message could be improved, how about
is assigned to Owner %s (which?) without disposing of owned value %s (what?)
There was a problem hiding this comment.
Also, a supplemental error, hinting at how we can workaround this error might be helpful.
| @@ -17,6 +17,7 @@ fail_compilation/fob1.d(104): Error: variable `fob1.foo1.p` is returned but is U | |||
|
|
|||
| /* TEST_OUTPUT: | |||
| --- | |||
| fail_compilation/fob1.d(204): Error: variable `fob1.foo2.p` assigning to Owner without disposing of owned value | |||
There was a problem hiding this comment.
It's weird that a null pointer has to be disposed. Is this really what we want?
There was a problem hiding this comment.
It's also weird that a null pointer can be left dangling, but currently null is seen equally as any other pointer.
|
@WalterBright SITREP? |
There was a problem hiding this comment.
@WalterBright There are a couple of points that the reviewers raised that you have not addressed:
- Improving the error message and offering information on how to fix the code
- Do we really need to error on a null pointer?
…osing of previous owned value
|
I did actually implement a kind of X was invalidated at Y style error message for @LiVe but I can't remember what I did with the code |
…osing of previous owned value