Skip to content

Commit d63a9b8

Browse files
nevkontaktenevkontakte
committed
Include original function names into source maps.
The main change in this commit is an ability to use identifier name mapping, which we use to show original function names in the source map. This addresses the long-standing #1085, where GopherJS call stacks were somewhat difficult to interpret due to function name mangling, especially in minified form. Now we emit an additional source map hit with the original function name, which Node is able to pick up. While at it, I moved source map hinting logic into its own package with tests and added some documentation on how it works. Now it should be easy to extend this mechanism for even richer source maps if we want to.
1 parent ad87dd6 commit d63a9b8

File tree

14 files changed

+626
-112
lines changed

14 files changed

+626
-112
lines changed

‎build/build.go‎

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/gopherjs/gopherjs/compiler/jsFile"
3131
"github.com/gopherjs/gopherjs/compiler/sources"
3232
"github.com/gopherjs/gopherjs/internal/errorList"
33+
"github.com/gopherjs/gopherjs/internal/sourcemapx"
3334
"github.com/gopherjs/gopherjs/internal/testmain"
3435
log "github.com/sirupsen/logrus"
3536

@@ -1216,9 +1217,9 @@ func (s *Session) ImportResolverFor(srcDir string) func(string) (*compiler.Archi
12161217
}
12171218
}
12181219

1219-
// SourceMappingCallback returns a call back for compiler.SourceMapFilter
1220+
// SourceMappingCallback returns a callback for [github.com/gopherjs/gopherjs/compiler.SourceMapFilter]
12201221
// configured for the current build session.
1221-
func (s *Session) SourceMappingCallback(m *sourcemap.Map) func(generatedLine, generatedColumn int, originalPos token.Position) {
1222+
func (s *Session) SourceMappingCallback(m *sourcemap.Map) func(generatedLine, generatedColumn int, originalPos token.Position, originalName string) {
12221223
return NewMappingCallback(m, s.xctx.Env().GOROOT, s.xctx.Env().GOPATH, s.options.MapToLocalDisk)
12231224
}
12241225

@@ -1233,7 +1234,7 @@ func (s *Session) WriteCommandPackage(archive *compiler.Archive, pkgObj string)
12331234
}
12341235
defer codeFile.Close()
12351236

1236-
sourceMapFilter := &compiler.SourceMapFilter{Writer: codeFile}
1237+
sourceMapFilter := &sourcemapx.Filter{Writer: codeFile}
12371238
if s.options.CreateMapFile {
12381239
m := &sourcemap.Map{File: filepath.Base(pkgObj)}
12391240
mapFile, err := os.Create(pkgObj + ".map")
@@ -1258,27 +1259,33 @@ func (s *Session) WriteCommandPackage(archive *compiler.Archive, pkgObj string)
12581259
}
12591260

