fixes #24504; fixes ensureMove for refs
#24505
Conversation
compiler/injectdestructors.nim
Outdated
| hasDestructor(n[1].typ): | ||
| result = p(n[1], c, s, normal) | ||
| else: | ||
| result = p(n[1], c, s, sinkArg) | ||
| dec c.inEnsureMove | ||
| return |
There was a problem hiding this comment.
I think the real problem here is your special case and the return. Instead the usual logic should be performed where the diagnosis intercepts c.inEnsureMove.
There was a problem hiding this comment.
Most logic of normal calls are not needed for ensureMove. The bug is that when a value is moved to a location (sinkArg), the location should own the value and ensure its destruction. (Another way is to translate ensureMove(x) into move(x) instead of =sink(x))
There was a problem hiding this comment.
So where did you get this logic from?
There was a problem hiding this comment.
Instead the usual logic should be performed where the diagnosis intercepts c.inEnsureMove.
Okay, I removed the special treatment. Though I need to pass the paramenter of ensureMove with sinkArg?
|
@narimiran this needs to be backported to every relevant version. |
|
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
fixes #24504