Skip to content

Commit

Permalink
cmd/compile: skip emitting dictionaries with missing method expressions
Browse files Browse the repository at this point in the history
The nounified frontend currently tries to construct dictionaries that
correspond to invalid instantiations (i.e., instantiations T[X] where
X does not satisfy the constraints specified on T's type parameter).
As a consequence, we may fail to find method expressions needed by the
dictionary.

The real fix for this is to avoid creating those dictionaries in the
first place, because they should never actually be needed at runtime.
But that seems scary for a backport: we've repeatedly attempted to
backport generics fixes, which have fixed one issue but introduced
another.

This CL is a minimally invasive solution to #54225, which avoids the
ICE by instead skipping emitting the invalid dictionary. If the
dictionary ends up not being needed (which I believe will always be
the case), then the linker's reachability analysis will simply ignore
its absence.

Or worst case, if the dictionary *is* reachable somehow, we've simply
turned an ICE into a link-time missing symbol failure. That's not
great for user experience, but it seems like a small trade off to
avoid risking breaking any other currently working code.

Updates #54225.

Change-Id: Ic379696079f4729b1dd6a66994a58cca50281a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/429655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
  • Loading branch information
mdempsky committed Sep 19, 2022
1 parent 7234c90 commit d31f850
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
26 changes: 25 additions & 1 deletion src/cmd/compile/internal/noder/stencil.go
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,7 @@ func (g *genInst) getSymForMethodCall(se *ir.SelectorExpr, subst *typecheck.Tsub
// instantiations have been created.
// Also handles writing method expression closures into the dictionaries.
func (g *genInst) finalizeSyms() {
Outer:
for _, d := range g.dictSymsToFinalize {
infoPrint("=== Finalizing dictionary %s\n", d.sym.Name)

Expand Down Expand Up @@ -1858,7 +1859,30 @@ func (g *genInst) finalizeSyms() {
}
}
if !found {
base.Fatalf("method %s on %v not found", bf.name, rcvr)
// We failed to find a method expression needed for this
// dictionary. This may happen because we tried to create a
// dictionary for an invalid instantiation.
//
// For example, in test/typeparam/issue54225.go, we attempt to
// construct a dictionary for "Node[struct{}].contentLen",
// even though "struct{}" does not implement "Value", so it
// cannot actually be used as a type argument to "Node".
//
// The real issue here is we shouldn't be attempting to create
// those dictionaries in the first place (e.g., CL 428356),
// but that fix is scarier for backporting to Go 1.19. Too
// many backport CLs to this code have fixed one issue while
// introducing another.
//
// So as a hack, instead of calling Fatalf, we simply skip
// calling objw.Global below, which prevents us from emitting
// the broken dictionary. The linker's dead code elimination
// should then naturally prune this invalid, unneeded
// dictionary. Worst case, if the dictionary somehow *is*
// needed by the final executable, we've just turned an ICE
// into a link-time missing symbol failure.
infoPrint(" ! abandoning dictionary %v; missing method expression %v.%s\n", d.sym.Name, rcvr, bf.name)
continue Outer
}
}

Expand Down
33 changes: 33 additions & 0 deletions test/typeparam/issue54225.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// run

// Copyright 2022 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package main

func main() {
One[TextValue]()
}

func One[V Value]() { Two[Node[V]]() }

func Two[V interface{ contentLen() int }]() {
var v V
v.contentLen()
}

type Value interface {
Len() int
}

type Node[V Value] struct{}

func (Node[V]) contentLen() int {
var value V
return value.Len()
}

type TextValue struct{}

func (TextValue) Len() int { return 0 }

0 comments on commit d31f850

Please sign in to comment.