[release-branch.go1.19] cmd/compile: skip emitting dictionaries with missing method expressions

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.

Fixes #55270.

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>
Reviewed-on: https://go-review.googlesource.com/c/go/+/429895
Reviewed-by: Keith Randall <khr@google.com>

2 files changed, 58 insertions, 1 deletions

diff --git a/src/cmd/compile/internal/noder/stencil.go b/src/cmd/compile/internal/noder/stencil.go
index 6fcb31b472..73728d5e16 100644
--- a/src/cmd/compile/internal/noder/stencil.go
+++ b/src/cmd/compile/internal/noder/stencil.go
@@ -1787,6 +1787,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)
 
@@ -1856,7 +1857,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
 			}
 		}
 
diff --git a/test/typeparam/issue54225.go b/test/typeparam/issue54225.go
new file mode 100644
index 0000000000..4de3efcaaf
--- /dev/null
+++ b/test/typeparam/issue54225.go
@@ -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 }