-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove Reflection.LambdaType #9861
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
Remove Reflection.LambdaType #9861
Conversation
@@ -1179,7 +1179,7 @@ class SourceCodePrinter[R <: Reflection & Singleton](val tasty: R)(syntaxHighlig | |||
printType(body) | |||
|
|||
case ParamRef(lambda, idx) => | |||
lambda match { | |||
(lambda: @unchecked) match { |
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.
@liufengyun I was getting the following exhaustivity fatal warning where lambda: MethodType | PolyType | TypeLambda
. Same if did the cases with case lambda: MethodType => ...
. It might be related to having a class tag preforming the type test.
[error] 1182 | lambda match {
[error] | ^^^^^^
[error] |match may not be exhaustive.
[error] |
[error] |It would fail on pattern case: _: MethodType, _: PolyType, _: TypeLambda
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.
Yes, it's unknown that the extractors are irrefutable for the corresponding type MethodType
or PolyType
.
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.
Should we make ClassTag.unapply
(and later TypeTest.unapply
) extractors irrefutable? It feels like they should be.
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.
ClassTag.unapply
is defined as follows:
def unapply(x: Any): Option[T] =
if (runtimeClass.isInstance(x)) Some(x.asInstanceOf[T])
else None
It's not irrefutable in general. Semantically, we may assume it's irrefutable if we know x
takes the type T
.
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.
That is the contract of the method. If x
does not take the type T
then implementation is broken and we cannot reason about the patterns correctly anyway.
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.
Is the following supposed to be irrefutable?
lambda match
case lambda: MethodType =>
case lambda: PolyType =>
case lambda: TypeLambda =>
} | ||
|
||
given ParamRefMethods as ParamRefMethods = ParamRefMethodsImpl | ||
protected val ParamRefMethodsImpl: ParamRefMethods | ||
|
||
trait ParamRefMethods: | ||
extension (self: ParamRef): | ||
def binder: LambdaType | ||
def binder: MethodType | PolyType | TypeLambda |
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.
We could also just use
def binder: Type
In theory, this should only help with pattern matching exhaustivity which currently fails. See other comments.
This can be encoded with intersection types in the few places where it is relevant
b962805
to
4a8a1a7
Compare
@@ -1966,8 +1967,6 @@ class QuoteContextImpl private (ctx: Context) extends QuoteContext: | |||
end extension | |||
end RecursiveTypeMethodsImpl | |||
|
|||
type LambdaType = dotc.core.Types.LambdaType |
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.
What do we gain by removing this? And if a union is preferable but needs to be used in multiple context, maybe we could have type LambdaType = MethodType | PolyType | TypeLambda
?
This can be encoded with intersection types in the few places where it is relevant
Based on #9818