New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: inconsistent behaviors in judging whether or not two types are identical #24721

Open
dotaheor opened this Issue Apr 6, 2018 · 10 comments

Comments

Projects
None yet
6 participants
@dotaheor

dotaheor commented Apr 6, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10.1 linux/amd64

Does this issue reproduce with the latest release?

yes

What did you do?

package m3

import "time"

type t = time.Time
type T = time.Time

type A = struct{t}
type B = struct{T}
import "m3"
import "time"

type t = time.Time
type T = time.Time

type A = struct{t}
type B = struct{T}

func main() {
   var a, b interface{} = m3.A{}, m3.B{}
   var x, y interface{} = A{}, B{}
   println(a == x) // true
   println(b == y) // true
   
   _ = A(m3.A{}) // cannot convert struct { time.Time } literal (type struct { time.Time }) to type struct { time.Time }
   _ = B(m3.B{}) // ok
   
   println(a == b) // true
   println(x == y) // true
   _ = B(A{})       // cannot convert struct { time.Time } literal (type struct { time.Time }) to type struct { time.Time }
   _ = m3.A(m3.B{}) // cannot convert struct { time.Time } literal (type struct { time.Time }) to type struct { time.Time }
}

What did you expect to see?

No errors, [edit: or all println calls, except the second, print false].

What did you see instead?

./main.go:18:9: cannot convert struct { time.Time } literal (type struct { time.Time }) to type struct { time.Time }
./main.go:23:9: cannot convert struct { time.Time } literal (type struct { time.Time }) to type struct { time.Time }
./main.go:24:12: cannot convert struct { time.Time } literal (type struct { time.Time }) to type struct { time.Time }
@dotaheor

This comment has been minimized.

dotaheor commented Apr 6, 2018

btw, should struct{t} and struct{T} be viewed as the same type?

@dotaheor

This comment has been minimized.

dotaheor commented Apr 6, 2018

The following program print the same output for the two types:

package main

import "fmt"
import "time"
import "reflect"

type t = time.Time
type T = time.Time

type A = struct{t}
type B = struct{T}

func main() {
	t := reflect.TypeOf(A{}) // the Singer type
	fmt.Println(t, "has", t.NumField(), "fields:")
	for i := 0; i < t.NumField(); i++ {
		fmt.Print("   filed#", i, ": ", t.Field(i).Name, "\n")
	}
	
	/*
	struct { time.Time } has 1 fields:
	filed#0: t
	*/
	
	t = reflect.TypeOf(B{}) // the Singer type
	fmt.Println(t, "has", t.NumField(), "fields:")
	for i := 0; i < t.NumField(); i++ {
		fmt.Print("   filed#", i, ": ", t.Field(i).Name, "\n")
	}
	
	/*
	struct { time.Time } has 1 fields:
	filed#0: t
	*/
}

I feel the outputs are wrong, at least the outputs should be filed#0: Time.

But I change the type A and B to non-alias types, then the outputs will be different
(I think the outputs are correct):

main.A has 1 fields:
   filed#0: t
main.B has 1 fields:
   filed#0: T
@bcmills

This comment has been minimized.

Member

bcmills commented Apr 6, 2018

A simpler repro does not require separate packages: https://play.golang.org/p/_ZimDvvVU0u

The compiler is correct to emit an error, because the fields of the two struct types have different names (t and T, respectively).
The fact that the error does not include the correct field names is a bug.

@bcmills bcmills changed the title from cmd/compile: false positive error to cmd/compile: misleading error message for struct assignment with different embedded field names of the same type Apr 6, 2018

@bcmills bcmills added this to the Go1.11 milestone Apr 6, 2018

@bcmills bcmills added the NeedsFix label Apr 6, 2018

@bcmills

This comment has been minimized.

Member

bcmills commented Apr 6, 2018

The fix is presumably to use the name of the field in the error message instead of the name of the underlying type.

@bcmills

This comment has been minimized.

Member

bcmills commented Apr 6, 2018

(CC @griesemer, maybe?)

@dotaheor

This comment has been minimized.

dotaheor commented Apr 7, 2018

If the error messages are correct, the problem is bigger:

package main

import (
	"fmt"
	"time"
)

type t = time.Time
type T = time.Time

type A = struct{ t }
type B = struct{ T }

func main() {
	var a A
	var b B
	fmt.Println(interface{}(b) == interface{}(a)) // true
	//fmt.Println(b == a)
}
@bcmills

This comment has been minimized.

Member

bcmills commented Apr 7, 2018

Huh, that's a neat bug you've found!
(https://play.golang.org/p/LSwVZ_aoNmc)

@jimmyfrasche

This comment has been minimized.

Member

jimmyfrasche commented Apr 7, 2018

@dotaheor dotaheor changed the title from cmd/compile: misleading error message for struct assignment with different embedded field names of the same type to cmd/compile: inconsistent behaviors in judging whether or not two types are identical Apr 8, 2018

@griesemer

This comment has been minimized.

Contributor

griesemer commented Apr 9, 2018

  1. Two struct types with differently named (corresponding) embedded fields are different types, even if the embedded field types are identical. So the assignments a = b are expected to be reported as invalid at compile time.

  2. Because of 1), runtime interface comparison of a and b values should return false, but the issue is that (basically) the same printing code that's used in the error message (and which prints both types as struct{ int }) is used to identify those types' symbols, so the type information is incorrect.

@griesemer

This comment has been minimized.

Contributor

griesemer commented Dec 5, 2018

Too late for 1.12.

@griesemer griesemer modified the milestones: Go1.12, Go1.13 Dec 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment