Skip to content

[Experiment] Do not widen singletons in hard unions #14347

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

[Experiment] Do not widen singletons in hard unions #14347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f81d2c8
Do not widen hard unions
mbovel Jan 25, 2022
fa7d107
Update documentation
mbovel Jan 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,8 @@ trait ConstraintHandling {
// Widening can add extra constraints, in particular the widened type might
// be a type variable which is now instantiated to `param`, and therefore
// cannot be used as an instantiation of `param` without creating a loop.
// If that happens, we run `instanceType` again to find a new instantation.
// (we do not check for non-toplevel occurences: those should never occur
// If that happens, we run `instanceType` again to find a new instantiation.
// (we do not check for non-toplevel occurrences: those should never occur
// since `addOneBound` disallows recursive lower bounds).
if constraint.occursAtToplevel(param, widened) then
instanceType(param, fromBelow)
Expand Down
22 changes: 14 additions & 8 deletions compiler/src/dotty/tools/dotc/core/Types.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1266,9 +1266,10 @@ object Types {
/** Widen this type and if the result contains embedded soft union types, replace
* them by their joins.
* "Embedded" means: inside type lambdas, intersections or recursive types,
* in prefixes of refined types, or in hard union types.
* or in prefixes of refined types. Softs unions inside hard unions are
* left untouched.
* If an embedded soft union is found, we first try to simplify or eliminate it by
* re-lubbing it while allowing type parameters to be constrained further.
* re-lubing it while allowing type parameters to be constrained further.
* Any remaining union types are replaced by their joins.
*
* For instance, if `A` is an unconstrained type variable, then
Expand Down Expand Up @@ -1303,8 +1304,8 @@ object Types {
case tp =>
tp

/** Widen all top-level singletons reachable by dealiasing
* and going to the operands of & and |.
/** Widen all top-level singletons reachable by dealiasing and going to the
* operands of intersections and soft unions.
* Overridden and cached in OrType.
*/
def widenSingletons(using Context): Type = dealias match {
Expand Down Expand Up @@ -3266,7 +3267,7 @@ object Types {
TypeComparer.lub(tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, canConstrain = true) match
case union: OrType => union.join
case res => res
else derivedOrType(tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull)
else this
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth flipping the order with your change:

if isSoft then
  if myUnionPeriod != ctx.period then
    ...
  myUnion
else this

if !isProvisional then myUnionPeriod = ctx.period
myUnion

Expand All @@ -3280,11 +3281,16 @@ object Types {
if tp1.hasClassSymbol(defn.NothingClass) then tp2.atoms
else if tp2.hasClassSymbol(defn.NothingClass) then tp1.atoms
else tp1.atoms | tp2.atoms
val tp1w = tp1.widenSingletons
val tp2w = tp2.widenSingletons
myWidened = if ((tp1 eq tp1w) && (tp2 eq tp2w)) this else tp1w | tp2w
atomsRunId = ctx.runId

myWidened =
if isSoft then
val tp1w = tp1.widenSingletons
val tp2w = tp2.widenSingletons
if ((tp1 eq tp1w) && (tp2 eq tp2w)) this else tp1w | tp2w
else
this

override def atoms(using Context): Atoms =
ensureAtomsComputed()
myAtoms
Expand Down
11 changes: 10 additions & 1 deletion tests/pos/widen-union.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,21 @@ object Test2:
|| xs.corresponds(ys)(consistent(_, _)) // error, found: Any, required: Int | String

object Test3:

def g[X](x: X | String): Int = ???
def y: Boolean | String = ???
g[Boolean](y)
g(y)
g[Boolean](identity(y))
g(identity(y))

object Test4:
def f(a: 2 | 3) = a

def test() =
Copy link
Contributor

@odersky odersky Jan 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Consider adding a test where the union is the result type of a def.

val x: 2 | 3 = 2
val y = x
f(y)

def g: 2 | 3 = 2
val z = g
f(z)