Skip to content

Drop ConstrainResult mode bit #9641

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

Merged
merged 1 commit into from
Aug 26, 2020
Merged

Drop ConstrainResult mode bit #9641

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
3 changes: 0 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Mode.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ object Mode {
*/
val Printing: Mode = newMode(10, "Printing")

/** We are constraining a method based on its expected type. */
val ConstrainResult: Mode = newMode(11, "ConstrainResult")

/** We are currently in a `viewExists` check. In that case, ambiguous
* implicits checks are disabled and we succeed with the first implicit
* found.
Expand Down
13 changes: 12 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TypeComparer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
private var myInstance: TypeComparer = this
def currentInstance: TypeComparer = myInstance

private var useNecessaryEither = false

/** Is a subtype check in progress? In that case we may not
* permanently instantiate type variables, because the corresponding
* constraint might still be retracted and the instantiation should
Expand Down Expand Up @@ -129,6 +131,12 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
}
}

def necessarySubType(tp1: Type, tp2: Type): Boolean =
val saved = useNecessaryEither
useNecessaryEither = true
try topLevelSubType(tp1, tp2)
finally useNecessaryEither = saved

def testSubType(tp1: Type, tp2: Type): CompareResult =
GADTused = false
if !topLevelSubType(tp1, tp2) then CompareResult.Fail
Expand Down Expand Up @@ -1481,7 +1489,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
* @see [[sufficientEither]] for the normal case
*/
protected def either(op1: => Boolean, op2: => Boolean): Boolean =
if ctx.mode.is(Mode.GadtConstraintInference) || ctx.mode.is(Mode.ConstrainResult) then
if ctx.mode.is(Mode.GadtConstraintInference) || useNecessaryEither then
necessaryEither(op1, op2)
else
sufficientEither(op1, op2)
Expand Down Expand Up @@ -2528,6 +2536,9 @@ object TypeComparer {
def topLevelSubType(tp1: Type, tp2: Type)(using Context): Boolean =
comparing(_.topLevelSubType(tp1, tp2))

def necessarySubType(tp1: Type, tp2: Type)(using Context): Boolean =
comparing(_.necessarySubType(tp1, tp2))

def isSubType(tp1: Type, tp2: Type)(using Context): Boolean =
comparing(_.isSubType(tp1, tp2))

Expand Down
41 changes: 24 additions & 17 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Uniques._
import config.Printers.typr
import util.SourceFile
import util.Property
import TypeComparer.necessarySubType

import scala.annotation.internal.sharable

Expand All @@ -37,6 +38,14 @@ object ProtoTypes {
def isCompatible(tp: Type, pt: Type)(using Context): Boolean =
(tp.widenExpr relaxed_<:< pt.widenExpr) || viewExists(tp, pt)

/** Like isCompatibe, but using a subtype comparison with necessary eithers
* that don't unnecessarily truncate the constraint space, returning false instead.
*/
def necessarilyCompatible(tp: Type, pt: Type)(using Context): Boolean =
val tpw = tp.widenExpr
val ptw = pt.widenExpr
necessarySubType(tpw, ptw) || tpw.isValueSubType(ptw) || viewExists(tp, pt)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it matter?

Suggested change
necessarySubType(tpw, ptw) || tpw.isValueSubType(ptw) || viewExists(tp, pt)
necessarySubType(tpw, ptw) || tpw.isValueSubType(ptw) || viewExists(tpw, ptw)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would matter if there is an implicit conversion between singleton types. In this case it should be viewExists(tp, pt)


/** Test compatibility after normalization.
* Do this in a fresh typerstate unless `keepConstraint` is true.
*/
Expand Down Expand Up @@ -67,24 +76,22 @@ object ProtoTypes {
* fits the given expected result type.
*/
def constrainResult(mt: Type, pt: Type)(using Context): Boolean =
withMode(Mode.ConstrainResult) {
val savedConstraint = ctx.typerState.constraint
val res = pt.widenExpr match {
case pt: FunProto =>
mt match {
case mt: MethodType => constrainResult(resultTypeApprox(mt), pt.resultType)
case _ => true
}
case _: ValueTypeOrProto if !disregardProto(pt) =>
isCompatible(normalize(mt, pt), pt)
case pt: WildcardType if pt.optBounds.exists =>
isCompatible(normalize(mt, pt), pt)
case _ =>
true
}
if !res then ctx.typerState.constraint = savedConstraint
res
val savedConstraint = ctx.typerState.constraint
val res = pt.widenExpr match {
case pt: FunProto =>
mt match {
case mt: MethodType => constrainResult(resultTypeApprox(mt), pt.resultType)
case _ => true
}
case _: ValueTypeOrProto if !disregardProto(pt) =>
necessarilyCompatible(normalize(mt, pt), pt)
case pt: WildcardType if pt.optBounds.exists =>
necessarilyCompatible(normalize(mt, pt), pt)
case _ =>
true
}
if !res then ctx.typerState.constraint = savedConstraint
res

/** Constrain result with special case if `meth` is an inlineable method in an inlineable context.
* In that case, we should always succeed and not constrain type parameters in the expected type,
Expand Down