Skip to content
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

go/types: failure to reject interface{}(nil) == []int(nil) #28164

Closed
mdempsky opened this issue Oct 11, 2018 · 9 comments
Closed

go/types: failure to reject interface{}(nil) == []int(nil) #28164

mdempsky opened this issue Oct 11, 2018 · 9 comments
Assignees
Milestone

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Oct 11, 2018

// 1
var _ = interface{}(nil) == interface{}([]int(nil))

// 2
var _ = interface{}(nil) == []int(nil)

cmd/compile, gccgo, and go/types all accept 1.

However, only go/types accepts 2. cmd/compile and gccgo reject 2 because []int is not comparable (except directly against nil).

/cc @griesemer

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 11, 2018

It seems to me that this is a bug in go/types: In 2), the left operand is of type interface{} and an []int is assignable to that, so one requirement is satisfied. The assignability is due to the fact that the left operand is an (empty) interface type and []int implements it. I'm not seeing an implicit conversion here. So we have a comparison between an interface and an []int, and []int's are not comparable against interfaces (only nil).

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 11, 2018

That is to say, I don't see the spec issue, but maybe I am missing something.

Loading

@mdempsky
Copy link
Member Author

@mdempsky mdempsky commented Oct 11, 2018

Sorry for not elaborating further in my initial post.

I admit the Go spec strictly only says that the comparison operands must be assignable, but I think the implementations all understand that to mean the assignable operand is implicitly converted to other's type.

Consider interface{}(true) == true, which cmd/compile and gccgo both evaluate to true. The only way this expression makes sense to me is if true is first implicitly converted to interface{}(true), and then the resulting two operands are compared according to the rule for two interface values.

If we assume there's no implicit bool-to-interface conversion, then this is a comparison between an interface value and a boolean value. The spec says when two interface values are equal and when two boolean values are equal, but not when an interface value and a boolean value are equal.

By the same logic, I reason that interface{}(nil) == []int(nil) involves an implicit slice-to-interface conversion; the comparison is handled according to the rules for comparing interface values; and the restrictions about comparing slice values are inapplicable.

Loading

@go101
Copy link

@go101 go101 commented Oct 11, 2018

Go spec says:

In any comparison, the first operand must be assignable to the type of the second operand, or vice versa.

and

Slice, map, and function values are not comparable. However, as a special case, a slice, map, or function value may be compared to the predeclared identifier nil.

So, I looks it is a problem of which of the above rules has a higher priority.

Loading

@griesemer griesemer self-assigned this Oct 11, 2018
@griesemer griesemer added this to the Go1.12 milestone Oct 11, 2018
@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 11, 2018

Marked for 1.12 but not urgent. This has been like this for a while.

Loading

@rsc
Copy link
Contributor

@rsc rsc commented Oct 17, 2018

We've rejected this code literally forever. I don't see the point to starting to accept it when we know it's a malformed test. It seems like we should fix go/types.

Fine to clear up the spec language of course, but it seems like everyone (the compiler authors, and gri above) agrees that what we meant was to reject it.

Loading

@rsc rsc added the NeedsFix label Oct 17, 2018
@rsc rsc changed the title spec: can maps/funcs/slices that are implicitly converted to interface type be compared? spec: clarify invalidity of interface == slice comparison Oct 17, 2018
@deanveloper
Copy link

@deanveloper deanveloper commented Oct 18, 2018

There are three parts of the spec to consider.

In any comparison, the first operand must be assignable to the type of the second operand, or vice versa.

This statement is not an "if and only if", it simply states a "barrier" or sorts to be able to be comparable. The operands must be assignable but that does not mean that all assignable pairs are comparable.

A value x of non-interface type X and a value t of interface type T are comparable when values of type X are comparable and X implements T. They are equal if t's dynamic type is identical to X and t's dynamic value is equal to x.

This states that they are comparable if X is comparable and X implements T. X is a slice in this case, so this is fine.

Slice, map, and function values are not comparable. However, as a special case, a slice, map, or function value may be compared to the predeclared identifier nil.

This says slice, map, and function values are not comparable to anything except the identifier nil. An interface{} value cannot be the predeclared identifier nil, so they are concretely are not comparable.

The spec is 100% correct with what it says. However, it may be good to make it more clear given that this issue was created and there was a bit of debate as to what the spec said.

EDIT - Did not see that this already has NeedsFix, my bad.

Loading

@mdempsky
Copy link
Member Author

@mdempsky mdempsky commented Oct 18, 2018

@deanveloper Thanks for pointing out the rule for comparing mixed interface and non-interface types. I completely missed that.

I withdraw my arguments that there's an implicit conversion, and I agree that go/types is erroneous in accepting example 2.

Loading

@mdempsky mdempsky changed the title spec: clarify invalidity of interface == slice comparison go/types: failure to reject interface{}(nil) == []int(nil) Oct 18, 2018
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 19, 2018

Change https://golang.org/cl/143277 mentions this issue: go/types: fix unsymmetric test when typechecking comparisons

Loading

@gopherbot gopherbot closed this in 34585ba Oct 19, 2018
@golang golang locked and limited conversation to collaborators Oct 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants