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 <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: griesemer

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,20 +231,21 @@ 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
 	}
 
 	if x == nil {
 		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

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,20 +230,21 @@ 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
 	}
 
 	if x == nil {
 		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

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
+}