From f4e6815652cf3c4803001dd9ab7f0d7f125a466c Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 16 Aug 2023 21:16:29 -0700 Subject: [PATCH] cmd/compile/internal/noder: remove inlined closure naming hack I previously used a clumsy hack to copy Closgen back and forth while inlining, to handle when an inlined function contains closures, which need to each be uniquely numbered. The real solution was to name the closures using r.inlCaller, rather than r.curfn. This CL adds a helper method to do exactly this. Change-Id: I510553b5d7a8f6581ea1d21604e834fd6338cb06 Reviewed-on: https://go-review.googlesource.com/c/go/+/520339 Run-TryBot: Matthew Dempsky Auto-Submit: Matthew Dempsky TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/noder/reader.go | 37 ++++++++++++++---------- test/closure3.dir/main.go | 8 ++--- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index 59b10b3b33..a07fec68ec 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/src/cmd/compile/internal/noder/reader.go @@ -2722,15 +2722,12 @@ func (r *reader) syntheticClosure(origPos src.XPos, typ *types.Type, ifaceHack b return false } - // The ODCLFUNC and its body need to use the original position, but - // the OCLOSURE node and any Init statements should use the inlined - // position instead. See also the explanation in reader.funcLit. - inlPos := r.inlPos(origPos) - - // TODO(mdempsky): Remove hard-coding of typecheck.Target. - fn := ir.NewClosureFunc(origPos, inlPos, typ, r.curfn, typecheck.Target) + fn := r.inlClosureFunc(origPos, typ) fn.SetWrapper(true) + clo := fn.OClosure + inlPos := clo.Pos() + var init ir.Nodes for i, n := range captures { if isSafe(n) { @@ -2766,7 +2763,7 @@ func (r *reader) syntheticClosure(origPos src.XPos, typ *types.Type, ifaceHack b bodyReader[fn] = pri pri.funcBody(fn) - return ir.InitExpr(init, fn.OClosure) + return ir.InitExpr(init, clo) } // syntheticSig duplicates and returns the params and results lists @@ -3114,15 +3111,16 @@ func (r *reader) funcLit() ir.Node { // OCLOSURE node, because that position represents where any heap // allocation of the closure is credited (#49171). r.suppressInlPos++ - pos := r.pos() - xtype2 := r.signature(nil) + origPos := r.pos() + sig := r.signature(nil) r.suppressInlPos-- - // TODO(mdempsky): Remove hard-coding of typecheck.Target. - fn := ir.NewClosureFunc(pos, r.inlPos(pos), xtype2, r.curfn, typecheck.Target) + fn := r.inlClosureFunc(origPos, sig) fn.ClosureVars = make([]*ir.Name, 0, r.Len()) for len(fn.ClosureVars) < cap(fn.ClosureVars) { + // TODO(mdempsky): I think these should be original positions too + // (i.e., not inline-adjusted). ir.NewClosureVar(r.pos(), fn, r.useLocal()) } if param := r.dictParam; param != nil { @@ -3136,6 +3134,18 @@ func (r *reader) funcLit() ir.Node { return fn.OClosure } +// inlClosureFunc constructs a new closure function, but correctly +// handles inlining. +func (r *reader) inlClosureFunc(origPos src.XPos, sig *types.Type) *ir.Func { + curfn := r.inlCaller + if curfn == nil { + curfn = r.curfn + } + + // TODO(mdempsky): Remove hard-coding of typecheck.Target. + return ir.NewClosureFunc(origPos, r.inlPos(origPos), sig, curfn, typecheck.Target) +} + func (r *reader) exprList() []ir.Node { r.Sync(pkgbits.SyncExprList) return r.exprs() @@ -3451,10 +3461,7 @@ func unifiedInlineCall(call *ir.CallExpr, fn *ir.Func, inlIndex int) *ir.Inlined r := pri.asReader(pkgbits.RelocBody, pkgbits.SyncFuncBody) - // TODO(mdempsky): This still feels clumsy. Can we do better? tmpfn := ir.NewFunc(fn.Pos(), fn.Nname.Pos(), callerfn.Sym(), fn.Type()) - tmpfn.Closgen = callerfn.Closgen - defer func() { callerfn.Closgen = tmpfn.Closgen }() r.curfn = tmpfn diff --git a/test/closure3.dir/main.go b/test/closure3.dir/main.go index 07629bfec0..441da70105 100644 --- a/test/closure3.dir/main.go +++ b/test/closure3.dir/main.go @@ -256,10 +256,10 @@ func main() { b := 3 return func(y int) int { // ERROR "can inline main.func27.1" "can inline main.main.func27.func34" c := 5 - return func(z int) int { // ERROR "can inline main.func27.1.1" "can inline main.main.func27.func34.1" "can inline main.func27.main.func27.1.func2" "can inline main.main.func27.main.main.func27.func34.func36" + return func(z int) int { // ERROR "can inline main.func27.1.1" "can inline main.main.func27.func34.1" "can inline main.func27.main.func27.1.2" "can inline main.main.func27.main.main.func27.func34.func36" return a*x + b*y + c*z }(10) // ERROR "inlining call to main.func27.1.1" - }(100) // ERROR "inlining call to main.func27.1" "inlining call to main.func27.main.func27.1.func2" + }(100) // ERROR "inlining call to main.func27.1" "inlining call to main.func27.main.func27.1.2" }(1000); r != 2350 { // ERROR "inlining call to main.func27" "inlining call to main.main.func27.func34" "inlining call to main.main.func27.main.main.func27.func34.func36" ppanic("r != 2350") } @@ -271,13 +271,13 @@ func main() { b := 3 return func(y int) int { // ERROR "can inline main.func28.1" "can inline main.main.func28.func35" c := 5 - func(z int) { // ERROR "can inline main.func28.1.1" "can inline main.func28.main.func28.1.func2" "can inline main.main.func28.func35.1" "can inline main.main.func28.main.main.func28.func35.func37" + func(z int) { // ERROR "can inline main.func28.1.1" "can inline main.func28.main.func28.1.2" "can inline main.main.func28.func35.1" "can inline main.main.func28.main.main.func28.func35.func37" a = a * x b = b * y c = c * z }(10) // ERROR "inlining call to main.func28.1.1" return a + c - }(100) + b // ERROR "inlining call to main.func28.1" "inlining call to main.func28.main.func28.1.func2" + }(100) + b // ERROR "inlining call to main.func28.1" "inlining call to main.func28.main.func28.1.2" }(1000); r != 2350 { // ERROR "inlining call to main.func28" "inlining call to main.main.func28.func35" "inlining call to main.main.func28.main.main.func28.func35.func37" ppanic("r != 2350") }