12601261
// NewMappingCallback creates a new callback for source map generation.
1261-
func NewMappingCallback(m *sourcemap.Map, goroot, gopath string, localMap bool) func(generatedLine, generatedColumn int, originalPos token.Position) {
1262-
return func(generatedLine, generatedColumn int, originalPos token.Position) {
1263-
if !originalPos.IsValid() {
1264-
m.AddMapping(&sourcemap.Mapping{GeneratedLine: generatedLine, GeneratedColumn: generatedColumn})
1265-
return
1262+
func NewMappingCallback(m *sourcemap.Map, goroot, gopath string, localMap bool) func(generatedLine, generatedColumn int, originalPos token.Position, originalName string) {
1263+
return func(generatedLine, generatedColumn int, originalPos token.Position, originalName string) {
1264+
mapping := &sourcemap.Mapping{GeneratedLine: generatedLine, GeneratedColumn: generatedColumn}
1265+
1266+
if originalPos.IsValid() {
1267+
file := originalPos.Filename
1268+
1269+
switch hasGopathPrefix, prefixLen := hasGopathPrefix(file, gopath); {
1270+
case localMap:
1271+
// no-op: keep file as-is
1272+
case hasGopathPrefix:
1273+
file = filepath.ToSlash(file[prefixLen+4:])
1274+
case strings.HasPrefix(file, goroot):
1275+
file = filepath.ToSlash(file[len(goroot)+4:])
1276+
default:
1277+
file = filepath.Base(file)
1278+
}
1279+
mapping.OriginalFile = file
1280+
mapping.OriginalLine = originalPos.Line
1281+
mapping.OriginalColumn = originalPos.Column
12661282
}
12671283

1268-
file := originalPos.Filename
1269-
1270-
switch hasGopathPrefix, prefixLen := hasGopathPrefix(file, gopath); {
1271-
case localMap:
1272-
// no-op: keep file as-is
1273-
case hasGopathPrefix:
1274-
file = filepath.ToSlash(file[prefixLen+4:])
1275-
case strings.HasPrefix(file, goroot):
1276-
file = filepath.ToSlash(file[len(goroot)+4:])
1277-
default:
1278-
file = filepath.Base(file)
1284+
if originalName != "" {
1285+
mapping.OriginalName = originalName
12791286
}
12801287

1281-
m.AddMapping(&sourcemap.Mapping{GeneratedLine: generatedLine, GeneratedColumn: generatedColumn, OriginalFile: file, OriginalLine: originalPos.Line, OriginalColumn: originalPos.Column})
1288+
m.AddMapping(mapping)
12821289
}
12831290
}
12841291

‎compiler/compiler.go‎

Lines changed: 12 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package compiler
77

88
import (
99
"bytes"
10-
"encoding/binary"
1110
"encoding/gob"
1211
"encoding/json"
1312
"fmt"
@@ -20,6 +19,7 @@ import (
2019
"github.com/gopherjs/gopherjs/compiler/internal/dce"
2120
"github.com/gopherjs/gopherjs/compiler/linkname"
2221
"github.com/gopherjs/gopherjs/compiler/prelude"
22+
"github.com/gopherjs/gopherjs/internal/sourcemapx"
2323
"golang.org/x/tools/go/gcexportdata"
2424
)
2525

@@ -108,7 +108,13 @@ func ImportDependencies(archive *Archive, importPkg func(string) (*Archive, erro
108108
return deps, nil
109109
}
110110

111-
func WriteProgramCode(pkgs []*Archive, w *SourceMapFilter, goVersion string) error {
111+
type dceInfo struct {
112+
decl *Decl
113+
objectFilter string
114+
methodFilter string
115+
}
116+
117+
func WriteProgramCode(pkgs []*Archive, w *sourcemapx.Filter, goVersion string) error {
112118
mainPkg := pkgs[len(pkgs)-1]
113119
minify := mainPkg.Minified
114120

@@ -165,9 +171,9 @@ func WriteProgramCode(pkgs []*Archive, w *SourceMapFilter, goVersion string) err
165171
return nil
166172
}
167173

168-
func WritePkgCode(pkg *Archive, dceSelection map[*Decl]struct{}, gls linkname.GoLinknameSet, minify bool, w *SourceMapFilter) error {
174+
func WritePkgCode(pkg *Archive, dceSelection map[*Decl]struct{}, gls linkname.GoLinknameSet, minify bool, w *sourcemapx.Filter) error {
169175
if w.MappingCallback != nil && pkg.FileSet != nil {
170-
w.fileSet = pkg.FileSet
176+
w.FileSet = pkg.FileSet
171177
}
172178
if _, err := w.Write(pkg.IncJSCode); err != nil {
173179
return err
@@ -316,77 +322,6 @@ func ReadArchive(importPath string, r io.Reader, srcModTime time.Time, imports m
316322
}
317323

318324
// WriteArchive writes compiled package archive on disk for later reuse.
319-
//
320-
// The passed in buildTime is used to determine if the archive is out-of-date.
321-
// Typically it should be set to the srcModTime or time.Now() but it is exposed for testing purposes.
322-
func WriteArchive(a *Archive, buildTime time.Time, w io.Writer) error {
323-
exportData := new(bytes.Buffer)
324-
if a.Package != nil {
325-
if err := gcexportdata.Write(exportData, nil, a.Package); err != nil {
326-
return fmt.Errorf("failed to write export data: %w", err)
327-
}
328-
}
329-
330-
encodedFileSet := new(bytes.Buffer)
331-
if a.FileSet != nil {
332-
if err := a.FileSet.Write(json.NewEncoder(encodedFileSet).Encode); err != nil {
333-
return err
334-
}
335-
}
336-
337-
sa := serializableArchive{
338-
ImportPath: a.ImportPath,
339-
Name: a.Name,
340-
Imports: a.Imports,
341-
ExportData: exportData.Bytes(),
342-
Declarations: a.Declarations,
343-
IncJSCode: a.IncJSCode,
344-
FileSet: encodedFileSet.Bytes(),
345-
Minified: a.Minified,
346-
GoLinknames: a.GoLinknames,
347-
BuildTime: buildTime,
348-
}
349-
350-
return gob.NewEncoder(w).Encode(sa)
351-
}
352-
353-
type SourceMapFilter struct {
354-
Writer io.Writer
355-
MappingCallback func(generatedLine, generatedColumn int, originalPos token.Position)
356-
line int
357-
column int
358-
fileSet *token.FileSet
359-
}
360-
361-
func (f *SourceMapFilter) Write(p []byte) (n int, err error) {
362-
var n2 int
363-
for {
364-
i := bytes.IndexByte(p, '\b')
365-
w := p
366-
if i != -1 {
367-
w = p[:i]
368-
}
369-
370-
n2, err = f.Writer.Write(w)
371-
n += n2
372-
for {
373-
i := bytes.IndexByte(w, '\n')
374-
if i == -1 {
375-
f.column += len(w)
376-
break
377-
}
378-
f.line++
379-
f.column = 0
380-
w = w[i+1:]
381-
}
382-
383-
if err != nil || i == -1 {
384-
return
385-
}
386-
if f.MappingCallback != nil {
387-
f.MappingCallback(f.line+1, f.column, f.fileSet.Position(token.Pos(binary.BigEndian.Uint32(p[i+1:i+5]))))
388-
}
389-
p = p[i+5:]
390-
n += 5
391-
}
325+
func WriteArchive(a *Archive, w io.Writer) error {
326+
return gob.NewEncoder(w).Encode(a)
392327
}

‎compiler/compiler_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"github.com/google/go-cmp/cmp"
12+
"github.com/gopherjs/gopherjs/internal/sourcemapx"
1213
"golang.org/x/tools/go/packages"
1314

1415
"github.com/gopherjs/gopherjs/compiler/internal/dce"
@@ -846,11 +847,10 @@ func reloadCompiledProject(t *testing.T, archives map[string]*Archive, rootPkgPa
846847
// the old recursive archive loading that is no longer used since it
847848
// doesn't allow cross package analysis for generings.
848849

849-
buildTime := newTime(5.0)
850850
serialized := map[string][]byte{}
851851
for path, a := range archives {
852852
buf := &bytes.Buffer{}
853-
if err := WriteArchive(a, buildTime, buf); err != nil {
853+
if err := WriteArchive(a, buf); err != nil {
854854
t.Fatalf(`failed to write archive for %s: %v`, path, err)
855855
}
856856
serialized[path] = buf.Bytes()
@@ -903,7 +903,7 @@ func renderPackage(t *testing.T, archive *Archive, minify bool) string {
903903

904904
buf := &bytes.Buffer{}
905905

906-
if err := WritePkgCode(archive, selection, linkname.GoLinknameSet{}, minify, &SourceMapFilter{Writer: buf}); err != nil {
906+
if err := WritePkgCode(archive, selection, linkname.GoLinknameSet{}, minify, &sourcemapx.Filter{Writer: buf}); err != nil {
907907
t.Fatal(err)
908908
}
909909

‎compiler/functions.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/gopherjs/gopherjs/compiler/internal/analysis"
1717
"github.com/gopherjs/gopherjs/compiler/internal/typeparams"
1818
"github.com/gopherjs/gopherjs/compiler/typesutil"
19+
"github.com/gopherjs/gopherjs/internal/sourcemapx"
1920
)
2021

2122
// nestedFunctionContext creates a new nested context for a function corresponding
@@ -64,7 +65,11 @@ func (fc *funcContext) nestedFunctionContext(info *analysis.FuncInfo, inst typep
6465
if recvType := typesutil.RecvType(sig); recvType != nil {
6566
funcRef = recvType.Obj().Name() + midDot + funcRef
6667
}
67-
c.funcRef = c.newVariable(funcRef, true /*pkgLevel*/)
68+
c.funcRef = sourcemapx.Identifier{
69+
Name: c.newVariable(funcRef, true /*pkgLevel*/),
70+
OriginalName: o.FullName(),
71+
OriginalPos: o.Pos(),
72+
}
6873

6974
return c
7075
}
@@ -299,11 +304,11 @@ func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident,
299304
localVars = append(localVars, "$r")
300305
// funcRef identifies the function object itself, so it doesn't need to be saved
301306
// or restored.
302-
localVars = removeMatching(localVars, fc.funcRef)
307+
localVars = removeMatching(localVars, fc.funcRef.Name)
303308
// If a blocking function is being resumed, initialize local variables from the saved context.
304309
localVarDefs = fmt.Sprintf("var {%s, $c} = $restore(this, {%s});\n", strings.Join(localVars, ", "), strings.Join(args, ", "))
305310
// If the function gets blocked, save local variables for future.
306-
saveContext := fmt.Sprintf("var $f = {$blk: "+fc.funcRef+", $c: true, $r, %s};", strings.Join(fc.localVars, ", "))
311+
saveContext := fmt.Sprintf("var $f = {$blk: %s, $c: true, $r, %s};", fc.funcRef, strings.Join(fc.localVars, ", "))
307312

308313
suffix = " " + saveContext + "return $f;" + suffix
309314
} else if len(fc.localVars) > 0 {
@@ -351,5 +356,5 @@ func (fc *funcContext) translateFunctionBody(typ *ast.FuncType, recv *ast.Ident,
351356

352357
fc.pkgCtx.escapingVars = prevEV
353358

354-
return fmt.Sprintf("function %s(%s) {\n%s%s}", fc.funcRef, strings.Join(args, ", "), bodyOutput, fc.Indentation(0))
359+
return fmt.Sprintf("%sfunction %s(%s) {\n%s%s}", fc.funcRef.EncodeHint(), fc.funcRef, strings.Join(args, ", "), bodyOutput, fc.Indentation(0))
355360
}

‎compiler/package.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/gopherjs/gopherjs/compiler/sources"
1616
"github.com/gopherjs/gopherjs/compiler/typesutil"
1717
"github.com/gopherjs/gopherjs/internal/errorList"
18+
"github.com/gopherjs/gopherjs/internal/sourcemapx"
1819
)
1920

2021
// pkgContext maintains compiler context for a specific package.
@@ -60,7 +61,7 @@ type funcContext struct {
6061
// "function" keyword in the generated code). This identifier can be used
6162
// within the function scope to reference the function object. It will also
6263
// appear in the stack trace.
63-
funcRef string
64+
funcRef sourcemapx.Identifier
6465
// Surrounding package context.
6566
pkgCtx *pkgContext
6667
// Function context, surrounding this function definition. For package-level

‎compiler/utils.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package compiler
22

33
import (
44
"bytes"
5-
"encoding/binary"
65
"errors"
76
"fmt"
87
"go/ast"
@@ -21,6 +20,7 @@ import (
2120
"github.com/gopherjs/gopherjs/compiler/internal/analysis"
2221
"github.com/gopherjs/gopherjs/compiler/internal/typeparams"
2322
"github.com/gopherjs/gopherjs/compiler/typesutil"
23+
"github.com/gopherjs/gopherjs/internal/sourcemapx"
2424
)
2525

2626
// We use this character as a separator in synthetic identifiers instead of a
@@ -71,8 +71,13 @@ func (fc *funcContext) SetPos(pos token.Pos) {
7171
func (fc *funcContext) writePos() {
7272
if fc.posAvailable {
7373
fc.posAvailable = false
74-
fc.Write([]byte{'\b'})
75-
binary.Write(fc, binary.BigEndian, uint32(fc.pos))
74+
h := sourcemapx.Hint{}
75+
if err := h.Pack(fc.pos); err != nil {
76+
panic(bailout(fmt.Errorf("failed to pack source map position: %w", err)))
77+
}
78+
if _, err := h.WriteTo(fc); err != nil {
79+
panic(bailout(fmt.Errorf("failed to write source map hint: %w", err)))
80+
}
7681
}
7782
}
7883

@@ -832,7 +837,7 @@ func getJsTag(tag string) string {
832837
}
833838

834839
func needsSpace(c byte) bool {
835-
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || c == '$'
840+
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || c == '$' || c == '\b'
836841
}
837842

838843
func removeWhitespace(b []byte, minify bool) []byte {
@@ -845,8 +850,9 @@ func removeWhitespace(b []byte, minify bool) []byte {
845850
for len(b) > 0 {
846851
switch b[0] {
847852
case '\b':
848-
out = append(out, b[:5]...)
849-
b = b[5:]
853+
_, length := sourcemapx.ReadHint(b)
854+
out = append(out, b[:length]...)
855+
b = b[length:]
850856
continue
851857
case ' ', '\t', '\n':
852858
if (!needsSpace(previous) || !needsSpace(b[1])) && !(previous == '-' && b[1] == '-') {

‎internal/sourcemapx/doc.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Package sourcemapx contains utilities for passing source map information
2+
// around, intended to work with github.com/neelance/sourcemap.
3+
//
4+
// GopherJS code generator outputs hints about correspondence between the
5+
// generated code and original sources inline. Such hints are marked by the
6+
// special `\b` (0x08) magic byte, followed by a variable-length sequence of
7+
// bytes, which can be extracted from the byte slice using ReadHint() function.
8+
//
9+
// '\b' was chosen as a magic symbol because it would never occur unescaped in
10+
// the generated code, other than when explicitly inserted by the source mapping
11+
// hint. See Hint type documentation for the details of the encoded format.
12+
//
13+
// The hinting mechanism is designed to be extensible, the Hint type able to
14+
// wrap different types containing different information:
15+
//
16+
// - go/token.Pos indicates position in the original source the current
17+
// location in the generated code corresponds to.
18+
// - Identifier maps a JS identifier to the original Go identifier it
19+
// represents.
20+
//
21+
// More types may be added in future if necessary.
22+
//
23+
// Filter type is used to extract the hints from the written code stream and
24+
// pass them into source map generator. It also ensures that the encoded inline
25+
// hints don't make it into the final output, since they are not valid JS.
26+
package sourcemapx

0 commit comments

Comments
 (0)