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.

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

go/types: erroneously accepts recursive type aliases #25141

Closed
mdempsky opened this issue Apr 27, 2018 · 5 comments
Closed

go/types: erroneously accepts recursive type aliases #25141

mdempsky opened this issue Apr 27, 2018 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@mdempsky
Copy link
Contributor

18130-type-alias explicitly states that type T = *T is invalid, but go/types accepts it:

$ cat v.go
package p
type T = *T
$ gotype v.go
$ echo $?
0

cmd/compile does not.

Notably though, I don't see any wording in the text actually added to the Go spec that precludes this.

/cc @griesemer
@mdempsky
Copy link
Contributor Author

Tangentially, I'm questioning what the fundamental difference (if any) is between type T = *T and type U interface { F() interface { U } }. Why disallow one, but allow the other?

We sorta support the latter, but it's sketchy and a recurring thorn. I think if we properly fix support for it, it would make supporting the former trivial. But maybe it's just better to disallow both.

@zigo101
Copy link

zigo101 commented Apr 28, 2018

Looks the difference is whether or not the declaration is an alias declaration.
type U = interface { F() interface { U } } is also denied by gc, but type T *T is also accepted by gc.

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 30, 2018
@andybons andybons added this to the Unplanned milestone Apr 30, 2018
@griesemer griesemer self-assigned this Apr 30, 2018
@griesemer griesemer added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 30, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 30, 2018
@griesemer griesemer modified the milestones: Unplanned, Go1.11 Apr 30, 2018
@griesemer
Copy link
Contributor

Somewhat related: #23139 .

go/types has issues with detecting certain cycles. I hope to address this for 1.11.

Per the original type-alias design doc from @rsc, it must be possible to "expand out" type alias declarations; this wouldn't be possible for type T = *T without expanding endlessly. You are correct that the spec doesn't state that, though. I filed #25187.

I think it would be nice to accept all these, but our depth-first based type-checking approaches have troubles with them; and ad-hoc approaches tend to be incorrect. We need to either disallow these cases or have a sound way to describe what we allow and what we don't, and also a (understood and correct) way of type-checking them w/o incurring undue cost.

At the same time, these are probably not urgent.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/114517 mentions this issue: go/types: initial framework for marking-based cycle detection

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/115455 mentions this issue: go/types: use color-marking based cycle detection at package level

gopherbot pushed a commit that referenced this issue May 31, 2018
@griesemer
go/types: initial framework for marking-based cycle detection
e57cdd8 
The existing code explicitly passes a (type name) path around
to determine cycles; it also restarts the path for types that
"break" a cycle (such as a pointer, function, etc.). This does
not work for alias types (whose cycles are broken in a different
way). Furthermore, because the path is not passed through all
type checker functions that need it, we can't see the path or
use it for detection of some cycles (e.g. cycles involving array
lengths), which required ad-hoc solutions in those cases.

This change introduces an explicit marking scheme for any kind
of object; an object is painted in various colors indicating
its state. It also introduces an object path (a stack) main-
tained with the Checker state, which is available in all type
checker functions that need access to it.

The change only introduces these mechanisms and exercises the
basic functionality, with no effect on the existing code for
now.

For #25141.

Change-Id: I7c28714bdafe6c8d9afedf12a8a887554237337c
Reviewed-on: https://go-review.googlesource.com/114517
Reviewed-by: Alan Donovan <adonovan@google.com>
@golang golang locked and limited conversation to collaborators May 31, 2019
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.11
Development

No branches or pull requests
5 participants
@mdempsky @andybons @griesemer @gopherbot @zigo101