Skip to content

Commit 435ccb0

Browse files
nevkontaktenevkontakte
committed
Update and re-enable tests that rely on Go function names.
The tests that were previously failing should work again with the new function name source maps. There are only two caveats at this point: - For some reasons a function that's currently executing deferrals doesn't map to its original name. - Synthetic names for literal functions differ in format slightly from vanilla Go, which still breaks the test in net/http. The difference is minor enough it's not forth fixing though.
1 parent 66dcffd commit 435ccb0

File tree

8 files changed

+72
-82
lines changed

8 files changed

+72
-82
lines changed

‎compiler/functions.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,11 @@ func (fc *funcContext) nestedFunctionContext(info *analysis.FuncInfo, inst typep
6161
// Synthesize an identifier by which the function may reference itself. Since
6262
// it appears in the stack trace, it's useful to include the receiver type in
6363
// it.
64-
funcRef := o.Name()
65-
if recvType := typesutil.RecvType(sig); recvType != nil {
66-
funcRef = recvType.Obj().Name() + midDot + funcRef
67-
}
64+
funcRef := strings.ReplaceAll(o.Name(), ".", midDot)
6865
c.funcRef = sourcemapx.Identifier{
69-
Name: c.newVariable(funcRef, true /*pkgLevel*/),
70-
OriginalName: o.FullName(),
66+
Name: c.newVariable(funcRef, true /*pkgLevel*/),
67+
// o.FullName() decorates pointer receivers as `(*T).method`, we want simply `T.method`.
68+
OriginalName: strings.NewReplacer("(", "", ")", "", "*", "").Replace(o.FullName()),
7169
OriginalPos: o.Pos(),
7270
}
7371

@@ -269,7 +267,7 @@ func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident,
269267

270268
fc.translateStmtList(body.List)
271269
if len(fc.Flattened) != 0 && !astutil.EndsWithReturn(body.List) {
272-
fc.translateStmt(&ast.ReturnStmt{}, nil)
270+
fc.translateStmt(&ast.ReturnStmt{Return: body.Rbrace}, nil)
273271
}
274272
}))
275273

‎compiler/natives/src/net/http/server_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,8 @@ package http_test
66
import "testing"
77

88
func TestTimeoutHandlerSuperfluousLogs(t *testing.T) {
9-
t.Skip("https://github.com/gopherjs/gopherjs/issues/1085")
9+
// The test expects nested anonymous functions to be named "Foo.func1.2",
10+
// bug GopherJS generates "Foo.func1.func2". Otherwise the test works as
11+
// expected.
12+
t.Skip("GopherJS uses different synthetic function names.")
1013
}

‎compiler/natives/src/reflect/reflect.go

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package reflect
55

