From 8592af1c815c34078d89c08536addbe757550179 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Mon, 7 Nov 2016 23:10:01 +0100 Subject: [PATCH 1/7] fix #1670: move the check of value class to typer --- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 3 +-- compiler/src/dotty/tools/dotc/typer/Typer.scala | 5 +++++ tests/neg/i1670.scala | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 tests/neg/i1670.scala diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 46bdbf3b32d6..feeb80547426 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -689,7 +689,7 @@ object RefChecks { } /** Verify classes extending AnyVal meet the requirements */ - private def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = { + def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = { def checkValueClassMember(stat: Tree) = stat match { case _: ValDef if !stat.symbol.is(ParamAccessor) => ctx.error(s"value class may not define non-parameter field", stat.pos) @@ -836,7 +836,6 @@ class RefChecks extends MiniPhase { thisTransformer => checkParents(cls) checkCompanionNameClashes(cls) checkAllOverrides(cls) - checkDerivedValueClass(cls, tree.body) tree } catch { case ex: MergeError => diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9f5a942d6c3a..34300a5e353f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1269,6 +1269,11 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit ctx.featureWarning(nme.dynamics.toString, "extension of type scala.Dynamic", isScala2Feature = true, cls, isRequired, cdef.pos) } + + + // check value class constraints + RefChecks.checkDerivedValueClass(cls, body1) + cdef1 // todo later: check that diff --git a/tests/neg/i1670.scala b/tests/neg/i1670.scala new file mode 100644 index 000000000000..46f283e4848c --- /dev/null +++ b/tests/neg/i1670.scala @@ -0,0 +1 @@ +class A(a:Int, b:Int) extends AnyVal \ No newline at end of file From abb9aa802789e6b43ff07c45281eedbb09512585 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Mon, 7 Nov 2016 23:57:06 +0100 Subject: [PATCH 2/7] fix #1642: disallow value classe wrapping value class --- compiler/src/dotty/tools/dotc/core/TypeErasure.scala | 2 +- .../dotty/tools/dotc/transform/ExtensionMethods.scala | 8 -------- .../src/dotty/tools/dotc/transform/ValueClasses.scala | 10 ++++++++++ compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 7 +++++-- compiler/src/dotty/tools/dotc/typer/Typer.scala | 1 - tests/neg/i1642.scala | 1 + tests/neg/i1670.scala | 2 +- tests/pos/i1642.scala | 2 ++ 8 files changed, 20 insertions(+), 13 deletions(-) create mode 100644 tests/neg/i1642.scala create mode 100644 tests/pos/i1642.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index abbacee49628..28dbed8f6625 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -427,7 +427,7 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean private def eraseDerivedValueClassRef(tref: TypeRef)(implicit ctx: Context): Type = { val cls = tref.symbol.asClass val underlying = underlyingOfValueClass(cls) - if (underlying.exists) ErasedValueType(tref, valueErasure(underlying)) + if (underlying.exists && !isCyclic(cls)) ErasedValueType(tref, valueErasure(underlying)) else NoType } diff --git a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala index 5ae4e8a54de9..925ec08b2178 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala @@ -135,14 +135,6 @@ class ExtensionMethods extends MiniPhaseTransform with DenotTransformer with Ful // TODO: this is state and should be per-run // todo: check that when transformation finished map is empty - private def checkNonCyclic(pos: Position, seen: Set[Symbol], clazz: ClassSymbol)(implicit ctx: Context): Unit = - if (seen contains clazz) - ctx.error("value class may not unbox to itself", pos) - else { - val unboxed = underlyingOfValueClass(clazz).typeSymbol - if (isDerivedValueClass(unboxed)) checkNonCyclic(pos, seen + clazz, unboxed.asClass) - } - override def transformTemplate(tree: tpd.Template)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { if (isDerivedValueClass(ctx.owner)) { /* This is currently redundant since value classes may not diff --git a/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala b/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala index 93005c57ae26..b16d05644706 100644 --- a/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala +++ b/compiler/src/dotty/tools/dotc/transform/ValueClasses.scala @@ -53,4 +53,14 @@ object ValueClasses { def underlyingOfValueClass(d: ClassDenotation)(implicit ctx: Context): Type = valueClassUnbox(d).info.resultType + /** Whether a value class wraps itself */ + def isCyclic(cls: ClassSymbol)(implicit ctx: Context): Boolean = { + def recur(seen: Set[Symbol], clazz: ClassSymbol)(implicit ctx: Context): Boolean = + (seen contains clazz) || { + val unboxed = underlyingOfValueClass(clazz).typeSymbol + (isDerivedValueClass(unboxed)) && recur(seen + clazz, unboxed.asClass) + } + + recur(Set[Symbol](), cls) + } } diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index feeb80547426..f9c9ddee47e0 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -18,7 +18,7 @@ import config.{ScalaVersion, NoScalaVersion} import Decorators._ import typer.ErrorReporting._ import DenotTransformers._ -import ValueClasses.isDerivedValueClass +import ValueClasses.{isDerivedValueClass, isCyclic} object RefChecks { import tpd._ @@ -707,12 +707,15 @@ object RefChecks { ctx.error("`abstract' modifier cannot be used with value classes", clazz.pos) if (!clazz.isStatic) ctx.error(s"value class may not be a ${if (clazz.owner.isTerm) "local class" else "member of another class"}", clazz.pos) + if (isCyclic(clazz.asClass)) + ctx.error("value class cannot wrap itself", clazz.pos) else { - val clParamAccessors = clazz.asClass.paramAccessors.filter(sym => sym.isTerm && !sym.is(Method)) + val clParamAccessors = clazz.asClass.paramAccessors.filter(_.isTerm) clParamAccessors match { case List(param) => if (param.is(Mutable)) ctx.error("value class parameter must not be a var", param.pos) + case _ => ctx.error("value class needs to have exactly one val parameter", clazz.pos) } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 34300a5e353f..9d29c3a4e600 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1270,7 +1270,6 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit cls, isRequired, cdef.pos) } - // check value class constraints RefChecks.checkDerivedValueClass(cls, body1) diff --git a/tests/neg/i1642.scala b/tests/neg/i1642.scala new file mode 100644 index 000000000000..a74078da5f2f --- /dev/null +++ b/tests/neg/i1642.scala @@ -0,0 +1 @@ +class Test2(val valueVal: Test2) extends AnyVal diff --git a/tests/neg/i1670.scala b/tests/neg/i1670.scala index 46f283e4848c..f4df5ae52cca 100644 --- a/tests/neg/i1670.scala +++ b/tests/neg/i1670.scala @@ -1 +1 @@ -class A(a:Int, b:Int) extends AnyVal \ No newline at end of file +class A(a:Int, b:Int) extends AnyVal diff --git a/tests/pos/i1642.scala b/tests/pos/i1642.scala new file mode 100644 index 000000000000..2fe67cf18af7 --- /dev/null +++ b/tests/pos/i1642.scala @@ -0,0 +1,2 @@ +class Test1(val x: Int) extends AnyVal +class Test2(val y: Test1) extends AnyVal From af3e581a55a5be92e3a9001a887220fda5bcb54f Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 9 Nov 2016 10:05:35 +0100 Subject: [PATCH 3/7] don't recheck in -Ycheck --- .../tools/dotc/transform/TreeChecker.scala | 2 +- .../src/dotty/tools/dotc/typer/Checking.scala | 49 +++++++++++++++++-- .../dotty/tools/dotc/typer/RefChecks.scala | 37 -------------- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- 4 files changed, 48 insertions(+), 42 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index aa4eefe43fac..7da8bde0f1a4 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -135,7 +135,7 @@ class TreeChecker extends Phase with SymTransformer { } } - class Checker(phasesToCheck: Seq[Phase]) extends ReTyper { + class Checker(phasesToCheck: Seq[Phase]) extends ReTyper with NoChecking { val nowDefinedSyms = new mutable.HashSet[Symbol] val everDefinedSyms = new mutable.HashMap[Symbol, Tree] diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 0c5a8e5dbcdf..5e1d8622a8af 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -29,6 +29,7 @@ import ErrorReporting.{err, errorType} import config.Printers.typr import collection.mutable import SymDenotations.NoCompleter +import dotty.tools.dotc.transform.ValueClasses._ object Checking { import tpd._ @@ -56,7 +57,7 @@ object Checking { checkBounds(args, poly.paramBounds, _.substParams(poly, _)) /** Check applied type trees for well-formedness. This means - * - all arguments are within their corresponding bounds + * - all arguments are within their corresponding bounds * - if type is a higher-kinded application with wildcard arguments, * check that it or one of its supertypes can be reduced to a normal application. * Unreducible applications correspond to general existentials, and we @@ -88,12 +89,12 @@ object Checking { checkWildcardHKApply(tp.superType, pos) } case _ => - } + } def checkValidIfHKApply(implicit ctx: Context): Unit = checkWildcardHKApply(tycon.tpe.appliedTo(args.map(_.tpe)), tree.pos) checkValidIfHKApply(ctx.addMode(Mode.AllowLambdaWildcardApply)) } - + /** Check that `tp` refers to a nonAbstract class * and that the instance conforms to the self type of the created class. */ @@ -406,6 +407,43 @@ object Checking { notPrivate.errors.foreach { case (msg, pos) => ctx.errorOrMigrationWarning(msg, pos) } info } + + /** Verify classes extending AnyVal meet the requirements */ + def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = { + def checkValueClassMember(stat: Tree) = stat match { + case _: ValDef if !stat.symbol.is(ParamAccessor) => + ctx.error(s"value class may not define non-parameter field", stat.pos) + case d: DefDef if d.symbol.isConstructor => + ctx.error(s"value class may not define secondary constructor", stat.pos) + case _: MemberDef | _: Import | EmptyTree => + // ok + case _ => + ctx.error(s"value class may not contain initialization statements", stat.pos) + } + if (isDerivedValueClass(clazz)) { + if (clazz.is(Trait)) + ctx.error("Only classes (not traits) are allowed to extend AnyVal", clazz.pos) + if (clazz.is(Abstract)) + ctx.error("`abstract' modifier cannot be used with value classes", clazz.pos) + if (!clazz.isStatic) + ctx.error(s"value class may not be a ${if (clazz.owner.isTerm) "local class" else "member of another class"}", clazz.pos) + if (isCyclic(clazz.asClass)) + ctx.error("value class cannot wrap itself", clazz.pos) + else { + val clParamAccessors = clazz.asClass.paramAccessors.filter(_.isTerm) + clParamAccessors match { + case List(param) => + if (param.is(Mutable)) + ctx.error("value class parameter must not be a var", param.pos) + + case _ => + ctx.error("value class needs to have exactly one val parameter", clazz.pos) + } + } + stats.foreach(checkValueClassMember) + } + + } } trait Checking { @@ -553,6 +591,10 @@ trait Checking { errorTree(tpt, ex"Singleton type ${tpt.tpe} is not allowed $where") } else tpt + + /** Verify classes extending AnyVal meet the requirements */ + def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = + Checking.checkDerivedValueClass(clazz, stats) } trait NoChecking extends Checking { @@ -568,4 +610,5 @@ trait NoChecking extends Checking { override def checkParentCall(call: Tree, caller: ClassSymbol)(implicit ctx: Context) = () override def checkSimpleKinded(tpt: Tree)(implicit ctx: Context): Tree = tpt override def checkNotSingleton(tpt: Tree, where: String)(implicit ctx: Context): Tree = tpt + override def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = () } diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index f9c9ddee47e0..dcbd444f951c 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -18,7 +18,6 @@ import config.{ScalaVersion, NoScalaVersion} import Decorators._ import typer.ErrorReporting._ import DenotTransformers._ -import ValueClasses.{isDerivedValueClass, isCyclic} object RefChecks { import tpd._ @@ -688,42 +687,6 @@ object RefChecks { } } - /** Verify classes extending AnyVal meet the requirements */ - def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = { - def checkValueClassMember(stat: Tree) = stat match { - case _: ValDef if !stat.symbol.is(ParamAccessor) => - ctx.error(s"value class may not define non-parameter field", stat.pos) - case _: DefDef if stat.symbol.isConstructor => - ctx.error(s"value class may not define secondary constructor", stat.pos) - case _: MemberDef | _: Import | EmptyTree => - // ok - case _ => - ctx.error(s"value class may not contain initialization statements", stat.pos) - } - if (isDerivedValueClass(clazz)) { - if (clazz.is(Trait)) - ctx.error("Only classes (not traits) are allowed to extend AnyVal", clazz.pos) - if (clazz.is(Abstract)) - ctx.error("`abstract' modifier cannot be used with value classes", clazz.pos) - if (!clazz.isStatic) - ctx.error(s"value class may not be a ${if (clazz.owner.isTerm) "local class" else "member of another class"}", clazz.pos) - if (isCyclic(clazz.asClass)) - ctx.error("value class cannot wrap itself", clazz.pos) - else { - val clParamAccessors = clazz.asClass.paramAccessors.filter(_.isTerm) - clParamAccessors match { - case List(param) => - if (param.is(Mutable)) - ctx.error("value class parameter must not be a var", param.pos) - - case _ => - ctx.error("value class needs to have exactly one val parameter", clazz.pos) - } - } - stats.foreach(checkValueClassMember) - } - } - type LevelAndIndex = immutable.Map[Symbol, (LevelInfo, Int)] class OptLevelInfo extends DotClass { diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9d29c3a4e600..bc335299b4ec 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1271,7 +1271,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit } // check value class constraints - RefChecks.checkDerivedValueClass(cls, body1) + checkDerivedValueClass(cls, body1) cdef1 From 93f04c863379fa1d73c1244f54d8fd5e136ac9c2 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 9 Nov 2016 11:31:48 +0100 Subject: [PATCH 4/7] remove invalid field in value class --- tests/pickling/extmethods.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/pickling/extmethods.scala b/tests/pickling/extmethods.scala index 1cbc9f2eef85..af1882f7a26d 100644 --- a/tests/pickling/extmethods.scala +++ b/tests/pickling/extmethods.scala @@ -3,6 +3,5 @@ package extMethods trait That1[A] class T[A, This <: That1[A]](val x: Int) extends AnyVal { self: This => - var next: This = _ final def loop(x: This, cnt: Int): Int = loop(x, cnt + 1) } From 7f7d2cc8617b8d969d1399f4616f5d657fe6cdfd Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Wed, 9 Nov 2016 13:07:31 +0100 Subject: [PATCH 5/7] fix failing neg tests --- tests/neg/i1642.scala | 2 +- tests/neg/i1670.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/neg/i1642.scala b/tests/neg/i1642.scala index a74078da5f2f..2f7644698f0f 100644 --- a/tests/neg/i1642.scala +++ b/tests/neg/i1642.scala @@ -1 +1 @@ -class Test2(val valueVal: Test2) extends AnyVal +class Test2(val valueVal: Test2) extends AnyVal // error: value class cannot wrap itself diff --git a/tests/neg/i1670.scala b/tests/neg/i1670.scala index f4df5ae52cca..69c47eda7997 100644 --- a/tests/neg/i1670.scala +++ b/tests/neg/i1670.scala @@ -1 +1 @@ -class A(a:Int, b:Int) extends AnyVal +class A(a:Int, b:Int) extends AnyVal // error: value class needs to have exactly one val parameter From 8d97bc24cd26a5f06a07e733b59e82ef77b2e896 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Tue, 15 Nov 2016 18:00:28 +0100 Subject: [PATCH 6/7] renable checking for TreeChecker --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 7da8bde0f1a4..328c8204def5 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -135,11 +135,14 @@ class TreeChecker extends Phase with SymTransformer { } } - class Checker(phasesToCheck: Seq[Phase]) extends ReTyper with NoChecking { + class Checker(phasesToCheck: Seq[Phase]) extends ReTyper with Checking { val nowDefinedSyms = new mutable.HashSet[Symbol] val everDefinedSyms = new mutable.HashMap[Symbol, Tree] + // don't check value classes after typer, as the constraint about constructors doesn't hold after transform + override def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = () + def withDefinedSym[T](tree: untpd.Tree)(op: => T)(implicit ctx: Context): T = tree match { case tree: DefTree => val sym = tree.symbol From 9b8aaaf899474d3b6ce7b73f637866953f33fd48 Mon Sep 17 00:00:00 2001 From: liu fengyun Date: Thu, 17 Nov 2016 08:14:38 +0100 Subject: [PATCH 7/7] fix test i705-inner-value-class2.scala --- tests/neg/i705-inner-value-class2.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/neg/i705-inner-value-class2.scala b/tests/neg/i705-inner-value-class2.scala index a084da338583..d59449b6be51 100644 --- a/tests/neg/i705-inner-value-class2.scala +++ b/tests/neg/i705-inner-value-class2.scala @@ -1,5 +1,5 @@ class Foo { - class B(val a: Int) extends AnyVal + class B(val a: Int) extends AnyVal // error: value class may not be a member of another class` } object Test {