Bug 14264 – Destructor not called when struct is returned from a parenthesis-less function call

struct Foo { ~this() { import std.stdio; writeln("I am destroyed"); } } Foo makeFoo() { return Foo(); } void main() { makeFoo(); // I am destroyed makeFoo; // -> destructor is not called }

this is due to inconsistency in `ExpStatement::semantic()`. the code is: exp = exp->semantic(sc); // (1) exp = exp->addDtorHook(sc); // (2) exp = resolveProperties(sc, exp); // (3) for "bracket call" `exp` in (1) CallExp, and `addDtorHook` is correctly adds temporary to store `Foo` and destroy it later (actually, rewrites call as comma expression). but for "bracketless call" `exp` in (1) is `IdentifierExp`, which is changed to `VarExp` by semantic analyzer. so in (2) we have simply `VarExp`, which, obviously, not a call and doesn't need any destructors to be added. and later `resolveProperties` see that our `VarExp` is really a call, and returns `CallExp`, but alas, it's too late to add dtors. but i'm still not Kenji ;-), so i don't know if it's ok to just call `resolveProperties` before adding destructor hook. i bet it is wrong though, 'cause `resolveProperties` is a complex thing and i still have to found what it really does. there are no helpful comments in DMD source about it. :-(

also, raising this to critical, as it can easily kill any RC-counting scheme.

Yes, it was discovered in RC code. I agree it's kind of critical as RC is readily compromised if you happen to like optional parens. Yet the workaround is quite simple.

DMD hackfix is around three lines too (i did that in my branch), but it's a dirty hack that shouldn't be in the codebase at all. ;-) actually, there are some other places in DMD where such code is used, but it seems that this is the only place where bug can be triggered, as the other cases triggers errors anyway (such as `if (makeFoo)`, for example). but i'm still not sure if moving `addDtorHook()` to `resolveProperties()` is the right thing to do. probably not, as `resolveProperties()` seems to be used for CTFE too, and CTFE doesn't do `addDtorHook()` call. i dreaming of good guide to compiler internals...

(In reply to Ketmar Dark from comment #4) > DMD hackfix is around three lines too (i did that in my branch), but it's a > dirty hack that shouldn't be in the codebase at all. ;-) > > actually, there are some other places in DMD where such code is used, but it > seems that this is the only place where bug can be triggered, as the other > cases triggers errors anyway (such as `if (makeFoo)`, for example). > > but i'm still not sure if moving `addDtorHook()` to `resolveProperties()` is > the right thing to do. probably not, as `resolveProperties()` seems to be > used for CTFE too, and CTFE doesn't do `addDtorHook()` call. > > i dreaming of good guide to compiler internals... resolvePropeties() converts a sort of function expression (VarExp with FuncDeclaration, DotVarExp with FuncDeclaration, OverExp, etc) to a CallExp if possible. addDtorHook() copies the given expression to a temporary to call destructor, if the expression type is struct. Those functions are used only in the semantic analysis stage. After the semantic analysis finished, CTFE engine will interpret the generated AST. In this case, addDtorHook() should take the result of resolveProperties(). The wrong call order has been there before the my first contribution for dmd codebase. https://github.com/D-Programming-Language/dmd/pull/4474

> The wrong call order has been there before the my first contribution for dmd > codebase. to be clear: i didn't mean that this was the code you wrote. what i mean is that you have excellent and very deep understanding of compiler code, so you can answer on such simple questions as mine in no time. so "i'm not Kenji" means that "i don't have Kenji's understanding of the codebase". ;-) sorry if it looks like i'm blaiming you. it was the exactly opposite, i was praising your knowledge and contributions. ;-)

(In reply to Ketmar Dark from comment #6) > > The wrong call order has been there before the my first contribution for dmd > > codebase. > > to be clear: i didn't mean that this was the code you wrote. what i mean is > that you have excellent and very deep understanding of compiler code, so you > can answer on such simple questions as mine in no time. > > so "i'm not Kenji" means that "i don't have Kenji's understanding of the > codebase". ;-) > > sorry if it looks like i'm blaiming you. it was the exactly opposite, i was > praising your knowledge and contributions. ;-) Don't worry, I'm precisely understanding your intention of the comment. I meant that I didn't notice the bug until now, and I'm not perfect. If you consider to start contribution to the dmd codebase for the bugfix, It's very glad to us.

(In reply to Kenji Hara from comment #7) > If you consider to start contribution to the dmd codebase for the bugfix, > It's very glad to us. i'm slowly working on my "aliced" patchset, developing deeper understanding of codebase on my way. when i see the bug in DMD, it affects both DMD and Aliced, so i'm trying to at least localise it. writing my findings here, by the way, greatly helps, as usually someone more knowledgeable than me tells me which of my assumptions were correct and which were wrong. thank you and all other people for that. sadly, i don't want to use github, so i can't actively contribute. but i like to read your patches (and i usually incorporating most of them into Aliced almost immediately ;-), sometimes even a simple fix makes "aha! so *that* is the way it supposed to work!"

Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/753d6c9a19cf342d59ebae1c98eaa225d7c5f1ad fix Issue 14264 - Destructor not called when struct is returned from a parenthesis-less function call https://github.com/D-Programming-Language/dmd/commit/f5ca6cfc53e5c7a189bf05b06b4129fefa3bf617 Merge pull request #4474 from 9rnsr/fix14264 Issue 14264 - Destructor not called when struct is returned from a parenthesis-less function call

Commits pushed to stable at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/753d6c9a19cf342d59ebae1c98eaa225d7c5f1ad fix Issue 14264 - Destructor not called when struct is returned from a parenthesis-less function call https://github.com/D-Programming-Language/dmd/commit/f5ca6cfc53e5c7a189bf05b06b4129fefa3bf617 Merge pull request #4474 from 9rnsr/fix14264