66
import (
77
"errors"
8-
"runtime"
98
"strconv"
109
"unsafe"
1110

@@ -1774,37 +1773,37 @@ func stringsHasPrefix(s, prefix string) bool {
17741773
return len(s) >= len(prefix) && s[:len(prefix)] == prefix
17751774
}
17761775

1777-
func valueMethodName() string {
1778-
var pc [5]uintptr
1779-
n := runtime.Callers(1, pc[:])
1780-
frames := runtime.CallersFrames(pc[:n])
1781-
valueTyp := TypeOf(Value{})
1782-
var frame runtime.Frame
1783-
for more := true; more; {
1784-
frame, more = frames.Next()
1785-
name := frame.Function
1786-
// Function name extracted from the call stack can be different from
1787-
// vanilla Go, so is not prefixed by "reflect.Value." as needed by the original.
1788-
// See https://cs.opensource.google/go/go/+/refs/tags/go1.19.13:src/reflect/value.go;l=173-191
1789-
// This workaround may become obsolete after
1790-
// https://github.com/gopherjs/gopherjs/issues/1085 is resolved.
1791-
1792-
methodName := name
1793-
if idx := stringsLastIndex(name, '.'); idx >= 0 {
1794-
methodName = name[idx+1:]
1795-
}
1796-
1797-
// Since function name in the call stack doesn't contain receiver name,
1798-
// we are looking for the first exported function name that matches a
1799-
// known Value method.
1800-
if _, ok := valueTyp.MethodByName(methodName); ok {
1801-
if len(methodName) > 0 && 'A' <= methodName[0] && methodName[0] <= 'Z' {
1802-
return `reflect.Value.` + methodName
1803-
}
1804-
}
1805-
}
1806-
return "unknown method"
1807-
}
1776+
// func valueMethodName() string {
1777+
// var pc [5]uintptr
1778+
// n := runtime.Callers(1, pc[:])
1779+
// frames := runtime.CallersFrames(pc[:n])
1780+
// valueTyp := TypeOf(Value{})
1781+
// var frame runtime.Frame
1782+
// for more := true; more; {
1783+
// frame, more = frames.Next()
1784+
// name := frame.Function
1785+
// // Function name extracted from the call stack can be different from
1786+
// // vanilla Go, so is not prefixed by "reflect.Value." as needed by the original.
1787+
// // See https://cs.opensource.google/go/go/+/refs/tags/go1.19.13:src/reflect/value.go;l=173-191
1788+
// // This workaround may become obsolete after
1789+
// // https://github.com/gopherjs/gopherjs/issues/1085 is resolved.
1790+
1791+
// methodName := name
1792+
// if idx := stringsLastIndex(name, '.'); idx >= 0 {
1793+
// methodName = name[idx+1:]
1794+
// }
1795+
1796+
// // Since function name in the call stack doesn't contain receiver name,
1797+
// // we are looking for the first exported function name that matches a
1798+
// // known Value method.
1799+
// if _, ok := valueTyp.MethodByName(methodName); ok {
1800+
// if len(methodName) > 0 && 'A' <= methodName[0] && methodName[0] <= 'Z' {
1801+
// return `reflect.Value.` + methodName
1802+
// }
1803+
// }
1804+
// }
1805+
// return "unknown method"
1806+
// }
18081807

18091808
func verifyNotInHeapPtr(p uintptr) bool {
18101809
// Go runtime uses this method to make sure that a uintptr won't crash GC if

‎compiler/natives/src/reflect/reflect_test.go

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -298,23 +298,3 @@ func TestIssue50208(t *testing.T) {
298298
func TestStructOfTooLarge(t *testing.T) {
299299
t.Skip("This test is dependent on field alignment to determine if a struct size would exceed virtual address space.")
300300
}
301-
302-
func TestSetLenCap(t *testing.T) {
303-
t.Skip("Test depends on call stack function names: https://github.com/gopherjs/gopherjs/issues/1085")
304-
}
305-
306-
func TestSetPanic(t *testing.T) {
307-
t.Skip("Test depends on call stack function names: https://github.com/gopherjs/gopherjs/issues/1085")
308-
}
309-
310-
func TestCallPanic(t *testing.T) {
311-
t.Skip("Test depends on call stack function names: https://github.com/gopherjs/gopherjs/issues/1085")
312-
}
313-
314-
func TestValuePanic(t *testing.T) {
315-
t.Skip("Test depends on call stack function names: https://github.com/gopherjs/gopherjs/issues/1085")
316-
}
317-
318-
func TestSetIter(t *testing.T) {
319-
t.Skip("Test depends on call stack function names: https://github.com/gopherjs/gopherjs/issues/1085")
320-
}

‎compiler/natives/src/runtime/runtime.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func ParseCallFrame(info *js.Object) basicFrame {
214214
var file, funcName string
215215
var line, col int
216216

217-
pos := info.Call("substring", openIdx+1, info.Call("indexOf", ")").Int())
217+
pos := info.Call("substring", openIdx+1, info.Call("lastIndexOf", ")").Int())
218218
parts := pos.Call("split", ":")
219219

220220
if pos.String() == "<anonymous>" {
@@ -224,7 +224,7 @@ func ParseCallFrame(info *js.Object) basicFrame {
224224
line = parts.Index(parts.Length() - 2).Int()
225225
col = parts.Index(parts.Length() - 1).Int()
226226
}
227-
fn := info.Call("substring", info.Call("indexOf", "at ").Int()+3, info.Call("indexOf", " (").Int())
227+
fn := info.Call("substring", info.Call("indexOf", "at ").Int()+3, info.Call("lastIndexOf", " (").Int())
228228
if idx := fn.Call("indexOf", "[as ").Int(); idx > 0 {
229229
fn = fn.Call("substring", idx+4, fn.Call("indexOf", "]"))
230230
}

‎compiler/utils.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,10 +396,10 @@ func (fc *funcContext) newLitFuncName() string {
396396
if fc.instance.Object != nil {
397397
if recvType := typesutil.RecvType(fc.sig.Sig); recvType != nil {
398398
name.WriteString(recvType.Obj().Name())
399-
name.WriteString(midDot)
399+
name.WriteString(".")
400400
}
401401
name.WriteString(fc.instance.Object.Name())
402-
name.WriteString(midDot)
402+
name.WriteString(".")
403403
}
404404
fmt.Fprintf(name, "func%d", fc.funcLitCounter)
405405
return name.String()

‎tests/gorepo/run.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ var knownFails = map[string]failReason{
9898
"fixedbugs/issue19246.go": {desc: "expected nil pointer dereference panic"}, // Issue https://golang.org/issues/19246: Failed to evaluate some zero-sized values when converting them to interfaces.
9999

100100
// These are new tests in Go 1.10.
101-
"fixedbugs/issue21879.go": {desc: "incorrect output related to runtime.Callers, runtime.CallersFrames, etc."},
102101
"fixedbugs/issue21887.go": {desc: "incorrect output (although within spec, not worth fixing) for println(^uint64(0)). got: { '$high': 4294967295, '$low': 4294967295, '$val': [Circular] } want: 18446744073709551615"},
103102
"fixedbugs/issue22660.go": {category: notApplicable, desc: "test of gc compiler, uses os/exec.Command"},
104103
"fixedbugs/issue23305.go": {desc: "GopherJS fails to compile println(0xffffffff), maybe because 32-bit arch"},
@@ -125,8 +124,7 @@ var knownFails = map[string]failReason{
125124
"fixedbugs/issue24491a.go": {category: notApplicable, desc: "tests interaction between unsafe and GC; uses runtime.SetFinalizer()"},
126125
"fixedbugs/issue24491b.go": {category: notApplicable, desc: "tests interaction between unsafe and GC; uses runtime.SetFinalizer()"},
127126
"fixedbugs/issue29504.go": {category: notApplicable, desc: "requires source map support beyond what GopherJS currently provides"},
128-
// This test incorrectly passes because main function's name is returned as "main" and not "main.main". Even number of bugs cancel each other out ¯\_(ツ)_/¯
129-
// "fixedbugs/issue29735.go": {category: usesUnsupportedPackage, desc: "GopherJS only supports runtime.FuncForPC() with position counters previously returned by runtime.Callers() or runtime.Caller()"},
127+
"fixedbugs/issue29735.go": {category: lowLevelRuntimeDifference, desc: "GopherJS only supports runtime.FuncForPC() with position counters previously returned by runtime.Callers() or runtime.Caller()"},
130128
"fixedbugs/issue30116.go": {desc: "GopherJS doesn't specify the array/slice index selector in the out-of-bounds message"},
131129
"fixedbugs/issue30116u.go": {desc: "GopherJS doesn't specify the array/slice index selector in the out-of-bounds message"},
132130
"fixedbugs/issue34395.go": {category: neverTerminates, desc: "https://github.com/gopherjs/gopherjs/issues/1007"},

‎tests/runtime_test.go

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,11 @@ func (c *callStack) capture() {
101101
}
102102

103103
func TestCallers(t *testing.T) {
104-
got := callStack{}
105-
106104
// Some of the GopherJS function names don't match upstream Go, or even the
107105
// function names in the Go source when minified.
108106
// Until https://github.com/gopherjs/gopherjs/issues/1085 is resolved, the
109107
// mismatch is difficult to avoid, but we can at least use "masked" frames to
110108
// make sure the number of frames matches expected.
111-
want := callStack{
112-
masked("runtime.Callers"),
113-
masked("github.com/gopherjs/gopherjs/tests.(*callerNames).capture"),
114-
masked("github.com/gopherjs/gopherjs/tests.TestCallers.func{1,2}"),
115-
masked("testing.tRunner"),
116-
"runtime.goexit",
117-
}
118-
119109
opts := cmp.Comparer(func(a, b funcName) bool {
120110
if a == masked("") || b == masked("") {
121111
return true
@@ -124,13 +114,34 @@ func TestCallers(t *testing.T) {
124114
})
125115

126116
t.Run("Normal", func(t *testing.T) {
117+
got := callStack{}
118+
want := callStack{
119+
"runtime.Callers",
120+
"github.com/gopherjs/gopherjs/tests.callStack.capture",
121+
"github.com/gopherjs/gopherjs/tests.TestCallers.func2",
122+
"testing.tRunner",
123+
"runtime.goexit",
124+
}
125+
127126
got.capture()
128127
if diff := cmp.Diff(want, got, opts); diff != "" {
129128
t.Errorf("runtime.Callers() returned a diff (-want,+got):\n%s", diff)
130129
}
131130
})
132131

133132
t.Run("Deferred", func(t *testing.T) {
133+
got := callStack{}
134+
want := callStack{
135+
"runtime.Callers",
136+
"github.com/gopherjs/gopherjs/tests.callStack.capture",
137+
// For some reason function epilog where deferred calls are invoked doesn't
138+
// get source-mapped to the original source properly, which causes node
139+
// not to map the function name to the original.
140+
masked("github.com/gopherjs/gopherjs/tests.TestCallers.func3"),
141+
"testing.tRunner",
142+
"runtime.goexit",
143+
}
144+
134145
defer func() {
135146
if diff := cmp.Diff(want, got, opts); diff != "" {
136147
t.Errorf("runtime.Callers() returned a diff (-want,+got):\n%s", diff)
@@ -140,17 +151,18 @@ func TestCallers(t *testing.T) {
140151
})
141152

142153
t.Run("Recover", func(t *testing.T) {
154+
got := callStack{}
143155
defer func() {
144156
recover()
145157
got.capture()
146158

147159
want := callStack{
148-
masked("runtime.Callers"),
149-
masked("github.com/gopherjs/gopherjs/tests.(*callerNames).capture"),
150-
masked("github.com/gopherjs/gopherjs/tests.TestCallers.func3.1"),
160+
"runtime.Callers",
161+
"github.com/gopherjs/gopherjs/tests.callStack.capture",
162+
"github.com/gopherjs/gopherjs/tests.TestCallers.func4.func1",
151163
"runtime.gopanic",
152-
masked("github.com/gopherjs/gopherjs/tests.TestCallers.func{1,2}"),
153-
masked("testing.tRunner"),
164+
"github.com/gopherjs/gopherjs/tests.TestCallers.func4",
165+
"testing.tRunner",
154166
"runtime.goexit",
155167
}
156168
if diff := cmp.Diff(want, got, opts); diff != "" {

0 commit comments

Comments
 (0)