-
Notifications
You must be signed in to change notification settings - Fork 18k
syntax: parser doesn't recognize valid type parameter list #49482
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
Comments
Another work-around for the single type-parameter case would be a trailing comma, as in type T[P *int,] struct{} because type parameter lists (like all parameter lists) permit a trailing comma, while array type declarations don't. |
I assume that this also works: type T[P (*int)] struct{} |
It should but currently doesn't, it appears. Filed a separate issue #49485. |
It turns out that using parentheses does not work in general. |
Change https://golang.org/cl/370774 mentions this issue: |
Change https://golang.org/cl/372874 mentions this issue: |
…isely The new description matches the implementation (CL 370774). Also, in the section on type constraints, use "defines" instead of "determines" because the constraint interface defines the type set which is precisely the set of acceptable type arguments. For #49482. Change-Id: I6f30f49100e8ba8bec0a0f1b450f88cae54312eb Reviewed-on: https://go-review.googlesource.com/c/go/+/372874 Trust: Robert Griesemer <[email protected]> Run-TryBot: Robert Griesemer <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
Change https://go.dev/cl/385575 mentions this issue: |
Reopening as while auditing our port sheet I realized this fix did not get ported to go/parser. I have mailed the port in https://go.dev/cl/385575. Tentatively marking as a release blocker: the original bug may not have been a release blocker, but it seems like a release blocker that the compiler accepts these expressions and go/parser does not, even if they will be uncommon in practice. |
CC @golang/release @mdempsky As mentioned above, I think this may be a release blocker. Since the CL is mailed, I have added the release-blocker label in hopes that it can get merged before the RC1. Please reach out to me if any concerns. |
While working on the port, I realized that there are valid cases that both parsers can't currently handle. For example:
Example 1 is parsed as an array type, though it can be unambiguously parsed as a generic type declaration. Example 2 is a parser error. I have a fix for both cases, but since this is not trivial I think it must wait until Go 1.19. I believe the CL above makes go/parser bug-compatible with the compiler. |
Change https://go.dev/cl/385756 mentions this issue: |
Change https://go.dev/cl/385757 mentions this issue: |
This is a port of CL 370774 to go/parser and go/printer. It is adjusted for the slightly different factoring of parameter list parsing and printing in go/parser and go/printer. For #49482 Change-Id: I1c5b1facddbfcb7f7b2be356c817fc7e608223f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/385575 Trust: Robert Findley <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]>
Thanks to @mdempsky's help, we landed the fix to make |
Add tests that verify consistent behavior of go/types and types2 with respect to potentially ambiguous type parameter lists. For #49482 Change-Id: I3386d4fa3eb91f2a8ea0987372ca40a6962de886 Reviewed-on: https://go-review.googlesource.com/c/go/+/385756 Trust: Robert Findley <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]>
Change https://go.dev/cl/402255 mentions this issue: |
Change https://go.dev/cl/402256 mentions this issue: |
Accept ~x as ordinary unary expression in the parser but recognize such expressions as invalid in the type checker. This change opens the door to recognizing complex type constraint literals such as `*E|~int` in `[P *E|~int]` and parse them correctly instead of reporting a parse error because `P*E|~int` syntactically looks like an incorrect array length expression (binary expression where the RHS of | is an invalid unary expression ~int). As a result, the parser is more forgiving with expressions but the type checker will reject invalid uses as before. We could pass extra information into the binary/unary expression parse functions to prevent the use of ~ in invalid situations but it doesn't seem worth the trouble. In fact it may be advantageous to allow a more liberal expression syntax especially in the presence of errors (better parser synchronization after an error). Preparation for fixing #49482. Change-Id: I119e8bd9445dfa6460fcd7e0658e3554a34b2769 Reviewed-on: https://go-review.googlesource.com/c/go/+/402255 Run-TryBot: Robert Griesemer <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> Auto-Submit: Robert Griesemer <[email protected]>
Change https://go.dev/cl/404397 mentions this issue: |
…erals Without this change, the type parameter list "[P T | T]" is printed as "[P T | T,]" in an attempt to avoid an ambiguity. But the type parameter P cannot syntactically combine with the constraint T | T and make a new valid expression. This change introduces a specific combinesWithName predicate that reports whether a constraint expression can combine with a type parameter name to form a new valid (value) expression. Use combinesWithName to accurately determine when a comma is needed. For #49482. Change-Id: Id1d17a18f0c9af04495da7b0453e83798f32b04a Reviewed-on: https://go-review.googlesource.com/c/go/+/404397 Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Robert Findley <[email protected]>
There is a (known) ambiguity for type parameter lists. For
the parser cannot tell if this is a generic type declaration or an array type declaration with length
P*int
. In general, people will write~*int
and there is the work-aroundinterface{*int}
; one just has to be aware of it.But the parser also assumes that
is starting an array type declaration, yet this is clearly a valid type parameter list.
Should be fixed for 1.18 but is not a release blocker as there are work-arounds.
cc: @findleyr @ianlancetaylor
The text was updated successfully, but these errors were encountered: