Skip to content

Fix #4098: Check that refinements have good bounds #4122

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
Mar 28, 2018
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
33 changes: 26 additions & 7 deletions compiler/src/dotty/tools/dotc/core/CheckRealizable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ object CheckRealizable {
class NotFinal(sym: Symbol)(implicit ctx: Context)
extends Realizability(i" refers to nonfinal $sym")

class HasProblemBounds(typ: SingleDenotation)(implicit ctx: Context)
extends Realizability(i" has a member $typ with possibly conflicting bounds ${typ.info.bounds.lo} <: ... <: ${typ.info.bounds.hi}")
class HasProblemBounds(name: Name, info: Type)(implicit ctx: Context)
extends Realizability(i" has a member $name with possibly conflicting bounds ${info.bounds.lo} <: ... <: ${info.bounds.hi}")

class HasProblemBaseArg(typ: Type, argBounds: TypeBounds)(implicit ctx: Context)
extends Realizability(i" has a base type $typ with possibly conflicting parameter bounds ${argBounds.lo} <: ... <: ${argBounds.hi}")
Expand Down Expand Up @@ -96,6 +96,14 @@ class CheckRealizable(implicit ctx: Context) {
else boundsRealizability(tp).andAlso(memberRealizability(tp))
}

private def refinedNames(tp: Type): Set[Name] = tp.dealias match {
case tp: RefinedType => refinedNames(tp.parent) + tp.refinedName
case tp: AndType => refinedNames(tp.tp1) ++ refinedNames(tp.tp2)
case tp: OrType => refinedNames(tp.tp1) ++ refinedNames(tp.tp2)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that A | B would need a test? Confused what happens there with disjoint ranges.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refinedNames only collects names that appear in a top-level refinement. So that's the same for & and |. The bounds checking is done later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant I don't see a testcase exercising this path.

case tp: TypeProxy => refinedNames(tp.underlying)
case _ => Set.empty
}

/** `Realizable` if `tp` has good bounds, a `HasProblem...` instance
* pointing to a bad bounds member otherwise. "Has good bounds" means:
*
Expand All @@ -107,12 +115,22 @@ class CheckRealizable(implicit ctx: Context) {
* also lead to base types with bad bounds).
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, the comment is now correct. The previous implementation did less than the comment implied.

*/
private def boundsRealizability(tp: Type) = {
val mbrProblems =

val memberProblems =
for {
mbr <- tp.nonClassTypeMembers
if !(mbr.info.loBound <:< mbr.info.hiBound)
}
yield new HasProblemBounds(mbr)
yield new HasProblemBounds(mbr.name, mbr.info)

val refinementProblems =
for {
name <- refinedNames(tp)
if (name.isTypeName)
mbr <- tp.member(name).alternatives
if !(mbr.info.loBound <:< mbr.info.hiBound)
}
yield new HasProblemBounds(name, mbr.info)

def baseTypeProblems(base: Type) = base match {
case AndType(base1, base2) =>
Expand All @@ -126,12 +144,13 @@ class CheckRealizable(implicit ctx: Context) {
val baseProblems =
tp.baseClasses.map(_.baseTypeOf(tp)).flatMap(baseTypeProblems)

(((Realizable: Realizability)
/: mbrProblems)(_ andAlso _)
((((Realizable: Realizability)
/: memberProblems)(_ andAlso _)
/: refinementProblems)(_ andAlso _)
/: baseProblems)(_ andAlso _)
}

/** `Realizable` if all of `tp`'s non-struct fields have realizable types,
/** `Realizable` if all of `tp`'s non-strict fields have realizable types,
* a `HasProblemField` instance pointing to a bad field otherwise.
*/
private def memberRealizability(tp: Type) = {
Expand Down
37 changes: 19 additions & 18 deletions compiler/src/dotty/tools/dotc/typer/Dynamic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import dotty.tools.dotc.core.Names.{Name, TermName}
import dotty.tools.dotc.core.StdNames._
import dotty.tools.dotc.core.Types._
import dotty.tools.dotc.core.Decorators._
import util.Positions._
import core.Symbols._
import core.Definitions
import Inferencing._
Expand Down Expand Up @@ -49,7 +50,7 @@ trait Dynamic { self: Typer with Applications =>
* foo.bar[T0, ...](x = bazX, y = bazY, baz, ...) ~~> foo.applyDynamicNamed[T0, ...]("bar")(("x", bazX), ("y", bazY), ("", baz), ...)
*/
def typedDynamicApply(tree: untpd.Apply, pt: Type)(implicit ctx: Context): Tree = {
def typedDynamicApply(qual: untpd.Tree, name: Name, targs: List[untpd.Tree]): Tree = {
def typedDynamicApply(qual: untpd.Tree, name: Name, selPos: Position, targs: List[untpd.Tree]): Tree = {
def isNamedArg(arg: untpd.Tree): Boolean = arg match { case NamedArg(_, _) => true; case _ => false }
val args = tree.args
val dynName = if (args.exists(isNamedArg)) nme.applyDynamicNamed else nme.applyDynamic
Expand All @@ -62,19 +63,19 @@ trait Dynamic { self: Typer with Applications =>
case arg => namedArgTuple("", arg)
}
val args1 = if (dynName == nme.applyDynamic) args else namedArgs
typedApply(untpd.Apply(coreDynamic(qual, dynName, name, targs), args1), pt)
typedApply(untpd.Apply(coreDynamic(qual, dynName, name, selPos, targs), args1), pt)
}
}

tree.fun match {
case Select(qual, name) if !isDynamicMethod(name) =>
typedDynamicApply(qual, name, Nil)
case TypeApply(Select(qual, name), targs) if !isDynamicMethod(name) =>
typedDynamicApply(qual, name, targs)
case sel @ Select(qual, name) if !isDynamicMethod(name) =>
typedDynamicApply(qual, name, sel.pos, Nil)
case TypeApply(sel @ Select(qual, name), targs) if !isDynamicMethod(name) =>
typedDynamicApply(qual, name, sel.pos, targs)
case TypeApply(fun, targs) =>
typedDynamicApply(fun, nme.apply, targs)
typedDynamicApply(fun, nme.apply, fun.pos, targs)
case fun =>
typedDynamicApply(fun, nme.apply, Nil)
typedDynamicApply(fun, nme.apply, fun.pos, Nil)
}
}

Expand All @@ -86,26 +87,26 @@ trait Dynamic { self: Typer with Applications =>
* through an existing transformation of in typedAssign [foo.bar(baz) = quux ~~> foo.bar.update(baz, quux)].
*/
def typedDynamicSelect(tree: untpd.Select, targs: List[Tree], pt: Type)(implicit ctx: Context): Tree =
typedApply(coreDynamic(tree.qualifier, nme.selectDynamic, tree.name, targs), pt)
typedApply(coreDynamic(tree.qualifier, nme.selectDynamic, tree.name, tree.pos, targs), pt)

/** Translate selection that does not typecheck according to the normal rules into a updateDynamic.
* foo.bar = baz ~~> foo.updateDynamic(bar)(baz)
*/
def typedDynamicAssign(tree: untpd.Assign, pt: Type)(implicit ctx: Context): Tree = {
def typedDynamicAssign(qual: untpd.Tree, name: Name, targs: List[untpd.Tree]): Tree =
typedApply(untpd.Apply(coreDynamic(qual, nme.updateDynamic, name, targs), tree.rhs), pt)
def typedDynamicAssign(qual: untpd.Tree, name: Name, selPos: Position, targs: List[untpd.Tree]): Tree =
typedApply(untpd.Apply(coreDynamic(qual, nme.updateDynamic, name, selPos, targs), tree.rhs), pt)
tree.lhs match {
case Select(qual, name) if !isDynamicMethod(name) =>
typedDynamicAssign(qual, name, Nil)
case TypeApply(Select(qual, name), targs) if !isDynamicMethod(name) =>
typedDynamicAssign(qual, name, targs)
case sel @ Select(qual, name) if !isDynamicMethod(name) =>
typedDynamicAssign(qual, name, sel.pos, Nil)
case TypeApply(sel @ Select(qual, name), targs) if !isDynamicMethod(name) =>
typedDynamicAssign(qual, name, sel.pos, targs)
case _ =>
errorTree(tree, ReassignmentToVal(tree.lhs.symbol.name))
}
}

private def coreDynamic(qual: untpd.Tree, dynName: Name, name: Name, targs: List[untpd.Tree])(implicit ctx: Context): untpd.Apply = {
val select = untpd.Select(qual, dynName)
private def coreDynamic(qual: untpd.Tree, dynName: Name, name: Name, selPos: Position, targs: List[untpd.Tree])(implicit ctx: Context): untpd.Apply = {
val select = untpd.Select(qual, dynName).withPos(selPos)
val selectWithTypes =
if (targs.isEmpty) select
else untpd.TypeApply(select, targs)
Expand Down Expand Up @@ -134,7 +135,7 @@ trait Dynamic { self: Typer with Applications =>
def structuralCall(selectorName: TermName, formals: List[Tree]) = {
val selectable = adapt(qual, defn.SelectableType)
val scall = untpd.Apply(
untpd.TypedSplice(selectable.select(selectorName)),
untpd.TypedSplice(selectable.select(selectorName)).withPos(tree.pos),
(Literal(Constant(name.toString)) :: formals).map(untpd.TypedSplice(_)))
typed(scall)
}
Expand Down
16 changes: 16 additions & 0 deletions tests/neg/i4098.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
object App {
import scala.reflect.Selectable.reflectiveSelectable

def coerce[U, V](u: U): V = {
type X = { type R >: U }
type Y = { type R = V }
type Z = X & Y
val u1: Z#R = u // error: not a legal path
u1
}

def main(args: Array[String]): Unit = {
val x: Int = coerce[String, Int]("a")
println(x + 1)
}
}
16 changes: 16 additions & 0 deletions tests/neg/i4098a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
object App {
import scala.reflect.Selectable.reflectiveSelectable

def coerce[U, V](u: U): V = {
type X = { val x: { type R >: U } }
type Y = { val x: { type R = V } }
lazy val z: X & Y = z
val u1: z.x.R = u // error: Object { type R >: U | V <: V } is not stable (with arrows under z.x)
u1
}

def main(args: Array[String]): Unit = {
val x: Int = coerce[String, Int]("a")
println(x + 1)
}
}