fix: prohibit capturing variables with destructors in closures (fixes #21497 #18704)

[gorsing]

This fixes issue #21497 by making it a compile-time error to capture variables with destructors in closures. The current behavior is inconsistent and leads to double destruction or use-after-free. Until closures can safely support such captures, this strict
prohibition prevents undefined behavior.

update: #21497 has been officially closed and marked as a duplicate/sub-issue of #18704.

[dlang-bot]

Thanks for your pull request and interest in making D better, @gorsing! 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: and Returns:)

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

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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 "master + dmd#21514"

[gorsing]

Added a set of new tests covering closure capture restrictions introduced in this PR (dmd#21514):

Positive tests:

• compilable/closure_no_dtor.d: Capturing a struct without destructor is allowed.
• compilable/closure_class_no_dtor.d: Capturing a class without destructor is allowed.
• compilable/closure_local_struct_no_dtor.d: Capturing a local struct without destructor is allowed.
• compilable/closure_this_no_dtor.d: Capturing this of a struct without destructor is allowed.
• compilable/closure_nested_func_no_dtor.d: Capturing a variable without destructor inside a nested function is allowed.

Negative tests:

• fail_compilation/closure_with_dtor.d: Capturing a struct with destructor is forbidden.
• fail_compilation/closure_class_dtor.d: Capturing a class with destructor is forbidden.
• fail_compilation/closure_local_struct_dtor.d: Capturing a local struct with destructor is forbidden.
• fail_compilation/closure_this_dtor.d: Capturing this of a struct with destructor is forbidden.
• fail_compilation/closure_nested_func_dtor.d: Capturing a variable with destructor inside a nested function is forbidden.

[gorsing]

Please note that this patch reflects my personal initiative and subjective view as an enthusiast of the D language.
I fully understand that opinions within the community may differ on whether such stricter compiler checks are desirable or appropriate at this stage.

I do not claim this is the only correct approach, but I believe surfacing hidden issues early — even at the cost of stricter compilation rules — contributes to the long-term stability and predictability of the language.

I welcome feedback and alternative suggestions to address the problem described in #21497.

[rikkimax]

Strictly speaking, there is nothing wrong with a closure capturing a variable that has a destructor and is allocated on the stack.
It is possible for the delegate, and a called function to make use of one of these variables safely.

Any issues that I can come up with right now, stem from the lifetime of the variable not being respected. We attempted to solve this with DIP1000, and I am in the process of attempting a replacement DFA + language feature solution.

As this is a very marginal at best solution to a much more complex set of problems and will break user code, I suggest it would be best that we do not go in this direction, and await to see if my DFA approach will pay off. If not, then we can revisit it with editions where breaking changes like this would be allowed.

[ibuclaw]

I think this is not the direction that should be taken. Rather, investigate whether destruction can be propagated when captures escape.

1. If the intent is to be RAII as per structs, then as D delegates are polymorphic (the .ptr interchangeably reference any closure object, class, or struct), my suspicion is any basic var tracking will break down quickly.

Perhaps delegate ptr could be changed to point to a capture object

struct capture
{
    void* context;
    void function() xdtor;
    int refcount;
    // any other metadata...
}

Delegates will require copy/move construction to interface with this type to ensure xdtor is called at the correct time.

2. Because closures are GC allocated. Things can simplified greatly if we just accept eventual destruction as is the case with classes or struct pointers.

Closure types go from POD to

struct closure
{
    ~this();  // __dtor(&field1), etc...
    // any captured fields
}

All that's required is for each closure frame to get its own autogenerated dtor, and replace the GC allocation _d_allocmemory(closure.sizeof) with _d_newitemT(typeid(closure)). The GC takes control of calling dtor when no live references exist.

@rikkimax does this make sense?

[ibuclaw]

FAOD, in all these examples, none of the closures escape, so calling dtor on s at the end of scope is correct behaviour and should remain valid.

[rikkimax]

I think this is not the direction that should be taken. Rather, investigate whether destruction can be propagated when captures escape.

1. If the intent is to be RAII as per structs, then as D delegates are polymorphic (the .ptr interchangeably reference any closure object, class, or struct), my suspicion is any basic var tracking will break down quickly.

Perhaps delegate ptr could be changed to point to a capture object

struct capture
{
    void* context;
    void function() xdtor;
    int refcount;
    // any other metadata...
}

Delegates will require copy/move construction to interface with this type to ensure xdtor is called at the correct time.

2. Because closures are GC allocated. Things can simplified greatly if we just accept eventual destruction as is the case with classes or struct pointers.

Closure types go from POD to

struct closure
{
    ~this();  // __dtor(&field1), etc...
    // any captured fields
}

All that's required is for each closure frame to get its own autogenerated dtor, and replace the GC allocation _d_allocmemory(closure.sizeof) with _d_newitemT(typeid(closure)). The GC takes control of calling dtor when no live references exist.

@rikkimax does this make sense?

It does, however building an actual struct is the slow way to do it. That would require running semantic analysis and generating a bunch of symbols that are not required.

Right now a closure, does create a struct as far as the backend is concerned.
But this isn't a real struct, the frontend never sees this.
As a result it never triggers a TypeInfo to generate.

dmd/compiler/src/dmd/toir.d

Line 786 in bf25346

void buildClosure(FuncDeclaration fd, ref IRState irs)

Unfortunately the closure is also appended to, to handle structs that have an alignment size larger than the stack's.

dmd/compiler/src/dmd/toir.d

Line 1050 in bf25346

void buildAlignSection(FuncDeclaration fd, ref IRState irs)

As for the cleanup function, it wouldn't be a struct's __dtor, it would be __xdtor calling other __xdtor or __dtor depending upon what exists.

If it weren't for alignment, I'd suggest templating _d_allocmemory with the types and pass in the closure pointer as a context.
However since that isn't possible, its going to have to take a TypeInfo instead.
The compiler hook itself doesn't appear to be used in dmd for anything other than this, so it should be possible to remove it on dmd's side and replace it with a purpose specific hook.

The TypeInfo being generated for a closure should probably be closure specific, since all it needs is the destructor function specified.

There are some dmd specific flags for closure + alignment addition. Perhaps these can be abstracted out of being dmd specific.

dmd/compiler/src/dmd/declaration.d

Line 835 in bf25346

version (MARS)

My biggest concern isn't so much the closure cleanup, its the finally statement. I would prefer it to be rewritten out if it isn't being used. But I am worried that the information to do that won't be available when its required. Worse case scenario, a flag on the finally statement to say the final body is disabled would be good enough. Without this frontend dataflow analysis is going to have to know to check for it and then go check for it which is a lot to ask.

[rikkimax]

Now that I'm thinking of it, the code to handle finally statements rewrite should be pretty easy to implement.

dmd/compiler/src/dmd/statementsem.d

Line 3428 in 3d06a91

if ((blockexit & ~BE.halt) == BE.fallthru)

if (auto de = tfs.finalbody.isDtorExpStatement)
{
    if (de.var.inClosure || de.var.inAlignSection)
    {
            result = tfs._body;
            return;
    }
}

I should've known this is where it would've gone, I've had that link in my notes for like a week for exception handling (meeting on the 25th) lol.

[ibuclaw]

Can you put this in a relevant issue instead?

[rikkimax]

Ok, done. #18704

[gorsing]

Another important point is that heap allocation through closures often happens unconsciously. Since closures in D automatically allocate memory on the heap as soon as they capture variables from an outer context, newcomers can easily write inefficient code without realizing that they are:

• making hidden heap allocations;
• losing control over object lifetimes;
• causing additional garbage collections;
• violating RAII principles and reducing predictability.

By preventing or limiting the capture of variables with destructors, we can also help new users avoid unintentionally switching from value semantics to GC semantics. This reduces cognitive load and lowers the risk of "shooting themselves in the foot."

From this perspective, prohibiting or warning about such captures is not only a safeguard against undefined behavior, but also a way to improve the transparency and efficiency of code, especially in performance-critical sections.

[gorsing]

I understand that a hard ban may be too radical. As a compromise, I suggest considering the following options:

• Adding a -preview=closureNoDtor flag to allow the community to test this behavior without impacting existing code;
• Issuing a warning instead of an error, to explicitly highlight potential problems without breaking compilation;
• Possibly extracting the idea into a separate DIP, in order to explore alternative implementation strategies—such as stricter compile-time checks, lifetime tracking, or introducing capture structures.