Skip to content

Conversation

@nicolasstucki
Copy link
Contributor

This can be encoded with intersection types in the few places where it is relevant

Based on #9818


case ParamRef(lambda, idx) =>
lambda match {
(lambda: @unchecked) match {
Copy link
Contributor Author

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

Copy link
Contributor

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 .

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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 =>

trait ParamRefMethods:
extension (self: ParamRef):
def binder: LambdaType
def binder: MethodType | PolyType | TypeLambda
Copy link
Contributor Author

@nicolasstucki nicolasstucki Sep 24, 2020

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
@nicolasstucki nicolasstucki force-pushed the remove-reflection-lambdatype branch from b962805 to 4a8a1a7 Compare September 24, 2020 14:14
end extension
end RecursiveTypeMethodsImpl

type LambdaType = dotc.core.Types.LambdaType
Copy link
Member

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants