Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix of #50 - volatile #53

Merged
merged 1 commit into from

3 participants

@odersky
Owner

Volatile checking needs to take all intersections into account; previously these
could be discarded through needsChecking.

Plus several refactorings and additions.

1) Module vals now have Final and Stable flags set
2) All logic around isVolatile is now in TypeOps; some of it was moved from Types.
3) Added stability checking to Select and SelectFromType typings.

Todo: We should find a better name for isVolatile. Maybe define the negation instead under the name
"isRealizable"?.

@odersky odersky Fix of #50 - volatile
Volatile checking needs to take all intersections into account; previously these
could be discarded through needsChecking.

Plus several refactorings and additions.

1) Module vals now have Final and Stable flags set
2) All logic around isVolatile is now in TypeOps; some of it was moved from Types.
3) Added stability checking to Select and SelectFromType typings.

Todo: We should find a better name for isVolatile. Maybe define the negation instead under the name
"isRealizable"?.
af337f0
@odersky
Owner
@samuelgruetter samuelgruetter commented on the diff
src/dotty/tools/dotc/core/Types.scala
@@ -127,25 +128,6 @@ object Types {
/** Is some part of this type produced as a repair for an error? */
final def isErroneous(implicit ctx: Context): Boolean = existsPart(_.isError)
- /** A type is volatile if its DNF contains an alternative of the form
- * {P1, ..., Pn}, {N1, ..., Nk}, where the Pi are parent typerefs and the
- * Nj are refinement names, and one the 4 following conditions is met:
- *
- * 1. At least two of the parents Pi are abstract types.
- * 2. One of the parents Pi is an abstract type, and one other type Pj,
- * j != i has an abstract member which has the same name as an
- * abstract member of the whole type.
- * 3. One of the parents Pi is an abstract type, and one of the refinement
- * names Nj refers to an abstract member of the whole type.
- * 4. One of the parents Pi is an abstract type with a volatile upper bound.
- *
- * Lazy values are not allowed to have volatile type, as otherwise
- * unsoundness can result.
- */
@samuelgruetter Owner

this (important) documentation is deleted, but not added anywhere

@odersky Owner
odersky added a note

Good catch. It should have been added to isVolatile in TypeOps, but that change got lost somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samuelgruetter

Todo: We should find a better name for isVolatile. Maybe define the negation instead under the name "isRealizable"?.

+1

@samuelgruetter

I just pulled in these changes and ran the example I gave in #50 and it's still accepted...

@samuelgruetter

I've started my own isRealizable function, see here.
It passes all tests, except one that I've added just now. The problem is that isRealizable(NoType) is called, and I don't know (yet) why...

@odersky
Owner

The example is in #50 is still accepted because isVolatile is not supposed to flag this one. is Volatile is an overapproximation of: The type is feasible now but it might become unfeasible by refining some of its abstract components. Feasibility of types has to be tested elsewhere. The right thing to do is to design and write a phase (what sued to be refChecks) that does this.

@samuelgruetter

That makes sense. I've just found out myself why we need to put this into refChecks ;-) It's because my isRealizable function (which tries to check everything) depends on the types of not yet typed trees (that's why isRealizable(NoType) was called...).

@samuelgruetter

LGTM [modulo the lost documentation for isVolatile ;-)]

@namin
Owner

LGTM, though shouldn't we leave issue #50 open?
Also, is the documentation that was deleted up-to-date, anyways? Is an aliased type member considered abstract?

@odersky odersky merged commit af337f0 into lampepfl:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 9, 2014
  1. @odersky

    Fix of #50 - volatile

    odersky authored
    Volatile checking needs to take all intersections into account; previously these
    could be discarded through needsChecking.
    
    Plus several refactorings and additions.
    
    1) Module vals now have Final and Stable flags set
    2) All logic around isVolatile is now in TypeOps; some of it was moved from Types.
    3) Added stability checking to Select and SelectFromType typings.
    
    Todo: We should find a better name for isVolatile. Maybe define the negation instead under the name
    "isRealizable"?.
This page is out of date. Refresh to see the latest.
View
1  src/dotty/tools/dotc/config/Printers.scala
@@ -14,6 +14,7 @@ object Printers {
val core: Printer = noPrinter
val typr: Printer = noPrinter
val constr: Printer = noPrinter
+ val checks: Printer = noPrinter
val overload: Printer = noPrinter
val implicits: Printer = noPrinter
val implicitsDetailed: Printer = noPrinter
View
2  src/dotty/tools/dotc/core/Flags.scala
@@ -422,7 +422,7 @@ object Flags {
final val RetainedTypeArgFlags = VarianceFlags | ExpandedName | Protected | Local
/** Modules always have these flags set */
- final val ModuleCreationFlags = ModuleVal
+ final val ModuleCreationFlags = ModuleVal | Final | Stable
/** Module classes always have these flags set */
final val ModuleClassCreationFlags = ModuleClass | Final
View
2  src/dotty/tools/dotc/core/SymDenotations.scala
@@ -355,7 +355,7 @@ object SymDenotations {
final def isStable(implicit ctx: Context) = {
val isUnstable =
(this is UnstableValue) ||
- info.isVolatile && !hasAnnotation(defn.uncheckedStableClass)
+ ctx.isVolatile(info) && !hasAnnotation(defn.uncheckedStableClass)
(this is Stable) || isType || {
if (isUnstable) false
else { setFlag(Stable); true }
View
72 src/dotty/tools/dotc/core/TypeOps.scala
@@ -3,6 +3,8 @@ package core
import Contexts._, Types._, Symbols._, Names._, Flags._, Scopes._
import SymDenotations._
+import config.Printers._
+import Decorators._
import util.SimpleMap
trait TypeOps { this: Context =>
@@ -75,7 +77,12 @@ trait TypeOps { this: Context =>
}
final def isVolatile(tp: Type): Boolean = {
- /** Pre-filter to avoid expensive DNF computation */
+
+ /** Pre-filter to avoid expensive DNF computation
+ * If needsChecking returns false it is guaranteed that
+ * DNF does not contain intersections, or abstract types with upper
+ * bounds that themselves need checking.
+ */
def needsChecking(tp: Type, isPart: Boolean): Boolean = tp match {
case tp: TypeRef =>
tp.info match {
@@ -88,31 +95,62 @@ trait TypeOps { this: Context =>
needsChecking(tp.parent, true)
case tp: TypeProxy =>
needsChecking(tp.underlying, isPart)
- case AndType(l, r) =>
- needsChecking(l, true) || needsChecking(r, true)
- case OrType(l, r) =>
- isPart || needsChecking(l, isPart) && needsChecking(r, isPart)
+ case tp: AndType =>
+ true
+ case tp: OrType =>
+ isPart || needsChecking(tp.tp1, isPart) && needsChecking(tp.tp2, isPart)
case _ =>
false
}
+
needsChecking(tp, false) && {
- tp.DNF forall { case (parents, refinedNames) =>
+ DNF(tp) forall { case (parents, refinedNames) =>
val absParents = parents filter (_.symbol is Deferred)
- absParents.size >= 2 || {
- val ap = absParents.head
- ((parents exists (p =>
- (p ne ap)
- || p.memberNames(abstractTypeNameFilter, tp).nonEmpty
- || p.memberNames(abstractTermNameFilter, tp).nonEmpty))
- || (refinedNames & tp.memberNames(abstractTypeNameFilter, tp)).nonEmpty
- || (refinedNames & tp.memberNames(abstractTermNameFilter, tp)).nonEmpty
- || isVolatile(ap)
- )
+ absParents.nonEmpty && {
+ absParents.lengthCompare(2) >= 0 || {
+ val ap = absParents.head
+ ((parents exists (p =>
+ (p ne ap)
+ || p.memberNames(abstractTypeNameFilter, tp).nonEmpty
+ || p.memberNames(abstractTermNameFilter, tp).nonEmpty))
+ || (refinedNames & tp.memberNames(abstractTypeNameFilter, tp)).nonEmpty
+ || (refinedNames & tp.memberNames(abstractTermNameFilter, tp)).nonEmpty
+ || isVolatile(ap))
+ }
}
}
}
}
+ /** The disjunctive normal form of this type.
+ * This collects a set of alternatives, each alternative consisting
+ * of a set of typerefs and a set of refinement names. Both sets are represented
+ * as lists, to obtain a deterministic order. Collected are
+ * all type refs reachable by following aliases and type proxies, and
+ * collecting the elements of conjunctions (&) and disjunctions (|).
+ * The set of refinement names in each alternative
+ * are the set of names in refinement types encountered during the collection.
+ */
+ final def DNF(tp: Type): List[(List[TypeRef], Set[Name])] = ctx.traceIndented(s"DNF($this)", checks) {
+ tp.dealias match {
+ case tp: TypeRef =>
+ (tp :: Nil, Set[Name]()) :: Nil
+ case RefinedType(parent, name) =>
+ for ((ps, rs) <- DNF(parent)) yield (ps, rs + name)
+ case tp: TypeProxy =>
+ DNF(tp.underlying)
+ case AndType(l, r) =>
+ for ((lps, lrs) <- DNF(l); (rps, rrs) <- DNF(r))
+ yield (lps | rps, lrs | rrs)
+ case OrType(l, r) =>
+ DNF(l) | DNF(r)
+ case tp =>
+ TypeOps.emptyDNF
+ }
+ }
+
+
+
private def enterArgBinding(formal: Symbol, info: Type, cls: ClassSymbol, decls: Scope) = {
val lazyInfo = new LazyType { // needed so we do not force `formal`.
def complete(denot: SymDenotation)(implicit ctx: Context): Unit = {
@@ -215,6 +253,6 @@ trait TypeOps { this: Context =>
}
object TypeOps {
-
+ val emptyDNF = (Nil, Set[Name]()) :: Nil
var track = false // !!!DEBUG
}
View
47 src/dotty/tools/dotc/core/Types.scala
@@ -106,7 +106,8 @@ object Types {
classSymbol.derivesFrom(cls)
/** A type T is a legal prefix in a type selection T#A if
- * T is stable or T contains no uninstantiated type variables.
+ * T is stable or T contains no abstract types
+ * !!! Todo: What about non-final vals that contain abstract types?
*/
final def isLegalPrefix(implicit ctx: Context): Boolean =
isStable || memberNames(abstractTypeNameFilter).isEmpty
@@ -127,25 +128,6 @@ object Types {
/** Is some part of this type produced as a repair for an error? */
final def isErroneous(implicit ctx: Context): Boolean = existsPart(_.isError)
- /** A type is volatile if its DNF contains an alternative of the form
- * {P1, ..., Pn}, {N1, ..., Nk}, where the Pi are parent typerefs and the
- * Nj are refinement names, and one the 4 following conditions is met:
- *
- * 1. At least two of the parents Pi are abstract types.
- * 2. One of the parents Pi is an abstract type, and one other type Pj,
- * j != i has an abstract member which has the same name as an
- * abstract member of the whole type.
- * 3. One of the parents Pi is an abstract type, and one of the refinement
- * names Nj refers to an abstract member of the whole type.
- * 4. One of the parents Pi is an abstract type with a volatile upper bound.
- *
- * Lazy values are not allowed to have volatile type, as otherwise
- * unsoundness can result.
- */
@samuelgruetter Owner

this (important) documentation is deleted, but not added anywhere

@odersky Owner
odersky added a note

Good catch. It should have been added to isVolatile in TypeOps, but that change got lost somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- final def isVolatile(implicit ctx: Context): Boolean = track("isVolatile") {
- ctx.isVolatile(this)
- }
-
/** Does the type carry an annotation that is an instance of `cls`? */
final def hasAnnotation(cls: ClassSymbol)(implicit ctx: Context): Boolean = stripTypeVar match {
case AnnotatedType(annot, tp) => (annot matches cls) || (tp hasAnnotation cls)
@@ -741,31 +723,6 @@ object Types {
def typeParamNamed(name: TypeName)(implicit ctx: Context): Symbol =
classSymbol.decls.lookup(name) orElse member(name).symbol
- /** The disjunctive normal form of this type.
- * This collects a set of alternatives, each alternative consisting
- * of a set of typerefs and a set of refinement names. Both sets are represented
- * as lists, to obtain a deterministic order. Collected are
- * all type refs reachable by following aliases and type proxies, and
- * collecting the elements of conjunctions (&) and disjunctions (|).
- * The set of refinement names in each alternative
- * are the set of names in refinement types encountered during the collection.
- */
- final def DNF(implicit ctx: Context): List[(List[TypeRef], Set[Name])] = dealias match {
- case tp: TypeRef =>
- (tp :: Nil, Set[Name]()) :: Nil
- case RefinedType(parent, name) =>
- for ((ps, rs) <- parent.DNF) yield (ps, rs + name)
- case tp: TypeProxy =>
- tp.underlying.DNF
- case AndType(l, r) =>
- for ((lps, lrs) <- l.DNF; (rps, rrs) <- r.DNF)
- yield (lps | rps, lrs | rrs)
- case OrType(l, r) =>
- l.DNF | r.DNF
- case tp =>
- emptyDNF
- }
-
// ----- Substitutions -----------------------------------------------------
/** Substitute all types that refer in their symbol attribute to
View
13 src/dotty/tools/dotc/typer/Checking.scala
@@ -23,6 +23,7 @@ trait NoChecking {
def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = tree
def checkBounds(args: List[tpd.Tree], poly: PolyType, pos: Position)(implicit ctx: Context): Unit = ()
def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit = ()
+ def checkLegalPrefix(tp: Type, pos: Position)(implicit ctx: Context): Unit = ()
def checkClassTypeWithStablePrefix(tp: Type, pos: Position, traitReq: Boolean)(implicit ctx: Context): Type = tp
def checkImplicitTptNonEmpty(defTree: untpd.ValOrDefDef)(implicit ctx: Context): Unit = ()
def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = ()
@@ -53,13 +54,17 @@ trait Checking extends NoChecking {
if (!(bounds.lo <:< arg.tpe)) notConforms("lower", bounds.lo)
}
- /** Check that type `tp` is stable.
+ /** Check that type `tp` is stable. */
+ override def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit =
+ if (!tp.isStable) ctx.error(i"$tp is not stable", pos)
+
+ /** Check that type `tp` is a legal prefix for '#'.
* @return The type itself
*/
- override def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit =
- if (!tp.isStable) ctx.error(i"Prefix of type ${tp.widenIfUnstable} is not stable", pos)
+ override def checkLegalPrefix(tp: Type, pos: Position)(implicit ctx: Context): Unit =
+ if (!tp.isLegalPrefix) ctx.error(i"$tp is not a valid prefix for '#'", pos)
- /** Check that `tp` is a class type with a stable prefix. Also, if `isFirst` is
+ /** Check that `tp` is a class type with a stable prefix. Also, if `isFirst` is
* false check that `tp` is a trait.
* @return `tp` itself if it is a class or trait ref, ObjectClass.typeRef if not.
*/
View
2  src/dotty/tools/dotc/typer/Typer.scala
@@ -265,11 +265,13 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
def typedSelect(tree: untpd.Select, pt: Type)(implicit ctx: Context): Tree = track("typedSelect") {
val qual1 = typedExpr(tree.qualifier, selectionProto(tree.name, pt, this))
+ if (tree.name.isTypeName) checkStable(qual1.tpe, qual1.pos)
checkValue(assignType(cpy.Select(tree, qual1, tree.name), qual1), pt)
}
def typedSelectFromTypeTree(tree: untpd.SelectFromTypeTree, pt: Type)(implicit ctx: Context): SelectFromTypeTree = track("typedSelectFromTypeTree") {
val qual1 = typedType(tree.qualifier, selectionProto(tree.name, pt, this))
+ checkLegalPrefix(qual1.tpe, qual1.pos)
assignType(cpy.SelectFromTypeTree(tree, qual1, tree.name), qual1)
}
View
2  src/dotty/tools/dotc/util/common.scala
@@ -7,8 +7,6 @@ import core.Types.WildcardType
/** Common values hoisted out for performance */
object common {
- val emptyDNF = (Nil, Set[Name]()) :: Nil
-
val alwaysTrue = Function.const(true) _
val alwaysZero = Function.const(0) _
val alwaysWildcardType = Function.const(WildcardType) _
View
1  test/dotc/tests.scala
@@ -52,6 +52,7 @@ class tests extends CompilerTest {
@Test def neg_rootImports = compileFile(negDir, "rootImplicits", xerrors = 2)
@Test def neg_templateParents() = compileFile(negDir, "templateParents", xerrors = 3)
@Test def neg_i39 = compileFile(negDir, "i39", xerrors = 1)
+ @Test def neg_i50_volatile = compileFile(negDir, "i50-volatile", xerrors = 4)
@Test def dotc = compileDir(dotcDir + "tools/dotc")
@Test def dotc_ast = compileDir(dotcDir + "tools/dotc/ast")
View
25 tests/neg/i50-volatile.scala
@@ -0,0 +1,25 @@
+object Test {
+ class Base {
+ class Inner
+ }
+ type A <: Base {
+ type X = String
+ }
+ type B <: {
+ type X = Int
+ }
+ lazy val o: A & B = ???
+
+ class Client extends o.Inner
+
+ def xToString(x: o.X): String = x
+
+ def intToString(i: Int): String = xToString(i)
+}
+object Test2 {
+
+ import Test.o._
+
+ def xToString(x: X): String = x
+
+}
Something went wrong with that request. Please try again.