Proposal Details
This proposal is to extend x/tools/go/ssa to support for defers when running with GOEXPERIMENT=rangefunc.
This adds a new Into field to Defer, a new ssa builtin function ssa:defers(), and new opaque type deferStack.
ssa:defers() returns a reference to the current function's defer stack when are executed by RunDefers. ssa:defers is a Builtin and the return type is a pointer to a deferStack. When Defer.Into != nil, it is a pointer to a deferStack and is pushed onto that defer stack.
It is worth comparing these to the new runtime intrinsics to implement defer with range-over-func yields:
var #defers = runtime.deferrangefunc()
f(func() {
runtime.deferprocat(func() { print("A") }, #defers)
})
ssa:defers() is analogous to runtime.deferrangefunc() and Defer.Into != nil is equivalent to runtime.deferprocat.
Better suggestions for names are welcomed.
A simplification we may also wish to do is to require Defer.Into to not be equal to nil (instead of optionally nil). This might not be backwards compatible with existing ssa users on existing code. They would see ssa:defers() calls and the new *deferStack type. The proposed implementation currently only emits a value for Into for range-over-func loops.
Comment From: timothy-king
https://go.dev/cl/555075 proposed implementation of GOEXPERIMENT=rangefunc for x/tools/go/ssa.
Comment From: adonovan
I agree that this is needed, and I agree it is cleaner to require the Defer{Into} field in all cases rather than make it implicitly "the current defer stack" when omitted.
Let's use the same name for both fields:
package ssa
type Defer struct {
+ Stack Value // stack (from ssa:deferstack() intrinsic) onto which this function is pushed
...
}
type RunDefers struct {
+ Stack Value // stack (from ssa:deferstack() intrinsic) of functions to call in pop order
...
}
(Aside: it would be nice if we could avoid the double indirection of vars plumbed through closures when they are never assigned from within the closure; in the case of deferstacks we know a priori that this is the case.)
Comment From: timothy-king
Another option is for ssa:deferstack() to be a new ssa Instruction, Stack. The new Instruction would take no arguments and produce a value referring to the stack frame of the current function. The type of the value would be a pointer to an opaque stack type. A new Instruction type would not be backwards compatible with type switches that attempt to be complete over Instructions.
If we start to require the Stack field in Defer and RunDefers, my preference would be to have Stack over ssa:deferstack(). It is marginally easier to deal with Instructions than calls to Builtins and we will expect to have a lot more of these in preexisting code. If we leave the Stack field optional, I do not have a strong preference between a new Builtin and a new Instruction.
Comment From: timothy-king
(cc @dominikh)
Comment From: adonovan
There's no need for a new instruction, so let's not add one. As we've seen from the types.Alias work, breaking existing switches is a very expensive business. Adding new fields (even required ones) to existing instructions is easy, as there is only one algorithm that creates instructions (the builder).
Comment From: gopherbot
Change https://go.dev/cl/555075 mentions this issue: go/ssa: support range-over-func
Comment From: rsc
The proposal is to add
package ssa
type Defer struct {
+ Stack Value // stack (from ssa:deferstack() intrinsic) onto which this function is pushed
...
}
type RunDefers struct {
+ Stack Value // stack (from ssa:deferstack() intrinsic) of functions to call in pop order
...
}
Comment From: rsc
There is significant opportunity for confusion between the call stack and the defer stack. I think the name 'Stack' is not specific enough. It should be 'Defers' or 'DeferStack' or 'DeferList' or something like that.
Comment From: rsc
This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group
Comment From: adonovan
There is significant opportunity for confusion between the call stack and the defer stack. I think the name 'Stack' is not specific enough. It should be 'Defers' or 'DeferStack' or 'DeferList' or something like that.
Users never populate this field, they only read it, and the doc comments will make clear that it refers to the logical (opaque) stack of deferred functions. (I can't see what stack-of-calls it could be confused with.)
Comment From: rsc
None of these seem like arguments against 'DeferStack' instead of 'Stack'.
Comment From: adonovan
Ok, deferstack it is. I don't see any real need for the RunDefers.DeferStack field since its value must always be the same as would be obtained by a call to ssa:deferstack(), so let's avoid doubling the size of that instruction.
@timothy-king @rsc Are we all happy with this:?
package ssa
type Defer struct {
+ DeferStack Value // stack of deferred functions (from ssa:deferstack() intrinsic) onto which this function is pushed
...
}
Comment From: timothy-king
I'm happy. There is nothing we do not need. And what is there is straightforward to extend later if we change our minds (like a Stack instruction).
Comment From: rsc
Based on the discussion above, this proposal seems like a likely accept. — rsc for the proposal review group
The proposal is to add
type Defer struct {
+ DeferStack Value // stack of deferred functions (from ssa:deferstack() intrinsic) onto which this function is pushed
...
}
Comment From: rsc
No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group
The proposal is to add
type Defer struct {
+ DeferStack Value // stack of deferred functions (from ssa:deferstack() intrinsic) onto which this function is pushed
...
}
Comment From: gopherbot
Change https://go.dev/cl/590975 mentions this issue: go/ssa: export Defer.DeferStack field