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.

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>

diff --git a/src/cmd/compile/internal/noder/stencil.go b/src/cmd/compile/internal/noder/stencil.go
index 5a41d2f1f0f732410dda56b9084a8f89a9bc8f1c..91c0edff96cf3dadf1e2922c56b38e02e761d82f 100644 (file)
--- a/src/cmd/compile/internal/noder/stencil.go
+++ b/src/cmd/compile/internal/noder/stencil.go
@@ -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)
 
@@ -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
                        }
                }
 

diff --git a/test/typeparam/issue54225.go b/test/typeparam/issue54225.go
new file mode 100644 (file)
index 0000000..4de3efc
--- /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 }