Skip to content

Fix how intersected bounds are added to Constraint #16591

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 1 commit into from
Closed

Fix how intersected bounds are added to Constraint #16591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion 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 @@ -861,12 +861,24 @@ trait ConstraintHandling {
addConstraintInvocations += 1
val saved = canWidenAbstract
canWidenAbstract = true
try bound match

def contains(bound: Type) = bound match
case tpr: TypeParamRef => constraint.contains(tpr)
case tv: TypeVar => constraint.contains(tv)
case _ => false

def add(bound: Type): Boolean = bound match
case AndType(bound1, bound2) if contains(bound1) ^ contains(bound2) =>
add(bound1) && add(bound2)
Comment on lines +871 to +872
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what's going on here but it seems incorrect. According to the documentation comment this case can only be hit if bound is a lower bound, so we're trying to add the following constraint:

param >: bound1 & bound2

And this case splits this into two two constraints:

param >: bound1
param >: bound2

Which is equivalent to:

param >: bound1 | bound2

So we are over-constraining which can prevent some possible solutions (I can't instantiate param to bound1 or bound2 or their intersection anymore, unless bound1 =:= bound2).

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I'm splitting P >: B & Q, where P and Q are params and B isn't, into P >: B constraint and Q < P ordering, which allows for P =:= Q unification to go through.

Copy link
Member

Choose a reason for hiding this comment

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

This still leads to overconstraining, here's an example which compiled before this change but not after:

class LT[-X, Y]
object LT {
 given lt[X <: Y, Y]: LT[X, Y] = ???
}

trait B

def foo[Q, P](using LT[B & Q, P])(p: P, q: Q): P = p

def test(b: B, any: Any): B =
  foo(b, any)

Setting P =:= Q means that the method is forced to return Any, even though the only thing we specified is that B & Any <: B

case bound: TypeParamRef if constraint contains bound =>
addParamBound(bound)
case bound: TypeVar if constraint.contains(bound) =>
addParamBound(bound.origin)
case _ =>
val pbound = avoidLambdaParams(bound)
kindCompatible(param, pbound) && addBoundTransitively(param, pbound, !fromBelow)

try add(bound)
finally
canWidenAbstract = saved
addConstraintInvocations -= 1
Expand Down
31 changes: 22 additions & 9 deletions compiler/test/dotty/tools/dotc/core/ConstraintsTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
package dotty.tools
package dotc.core
package dotc
package core

import vulpix.TestConfiguration

import dotty.tools.dotc.core.Contexts.{*, given}
import dotty.tools.dotc.core.Decorators.{*, given}
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.dotc.core.Types.*
import dotty.tools.dotc.ast.tpd.*
import dotty.tools.dotc.typer.ProtoTypes.constrained
import Contexts.{*, given}, Decorators.{*, given}, Symbols.*, Types.*
import ast.tpd.*
import typer.ProtoTypes.*
import util.SimpleIdentitySet

import org.junit.Test

import dotty.tools.DottyTest

class ConstraintsTest:

@Test def mergeParamsTransitivity: Unit =
Expand Down Expand Up @@ -91,3 +88,19 @@ class ConstraintsTest:
assert(!ctx.typerState.constraint.occursAtToplevel(tvar.origin, entry),
i"cyclic bound for ${tvar.origin}: ${entry} in ${ctx.typerState.constraint}")
}

@Test def splitIntersectedBounds: Unit = inCompilerContext(TestConfiguration.basicClasspath) {
val Foo = newTypeVar(TypeBounds.empty, "Foo".toTypeName)
val Bar = newTypeVar(TypeBounds.empty, "Bar".toTypeName)
val Int = defn.IntType

val tp1 = Foo & Int
val tp2 = Bar & Int
val log = TypeComparer.explained(_.isSameType(tp1, tp2))
//println(i"$log")
//println(i"${ctx.typerState.constraint}")

val tree = ref(newAnonFun(defn.RootClass, MethodType(Nil, defn.UnitType))).appliedToNone
ctx.typer.interpolateTypeVars(tree, WildcardType, SimpleIdentitySet.empty)
assert(Foo =:= Int && Bar =:= Int, i"Foo=$Foo Bar=$Bar")
}
14 changes: 14 additions & 0 deletions tests/pos/i16524.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
trait Axe
trait Foo[A]
trait Bar[B] { type Res }
class Ref[C](value: C)
trait Opt[+T]

abstract class Test:
def mash[X](ref: Ref[X], foo: Foo[X], bar: Bar[X]): bar.Res

def mkFoo[Y]: Foo[Opt[Y & Axe]]
def mkBar[Z]: Bar[Opt[Z & Axe]] { type Res = Z }
val optInt: Opt[Int & Axe]

def test = mash(new Ref(optInt), mkFoo, mkBar)