Skip to content

Handle binder type symbols in extractor patterns #4056

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

Merged
merged 1 commit into from
Mar 8, 2018

Conversation

liufengyun
Copy link
Contributor

@liufengyun liufengyun commented Mar 1, 2018

Handle binder type symbols in extractor patterns. Previously the following code will result in two warnings for ConstNullClass and Identity because of the interpolated type symbols _$1, _$2 and T, U don't match:

enum Fun[-T, +U >: Null] {
  def f: T => U = this match {
    case Identity(g) => g
    case ConstNull => (_ => null)
    case ConstNullClass(y) => (_ => null)
    case ConstNullSimple => null
  }

  case Identity[T, U >: Null](g: T => U) extends Fun[T, U]
  case ConstNull
  case ConstNullClass(x: T)
  case ConstNullSimple
}

This PR fixes this problem.

else
Prod(pat.tpe.stripAnnots, fun.tpe, fun.symbol, pats.map(project), irrefutable(fun))
Prod(erase(pat.tpe.stripAnnots), fun.tpe, fun.symbol, pats.map(project), irrefutable(fun))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be sure: basically this throws away _$1 and _$2, which is fine now but might be less fine if we had type arguments for extractor patterns (which at least I'd like eventually?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we check according to semantics, I think it's correct. I cannot come up with a counter-example. If some type cannot be checked at run-time, they get a warning instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; since type arguments to classes/traits are erased at runtime, the only use case is for annotating what GADT inference should deduce or for binding type variables generated by GADT inference. For instance:

trait Exp[T]
case class[A, B])[f: Exp[A => B])
def eval[T](e: Exp[T])(env: Env): T = e match {
    case Fun[a, b](x, body) =>
      (v: a) => body.eval(env + (x -> v))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the example doesn't compile at all, despite any repairing efforts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned upfront the issue is with a potential future feature, so of course my example doesn't work yet:

if we had type arguments for extractor patterns (which at least I'd like eventually?)

Copy link
Contributor

@Blaisorblade Blaisorblade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still uneasy with the strategy, but I guess I should add support for type arguments for extractor patterns before that becomes an issue.

The rest looks reasonable enough.

@Blaisorblade Blaisorblade merged commit cfef000 into scala:master Mar 8, 2018
@Blaisorblade Blaisorblade deleted the gadt-check branch March 8, 2018 20:16
@Blaisorblade Blaisorblade changed the title handle binder type symbols in extractor patterns Handle binder type symbols in extractor patterns Mar 8, 2018
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.

2 participants