Assign identity to all function literals and use them as funcRefs.

nevkontakte · nevkontakte · commit 4d2439585cd7 · 2023-04-08T14:01:36.000+01:00
The main change is that we assign explicit names to all function objects
that correspond to Go functions (named and literals). Function name is
declared as `var f = function nameHere() { ... }` and is visible inside
the function scope only. Doing so serves two purposes:

 - It is an identifier which we can use when saving state of a blocked
   function to know which function to call upon resumption.
 - It shows up in the stack trace, which helps distinguish
   similarly-named functions. For methods, we include the receiver type
   in the identifier to make A.String and B.String easily
   distinguishable.

The main trick is that we synthesize names for the function literals,
which are anonymous as far as go/types is concerned. The upstream Go
compiler does something very similar.

Lest but not least, with this change the generic-related code path in
the funcContext.translateFunctionBody becomes more self-contained, which
will become handy in the upcoming commits.
diff --git a/compiler/expressions.go b/compiler/expressions.go
@@ -177,7 +177,7 @@ func (fc *funcContext) translateExpr(expr ast.Expr) *expression {
 		}
 
 	case *ast.FuncLit:
-		fun := fc.nestedFunctionContext(fc.pkgCtx.FuncLitInfos[e], exprType.(*types.Signature)).translateFunctionBody(e.Type, nil, e.Body, "")
+		fun := fc.literalFuncContext(e).translateFunctionBody(e.Type, nil, e.Body)
 		if len(fc.pkgCtx.escapingVars) != 0 {
 			names := make([]string, 0, len(fc.pkgCtx.escapingVars))
 			for obj := range fc.pkgCtx.escapingVars {
diff --git a/compiler/functions.go b/compiler/functions.go
@@ -19,20 +19,21 @@ import (
 
 // newFunctionContext creates a new nested context for a function corresponding
 // to the provided info.
-func (fc *funcContext) nestedFunctionContext(info *analysis.FuncInfo, sig *types.Signature) *funcContext {
+func (fc *funcContext) nestedFunctionContext(info *analysis.FuncInfo, o *types.Func) *funcContext {
 	if info == nil {
 		panic(errors.New("missing *analysis.FuncInfo"))
 	}
-	if sig == nil {
-		panic(errors.New("missing *types.Signature"))
+	if o == nil {
+		panic(errors.New("missing *types.Func"))
 	}
 
 	c := &funcContext{
 		FuncInfo:    info,
+		funcObject:  o,
 		pkgCtx:      fc.pkgCtx,
 		genericCtx:  fc.genericCtx,
 		parent:      fc,
-		sigTypes:    &signatureTypes{Sig: sig},
+		sigTypes:    &signatureTypes{Sig: o.Type().(*types.Signature)},
 		allVars:     make(map[string]int, len(fc.allVars)),
 		localVars:   []string{},
 		flowDatas:   map[*types.Label]*flowData{nil: {}},
@@ -44,6 +45,42 @@ func (fc *funcContext) nestedFunctionContext(info *analysis.FuncInfo, sig *types
 		c.allVars[k] = v
 	}
 
+	// Synthesize an identifier by which the function may reference itself. Since
+	// it appears in the stack trace, it's useful to include the receiver type in
+	// it.
+	funcRef := o.Name()
+	if typeName := c.sigTypes.RecvTypeName(); typeName != "" {
+		funcRef = typeName + midDot + funcRef
+	}
+	c.funcRef = c.newVariable(funcRef, varPackage)
+
+	// If the function has type parameters, create a new generic context for it.
+	if c.sigTypes.IsGeneric() {
+		c.genericCtx = &genericCtx{}
+	}
+
+	return c
+}
+
+// namedFuncContext creates a new funcContext for a named Go function
+// (standalone or method).
+func (fc *funcContext) namedFuncContext(fun *ast.FuncDecl) *funcContext {
+	o := fc.pkgCtx.Defs[fun.Name].(*types.Func)
+	info := fc.pkgCtx.FuncDeclInfos[o]
+	c := fc.nestedFunctionContext(info, o)
+
+	return c
+}
+
+// literalFuncContext creates a new funcContext for a function literal. Since
+// go/types doesn't generate *types.Func objects for function literals, we
+// generate a synthetic one for it.
+func (fc *funcContext) literalFuncContext(fun *ast.FuncLit) *funcContext {
+	info := fc.pkgCtx.FuncLitInfos[fun]
+	sig := fc.pkgCtx.TypeOf(fun).(*types.Signature)
+	o := types.NewFunc(fun.Pos(), fc.pkgCtx.Pkg, fc.newLitFuncName(), sig)
+
+	c := fc.nestedFunctionContext(info, o)
 	return c
 }
 
@@ -68,9 +105,6 @@ func (fc *funcContext) translateTopLevelFunction(fun *ast.FuncDecl) []byte {
 // package context. Exported functions are also assigned to the `$pkg` object.
 func (fc *funcContext) translateStandaloneFunction(fun *ast.FuncDecl) []byte {
 	o := fc.pkgCtx.Defs[fun.Name].(*types.Func)
-	info := fc.pkgCtx.FuncDeclInfos[o]
-	sig := o.Type().(*types.Signature)
-
 	if fun.Recv != nil {
 		panic(fmt.Errorf("expected standalone function, got method: %s", o))
 	}
@@ -79,7 +113,7 @@ func (fc *funcContext) translateStandaloneFunction(fun *ast.FuncDecl) []byte {
 	if fun.Body == nil {
 		return []byte(fmt.Sprintf("\t%s = %s;\n", lvalue, fc.unimplementedFunction(o)))
 	}
-	body := fc.nestedFunctionContext(info, sig).translateFunctionBody(fun.Type, nil, fun.Body, lvalue)
+	body := fc.namedFuncContext(fun).translateFunctionBody(fun.Type, nil, fun.Body)
 
 	code := &bytes.Buffer{}
 	fmt.Fprintf(code, "\t%s = %s;\n", lvalue, body)
@@ -99,14 +133,11 @@ func (fc *funcContext) translateMethod(fun *ast.FuncDecl) []byte {
 	}
 
 	o := fc.pkgCtx.Defs[fun.Name].(*types.Func)
-	info := fc.pkgCtx.FuncDeclInfos[o]
-
-	sig := o.Type().(*types.Signature)
 	var recv *ast.Ident
 	if fun.Recv.List[0].Names != nil {
 		recv = fun.Recv.List[0].Names[0]
 	}
-	nestedFC := fc.nestedFunctionContext(info, sig)
+	nestedFC := fc.namedFuncContext(fun)
 
 	// primaryFunction generates a JS function equivalent of the current Go function
 	// and assigns it to the JS expression defined by lvalue.
@@ -115,11 +146,11 @@ func (fc *funcContext) translateMethod(fun *ast.FuncDecl) []byte {
 			return []byte(fmt.Sprintf("\t%s = %s;\n", lvalue, fc.unimplementedFunction(o)))
 		}
 
-		funDef := nestedFC.translateFunctionBody(fun.Type, recv, fun.Body, lvalue)
+		funDef := nestedFC.translateFunctionBody(fun.Type, recv, fun.Body)
 		return []byte(fmt.Sprintf("\t%s = %s;\n", lvalue, funDef))
 	}
 
-	recvType := sig.Recv().Type()
+	recvType := nestedFC.sigTypes.Sig.Recv().Type()
 	ptr, isPointer := recvType.(*types.Pointer)
 	namedRecvType, _ := recvType.(*types.Named)
 	if isPointer {
@@ -131,7 +162,7 @@ func (fc *funcContext) translateMethod(fun *ast.FuncDecl) []byte {
 	// Objects the method should be assigned to.
 	prototypeVar := fmt.Sprintf("%s.prototype.%s", typeName, funName)
 	ptrPrototypeVar := fmt.Sprintf("$ptrType(%s).prototype.%s", typeName, funName)
-	isGeneric := signatureTypes{Sig: sig}.IsGeneric()
+	isGeneric := nestedFC.sigTypes.IsGeneric()
 	if isGeneric {
 		// Generic method factories are assigned to the generic type factory
 		// properties, to be invoked at type construction time rather than method
@@ -194,23 +225,7 @@ func (fc *funcContext) unimplementedFunction(o *types.Func) string {
 	return fmt.Sprintf("function() {\n\t\t$throwRuntimeError(\"native function not implemented: %s\");\n\t}", o.FullName())
 }
 
-func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident, body *ast.BlockStmt, funcRef string) string {
-	functionName := "" // Function object name, i.e. identifier after the "function" keyword.
-	if funcRef == "" {
-		// Assign a name for the anonymous function.
-		funcRef = "$b"
-		functionName = " $b"
-	}
-
-	// For regular functions instance is directly in the function variable name.
-	instanceVar := funcRef
-	if fc.sigTypes.IsGeneric() {
-		fc.genericCtx = &genericCtx{}
-		// For generic function, funcRef refers to the generic factory function,
-		// allocate a separate variable for a function instance.
-		instanceVar = fc.newVariable("instance", varGenericFactory)
-	}
-
+func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident, body *ast.BlockStmt) string {
 	prevEV := fc.pkgCtx.escapingVars
 
 	params := fc.funcParamVars(typ)
@@ -272,10 +287,13 @@ func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident,
 		// $c indicates that a function is being resumed after a blocking call when set to true.
 		// $f is an object used to save and restore function context for blocking calls.
 		localVars = append(localVars, "$r")
+		// funcRef identifies the function object itself, so it doesn't need to be saved
+		// or restored.
+		localVars = removeMatching(localVars, fc.funcRef)
 		// If a blocking function is being resumed, initialize local variables from the saved context.
 		localVarDefs = fmt.Sprintf("var {%s, $c} = $restore(this, {%s});\n", strings.Join(localVars, ", "), strings.Join(params, ", "))
 		// If the function gets blocked, save local variables for future.
-		saveContext := fmt.Sprintf("var $f = {$blk: %s, $c: true, $r, %s};", instanceVar, strings.Join(fc.localVars, ", "))
+		saveContext := fmt.Sprintf("var $f = {$blk: %s, $c: true, %s};", fc.funcRef, strings.Join(localVars, ", "))
 
 		suffix = " " + saveContext + "return $f;" + suffix
 	} else if len(fc.localVars) > 0 {
@@ -324,9 +342,13 @@ func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident,
 	fc.pkgCtx.escapingVars = prevEV
 
 	if !fc.sigTypes.IsGeneric() {
-		return fmt.Sprintf("function%s(%s) {\n%s%s}", functionName, strings.Join(params, ", "), bodyOutput, fc.Indentation(0))
+		return fmt.Sprintf("function %s(%s) {\n%s%s}", fc.funcRef, strings.Join(params, ", "), bodyOutput, fc.Indentation(0))
 	}
 
+	// For generic function, funcRef refers to the generic factory function,
+	// allocate a separate variable for a function instance.
+	instanceVar := fc.newVariable("instance", varGenericFactory)
+
 	// Generic functions are generated as factories to allow passing type parameters
 	// from the call site.
 	// TODO(nevkontakte): Cache function instances for a given combination of type
@@ -341,9 +363,9 @@ func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident,
 	}
 
 	code := &strings.Builder{}
-	fmt.Fprintf(code, "function%s(%s){\n", functionName, strings.Join(typeParams, ", "))
+	fmt.Fprintf(code, "function(%s){\n", strings.Join(typeParams, ", "))
 	fmt.Fprintf(code, "%s", typesInit.String())
-	fmt.Fprintf(code, "%sconst %s = function(%s) {\n", fc.Indentation(1), instanceVar, strings.Join(params, ", "))
+	fmt.Fprintf(code, "%sconst %s = function %s(%s) {\n", fc.Indentation(1), instanceVar, fc.funcRef, strings.Join(params, ", "))
 	fmt.Fprintf(code, "%s", bodyOutput)
 	fmt.Fprintf(code, "%s};\n", fc.Indentation(1))
 	// TODO(nevkontakte): Method list entries for generic type methods should be
diff --git a/compiler/natives/src/reflect/reflect.go b/compiler/natives/src/reflect/reflect.go
@@ -1734,21 +1734,22 @@ func methodNameSkip() string {
 	}
 	// Function name extracted from the call stack can be different from vanilla
 	// Go. Here we try to fix stuff like "Object.$packages.reflect.Q.ptr.SetIterKey"
-	// into "Value.SetIterKey".
+	// or plain "SetIterKey" into "Value.SetIterKey".
 	// This workaround may become obsolete after https://github.com/gopherjs/gopherjs/issues/1085
 	// is resolved.
 	name := f.Name()
 	idx := len(name) - 1
 	for idx > 0 {
 		if name[idx] == '.' {
+			idx++
 			break
 		}
 		idx--
 	}
 	if idx < 0 {
 		return name
 	}
-	return "Value" + name[idx:]
+	return "Value." + name[idx:]
 }
 
 func verifyNotInHeapPtr(p uintptr) bool {
diff --git a/compiler/package.go b/compiler/package.go
@@ -94,6 +94,15 @@ func (sel *fakeSelection) Type() types.Type          { return sel.typ }
 // JavaScript code (as defined for `var` declarations).
 type funcContext struct {
 	*analysis.FuncInfo
+	// Function object this context corresponds to, or nil if the context is
+	// top-level or doesn't correspond to a function. For function literals, this
+	// is a synthetic object that assigns a unique identity to the function.
+	funcObject *types.Func
+	// JavaScript identifier assigned to the function object (the word after the
+	// "function" keyword in the generated code). This identifier can be used
+	// within the function scope to reference the function object. It will also
+	// appear in the stack trace.
+	funcRef string
 	// Surrounding package context.
 	pkgCtx *pkgContext
 	// Surrounding generic function context. nil if non-generic code.
@@ -137,6 +146,8 @@ type funcContext struct {
 	posAvailable bool
 	// Current position in the Go source code.
 	pos token.Pos
+	// Number of function literals encountered within the current function context.
+	funcLitCounter int
 }
 
 // newRootCtx creates a new package-level instance of a functionContext object.
diff --git a/compiler/utils.go b/compiler/utils.go
@@ -22,6 +22,11 @@ import (
 	"github.com/gopherjs/gopherjs/compiler/typesutil"
 )
 
+// We use this character as a separator in synthetic identifiers instead of a
+// regular dot. This character is safe for use in JS identifiers and helps to
+// visually separate components of the name when it appears in a stack trace.
+const midDot = "·"
+
 // IsRoot returns true for the package-level context.
 func (fc *funcContext) IsRoot() bool {
 	return fc.parent == nil
@@ -410,6 +415,25 @@ func (fc *funcContext) newIdentFor(obj types.Object) *ast.Ident {
 	return ident
 }
 
+// newLitFuncName generates a new synthetic name for a function literal.
+func (fc *funcContext) newLitFuncName() string {
+	fc.funcLitCounter++
+	name := &strings.Builder{}
+
+	// If function literal is defined inside another function, qualify its
+	// synthetic name with the outer function to make it easier to identify.
+	if fc.funcObject != nil {
+		if recvType := fc.sigTypes.RecvTypeName(); recvType != "" {
+			name.WriteString(recvType)
+			name.WriteString(midDot)
+		}
+		name.WriteString(fc.funcObject.Name())
+		name.WriteString(midDot)
+	}
+	fmt.Fprintf(name, "func%d", fc.funcLitCounter)
+	return name.String()
+}
+
 // typeParamVars returns a list of JS variable names representing type given
 // parameters.
 func (fc *funcContext) typeParamVars(params *types.TypeParamList) []string {
@@ -502,6 +526,7 @@ func (fc *funcContext) methodName(fun *types.Func) string {
 	}
 	return name
 }
+
 func (fc *funcContext) varPtrName(o *types.Var) string {
 	if getVarLevel(o) == varPackage && o.Exported() {
 		return fc.pkgVar(o.Pkg()) + "." + o.Name() + "$ptr"
@@ -903,7 +928,15 @@ func rangeCheck(pattern string, constantIndex, array bool) string {
 }
 
 func encodeIdent(name string) string {
-	return strings.Replace(url.QueryEscape(name), "%", "$", -1)
+	// Quick-and-dirty way to make any string safe for use as an identifier in JS.
+	name = url.QueryEscape(name)
+	// We use unicode middle dot as a visual separator in synthetic identifiers.
+	// It is safe for use in a JS identifier, so we un-encode it for readability.
+	name = strings.ReplaceAll(name, "%C2%B7", midDot)
+	// QueryEscape uses '%' before hex-codes of escaped characters, which is not
+	// allowed in a JS identifier, use '$' instead.
+	name = strings.ReplaceAll(name, "%", "$")
+	return name
 }
 
 // formatJSStructTagVal returns JavaScript code for accessing an object's property
@@ -999,6 +1032,23 @@ func (st signatureTypes) IsGeneric() bool {
 	return st.Sig.TypeParams().Len() > 0 || st.Sig.RecvTypeParams().Len() > 0
 }
 
+// RecvTypeName returns receiver type name for a method signature. For pointer
+// receivers the named type is unwrapped from the pointer type. For non-methods
+// an empty string is returned.
+func (st signatureTypes) RecvTypeName() string {
+	recv := st.Sig.Recv()
+	if recv == nil {
+		return ""
+	}
+
+	typ := recv.Type()
+	if ptrType, ok := typ.(*types.Pointer); ok {
+		typ = ptrType.Elem()
+	}
+
+	return typ.(*types.Named).Obj().Name()
+}
+
 // ErrorAt annotates an error with a position in the source code.
 func ErrorAt(err error, fset *token.FileSet, pos token.Pos) error {
 	return fmt.Errorf("%s: %w", fset.Position(pos), err)
@@ -1057,3 +1107,13 @@ func bailingOut(err interface{}) (*FatalError, bool) {
 	fe, ok := err.(*FatalError)
 	return fe, ok
 }
+
+func removeMatching[T comparable](haystack []T, needle T) []T {
+	var result []T
+	for _, el := range haystack {
+		if el != needle {
+			result = append(result, el)
+		}
+	}
+	return result
+}

Original file line number	Diff line number	Diff line change
`@@ -177,7 +177,7 @@ func (fc funcContext) translateExpr(expr ast.Expr) expression {`
`177`	`177`	`}`
`178`	`178`
`179`	`179`	`case *ast.FuncLit:`
`180`		`- fun := fc.nestedFunctionContext(fc.pkgCtx.FuncLitInfos[e], exprType.(*types.Signature)).translateFunctionBody(e.Type, nil, e.Body, "")`
	`180`	`+ fun := fc.literalFuncContext(e).translateFunctionBody(e.Type, nil, e.Body)`
`181`	`181`	`if len(fc.pkgCtx.escapingVars) != 0 {`
`182`	`182`	`names := make([]string, 0, len(fc.pkgCtx.escapingVars))`
`183`	`183`	`for obj := range fc.pkgCtx.escapingVars {`
Original file line number	Diff line number	Diff line change
`@@ -1734,21 +1734,22 @@ func methodNameSkip() string {`
`1734`	`1734`	`}`
`1735`	`1735`	`// Function name extracted from the call stack can be different from vanilla`
`1736`	`1736`	`// Go. Here we try to fix stuff like "Object.$packages.reflect.Q.ptr.SetIterKey"`
`1737`		`- // into "Value.SetIterKey".`
	`1737`	`+ // or plain "SetIterKey" into "Value.SetIterKey".`
`1738`	`1738`	`// This workaround may become obsolete after https://github.com/gopherjs/gopherjs/issues/1085`
`1739`	`1739`	`// is resolved.`
`1740`	`1740`	`name := f.Name()`
`1741`	`1741`	`idx := len(name) - 1`
`1742`	`1742`	`for idx > 0 {`
`1743`	`1743`	`if name[idx] == '.' {`
	`1744`	`+ idx++`
`1744`	`1745`	`break`
`1745`	`1746`	`}`
`1746`	`1747`	`idx--`
`1747`	`1748`	`}`
`1748`	`1749`	`if idx < 0 {`
`1749`	`1750`	`return name`
`1750`	`1751`	`}`
`1751`		`- return "Value" + name[idx:]`
	`1752`	`+ return "Value." + name[idx:]`
`1752`	`1753`	`}`
`1753`	`1754`
`1754`	`1755`	`func verifyNotInHeapPtr(p uintptr) bool {`