go/types, types2: don't panic for invalid assignments of comma-ok expressions

The relevant code was broken with CL 478218. Before that CL,
Checker.assignVar used to return the assigned type, or nil,
in case of failure. Checker.recordCommaOkTypes used to take
two types (not two operands), and if one of those types was
nil, it would simply not record. CL 478218, lost that (nil)
signal.

This change consistently reports an assignment check failure
by setting x.mode to invalid for initVar and assignVar and
then tests if x.mode != invalid before recording a comma-ok
expression.

Fixes #59371.

Change-Id: I193815ff3e4b43e3e510fe25bd0e72e0a6a816c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/486135
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/types2/assignments.go b/src/cmd/compile/internal/types2/assignments.go
index 30ed9ae701ccd471ee14f3e928c04ef2f9ebadde..59f29b8f6202b23fea51672270b25c87d1ce520a 100644 (file)
--- a/src/cmd/compile/internal/types2/assignments.go
+++ b/src/cmd/compile/internal/types2/assignments.go
@@ -17,7 +17,7 @@ import (
 // if necessary by attempting to convert untyped values to the appropriate
 // type. context describes the context in which the assignment takes place.
 // Use T == nil to indicate assignment to an untyped blank identifier.
-// x.mode is set to invalid if the assignment failed.
+// If the assignment check fails, x.mode is set to invalid.
 func (check *Checker) assignment(x *operand, T Type, context string) {
        check.singleValue(x)
 
@@ -135,11 +135,14 @@ func (check *Checker) initConst(lhs *Const, x *operand) {
 // initVar checks the initialization lhs = x in a variable declaration.
 // If lhs doesn't have a type yet, it is given the type of x,
 // or Typ[Invalid] in case of an error.
+// If the initialization check fails, x.mode is set to invalid.
 func (check *Checker) initVar(lhs *Var, x *operand, context string) {
        if x.mode == invalid || x.typ == Typ[Invalid] || lhs.typ == Typ[Invalid] {
                if lhs.typ == nil {
                        lhs.typ = Typ[Invalid]
                }
+               x.mode = invalid
+               return
        }
 
        // If lhs doesn't have a type yet, use the type of x.
@@ -150,6 +153,7 @@ func (check *Checker) initVar(lhs *Var, x *operand, context string) {
                        if typ == Typ[UntypedNil] {
                                check.errorf(x, UntypedNilUse, "use of untyped nil in %s", context)
                                lhs.typ = Typ[Invalid]
+                               x.mode = invalid
                                return
                        }
                        typ = Default(typ)
@@ -227,10 +231,14 @@ func (check *Checker) lhsVar(lhs syntax.Expr) Type {
 
 // assignVar checks the assignment lhs = rhs (if x == nil), or lhs = x (if x != nil).
 // If x != nil, it must be the evaluation of rhs (and rhs will be ignored).
+// If the assignment check fails and x != nil, x.mode is set to invalid.
 func (check *Checker) assignVar(lhs, rhs syntax.Expr, x *operand) {
        T := check.lhsVar(lhs) // nil if lhs is _
        if T == Typ[Invalid] {
                check.use(rhs)
+               if x != nil {
+                       x.mode = invalid
+               }
                return
        }
 
@@ -238,9 +246,6 @@ func (check *Checker) assignVar(lhs, rhs syntax.Expr, x *operand) {
                x = new(operand)
                check.expr(T, x, rhs)
        }
-       if x.mode == invalid {
-               return
-       }
 
        context := "assignment"
        if T == nil {
@@ -396,7 +401,9 @@ func (check *Checker) initVars(lhs []*Var, orig_rhs []syntax.Expr, returnStmt sy
                for i, lhs := range lhs {
                        check.initVar(lhs, rhs[i], context)
                }
-               if commaOk {
+               // Only record comma-ok expression if both initializations succeeded
+               // (go.dev/issue/59371).
+               if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid {
                        check.recordCommaOkTypes(orig_rhs[0], rhs)
                }
                return
@@ -458,7 +465,9 @@ func (check *Checker) assignVars(lhs, orig_rhs []syntax.Expr) {
                for i, lhs := range lhs {
                        check.assignVar(lhs, nil, rhs[i])
                }
-               if commaOk {
+               // Only record comma-ok expression if both assignments succeeded
+               // (go.dev/issue/59371).
+               if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid {
                        check.recordCommaOkTypes(orig_rhs[0], rhs)
                }
                return

diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go
index 71fbbea46ff48433bd4d03c9a547f8f34ff2fe11..f036142caacc039ccae57c8bab3815bdf0f077d4 100644 (file)
--- a/src/go/types/assignments.go
+++ b/src/go/types/assignments.go
@@ -17,7 +17,7 @@ import (
 // if necessary by attempting to convert untyped values to the appropriate
 // type. context describes the context in which the assignment takes place.
 // Use T == nil to indicate assignment to an untyped blank identifier.
-// x.mode is set to invalid if the assignment failed.
+// If the assignment check fails, x.mode is set to invalid.
 func (check *Checker) assignment(x *operand, T Type, context string) {
        check.singleValue(x)
 
@@ -73,6 +73,7 @@ func (check *Checker) assignment(x *operand, T Type, context string) {
                        check.updateExprType(x.expr, newType, false)
                }
        }
+       // x.typ is typed
 
        // A generic (non-instantiated) function value cannot be assigned to a variable.
        if sig, _ := under(x.typ).(*Signature); sig != nil && sig.TypeParams().Len() > 0 {
@@ -133,11 +134,14 @@ func (check *Checker) initConst(lhs *Const, x *operand) {
 // initVar checks the initialization lhs = x in a variable declaration.
 // If lhs doesn't have a type yet, it is given the type of x,
 // or Typ[Invalid] in case of an error.
+// If the initialization check fails, x.mode is set to invalid.
 func (check *Checker) initVar(lhs *Var, x *operand, context string) {
        if x.mode == invalid || x.typ == Typ[Invalid] || lhs.typ == Typ[Invalid] {
                if lhs.typ == nil {
                        lhs.typ = Typ[Invalid]
                }
+               x.mode = invalid
+               return
        }
 
        // If lhs doesn't have a type yet, use the type of x.
@@ -148,6 +152,7 @@ func (check *Checker) initVar(lhs *Var, x *operand, context string) {
                        if typ == Typ[UntypedNil] {
                                check.errorf(x, UntypedNilUse, "use of untyped nil in %s", context)
                                lhs.typ = Typ[Invalid]
+                               x.mode = invalid
                                return
                        }
                        typ = Default(typ)
@@ -225,10 +230,14 @@ func (check *Checker) lhsVar(lhs ast.Expr) Type {
 
 // assignVar checks the assignment lhs = rhs (if x == nil), or lhs = x (if x != nil).
 // If x != nil, it must be the evaluation of rhs (and rhs will be ignored).
+// If the assignment check fails and x != nil, x.mode is set to invalid.
 func (check *Checker) assignVar(lhs, rhs ast.Expr, x *operand) {
        T := check.lhsVar(lhs) // nil if lhs is _
        if T == Typ[Invalid] {
                check.use(rhs)
+               if x != nil {
+                       x.mode = invalid
+               }
                return
        }
 
@@ -236,9 +245,6 @@ func (check *Checker) assignVar(lhs, rhs ast.Expr, x *operand) {
                x = new(operand)
                check.expr(T, x, rhs)
        }
-       if x.mode == invalid {
-               return
-       }
 
        context := "assignment"
        if T == nil {
@@ -394,7 +400,9 @@ func (check *Checker) initVars(lhs []*Var, orig_rhs []ast.Expr, returnStmt ast.S
                for i, lhs := range lhs {
                        check.initVar(lhs, rhs[i], context)
                }
-               if commaOk {
+               // Only record comma-ok expression if both initializations succeeded
+               // (go.dev/issue/59371).
+               if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid {
                        check.recordCommaOkTypes(orig_rhs[0], rhs)
                }
                return
@@ -456,7 +464,9 @@ func (check *Checker) assignVars(lhs, orig_rhs []ast.Expr) {
                for i, lhs := range lhs {
                        check.assignVar(lhs, nil, rhs[i])
                }
-               if commaOk {
+               // Only record comma-ok expression if both assignments succeeded
+               // (go.dev/issue/59371).
+               if commaOk && rhs[0].mode != invalid && rhs[1].mode != invalid {
                        check.recordCommaOkTypes(orig_rhs[0], rhs)
                }
                return

diff --git a/src/internal/types/testdata/fixedbugs/issue59371.go b/src/internal/types/testdata/fixedbugs/issue59371.go
new file mode 100644 (file)
index 0000000..d60810a
--- /dev/null
+++ b/src/internal/types/testdata/fixedbugs/issue59371.go
@@ -0,0 +1,20 @@
+// Copyright 2023 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 p
+
+var m map[int]int
+
+func _() {
+       _, ok /* ERROR "undefined: ok" */ = m[0] // must not crash
+}
+
+func _() {
+       var ok = undef /* ERROR "undefined: undef" */
+       x, ok := m[0] // must not crash
+       _ = x
+       // The next line is only needed for go/types, not types2.
+       // TODO(gri) find cause and fix
+       _ = ok
+}