From 22677120e3fdbb9f9834b059b6473ab3fb10e810 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 4 Jul 2018 16:47:44 +0200 Subject: [PATCH 1/4] Don't warn if impure expressions are passed to erased vals `erased` is really the same thing as `lazy` or by-name. The semantics implies it will not be evaluated. So it is pointless to warn if an impure expression is passed to an erased val or parameter. Moreover, erased initializers are often complex expressions - after all if they were not, why erase them in the first place? These expressions are almost never recognized as pure given the weak effect checking we currently have. So the warnings are issued in the common case, which is bad. --- .../dotty/tools/dotc/typer/Applications.scala | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 11ed46942e08..bcf5493396b0 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -799,7 +799,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic => } app match { case Apply(fun, args) if fun.tpe.widen.isErasedMethod => - tpd.cpy.Apply(app)(fun = fun, args = args.map(arg => normalizeErasedExpr(arg, "This argument is given to an erased parameter. "))) + tpd.cpy.Apply(app)(fun = fun, args = args.map(arg => defaultValue(arg.tpe))) case _ => app } } @@ -1567,22 +1567,11 @@ trait Applications extends Compatibility { self: Typer with Dynamic => harmonizedElems } - /** Transforms the tree into a its default tree. - * Performed to shrink the tree that is known to be erased later. - */ - protected def normalizeErasedExpr(tree: Tree, msg: String)(implicit ctx: Context): Tree = { - if (!isPureExpr(tree)) - ctx.warning(msg + "This expression will not be evaluated.", tree.pos) - defaultValue(tree.tpe) - } - /** Transforms the rhs tree into a its default tree if it is in an `erased` val/def. * Performed to shrink the tree that is known to be erased later. */ - protected def normalizeErasedRhs(rhs: Tree, sym: Symbol)(implicit ctx: Context) = { - if (sym.is(Erased) && rhs.tpe.exists) normalizeErasedExpr(rhs, "Expression is on the RHS of an `erased` " + sym.showKind + ". ") - else rhs - } + protected def normalizeErasedRhs(rhs: Tree, sym: Symbol)(implicit ctx: Context) = + if (sym.is(Erased) && rhs.tpe.exists) defaultValue(rhs.tpe) else rhs /** If all `types` are numeric value types, and they are not all the same type, * pick a common numeric supertype and widen any constant types in `tpes` to it. From 6a120792a609deff234a5728798b5f08e15f8a8a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 4 Jul 2018 16:57:59 +0200 Subject: [PATCH 2/4] Erased values are stable but not realizable In that sense they behave exactly like lazy vals. Making them unstable threw out most use cases. We typically set up an erased value because it contains interesting types. But to get at the types the values have to be stable in the first place! --- compiler/src/dotty/tools/dotc/core/CheckRealizable.scala | 8 ++++---- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- tests/neg/erased-24.scala | 6 +++--- tests/neg/i4060.scala | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala index bac81942f8e3..4fcb4f317d59 100644 --- a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala +++ b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala @@ -63,10 +63,10 @@ class CheckRealizable(implicit ctx: Context) { */ private val checkedFields: mutable.Set[Symbol] = mutable.LinkedHashSet[Symbol]() - /** Is symbol's definitition a lazy val? + /** Is symbol's definitition a lazy or erased val? * (note we exclude modules here, because their realizability is ensured separately) */ - private def isLateInitialized(sym: Symbol) = sym.is(Lazy, butNot = Module) + private def isLateInitialized(sym: Symbol) = sym.is(Lazy | Erased, butNot = Module) /** The realizability status of given type `tp`*/ def realizability(tp: Type): Realizability = tp.dealias match { @@ -156,10 +156,10 @@ class CheckRealizable(implicit ctx: Context) { private def memberRealizability(tp: Type) = { def checkField(sofar: Realizability, fld: SingleDenotation): Realizability = sofar andAlso { - if (checkedFields.contains(fld.symbol) || fld.symbol.is(Private | Mutable | Lazy)) + if (checkedFields.contains(fld.symbol) || fld.symbol.is(Private | Mutable | Lazy | Erased)) // if field is private it cannot be part of a visible path // if field is mutable it cannot be part of a path - // if field is lazy it does not need to be initialized when the owning object is + // if field is lazy or erased it does not need to be initialized when the owning object is // so in all cases the field does not influence realizability of the enclosing object. Realizable else { diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 7fc436d79126..e06a4260831c 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -597,7 +597,7 @@ object SymDenotations { /** Is this a denotation of a stable term (or an arbitrary type)? */ final def isStable(implicit ctx: Context) = - isType || !is(Erased) && (is(Stable) || !(is(UnstableValue) || info.isInstanceOf[ExprType])) + isType || is(Stable) || !(is(UnstableValue) || info.isInstanceOf[ExprType]) /** Is this a "real" method? A real method is a method which is: * - not an accessor diff --git a/tests/neg/erased-24.scala b/tests/neg/erased-24.scala index 692ea19794cd..dc39d4f9b653 100644 --- a/tests/neg/erased-24.scala +++ b/tests/neg/erased-24.scala @@ -6,11 +6,11 @@ object Test { println(fun(new Bar)) } - def fun(erased foo: Foo): foo.X = { // error - null.asInstanceOf[foo.X] // error + def fun(erased foo: Foo): foo.X = { // ok + null.asInstanceOf[foo.X] // ok } - def fun2(erased foo: Foo)(erased bar: foo.B): bar.X = { // error // error + def fun2(erased foo: Foo)(erased bar: foo.B): bar.X = { // error null.asInstanceOf[bar.X] // error } } diff --git a/tests/neg/i4060.scala b/tests/neg/i4060.scala index 2d9321a5ed50..3d5c180b5d7b 100644 --- a/tests/neg/i4060.scala +++ b/tests/neg/i4060.scala @@ -1,5 +1,5 @@ class X { type R } -class T(erased val a: X)(val value: a.R) // error +class T(erased val a: X)(val value: a.R) object App { def coerce[U, V](u: U): V = { @@ -8,7 +8,7 @@ object App { class T[A <: X](erased val a: A)(val value: a.R) // error - object O { lazy val x : Y & X = ??? } // warning + object O { lazy val x : Y & X = ??? } val a = new T[Y & X](O.x)(u) a.value From e42b83e8101c98bba5fbaf5de2eee32fcebb092c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 4 Jul 2018 16:59:38 +0200 Subject: [PATCH 3/4] Unrelated hardening of ExpandPrivate I noticed a REPL crash because we triggered the error condition but there was no source associated with context.owner. --- compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala b/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala index ec82050366d9..5a4084012945 100644 --- a/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala +++ b/compiler/src/dotty/tools/dotc/transform/ExpandPrivate.scala @@ -90,6 +90,7 @@ class ExpandPrivate extends MiniPhase with IdentityDenotTransformer { thisPhase } assert(d.symbol.sourceFile != null && + ctx.owner.sourceFile != null && isSimilar(d.symbol.sourceFile.path, ctx.owner.sourceFile.path), s"private ${d.symbol.showLocated} in ${d.symbol.sourceFile} accessed from ${ctx.owner.showLocated} in ${ctx.owner.sourceFile}") d.ensureNotPrivate.installAfter(thisPhase) From 10f96b47822d2e2f174e8fab65ea92eae64599ff Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 4 Jul 2018 17:04:56 +0200 Subject: [PATCH 4/4] Small optimization Flag `|` is not super-fast, so it might pay to compute this once ahead of time. --- compiler/src/dotty/tools/dotc/core/CheckRealizable.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala index 4fcb4f317d59..e4009c863738 100644 --- a/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala +++ b/compiler/src/dotty/tools/dotc/core/CheckRealizable.scala @@ -52,6 +52,8 @@ object CheckRealizable { def boundsRealizability(tp: Type)(implicit ctx: Context) = new CheckRealizable().boundsRealizability(tp) + + private val LateInitialized = Lazy | Erased, } /** Compute realizability status */ @@ -66,7 +68,7 @@ class CheckRealizable(implicit ctx: Context) { /** Is symbol's definitition a lazy or erased val? * (note we exclude modules here, because their realizability is ensured separately) */ - private def isLateInitialized(sym: Symbol) = sym.is(Lazy | Erased, butNot = Module) + private def isLateInitialized(sym: Symbol) = sym.is(LateInitialized, butNot = Module) /** The realizability status of given type `tp`*/ def realizability(tp: Type): Realizability = tp.dealias match {