Skip to content

Fix #2762: Make sure constrainResult does not change context when false #2892

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.

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 3 commits into from
Jul 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,11 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
case methType: MethodType =>
// apply the result type constraint, unless method type is dependent
val resultApprox = resultTypeApprox(methType)
val savedConstraint = ctx.typerState.constraint
if (!constrainResult(resultApprox, resultType))
if (ctx.typerState.isCommittable)
// defer the problem until after the application;
// it might be healed by an implicit conversion
assert(ctx.typerState.constraint eq savedConstraint)
Copy link
Member

Choose a reason for hiding this comment

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

If this is removed then val savedConstraint above is unused.

()
else
fail(err.typeMismatchMsg(methType.resultType, resultType))
// match all arguments with corresponding formal parameters
Expand Down
31 changes: 17 additions & 14 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,23 @@ object ProtoTypes {
/** Check that the result type of the current method
* fits the given expected result type.
*/
def constrainResult(mt: Type, pt: Type)(implicit ctx: Context): Boolean = pt 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
def constrainResult(mt: Type, pt: Type)(implicit ctx: Context): Boolean = {
val savedConstraint = ctx.typerState.constraint
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be safer to do use a new TyperState? (ctx.fresh.setNewTyperState and then commit if we want to keep it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, except that constrainResult is called often, so copying the context each time would likely be a hit in performance. Also, the logic here is analogous to isSubType, which makes sense.

val res = pt 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) ctx.typerState.constraint = savedConstraint
res
}
}

Expand Down
16 changes: 16 additions & 0 deletions tests/neg/i2672.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class Foo[T]

trait Prepend {
type Out
}

object Test {
def foo()(implicit ev: Prepend): Foo[ev.Out] = ???

def test: Unit = {
foo(): Foo[Any] // error: found: Prepend => Foo[_] required: Foo[Any]
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a great error message by the way, since it might be displayed instead of a more informative message based on@implicitNotFound.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But even if an implicit was found, we'd still get an error because then the function would return a Foo[_] where a Foo[Any] was expected.


}

implicit val p: Prepend = ???
}
15 changes: 15 additions & 0 deletions tests/pos/i2672.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class Foo[+T]

trait Prepend {
type Out
}

object Test {
def foo()(implicit ev: Prepend): Foo[ev.Out] = ???

def test: Unit = {
foo(): Foo[Any]
}

implicit val p: Prepend = ???
}