scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.85k stars 1.06k forks source link

Varargs does not keep hardened union types #16818

Open soronpo opened 1 year ago

soronpo commented 1 year ago

It really depends how we specify it, but we could look at the union derived from varargs as hardened.

Compiler version

v3.3.0-RC2

Minimized code

class Foo
class U1 extends Foo
class U2 extends Foo
class U3 extends Foo
class U4 extends Foo
type U = U1 | U2
val u : U1 | U2 = ???
type U34 = U3 | U4
val u34 : U3 | U4 = ???
class Boxx[T](x: T*)
val b123 = Boxx(u, u34)
val b123Test: Boxx[U | U34] = b123 //error

Output

Found: (b123 : Boxx[Foo]) 
Required: Boxx[U | U34]

Expectation

No error.

mbovel commented 1 year ago

Is it specifically related to varargs? The same happens here for example:

class MyPair[T](a: T, b: T)
val p = MyPair(u, u34)
val p2: MyPair[U | U34] = p // error

It might be relevant to propagate unions hardness here as well.

soronpo commented 1 year ago

It might be relevant to propagate unions hardness here as well.

Yes, that is true.

scala-center-bot commented 1 year ago

This issue was picked for the Issue Spree No. 26 of 14 February 2023 which takes place in a week from now. @mbovel, @mprzysucha will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

mbovel commented 1 year ago

@odersky preparing tonight's spree; could we “propagate hardness” such that if nested union are hard, then the outer unions are considered hard as well?

diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
index d2fc225ff19..45bb0521cf8 100644
--- a/compiler/src/dotty/tools/dotc/core/Types.scala
+++ b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -3416,9 +3416,15 @@ object Types {
     private var myUnionPeriod: Period = Nowhere

     override def widenUnionWithoutNull(using Context): Type =
+      def containsHardUnions(tp: Type): Boolean = tp match
+        case tp: TypeProxy => containsHardUnions(tp.underlying)
+        case tp @ OrType(tp1, tp2) => !tp.isSoft || containsHardUnions(tp1) || containsHardUnions(tp2)
+        case AndType(tp1, tp2) => containsHardUnions(tp1) || containsHardUnions(tp2)
+        case _ => false
+
       if myUnionPeriod != ctx.period then
         myUnion =
-          if isSoft then
+          if isSoft && !containsHardUnions(tp1) && !containsHardUnions(tp2) then
             TypeComparer.lub(tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, canConstrain = true, isSoft = isSoft) match
               case union: OrType => union.join
               case res => res

This makes tests in this issue pass and doesn't seem to break existing compilation tests.

mbovel commented 1 year ago

If you have any insight into the issue or guidance on how to fix it, please leave it here.

  1. See documentation about hard/soft unions: https://github.com/lampepfl/dotty/blob/main/docs/_docs/reference/new-types/union-types-spec.md#hard-and-soft-union-types.
  2. Using -Yprint-debug, soft unions are displayed || and hard unions |.
sjrd commented 1 year ago

could we “propagate hardness” such that if nested union are hard, then the outer unions are considered hard as well?

No, that would not correctly produce a soft union for

val x: Int | String = 1 // hard
val y: Boolean | String = false // hard as well
val z = if foo then x else y

The type of the if should be a soft (Int | String) | (Boolean | String), so that the inferred type for z is a lub.

mbovel commented 1 year ago

No, that would not correctly produce a soft union

I see. Could we instead modify addOneBound so that the derived lower bound is a hard union if of the operands contains hard unions?

mbovel commented 1 year ago

I will continue to work on this. I therefore remove the “Spree” label.

I discussed it with Martin and he mentioned that a better fix would be to harden a type variables when it is lower-bounded by a hard union.