Fix #4557: Handle untpd.New with HKTypeLambda

[Medowhill]

No description provided.

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

[allanrenucci]

This still fails:

class A[X]
type T[X, Y] = A[X]
type Foo[X, Y] = T[X, Y]
class B extends Foo[Int, Int]

This now crashes:

class A[X]
type Foo[X, Y] = [Z] => A[X]
class B extends Foo[Int, Int]

[Medowhill]

To make the upper one work, I changed code to reduce a type recursively. If the result of reduction is AppliedType and its tycon is dealiased to HKTypeLambda, the compiler will reduce the type again.

To prevent crash, the compiler checks whether the reduced type is ValueType. If it is not, then the reduced one cannot be a prefix of Select, so that the compiler uses the original type.

[Blaisorblade]

This looks pretty good to me, tho probably an expert should review the new fixes.

• I see why you test against ValueType, but restoring the original type and letting the code fail later looks acceptable but a bit convoluted.

• Using recursion for normalizing seems reasonable, tho I worry if this risks an infinite loop when run against cycles; I tried adding a couple tests and this seems not the case, but a couple more won't hurt. Below, no. 1 fails elsewhere, no. 2 fails and has an imperfect report (tho it's maybe good enough).

object Test {
  class A[X]
  type T[X, Y] = A[X]
  type Bar[X, Y] = Foo[X, Y]
  type Foo[X, Y] = Bar[X, Y]
  class B extends Foo[Int, Int]
}

object Test {
  class A[X]
  type T[X, Y] = A[X]
  type Bar[X, Y] = Foo[X, Y]
  type Foo[X, Y] = Bar[X, Y]
  class B extends Foo[Int, Int]
}

Output — note that Foo gets dealiases to Bar in the error message, which isn't ideal. The error should maybe mention the original type. But that's probably not new in this patch: one might have to pass the type before dealiasing to checkClassType.

- Error: tests/neg/i4557b.scala:4:7 -------------------------------------------
4 |  type Bar[X, Y] = Foo[X, Y]
  |       ^
  |illegal cyclic reference: alias Test.Bar of type Bar refers back to the type itself
-- Error: tests/neg/i4557b.scala:6:18 ------------------------------------------
6 |  class B extends Foo[Int, Int]
  |                  ^^^^^^^^^^^^^
  |                  Test.Bar[Int, Int] is not a class type
two errors found

When you're done with these concern, we'll assign Odersky for the final review.

[Medowhill]

I found that the types of parents are calcualted already in ClassCompleter and can acquire the types by obtaining the dealiased type of the AppliedTypeTree inside TypedSplice. Now, the redundant calculation can be avoided.

I thought that the code can be more simplified as in https://github.com/Medowhill/dotty/commit/6c938a9cb40745b70d25d1cb9bd79025bfacb698 but it failed on tests dotty.tools.dotc.FromTastyTests.runTestFromTasty for the following files:
tests/run/valueclasses-pavlov.scala
tests/pos/simpleCaseClass-3.scala
tests/pos/i2104b.scala
The reason was that scala.Anyref became dealiased to Object since I used the dealiased type for every TypedSplice case. It resulted differences in decompiled files like:

$ diff tests/run/valueclasses-pavlov.decompiled tests/run/valueclasses-pavlov.decompiled.out
11c11
< object Box1 extends scala.AnyRef()
---
> object Box1
24c24
< object Box2 extends scala.AnyRef()
---
> object Box2

I believe that this behavior does not affect actual execution. However, since the tests fail, I do not include the simplified version in this PR.

When I simplified the pattern from tpt1 @ AppliedType(_, _) to tpt1: AppliedType as in https://github.com/Medowhill/dotty/commit/489b80b1050ffff63f284952fcd50d87a2877f21, it was compiled on the scalac but failed on the dotc with the below error message, so that failed on some tests.

[error] 322 |        val tp = tpt1.tpe.dealias
[error]     |                 ^^^^^^^^^^^^^^^^
[error]     |value `dealias` is not a member of dotty.tools.dotc.ast.Trees.Untyped @uncheckedVariance

I'll report this behavior as a seperate issue with a simpler code example.

[Medowhill]

I applied the change of https://github.com/Medowhill/dotty/commit/6c938a9cb40745b70d25d1cb9bd79025bfacb698 and removed extends scala.AnyRef() from the .decompiled files of the failing tests.

[allanrenucci]

I don't think you need this test if you do that:

diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
index 8a08be37b..3251f2735 100644
--- a/compiler/src/dotty/tools/dotc/typer/Typer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -516,7 +516,8 @@ class Typer extends Namer
           case TypeApplications.EtaExpansion(tycon) => tpt1 = tpt1.withType(tycon)
           case _ =>
         }
-        checkClassType(tpt1.tpe, tpt1.pos, traitReq = false, stablePrefixReq = true)
+        if (checkClassType(tpt1.tpe, tpt1.pos, traitReq = false, stablePrefixReq = true) eq defn.ObjectType)
+          tpt1 = TypeTree(defn.ObjectType)
 
         tpt1 match {
           case AppliedTypeTree(_, targs) =>

[Medowhill]

I checked that it passed all the tests. I've just pushed it. Thank you!

[allanrenucci]

not needed

[allanrenucci]

Can you squash the commits of this PR

[allanrenucci]

can be a val

[Blaisorblade]

Hm, creating a new tree like this will not set its position and so on, unlike tpt1 = tpt1.withType(defn.ObjectType) (like here) or TypeTree(defn.ObjectType) withPos tpt.pos (like in New). OTOH, this isn't a tree the user wrote. And maybe the point of using defn.ObjectType is to hope we get no other error here?

What makes more sense @allanrenucci?

[allanrenucci]

tpt1 = TypeTree(defn.ObjectType).withPos(tpt1.pos) I think

[Blaisorblade]

@Medowhill Does this need addressing?

[Medowhill]

I'll change it to tpt1 = TypeTree(defn.ObjectType).withPos(tpt1.pos). Thank you!

[Blaisorblade]

@Medowhill FWIW, the over-eager dealiasing has a separate issue (#4565).