Skip to content

Commit 5f9eeb8

Browse files
grantnelson-wfgrantnelson-wf
committed
Fix Build Issue
1 parent 38b4d3b commit 5f9eeb8

File tree

3 files changed

+212
-11
lines changed

3 files changed

+212
-11
lines changed

‎build/build.go

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ type overrideInfo struct {
130130
// If the method is defined in the overlays and therefore has its
131131
// own overrides, this will be ignored.
132132
purgeMethods bool
133+
134+
// overrideSignature is the function definition given in the overlays
135+
// that should be used to replace the signature in the originals.
136+
// Only receivers, type parameters, parameters, and results will be used.
137+
overrideSignature *ast.FuncDecl
133138
}
134139

135140
// parseAndAugment parses and returns all .go files of given pkg.
@@ -270,9 +275,14 @@ func augmentOverlayFile(file *ast.File, overrides map[string]overrideInfo) {
270275
switch d := decl.(type) {
271276
case *ast.FuncDecl:
272277
k := astutil.FuncKey(d)
273-
overrides[k] = overrideInfo{
278+
oi := overrideInfo{
274279
keepOriginal: astutil.KeepOriginal(d),
275280
}
281+
if astutil.OverrideSignature(d) {
282+
oi.overrideSignature = d
283+
purgeDecl = true
284+
}
285+
overrides[k] = oi
276286
case *ast.GenDecl:
277287
for j, spec := range d.Specs {
278288
purgeSpec := purgeDecl || astutil.Purge(spec)
@@ -323,11 +333,21 @@ func augmentOriginalFile(file *ast.File, overrides map[string]overrideInfo) {
323333
switch d := decl.(type) {
324334
case *ast.FuncDecl:
325335
if info, ok := overrides[astutil.FuncKey(d)]; ok {
336+
removeFunc := true
326337
if info.keepOriginal {
327338
// Allow overridden function calls
328339
// The standard library implementation of foo() becomes _gopherjs_original_foo()
329340
d.Name.Name = "_gopherjs_original_" + d.Name.Name
330-
} else {
341+
removeFunc = false
342+
}
343+
if overSig := info.overrideSignature; overSig != nil {
344+
d.Recv = overSig.Recv
345+
d.Type.TypeParams = overSig.Type.TypeParams
346+
d.Type.Params = overSig.Type.Params
347+
d.Type.Results = overSig.Type.Results
348+
removeFunc = false
349+
}
350+
if removeFunc {
331351
file.Decls[i] = nil
332352
}
333353
} else if recvKey := astutil.FuncReceiverKey(d); len(recvKey) > 0 {
@@ -391,21 +411,40 @@ func augmentOriginalFile(file *ast.File, overrides map[string]overrideInfo) {
391411
finalizeRemovals(file)
392412
}
393413

414+
// isOnlyImports determines if this file is empty except for imports.
415+
func isOnlyImports(file *ast.File) bool {
416+
for _, decl := range file.Decls {
417+
if gen, ok := decl.(*ast.GenDecl); !ok || gen.Tok != token.IMPORT {
418+
return false
419+
}
420+
}
421+
return true
422+
}
423+
394424
// pruneImports will remove any unused imports from the file.
395425
//
396-
// This will not remove any dot (`.`) or blank (`_`) imports.
426+
// This will not remove any dot (`.`) or blank (`_`) imports, unless
427+
// there are no declarations or directives meaning that all the imports
428+
// should be cleared.
397429
// If the removal of code causes an import to be removed, the init's from that
398430
// import may not be run anymore. If we still need to run an init for an import
399431
// which is no longer used, add it to the overlay as a blank (`_`) import.
400432
func pruneImports(file *ast.File) {
433+
if isOnlyImports(file) && !astutil.HasDirectivePrefix(file, `//go:linkname `) {
434+
// The file is empty, remove all imports including any `.` or `_` imports.
435+
file.Imports = nil
436+
file.Decls = nil
437+
return
438+
}
439+
401440
unused := make(map[string]int, len(file.Imports))
402441
for i, in := range file.Imports {
403442
if name := astutil.ImportName(in); len(name) > 0 {
404443
unused[name] = i
405444
}
406445
}
407446

408-
// Remove "unused import" for any import which is used.
447+
// Remove "unused imports" for any import which is used.
409448
ast.Inspect(file, func(n ast.Node) bool {
410449
if sel, ok := n.(*ast.SelectorExpr); ok {
411450
if id, ok := sel.X.(*ast.Ident); ok && id.Obj == nil {
@@ -418,6 +457,24 @@ func pruneImports(file *ast.File) {
418457
return
419458
}
420459

460+
// Remove "unused imports" for any import used for a directive.
461+
directiveImports := map[string]string{
462+
`unsafe`: `//go:linkname `,
463+
`embed`: `//go:embed `,
464+
}
465+
for name, index := range unused {
466+
in := file.Imports[index]
467+
path, _ := strconv.Unquote(in.Path.Value)
468+
directivePrefix, hasPath := directiveImports[path]
469+
if hasPath && astutil.HasDirectivePrefix(file, directivePrefix) {
470+
delete(unused, name)
471+
if len(unused) == 0 {
472+
return
473+
}
474+
break
475+
}
476+
}
477+
421478
// Remove all unused import specifications
422479
isUnusedSpec := map[*ast.ImportSpec]bool{}
423480
for _, index := range unused {
@@ -442,9 +499,8 @@ func pruneImports(file *ast.File) {
442499
}
443500

444501
// finalizeRemovals fully removes any declaration, specification, imports
445-
// that have been set to nil. This will also remove the file's top-level
446-
// comment group to remove any unassociated comments, including the comments
447-
// from removed code.
502+
// that have been set to nil. This will also remove any unassociated comment
503+
// groups, including the comments from removed code.
448504
func finalizeRemovals(file *ast.File) {
449505
fileChanged := false
450506
for i, decl := range file.Decls {
@@ -487,8 +543,18 @@ func finalizeRemovals(file *ast.File) {
487543
if fileChanged {
488544
file.Decls = astutil.Squeeze(file.Decls)
489545
}
546+
490547
file.Imports = astutil.Squeeze(file.Imports)
491-
file.Comments = nil
548+
549+
file.Comments = nil // clear this first so ast.Inspect doesn't walk it.
550+
remComments := []*ast.CommentGroup{}
551+
ast.Inspect(file, func(n ast.Node) bool {
552+
if cg, ok := n.(*ast.CommentGroup); ok {
553+
remComments = append(remComments, cg)
554+
}
555+
return true
556+
})
557+
file.Comments = remComments
492558
}
493559

494560
// Options controls build process behavior.

‎build/build_test.go

Lines changed: 106 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,36 @@ func TestOverlayAugmentation(t *testing.T) {
371371
`Sort`: {},
372372
`Equal`: {},
373373
},
374+
}, {
375+
desc: `remove unsafe and embed if not needed`,
376+
src: `import "unsafe"
377+
import "embed"
378+
379+
//gopherjs:purge
380+
var eFile embed.FS
381+
382+
//gopherjs:purge
383+
func SwapPointer(addr *unsafe.Pointer, new unsafe.Pointer) (old unsafe.Pointer)`,
384+
want: ``,
385+
expInfo: map[string]overrideInfo{
386+
`SwapPointer`: {},
387+
`eFile`: {},
388+
},
389+
}, {
390+
desc: `keep unsafe and embed for directives`,
391+
src: `import "unsafe"
392+
import "embed"
393+
394+
//go:embed hello.txt
395+
var eFile embed.FS
396+
397+
//go:linkname runtimeNano runtime.nanotime
398+
func runtimeNano() int64`,
399+
noCodeChange: true,
400+
expInfo: map[string]overrideInfo{
401+
`eFile`: {},
402+
`runtimeNano`: {},
403+
},
374404
},
375405
}
376406

@@ -539,14 +569,18 @@ func TestOriginalAugmentation(t *testing.T) {
539569
`Equal`: {},
540570
},
541571
src: `import "cmp"
572+
573+
// keeps the isOnlyImports from skipping what is being tested.
574+
func foo() {}
542575
543576
type Pointer[T any] struct {}
544577
545578
func Sort[S ~[]E, E cmp.Ordered](x S) {}
546579
547580
// overlay had stub "func Equal() {}"
548581
func Equal[S ~[]E, E any](s1, s2 S) bool {}`,
549-
want: ``,
582+
want: `// keeps the isOnlyImports from skipping what is being tested.
583+
func foo() {}`,
550584
}, {
551585
desc: `purge generics`,
552586
info: map[string]overrideInfo{
@@ -556,6 +590,9 @@ func TestOriginalAugmentation(t *testing.T) {
556590
},
557591
src: `import "cmp"
558592
593+
// keeps the isOnlyImports from skipping what is being tested.
594+
func foo() {}
595+
559596
type Pointer[T any] struct {}
560597
func (x *Pointer[T]) Load() *T {}
561598
func (x *Pointer[T]) Store(val *T) {}
@@ -564,12 +601,78 @@ func TestOriginalAugmentation(t *testing.T) {
564601
565602
// overlay had stub "func Equal() {}"
566603
func Equal[S ~[]E, E any](s1, s2 S) bool {}`,
567-
want: ``,
604+
want: `// keeps the isOnlyImports from skipping what is being tested.
605+
func foo() {}`,
568606
}, {
569607
desc: `prune an unused import`,
570608
info: map[string]overrideInfo{},
571-
src: `import foo "some/other/bar"`,
609+
src: `import foo "some/other/bar"
610+
611+
// keeps the isOnlyImports from skipping what is being tested.
612+
func foo() {}`,
613+
want: `// keeps the isOnlyImports from skipping what is being tested.
614+
func foo() {}`,
615+
}, {
616+
desc: `override signature of function`,
617+
info: map[string]overrideInfo{
618+
`Foo`: {
619+
overrideSignature: srctesting.ParseFuncDecl(t,
620+
`package whatever
621+
func Foo(a, b any) (any, bool) {}`),
622+
},
623+
},
624+
src: `func Foo[T comparable](a, b T) (T, bool) {
625+
if a == b {
626+
return a, true
627+
}
628+
return b, false
629+
}`,
630+
want: `func Foo(a, b any) (any, bool) {
631+
if a == b {
632+
return a, true
633+
}
634+
return b, false
635+
}`,
636+
}, {
637+
desc: `override signature of method`,
638+
info: map[string]overrideInfo{
639+
`Foo.Bar`: {
640+
overrideSignature: srctesting.ParseFuncDecl(t,
641+
`package whatever
642+
func (r *Foo) Bar(a, b any) (any, bool) {}`),
643+
},
644+
},
645+
src: `func (r *Foo[T]) Bar(a, b T) (T, bool) {
646+
if r.isSame(a, b) {
647+
return a, true
648+
}
649+
return b, false
650+
}`,
651+
want: `func (r *Foo) Bar(a, b any) (any, bool) {
652+
if r.isSame(a, b) {
653+
return a, true
654+
}
655+
return b, false
656+
}`,
657+
}, {
658+
desc: `empty file removes all imports`,
659+
info: map[string]overrideInfo{
660+
`foo`: {},
661+
},
662+
src: `import . "math/rand"
663+
func foo() int {
664+
return Int()
665+
}`,
572666
want: ``,
667+
}, {
668+
desc: `empty file with directive`,
669+
info: map[string]overrideInfo{
670+
`foo`: {},
671+
},
672+
src: `//go:linkname foo bar
673+
import _ "unsafe"`,
674+
want: `//go:linkname foo bar
675+
import _ "unsafe"`,
573676
},
574677
}
575678

‎compiler/astutil/astutil.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"reflect"
1010
"regexp"
1111
"strconv"
12+
"strings"
1213
)
1314

1415
func RemoveParens(e ast.Expr) ast.Expr {
@@ -148,6 +149,24 @@ func Purge(d ast.Node) bool {
148149
return hasDirective(d, `purge`)
149150
}
150151

152+
// OverrideSignature returns true if gopherjs:override-signature directive is
153+
// present on a function.
154+
//
155+
// `//gopherjs:override-signature` is a GopherJS-specific directive, which can
156+
// be applied in native overlays and will instruct the augmentation logic to
157+
// replace the original function signature which has the same FuncKey with the
158+
// signature defined in the native overlays.
159+
// This directive can be used to remove generics from a function signature or
160+
// to replace a receiver of a function with another one. The given native
161+
// overlay function will be removed, so no method body is needed in the overlay.
162+
//
163+
// The new signature may not contain types which require a new import since
164+
// the imports will not be automatically added when needed, only removed.
165+
// Use a type alias in the overlay to deal manage imports.
166+
func OverrideSignature(d *ast.FuncDecl) bool {
167+
return hasDirective(d, `override-signature`)
168+
}
169+
151170
// directiveMatcher is a regex which matches a GopherJS directive
152171
// and finds the directive action.
153172
var directiveMatcher = regexp.MustCompile(`^\/(?:\/|\*)gopherjs:([\w-]+)`)
@@ -179,6 +198,19 @@ func hasDirective(node ast.Node, directiveAction string) bool {
179198
return foundDirective
180199
}
181200

201+
// HasDirectivePrefix determines if any line in the given file
202+
// has the given directive prefix in it.
203+
func HasDirectivePrefix(file *ast.File, prefix string) bool {
204+
for _, cg := range file.Comments {
205+
for _, c := range cg.List {
206+
if strings.HasPrefix(c.Text, prefix) {
207+
return true
208+
}
209+
}
210+
}
211+
return false
212+
}
213+
182214
// FindLoopStmt tries to find the loop statement among the AST nodes in the
183215
// |stack| that corresponds to the break/continue statement represented by
184216
// branch.

0 commit comments

Comments
 (0)