Merge pull request gopherjs#1090 from nevkontakte/go-builtin

nevkontakte · web-flow · commit ec9572f70e60 · 2021-12-19T12:36:10.000Z
Correctly handle built-ins and `js.Object` methods with go keyword.
diff --git a/compiler/expressions.go b/compiler/expressions.go
@@ -794,6 +794,62 @@ func (fc *funcContext) translateCall(e *ast.CallExpr, sig *types.Signature, fun
 	return fc.formatExpr("%s(%s)", fun, strings.Join(args, ", "))
 }
 
+// delegatedCall returns a pair of JS expresions representing a callable function
+// and its arguments to be invoked elsewhere.
+//
+// This function is necessary in conjunction with keywords such as `go` and `defer`,
+// where we need to compute function and its arguments at the the keyword site,
+// but the call itself will happen elsewhere (hence "delegated").
+//
+// Built-in functions and cetrain `js.Object` methods don't translate into JS
+// function calls, and need to be wrapped before they can be delegated, which
+// this function handles and returns JS expressions that are safe to delegate
+// and behave like a regular JS function and a list of its argument values.
+func (fc *funcContext) delegatedCall(expr *ast.CallExpr) (callable *expression, arglist *expression) {
+	isBuiltin := false
+	isJs := false
+	switch fun := expr.Fun.(type) {
+	case *ast.Ident:
+		_, isBuiltin = fc.pkgCtx.Uses[fun].(*types.Builtin)
+	case *ast.SelectorExpr:
+		isJs = typesutil.IsJsPackage(fc.pkgCtx.Uses[fun.Sel].Pkg())
+	}
+	sig := fc.pkgCtx.TypeOf(expr.Fun).Underlying().(*types.Signature)
+	sigTypes := signatureTypes{Sig: sig}
+	args := fc.translateArgs(sig, expr.Args, expr.Ellipsis.IsValid())
+
+	if !isBuiltin && !isJs {
+		// Normal function calls don't require wrappers.
+		callable = fc.translateExpr(expr.Fun)
+		arglist = fc.formatExpr("[%s]", strings.Join(args, ", "))
+		return callable, arglist
+	}
+
+	// Since some builtins or js.Object methods may not transpile into
+	// callable expressions, we need to wrap then in a proxy lambda in order
+	// to push them onto the deferral stack.
+	vars := make([]string, len(expr.Args))
+	callArgs := make([]ast.Expr, len(expr.Args))
+	ellipsis := expr.Ellipsis
+
+	for i := range expr.Args {
+		v := fc.newVariable("_arg")
+		vars[i] = v
+		// Subtle: the proxy lambda argument needs to be assigned with the type
+		// that the original function expects, and not with the argument
+		// expression result type, or we may do implicit type conversion twice.
+		callArgs[i] = fc.newIdent(v, sigTypes.Param(i, ellipsis.IsValid()))
+	}
+	wrapper := &ast.CallExpr{
+		Fun:      expr.Fun,
+		Args:     callArgs,
+		Ellipsis: expr.Ellipsis,
+	}
+	callable = fc.formatExpr("function(%s) { %e; }", strings.Join(vars, ", "), wrapper)
+	arglist = fc.formatExpr("[%s]", strings.Join(args, ", "))
+	return callable, arglist
+}
+
 func (fc *funcContext) makeReceiver(e *ast.SelectorExpr) *expression {
 	sel, _ := fc.pkgCtx.SelectionOf(e)
 	if !sel.Obj().Exported() {
diff --git a/compiler/statements.go b/compiler/statements.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/constant"
+	"go/printer"
 	"go/token"
 	"go/types"
 	"strings"
@@ -39,6 +40,8 @@ func (fc *funcContext) translateStmt(stmt ast.Stmt, label *types.Label) {
 			pos = fc.pos
 		}
 		fmt.Fprintf(bail, "Occurred while compiling statement at %s:\n", fc.pkgCtx.fileSet.Position(pos))
+		(&printer.Config{Tabwidth: 2, Indent: 1, Mode: printer.UseSpaces}).Fprint(bail, fc.pkgCtx.fileSet, stmt)
+		fmt.Fprintf(bail, "\n\nDetailed AST:\n")
 		ast.Fprint(bail, fc.pkgCtx.fileSet, stmt, ast.NotNilFilter)
 		panic(bail) // Initiate orderly bailout.
 	}()
@@ -360,47 +363,8 @@ func (fc *funcContext) translateStmt(stmt ast.Stmt, label *types.Label) {
 		return
 
 	case *ast.DeferStmt:
-		isBuiltin := false
-		isJs := false
-		switch fun := s.Call.Fun.(type) {
-		case *ast.Ident:
-			var builtin *types.Builtin
-			builtin, isBuiltin = fc.pkgCtx.Uses[fun].(*types.Builtin)
-			if isBuiltin && builtin.Name() == "recover" {
-				fc.Printf("$deferred.push([$recover, []]);")
-				return
-			}
-		case *ast.SelectorExpr:
-			isJs = typesutil.IsJsPackage(fc.pkgCtx.Uses[fun.Sel].Pkg())
-		}
-		sig := fc.pkgCtx.TypeOf(s.Call.Fun).Underlying().(*types.Signature)
-		sigTypes := signatureTypes{Sig: sig}
-		args := fc.translateArgs(sig, s.Call.Args, s.Call.Ellipsis.IsValid())
-		if isBuiltin || isJs {
-			// Since some builtins or js.Object methods may not transpile into
-			// callable expressions, we need to wrap then in a proxy lambda in order
-			// to push them onto the deferral stack.
-			vars := make([]string, len(s.Call.Args))
-			callArgs := make([]ast.Expr, len(s.Call.Args))
-			ellipsis := s.Call.Ellipsis
-
-			for i := range s.Call.Args {
-				v := fc.newVariable("_arg")
-				vars[i] = v
-				// Subtle: the proxy lambda argument needs to be assigned with the type
-				// that the original function expects, and not with the argument
-				// expression result type, or we may do implicit type conversion twice.
-				callArgs[i] = fc.newIdent(v, sigTypes.Param(i, ellipsis.IsValid()))
-			}
-			call := fc.translateExpr(&ast.CallExpr{
-				Fun:      s.Call.Fun,
-				Args:     callArgs,
-				Ellipsis: s.Call.Ellipsis,
-			})
-			fc.Printf("$deferred.push([function(%s) { %s; }, [%s]]);", strings.Join(vars, ", "), call, strings.Join(args, ", "))
-			return
-		}
-		fc.Printf("$deferred.push([%s, [%s]]);", fc.translateExpr(s.Call.Fun), strings.Join(args, ", "))
+		callable, arglist := fc.delegatedCall(s.Call)
+		fc.Printf("$deferred.push([%s, %s]);", callable, arglist)
 
 	case *ast.AssignStmt:
 		if s.Tok != token.ASSIGN && s.Tok != token.DEFINE {
@@ -496,7 +460,8 @@ func (fc *funcContext) translateStmt(stmt ast.Stmt, label *types.Label) {
 		fc.translateStmt(s.Stmt, label)
 
 	case *ast.GoStmt:
-		fc.Printf("$go(%s, [%s]);", fc.translateExpr(s.Call.Fun), strings.Join(fc.translateArgs(fc.pkgCtx.TypeOf(s.Call.Fun).Underlying().(*types.Signature), s.Call.Args, s.Call.Ellipsis.IsValid()), ", "))
+		callable, arglist := fc.delegatedCall(s.Call)
+		fc.Printf("$go(%s, %s);", callable, arglist)
 
 	case *ast.SendStmt:
 		chanType := fc.pkgCtx.TypeOf(s.Chan).Underlying().(*types.Chan)
diff --git a/compiler/utils.go b/compiler/utils.go
@@ -10,6 +10,7 @@ import (
 	"go/token"
 	"go/types"
 	"net/url"
+	"regexp"
 	"runtime/debug"
 	"sort"
 	"strconv"
@@ -791,12 +792,14 @@ func (b *FatalError) Write(p []byte) (n int, err error) { return b.clues.Write(p
 
 func (b FatalError) Error() string {
 	buf := &strings.Builder{}
-	fmt.Fprintln(buf, "[compiler panic] ", b.Unwrap())
+	fmt.Fprintln(buf, "[compiler panic] ", strings.TrimSpace(b.Unwrap().Error()))
 	if b.clues.Len() > 0 {
-		fmt.Fprintln(buf, "\n", b.clues.String())
+		fmt.Fprintln(buf, "\n"+b.clues.String())
 	}
 	if len(b.stack) > 0 {
-		fmt.Fprintln(buf, "\n", string(b.stack))
+		// Shift stack track by 2 spaces for better readability.
+		stack := regexp.MustCompile("(?m)^").ReplaceAll(b.stack, []byte("  "))
+		fmt.Fprintln(buf, "\nOriginal stack trace:\n", string(stack))
 	}
 	return buf.String()
 }
diff --git a/tests/goroutine_test.go b/tests/goroutine_test.go
@@ -3,6 +3,7 @@ package tests
 import (
 	"context"
 	"fmt"
+	"runtime"
 	"testing"
 	"time"
 
@@ -265,3 +266,26 @@ func TestEventLoopStarvation(t *testing.T) {
 	}()
 	<-ctx.Done()
 }
+
+func TestGoroutineBuiltin(t *testing.T) {
+	// Test that a built-in function can be a goroutine body.
+	// https://github.com/gopherjs/gopherjs/issues/547.
+	c := make(chan bool)
+	go close(c)
+	<-c // Wait until goroutine executes successfully.
+}
+
+func TestGoroutineJsObject(t *testing.T) {
+	// Test that js.Object methods can be a goroutine body.
+	// https://github.com/gopherjs/gopherjs/issues/547.
+	if !(runtime.GOOS == "js" || runtime.GOARCH == "js") {
+		t.Skip("Test requires GopherJS")
+	}
+	o := js.Global.Get("Object").New()
+	go o.Set("x", "y")
+	// Wait until the goroutine executes successfully. Can't use locks here
+	// because goroutine body must be a bare js.Object method call.
+	for o.Get("x").String() != "y" {
+		runtime.Gosched()
+	}
+}
