-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} | ||
} | ||
|
||
|
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] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
} | ||
|
||
implicit val p: Prepend = ??? | ||
} |
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 = ??? | ||
} |
There was a problem hiding this comment.
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.