-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Require type var splats in included/inherited generic to match splats in definition #10240
base: master
Are you sure you want to change the base?
Require type var splats in included/inherited generic to match splats in definition #10240
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
|
||
case {type_vars.any?(TypeSplat), splat_index} | ||
in {true, Int32} | ||
# node is `(A, B, C, *D, E)`, definition is `(T, *U, V, W)`; *D would splat into V |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain this a bit more? Aren't all types on the left hand side need to be know to know how to splat them? I don't understand why D would splat only into V, which is not a splat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The arguments before and after the splat in the instantiation are always matched first, so A
, B
, and C
would match T
and *U
, followed by E
with W
, and finally *D
with *U
and V
, which would be disallowed regardless of what types the TypeParameter
s and TypeSplat
s have. Thus the definition must not have more than 3 non-splat parameters before *U
or more than 1 non-splat parameter after *U
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the PR description it says:
class Foo(T, *U, V); end
class Bar(*A) < Foo(*A, Int32, Int32); end # error, *A splats into T
But if I do:
Bar(Int32, Int32, Int32).new
then we get:
Foo(Int32, Int32, Int32, Int32, Int32)
and for:
Foo(T, *U, V)
we get:
- T: Int32
- U: Int32, Int32, Int32
- V: Int32
So that shouldn't be an error?
That said, I'm sure I'm misunderstanding something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess what I'm saying is... don't we need to expand/collect all the types on node
and then match that against the definition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In light of #3649 (comment), maybe this restriction isn't really necessary...
Resolves #3649.
Given a generic definition and a generic inheritance (or include), there are 4 distinct cases to consider:
Neither the superclass expression nor the definition includes a splat. The number of type arguments must match exactly. This is already covered.
The definition contains a splat, but the superclass expression doesn't. The number of type arguments must not be less than the number of non-splat type vars in the definition. This is also covered. (Note that the definition can only use at most one splat, but the superclass can use any number of splats, including splats of things other than type parameters.)
If the definition doesn't use a splat, any type var splat in the superclass expression now results in a compile-time error.
The definition has a splat, and at least one type var splat is also given in the superclass expression. If there are
m
type vars before the splat inFoo
's definition andn
type vars after the splat, then the firstm
and the lastn
type arguments of any included/inheritedFoo
must not contain any type var splats. The remaining type arguments will be collected by the splat parameter.