Skip to content

Commit

Permalink
runtime: don't say "different packages" if they may not be different
Browse files Browse the repository at this point in the history
Fix the panic message produced for an interface conversion error to
only say "types from different packages" if they are definitely from
different packges. If they may be from the same package, say "types
from different scopes."

Updates #18911
Fixes #26094

Change-Id: I0cea50ba31007d88e70c067b4680009ede69bab9
Reviewed-on: https://go-review.googlesource.com/123395
Reviewed-by: Austin Clements <austin@google.com>
  • Loading branch information
ianlancetaylor committed Jul 11, 2018
1 parent 12ed0dd commit cda1947
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 24 deletions.
33 changes: 19 additions & 14 deletions src/runtime/error.go
Expand Up @@ -19,32 +19,37 @@ type Error interface {

// A TypeAssertionError explains a failed type assertion.
type TypeAssertionError struct {
interfaceString string
concreteString string
assertedString string
missingMethod string // one method needed by Interface, missing from Concrete
_interface *_type
concrete *_type
asserted *_type
missingMethod string // one method needed by Interface, missing from Concrete
}

func (*TypeAssertionError) RuntimeError() {}

func (e *TypeAssertionError) Error() string {
inter := e.interfaceString
if inter == "" {
inter = "interface"
inter := "interface"
if e._interface != nil {
inter = e._interface.string()
}
if e.concreteString == "" {
return "interface conversion: " + inter + " is nil, not " + e.assertedString
as := e.asserted.string()
if e.concrete == nil {
return "interface conversion: " + inter + " is nil, not " + as
}
cs := e.concrete.string()
if e.missingMethod == "" {
msg := "interface conversion: " + inter + " is " + e.concreteString +
", not " + e.assertedString
if e.concreteString == e.assertedString {
msg := "interface conversion: " + inter + " is " + cs + ", not " + as
if cs == as {
// provide slightly clearer error message
msg += " (types from different packages)"
if e.concrete.pkgpath() != e.asserted.pkgpath() {
msg += " (types from different packages)"
} else {
msg += " (types from different scopes)"
}
}
return msg
}
return "interface conversion: " + e.concreteString + " is not " + e.assertedString +
return "interface conversion: " + cs + " is not " + as +
": missing method " + e.missingMethod
}

Expand Down
16 changes: 6 additions & 10 deletions src/runtime/iface.go
Expand Up @@ -41,7 +41,7 @@ func getitab(inter *interfacetype, typ *_type, canfail bool) *itab {
return nil
}
name := inter.typ.nameOff(inter.mhdr[0].name)
panic(&TypeAssertionError{"", typ.string(), inter.typ.string(), name.name()})
panic(&TypeAssertionError{nil, typ, &inter.typ, name.name()})
}

var m *itab
Expand Down Expand Up @@ -82,7 +82,7 @@ finish:
// The cached result doesn't record which
// interface function was missing, so initialize
// the itab again to get the missing function name.
panic(&TypeAssertionError{concreteString: typ.string(), assertedString: inter.typ.string(), missingMethod: m.init()})
panic(&TypeAssertionError{concrete: typ, asserted: &inter.typ, missingMethod: m.init()})
}

// find finds the given interface/type pair in t.
Expand Down Expand Up @@ -245,11 +245,7 @@ func itabsinit() {
// want = the static type we're trying to convert to.
// iface = the static type we're converting from.
func panicdottypeE(have, want, iface *_type) {
haveString := ""
if have != nil {
haveString = have.string()
}
panic(&TypeAssertionError{iface.string(), haveString, want.string(), ""})
panic(&TypeAssertionError{iface, have, want, ""})
}

// panicdottypeI is called when doing an i.(T) conversion and the conversion fails.
Expand All @@ -265,7 +261,7 @@ func panicdottypeI(have *itab, want, iface *_type) {
// panicnildottype is called when doing a i.(T) conversion and the interface i is nil.
// want = the static type we're trying to convert to.
func panicnildottype(want *_type) {
panic(&TypeAssertionError{"", "", want.string(), ""})
panic(&TypeAssertionError{nil, nil, want, ""})
// TODO: Add the static type we're converting from as well.
// It might generate a better error message.
// Just to match other nil conversion errors, we don't for now.
Expand Down Expand Up @@ -516,7 +512,7 @@ func assertI2I(inter *interfacetype, i iface) (r iface) {
tab := i.tab
if tab == nil {
// explicit conversions require non-nil interface value.
panic(&TypeAssertionError{"", "", inter.typ.string(), ""})
panic(&TypeAssertionError{nil, nil, &inter.typ, ""})
}
if tab.inter == inter {
r.tab = tab
Expand Down Expand Up @@ -549,7 +545,7 @@ func assertE2I(inter *interfacetype, e eface) (r iface) {
t := e._type
if t == nil {
// explicit conversions require non-nil interface value.
panic(&TypeAssertionError{"", "", inter.typ.string(), ""})
panic(&TypeAssertionError{nil, nil, &inter.typ, ""})
}
r.tab = getitab(inter, t, false)
r.data = e.data
Expand Down
19 changes: 19 additions & 0 deletions src/runtime/type.go
Expand Up @@ -131,6 +131,25 @@ func (t *_type) name() string {
return s[i+1:]
}

// pkgpath returns the path of the package where t was defined, if
// available. This is not the same as the reflect package's PkgPath
// method, in that it returns the package path for struct and interface
// types, not just named types.
func (t *_type) pkgpath() string {
if u := t.uncommon(); u != nil {
return t.nameOff(u.pkgpath).name()
}
switch t.kind & kindMask {
case kindStruct:
st := (*structtype)(unsafe.Pointer(t))
return st.pkgPath.name()
case kindInterface:
it := (*interfacetype)(unsafe.Pointer(t))
return it.pkgpath.name()
}
return ""
}

// reflectOffs holds type offsets defined at run time by the reflect package.
//
// When a type is defined at run time, its *rtype data lives on the heap.
Expand Down
49 changes: 49 additions & 0 deletions test/fixedbugs/issue26094.go
@@ -0,0 +1,49 @@
// run

// Copyright 2018 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

import "strings"

var X interface{}

type T struct{}

func scopes() {
p, ok := recover().(error)
if ok && strings.Contains(p.Error(), "different scopes") {
return
}
panic(p)
}

func F1() {
type T struct{}
X = T{}
}

func F2() {
type T struct{}
defer scopes()
_ = X.(T)
}

func F3() {
defer scopes()
_ = X.(T)
}

func F4() {
X = T{}
}

func main() {
F1() // set X to F1's T
F2() // check that X is not F2's T
F3() // check that X is not package T
F4() // set X to package T
F2() // check that X is not F2's T
}

0 comments on commit cda1947

Please sign in to comment.