Better resolution of implicits in patterns #13807
Conversation
| unapplyApp = typedExpr(untpd.TypedSplice(explicitCall)) | ||
| unapplyApp.tpe | ||
| } { (_, _) => | ||
| // Typing explicitCall may fail if there are implicits that can only |
There was a problem hiding this comment.
This is the core of the change.
| @@ -1373,6 +1397,7 @@ trait Applications extends Compatibility { | |||
| List.fill(argTypes.length - args.length)(WildcardType) | |||
| } | |||
| val unapplyPatterns = bunchedArgs.lazyZip(argTypes) map (typed(_, _)) | |||
| if (unapplyApp == null) unapplyApp = typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) | |||
There was a problem hiding this comment.
Re-infer types after type variables might have been further constrained.
| @@ -834,6 +834,9 @@ class Typer extends Namer | |||
| val expr1 = | |||
| if isWildcard then tree.expr.withType(underlyingTreeTpe.tpe) | |||
| else typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem) | |||
| if (ctx.mode.is(Mode.Pattern)) { | |||
There was a problem hiding this comment.
There must be something smarter here. Currently, pt seems to be nearly unused here, which seems wrong. This check has the side effect of e.g. constraining a type variable T in def unapply[T](...): Option[T] to be a subtype of a type ascription (for example, `case SomeUnapply(x: Foo)'). Needed to pass f242-3.scala. This is the thing I would like to do in a better way.
| @@ -2019,7 +2022,7 @@ class Typer extends Namer | |||
| } | |||
|
|
|||
| def typedBind(tree: untpd.Bind, pt: Type)(using Context): Tree = { | |||
| if !isFullyDefined(pt, ForceDegree.all) then | |||
| if !(ctx.mode.is(Mode.Pattern) || isFullyDefined(pt, ForceDegree.all)) then | |||
There was a problem hiding this comment.
Needed to pass f242-2.scala
| @@ -2,9 +2,3 @@ | |||
| 5 | case '{ StringContext(${Varargs(parts)}*) } => // error | |||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |||
| | no implicit argument of type scala.quoted.Quotes was found | |||
| -- [E006] Not Found Error: tests/neg-macros/i6436.scala:6:34 ----------------------------------------------------------- | |||
| 6 | val ps: Seq[Expr[String]] = parts // error | |||
There was a problem hiding this comment.
I think this is an improvement?
adampauls
force-pushed
the
implicit-unapply
branch
from
585cb87 to
6ccc7c7
Compare
|
cc @nicolasstucki since I think you last touched this code in #9003? |
| // Typing explicitCall may fail if there are implicits that can only | ||
| // be resolved with tighter bounds on the output types | ||
| // (e.g. def unapply[T](x: String)(using Foo[T]): Option[T]) | ||
| // If the call above fails to type, we try again here but with wildcards |
There was a problem hiding this comment.
Having read through https://contributors.scala-lang.org/t/better-implicit-resolution-in-pattern-matches/5308 I sort of understand the usecase, but I'm not sure it's worth it given this kind of weird business with wildcards needed in the implementation. It seems that pattern typing is fundamentally top-down and this code attempts to work around that in some specific cases, but I can't really tell what the implications of that change is.
There was a problem hiding this comment.
Thanks for follow up. I agree that this implementation is not ideal, but I think I would disagree that pattern typing ought to be fundamentally top-down. I think it is right now, but I don't see any good reason for it. I almost filed the feature request as a bug report because it's not really clear to me whether the current behavior is the spec.
If pattern matching were specified as syntactic sugar over a series of called to unapply/flatMap, etc., I think we might find that the behavior specified in the "feature request" would fall out naturally, and this PR would be more of a (possibly hacky) bug fix. I would like to fix the possibly hacky part, but I'll need a little consultation on what would seem less hacky and I suppose agreement on whether the syntactic sugar view of pattern matching is correct. Maybe it's worth starting a separate thread on what that view might be? Or is there already a spec for the bespoke type inference currently implemented beyond the implementation?
There was a problem hiding this comment.
Or is there already a spec for the bespoke type inference currently implemented beyond the implementation?
The Scala 2 spec contains this section: https://www.scala-lang.org/files/archive/spec/2.13/08-pattern-matching.html#pattern-matching-expressions but I honestly have no idea how closely it matches either the Scala 2 or 3 implementation.
There was a problem hiding this comment.
That spec doesn't say anything about how to type a pattern, just how to type a pattern in the context of type variables (IIUC). And if anything, it sounds a lot like the implementation in this PR, except with undefined for type parameters instead of wildcards for implicits?
First, it is attempted to type [pattern]
pwithTas its expected type. If this fails,pis instead typed with a modified expected typeT'which results fromTby replacing every occurrence of a type parametera_iby undefined.
That said, any view of pattern matching as syntactic sugar for a series of calls to flatMap/orElse/isInstanceOf that I could think of would not lead to the behavior that I implement in this PR, so I can't really argue this is a bug. I will close the PR for lack of interest in this new feature. Thanks for your time!
Attempts to address lampepfl/dotty-feature-requests#242. I'm still pretty new here, so apologies for any naive errors. At least one of the changes I made is probably not right. I'll leave comments in line. (cc @som-snytt ).