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,cmd/compile/internal/types2: type instantiation fails to distinguish similarly named defined types within a package #46592

Closed
mdempsky opened this issue Jun 5, 2021 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mdempsky
Copy link
Member

mdempsky commented Jun 5, 2021

This program is invalid, but go/types returns success for type checking it: https://go2goplay.golang.org/p/F-SWVR4f2dN (Note: The go2go diagnostic is only produced if/after type checking succeeded; contrast https://go2goplay.golang.org/p/o5NZXojNhMX.)

This appears to be because the string produced by instantiatedHash doesn't distinguish package-scope defined types from function-scope defined ones. In CL 324573, I've workaround around this by hacking writeTypeName to include TypeName.Pos when writing type names for instiantiatedHash:

https://go-review.googlesource.com/c/go/+/324573/8/src/cmd/compile/internal/types2/subst.go
https://go-review.googlesource.com/c/go/+/324573/8/src/cmd/compile/internal/types2/typestring.go

With that change, unified IR is able to successfully compile this test case that exercises a bunch of nested type parameters and similarly named defined types: https://go-review.googlesource.com/c/go/+/324573/8/test/typeparam/nested.go

/cc @griesemer @findleyr

@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 5, 2021
@mdempsky mdempsky added this to the Go1.18 milestone Jun 5, 2021
@griesemer griesemer self-assigned this Jun 6, 2021
@griesemer
Copy link
Contributor

instantiatedHash is incorrect for local types (and the comment in the source says so).

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/327170 mentions this issue: [dev.typeparams] cmd/compile/internal/types2: support local defined types

gopherbot pushed a commit that referenced this issue Jun 16, 2021
…ypes

This CL changes types2's instance hashing logic to include position
information for function-scope defined types as disambiguation. This
isn't ideal, but it worked for getting nested.go passing.

Updates #46592.

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

@mdempsky Is there anything left to do here (besides a clearer error message)?

For the example

package main

type T[_ any] int

type X int

func main() {
	type GlobalX = X
	type X int

	var _ T[X] = T[GlobalX](0)
}

the compiler now produces:

x.go:11:15: incompatible type: cannot use T[GlobalX](0) (constant 0 of type T[X]) as T[X] value

The error message remains confusing as the types appear to be the same. I filed #48295 for that.

@mdempsky
Copy link
Member Author

mdempsky commented Sep 9, 2021

Nope, sounds like this is fixed, and I think covered by test/typeparam/nested.go. Thanks for filing #48295.

@mdempsky mdempsky closed this as completed Sep 9, 2021
@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.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants