-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Handle dependent context functions #18443
Handle dependent context functions #18443
Conversation
0f39f38
to
408c92c
Compare
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.
Otherwise LGTM
*/ | ||
def unapply(ft: Type)(using Context): Option[MethodOrPoly] = { | ||
ft match | ||
case RefinedType(parent, nme.apply, mt: MethodOrPoly) |
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'd try to optimize it further by inlining the FunctionOf extractor and specializing it to this use case.
Also, maybe split the RefinedType
match into two, one for MethodType
refinements and the other for PolyType
refinements?
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.
maybe split the RefinedType match into two, one for MethodType refinements and the other for PolyType refinements?
We would end up with these cases
ft match
case RefinedType(parent, nme.apply, mt: MethodType)
if parent.derivesFrom(defn.PolyFunctionClass) || isFunctionNType(parent) =>
Some(mt)
case RefinedType(parent, nme.apply, pt: PolyType)
if parent.derivesFrom(defn.PolyFunctionClass) =>
Some(mt)
...
Note that in both cases we can have PolyFunctionClass
because we now use PolyFunction
for function types with erased parameters.
If it is for performance, we could do
ft match
case RefinedType(parent, nme.apply, mt: MethodOrPoly)
if parent.derivesFrom(defn.PolyFunctionClass) || (mt.isInstanceOf[MethodType] && isFunctionNType(parent)) =>
Some(mt)
...
I pushed this last option.
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.
408c92c
to
4df0b57
Compare
65f6dfa
to
12f5442
Compare
@nicolasstucki Can you have a look at the test failure? happy to review again when it is green. |
Add `FunctionTypeOfMethod` extractor that matches any kind of function and return its method type. We use this extractor instead of `ContextFunctionType` to all of * `ContextFunctionN[...]` * `ContextFunctionN[...] { def apply(using ...): R }` where `R` might be dependent on the parameters. * `PolyFunction { def apply(using ...): R }` where `R` might be dependent on the parameters. Currently this one would have at least one erased parameter.
12f5442
to
d5d8273
Compare
@odersky now the CI passed. |
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.
This will be very handy to have. Sorry for dropping the ball on the reviews.
Add
FunctionTypeOfMethod
extractor that matches any kind of function and return its method type.We use this extractor instead of
ContextFunctionType
to all ofContextFunctionN[...]
ContextFunctionN[...] { def apply(using ...): R }
whereR
might be dependent on the parameters.PolyFunction { def apply(using ...): R }
whereR
might be dependent on the parameters. Currently this one would have at least one erased parameter.The naming of the extractor follows the idea in #18305 (comment).