From 0f26bbae8fa22272ca31ed56647de3b327dbfd57 Mon Sep 17 00:00:00 2001
From: Alan Donovan <adonovan@google.com>
Date: Thu, 13 Jun 2013 17:31:32 -0400
Subject: [PATCH] go.tools/ssa: fix bug in code emitted for ast.TypeAssertExpr.

var x I = ...
x.(E) may fail dynamically (iff x is nil).

Added a testcase.

R=gri
CC=golang-dev
https://golang.org/cl/10237045
---
 ssa/emit.go                     |  5 ++---
 ssa/interp/interp.go            |  6 +++++-
 ssa/interp/testdata/coverage.go | 20 ++++++++++++++++++++
 ssa/ssa.go                      |  7 +++----
 4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/ssa/emit.go b/ssa/emit.go
index d91e435c..943b27f7 100644
--- a/ssa/emit.go
+++ b/ssa/emit.go
@@ -247,9 +247,8 @@ func emitTypeAssert(f *Function, x Value, t types.Type, pos token.Pos) Value {
 	// Simplify infallible assertions.
 	txi := x.Type().Underlying().(*types.Interface)
 	if ti, ok := t.Underlying().(*types.Interface); ok {
-		if types.IsIdentical(ti, txi) {
-			return x
-		}
+		// Even when ti==txi, we still need ChangeInterface
+		// since it performs a nil-check.
 		if isSuperinterface(ti, txi) {
 			c := &ChangeInterface{X: x}
 			c.setPos(pos)
diff --git a/ssa/interp/interp.go b/ssa/interp/interp.go
index 583b3420..95fdb709 100644
--- a/ssa/interp/interp.go
+++ b/ssa/interp/interp.go
@@ -161,7 +161,11 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation {
 		fr.env[instr] = call(fr.i, fr, instr.Pos(), fn, args)
 
 	case *ssa.ChangeInterface:
-		fr.env[instr] = fr.get(instr.X) // (can't fail)
+		x := fr.get(instr.X)
+		if x.(iface).t == nil {
+			panic(fmt.Sprintf("interface conversion: interface is nil, not %s", instr.Type()))
+		}
+		fr.env[instr] = x
 
 	case *ssa.ChangeType:
 		fr.env[instr] = fr.get(instr.X) // (can't fail)
diff --git a/ssa/interp/testdata/coverage.go b/ssa/interp/testdata/coverage.go
index 03e14275..1a9685fb 100644
--- a/ssa/interp/testdata/coverage.go
+++ b/ssa/interp/testdata/coverage.go
@@ -329,6 +329,26 @@ func init() {
 	}
 }
 
+// An I->I conversion always succeeds.
+func init() {
+	var x I
+	if I(x) != I(nil) {
+		panic("I->I conversion failed")
+	}
+}
+
+// An I->I type-assert fails iff the value is nil.
+func init() {
+	defer func() {
+		r := recover()
+		if r != "interface conversion: interface is nil, not main.I" {
+			panic("I->I type assertion succeeed for nil value")
+		}
+	}()
+	var x I
+	_ = x.(I)
+}
+
 //////////////////////////////////////////////////////////////////////
 // Variadic bridge methods and interface thunks.
 
diff --git a/ssa/ssa.go b/ssa/ssa.go
index 12d3eb85..db2ecb3f 100644
--- a/ssa/ssa.go
+++ b/ssa/ssa.go
@@ -588,16 +588,15 @@ type Convert struct {
 // ChangeInterface constructs a value of one interface type from a
 // value of another interface type known to be assignable to it.
 //
-// This operation cannot fail.  Use TypeAssert for interface
-// conversions that may fail dynamically.
+// This operation fails if the operand is nil.
+// For all other operands, well-typedness ensures success.
+// Use TypeAssert for interface conversions that are uncertain.
 //
 // Pos() returns the ast.CallExpr.Lparen if the instruction arose from
 // an explicit T(e) conversion; the ast.TypeAssertExpr.Lparen if the
 // instruction arose from an explicit e.(T) operation; or token.NoPos
 // otherwise.
 //
-// TODO(adonovan) what about the nil case of e = e.(E)?  Test.
-//
 // Example printed form:
 // 	t1 = change interface interface{} <- I (t0)
 //