Skip to content

go/types, types2: incorrect behavior of API methods on invalid unions #49739

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.

Sign up for GitHub

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

Jump to bottom
Closed
findleyr opened this issue Nov 23, 2021 · 7 comments
Closed

go/types, types2: incorrect behavior of API methods on invalid unions #49739

findleyr opened this issue Nov 23, 2021 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@findleyr
Copy link
Member

Consider the following code:

package p

type A int
type B int
type C interface{string}

type X interface{~A}
type Y interface{~B}
type Z interface{~int; C}

In this example, we have the following surprising behavior:

  • Identical(X.Type().Underlying(), Y.Type().Underlying()) == true
  • Implements(X.Type().Underlying(), Y.Type().Underlying()) == true
  • X.Type().Underlying().(*Interface).typeSet().IsEmpty() == false
  • Identical(X.Type().Underlying(), Z.Type().Underlying)) == false

The type set of X should be empty, per the update to the spec in https://golang.org/cl/362999. Are two interfaces with empty type sets identical? It's not clear to me, but we should be consistent.

CC @griesemer

*found via fuzzing.
@griesemer griesemer self-assigned this Nov 23, 2021
@griesemer griesemer added the Documentation Issues describing a change to documentation. label Nov 23, 2021
@griesemer griesemer added this to the Go1.18 milestone Nov 23, 2021
@griesemer
Copy link
Contributor

Interfaces that define the same type set are identical. This may be too abstract for the spec, so perhaps we can rephrase a bit.

@findleyr
Copy link
Member Author

Ok. But this is not just a documentation issue. There is a bug in our handling of interface{~A}, where A is defined: we raise an error but return an interface that is non-empty but has no concrete types that implement it (only interface types).

@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed Documentation Issues describing a change to documentation. labels Nov 23, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/366278 mentions this issue: cmd/compile/internal/types2: produce empty type set for invalid ~T

gopherbot pushed a commit that referenced this issue Nov 24, 2021
@griesemer
cmd/compile/internal/types2: produce empty type set for invalid ~T
67dd9ee 
If ~T is not permitted because the underlying type of T is not the
same as T, there is no type that satisfies ~T. Besides reporting an
error, also ensure that the corresponding type set is empty.

For #49739.

Change-Id: I127f75f170902e7989f7fe7b352dabda9f72e2a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/366278
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/367197 mentions this issue: go/types: produce empty type set for invalid ~T

gopherbot pushed a commit that referenced this issue Nov 29, 2021
@griesemer
go/types: produce empty type set for invalid ~T
1ab677a 
This is a clean port of CL 366278 from types2 to go/types.

For #49739.

Change-Id: I2e2cb739c02fcc07e012499c7b65b13b057875ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/367197
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
@griesemer
Copy link
Contributor

@findleyr Is there anything left to do here?

@griesemer
Copy link
Contributor

ping

@findleyr
Copy link
Member Author

findleyr commented Jan 7, 2022

Thanks, I think this is done. Closing.

@findleyr findleyr closed this as completed Jan 7, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
3 participants
@griesemer @gopherbot @findleyr