Special case inline unapply seq

[gorilskij]

Replacement for #15053

Resolves #8577

Fixed a bug where extension inline unapplySeq would fail to type by adding a few special cases in Inliner.scala

[nicolasstucki]

There is a bug to fix with the handling of type arguments. This is the cause of the failing/commented tests.

[nicolasstucki]

There is an issue here as we are reusing the same type arguments for the two type parameter lists. We can make this evident with the following test where the two type parameter lists have different sizes and we use them in the type of the unapply.

    extension (ctx: StringContext) def macroE: MacroE.StringContext = MacroE(ctx)
    extension [T,X <: DummyImplicit] (inline ctx: MacroE.StringContext) inline def unapplySeq[U](inline input: U)(using X): Option[Seq[U]] =
      ${ implUnapplyE('ctx, 'input) }

    val macroE"$xE" = 5

To fix this I propose something like the following where we extract the two type argument lists

    val (targs1, targs2) = fun match
      case TypeApply(Apply(TypeApply(_, targs1), _), targs2) => (targs1, targs2) // extension [U](y: Y) def unapply[Y](x: X)
      case TypeApply(_, targs) => (targs, Nil) // def unapply[U](x: X)
      case Apply(TypeApply(_, targs1), _) => (targs1, Nil) // extension (y: Y) def unapply[T](x: X)
      case _ => (Nil, Nil)

    val unapplyInfo = sym.info match
      case info: PolyType => info.instantiate(targs1.map(_.tpe)) match
        case MethodTpe(_, _, rt: PolyType) => rt.instantiate(targs2.map(_.tpe))
        case MethodTpe(_, _, rt) if sym.flags.is(ExtensionMethod) => rt
        case info => info
      case MethodTpe(_, _, rt: PolyType) => rt.instantiate(targs1.map(_.tpe))
      case MethodTpe(_, _, rt) if sym.flags.is(ExtensionMethod) => rt
      case info => info

[nicolasstucki]

With this change, all the tests you commented out start working.

[nicolasstucki]

For the cases MethodTpe(_, _, rt: PolyType) we assume that these are extension methods because no other definition have that type. I would make this a bit more explicit and add the flag test for this branch as well.

-        case MethodTpe(_, _, rt: PolyType) => rt.instantiate(targs2.map(_.tpe))
+        case MethodTpe(_, _, rt: PolyType) if sym.flags.is(ExtensionMethod) =>
+          rt.instantiate(targs2.map(_.tpe))

[nicolasstucki]

We could also refactor the common code and make the logic related to extensions clearer.

     def extensionInfo(info: Type, targs: List[Tree]): Type =
       info match
        case MethodTpe(_, _, rt: PolyType) => rt.instantiate(targs.map(_.tpe))
        case MethodTpe(_, _, rt) => rt
        case info => info
     val unapplyInfo = sym.info match
      case pt: PolyType =>
        val info = pt.instantiate(targs1.map(_.tpe))
        if sym.flags.is(ExtensionMethod) then extensionInfo(info, targs2) else info
      case info =>
        if sym.flags.is(ExtensionMethod) then extensionInfo(info, targs1) else info

[gorilskij]

It seems to work if we completely remove the whole targs and unapplyInfo ceremony and simply pass fun.tpe.widen instead (see last commit).

[nicolasstucki]

Widening might not be the correct thing to do. We might be missing some test cases. I imagine that if we have path dependent types widening will make us loose precision.

[gorilskij]

Do you have a particular test case in mind?

[nicolasstucki]

One that comes to my mind but is not supported due to some other bug is #13158 (comment). We could imagine a similar unapply with a path dependent type on a term that is not a parameter. Also, we might try to have a path dependent type in the trailing given parameters of the unapply.

[gorilskij]

I'm not sure about the example you give in the comment but the code example in #12991 compiles and runs correctly in this branch

[nicolasstucki]

Rebased and squashed commits

[smarter]

@nicolasstucki enabled auto-merge 2 days ago

In case you didn't notice, the merge is currently blocked because of merge conflicts.

[nicolasstucki]

Looks good to me.

It needed a rebase, but this generated some conflicts with another feature. Moved this into #16358 with the fix for the other issue.

[nicolasstucki]

Thank you @gorilskij