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, types2: bad "invalid composite literal type" error for type parameter with pointer structural type #50833

Closed
mdempsky opened this issue Jan 26, 2022 · 7 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@mdempsky
Copy link
Member

F below is valid non-generic code, so I'd expect G to compile successfully too. However, go/types and types2 currently reject it.

package p

func F() {
        type T = *struct{ f int }
        _ = []T{{f: 1}}
}

func G[T ~*struct{ f int }]() {
        _ = []T{{f: 1}}
}

/cc @griesemer @findleyr @ianlancetaylor

@mdempsky mdempsky added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 26, 2022
@mdempsky mdempsky added this to the Go1.18 milestone Jan 26, 2022
@gopherbot
Copy link

Change https://golang.org/cl/381074 mentions this issue: cmd/compile: support structural typing in unified IR

@gopherbot
Copy link

Change https://golang.org/cl/381075 mentions this issue: go/types, cmd/compile: fix composite literal structural typing

@mdempsky
Copy link
Member Author

CL 381075 appears to be a fairly straightforward fix to this issue, assuming we decide it is an issue.

@griesemer
Copy link
Contributor

I think we probably want to fix this.

gopherbot pushed a commit that referenced this issue Jan 27, 2022
This CL updates unified IR to look at the structural type of a
composite literal type, rather than merely the underlying type, to
determine if it's a structure. This fixes a number of currently
failing regress test cases.

Updates #50833.

Change-Id: I11c040c77ec86c23e8ffefcf1ce1aed548687dc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381074
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
@ianlancetaylor
Copy link
Contributor

I'm fine with fixing this for 1.18 but I also think it's OK to let it wait until 1.19.

I'm a little bit concerned that we are focusing a bit too much on structural types. At this point I expect them to be a tiny corner case. If anything I think we should focus more on making operations work if they are supported by all elements of a type set that does not have a structural type. But that is definitely for 1.19 or 1.20. If we can make that work, then all these structural type details will be subsumed by the more general, and more useful, case.

@griesemer
Copy link
Contributor

The fix does simplify the code and is arguably more regular, so there's that.

@ianlancetaylor
Copy link
Contributor

I'm fine with fixing this for 1.18.

jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
This CL updates unified IR to look at the structural type of a
composite literal type, rather than merely the underlying type, to
determine if it's a structure. This fixes a number of currently
failing regress test cases.

Updates golang#50833.

Change-Id: I11c040c77ec86c23e8ffefcf1ce1aed548687dc5
Reviewed-on: https://go-review.googlesource.com/c/go/+/381074
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
@golang golang locked and limited conversation to collaborators Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

4 participants