Bug 24838 – A closure with a layout of pointer size or below that is not modified, should not have a closure

error : function `MyClass.myMethod` is `@nogc` yet allocates closure for `myMethod()` with the GC delegate `MyClass.myMethod.__lambda3` closes over variable `this` class MyClass { void doSomething(); void myMethod() @nogc { acceptsCallback(&notLambda); // this works; delegate is created from the method acceptsCallback((){ doSomething(); }); // doesn't work! closure needlessly tries to allocate } void notLambda() { doSomething(); } } I think this is essentially a bug and a serious usability hindrance. The lambda seems to try and allocate a closure because it captures `this`, but that's no reason to allocate a closure! A function that receives `this` as context is called a method... in the extremely common case where a lambda closes `this` and nothing else, the lambda should just be synthesised as a method of `this` rather than a closure. The double-indirection from the closure is pointless and inefficient anyway. This would make lambda's about 100x more useful in @nogc code, because they can naturally access `this` as context without any closure allocation.

This is a misunderstanding of what ``@nogc`` is. It is a linting tool. It guarantees that no GC allocation will take place within the function. The reason why the compiler is not putting the closure on the stack is because it could escape the call stack. Mark the delegate parameter in ``acceptsCallback`` as ``scope`` and this will work. Without the escaping guarantees, using the GC is the correct solution in this situation, and therefore should error.

Please think that through.

This is independent of @nogc. Code not marked @nogc would also benefit from this.

Correct, all code will benefit. I just made the point because it's just particularly important for @nogc code, where it is currently impossible to implement event-based systems conveniently.

Lowering could be something like --- struct Context { T* ref_capture; //normal capture } void Closure(Context* ctx) { bool oneref=false; //change this T* local_ref; if(oneref)local_ref=cast(T*)ctx; else local_ref=ctx.ref_capture; //call local_ref.method(); } --- If one reference is captured, `oneref` variable could be changed(?) accordingly, and argument would be different, then backend will optimize it.

This is a valid enhancement request.

Let's rewrite: class MyClass { void doSomething(); void myMethod() @nogc { acceptsCallback((){ doSomething(); }); } } to make visible what is happening. class MyClass { void doSomething(MyClass this); void myMethod(MyClass this) @nogc { auto outer = &this; void func(MyClass* outer) { doSomething(*outer); } acceptsCallback(&outer.func); } } func is a member of myMethod, not a member of MyClass. func's "this" pointer is a reference to the stack frame of myMethod, not a reference to MyClass. acceptsCallback() is allowed to squirrel away the address of func(), and could access it after myMethod() has exited. This will result in corrupted memory. The solution is for acceptsCallback to annotate its parameter with `scope`, which says it will not preserve its argument past the lifetime of acceptsCallback.

I think you've completely missed the point... Yes, me code DOES squirrel away the address of func, that is literally the point of this bug report. The point is, the lambda doesn't access anything at all from the closure except 'this', and so rather than creating a closure like: struct Closure { MyClass this; } void func(Closure* closure) { closure.this.doSomething(); } The closure is pointlessly allocated; there's no reason to synthesise the function this way. If the Closure ONLY references `MyClass this`, then func has no reason to allocate a closure and bind it to the delegate; it could just synthesise this function: func(MyClass this) { this.doSomething(); } This bypasses the closure since it has no reason to exist, and just places `this` into the delegate, and so for all intents and purposes, the lambda is just a normal method. This optimisation is similarly valid for any closure which closes over exactly one pointer. It could even further be generalised such that any closure which closes over exactly a single size_t-sized POD just write that value to the 'this' pointer of the delegate (essentially small-struct optimisation embedded in the delegate), and then the lambda naturally receives it as first argument when the delegate it called. There's a quite broad category of optimisation here; it's inefficient and pointless in most cases for a closure to be allocated with just one member... and it also has the nice advantage that in cases where a delegate has only a single reference (it, 'this'), then it doesn't need to allocate a closure at all, and so it's naturally `@nogc`.

Okay now it starts to make sense what you are wanting. If the closure's layout is a pointer in size or less you do not want it to exist. It does require that this value is not modified, although what a pointer points to may be modified (head const). That leads me to the question for Walter, how much in the frontend needs to change for this optimization to be implemented by ldc/gdc? If it does not need to be changed, then this should be viewed as a ldc/gdc bug not a dmd one (which you shouldn't be using when optimizations are needed).

I promise you it made sense from the start ;)

This is a language thing; GDC/LDC have nothing to say about this. The frontend implements this, it's semantic to detect if a closure only contains a single thing and then delete it. Yes, you touched on one issue with mutability of references to stack objects; however `this` is not mutable, it's impossible for `this` to change during the lifetime of the closure, which is why my initial request applies to `this`... any expansion of this optimisation is theoretical, but the case for `this`, which is overwhelmingly the most common and the most useful case should be addressed initially. Also, why did you rename my issue? :/

I renamed it to translate it into compiler speak. The ``@nogc`` aspect has nothing to do with the enhancement. As for ldc/gdc, yes it is a them problem. Closure creation is part of the glue code, not the frontend itself. https://github.com/dlang/dmd/blob/b70e66033c7f53b3ac7def31e49d803ff87d6d30/compiler/src/dmd/toir.d#L779

There is no closure creation when this case is detected. @nogc does have something to do with the enhancement, because the enhancement will make code that doesn't currently work in @nogc work after the fix... and that's specifically why I need this fixed.

Created attachment 1923 Very crude patch, just to pin point the relevant code path. I made a whack a mole (literally) attempt on this issue. The code works on the provided code and closure tests in dmd "test suite" (to best of my knowledge how to run them). I think it is very fragile, and never got to do much more with it, but better to let it rot here than on my drive.

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/20540 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB