Skip to content

Commit 3c9b135

Browse files
authored
Merge pull request #1349 from Workiva/blockingDeferReturns
Improving return with defers blocking
2 parents ebb16a4 + 6e01eab commit 3c9b135

File tree

6 files changed

+796
-102
lines changed

6 files changed

+796
-102
lines changed

compiler/internal/analysis/defer.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package analysis
2+
3+
import (
4+
"go/ast"
5+
6+
"github.com/gopherjs/gopherjs/compiler/internal/typeparams"
7+
)
8+
9+
// deferStmt represents a defer statement that is blocking or not.
10+
//
11+
// A blocking defer statement will cause a return statement to be blocking
12+
// since the defer is called and potentially blocked while leaving the method.
13+
// We try to determine which defers affect which returns so that we only
14+
// mark returns as blocking if they are affected by a blocking defer.
15+
// In general we know that a defer will affect all returns that have been
16+
// declared after the defer statement.
17+
//
18+
// Since analysis doesn't create [CFG] basic blocks for full control
19+
// flow analysis we can't easily determine several cases:
20+
//
21+
// - Terminating if-statements(i.e. does the body of the if-statement always
22+
// return from the method) are difficult to determine. Any defer that is
23+
// added whilst inside a terminating if-statement body can only affect the
24+
// returns inside that if-statement body.
25+
// Otherwise, the defer may affect returns after the if-statement block has
26+
// rejoined the flow that it branched from. Since terminating if-statements
27+
// are difficult to determine without [CFG] blocks, we treat all
28+
// if-statements as if they are not terminating.
29+
// That means there may be some false positives, since returns declared
30+
// after a terminating branch will be marked as affected by a defer
31+
// declared in that branch, when in reality they are not.
32+
//
33+
// - Same as above but for else blocks, switch cases, and any branching.
34+
//
35+
// - Loops (i.e. for-statements and for-range-statements) can cause return
36+
// statements declared earlier in the loop to be affected by defers
37+
// declared after it in the loop. We can't determine which branches in a
38+
// loop may return to the start of the loop so we assume anywhere inside
39+
// of a loop can return to the start of the loop.
40+
// To handle this, all defers defined anywhere within a loop are assumed
41+
// to affect any return also defined in that loop.
42+
// We only need to track the top-level loop since nested loops will be
43+
// superseded by the top-level loop.
44+
//
45+
// - Labels and goto's are similar to loops in [CFG] blocks but without those
46+
// blocks it's harder to determine which defers will affect which returns.
47+
// To be safe, for any function with any blocking defers, returns, and
48+
// goto's, all the returns are defaulted to blocking.
49+
//
50+
// [CFG]: https://en.wikipedia.org/wiki/Control-flow_graph
51+
type deferStmt struct {
52+
inst *typeparams.Instance
53+
lit *ast.FuncLit
54+
}
55+
56+
// newBlockingDefer creates a new defer statement that is blocking.
57+
//
58+
// If the defer is calling a js.Object method then the defer is non-blocking.
59+
// If the defers calling an interface method or function pointer in a var
60+
// then the defer is blocking.
61+
func newBlockingDefer() *deferStmt {
62+
return &deferStmt{}
63+
}
64+
65+
// newInstDefer creates a new defer statement for an instances of a method.
66+
// The instance is used to look up the blocking information later.
67+
func newInstDefer(inst typeparams.Instance) *deferStmt {
68+
return &deferStmt{inst: &inst}
69+
}
70+
71+
// newLitDefer creates a new defer statement for a function literal.
72+
// The literal is used to look up the blocking information later.
73+
func newLitDefer(lit *ast.FuncLit) *deferStmt {
74+
return &deferStmt{lit: lit}
75+
}
76+
77+
// IsBlocking determines if the defer statement is blocking or not.
78+
func (d *deferStmt) IsBlocking(info *Info) bool {
79+
// If the instance or the literal is set then we can look up the blocking,
80+
// otherwise assume blocking because otherwise the defer wouldn't
81+
// have been recorded.
82+
if d.inst != nil {
83+
return info.FuncInfo(*d.inst).IsBlocking()
84+
}
85+
if d.lit != nil {
86+
return info.FuncLitInfo(d.lit).IsBlocking()
87+
}
88+
return true
89+
}
90+
91+
func isAnyDeferBlocking(deferStmts []*deferStmt, info *Info) bool {
92+
for _, def := range deferStmts {
93+
if def.IsBlocking(info) {
94+
return true
95+
}
96+
}
97+
return false
98+
}

0 commit comments

Comments
 (0)