From be8bceed8afeffe13970e297dd75cb935359c63f Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 19 Jun 2017 13:54:54 +0200 Subject: [PATCH 1/7] Fix #2772: Special case Devalify for java.lang.System.* Also update DropNoEffects to keep Select on static java field, to make sure a Java class doesn't end up in value position. That's another bug than the one reported in #2772, but it got triggered by the new test case. --- .../src/dotty/tools/dotc/core/Definitions.scala | 1 + .../dotc/transform/localopt/Devalify.scala | 17 +++++++++++++++-- .../dotc/transform/localopt/DropNoEffects.scala | 3 ++- tests/run/2772.scala | 14 ++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 tests/run/2772.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 5e58e60c3603..88335d1fe91b 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -483,6 +483,7 @@ class Definitions { lazy val BoxedFloatModule = ctx.requiredModule("java.lang.Float") lazy val BoxedDoubleModule = ctx.requiredModule("java.lang.Double") lazy val BoxedUnitModule = ctx.requiredModule("java.lang.Void") + lazy val SystemModule = ctx.requiredModule("java.lang.System") lazy val ByNameParamClass2x = enterSpecialPolyClass(tpnme.BYNAME_PARAM_CLASS, Covariant, Seq(AnyType)) lazy val EqualsPatternClass = enterSpecialPolyClass(tpnme.EQUALS_PATTERN, EmptyFlags, Seq(AnyType)) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala index 91fdaa51d35f..52c5449715b2 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala @@ -166,10 +166,12 @@ class Devalify extends Optimisation { def readingOnlyVals(t: Tree)(implicit ctx: Context): Boolean = dropCasts(t) match { case Typed(exp, _) => readingOnlyVals(exp) + case TypeApply(fun @ Select(rec, _), List(tp)) => if ((fun.symbol eq defn.Any_asInstanceOf) && rec.tpe.derivesFrom(tp.tpe.classSymbol)) readingOnlyVals(rec) else false + case Apply(Select(rec, _), Nil) => def isGetterOfAImmutableField = t.symbol.isGetter && !t.symbol.is(Mutable) def isCaseClassWithVar = t.symbol.info.decls.exists(_.is(Mutable)) @@ -182,8 +184,10 @@ class Devalify extends Optimisation { if (isGetterOfAImmutableField || isAccessingProductField || isImmutableCaseAccessor) readingOnlyVals(rec) else false + case Select(rec, _) if t.symbol.is(Method) => - if (t.symbol.isGetter && !t.symbol.is(Mutable)) readingOnlyVals(rec) // getter of a immutable field + if (t.symbol.isGetter && !t.symbol.is(Mutable)) + readingOnlyVals(rec) // getter of a immutable field else if (t.symbol.owner.derivesFrom(defn.ProductClass) && t.symbol.owner.is(CaseClass) && t.symbol.name.isSelectorName) { def isImmutableField = { val fieldId = t.symbol.name.toString.drop(1).toInt - 1 @@ -194,13 +198,22 @@ class Devalify extends Optimisation { } else if (t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable)) readingOnlyVals(rec) else false + case Select(qual, _) if !t.symbol.is(Mutable) => - readingOnlyVals(qual) + if (t.symbol == defn.SystemModule) { + // System.in is static final fields that, for legacy reasons, must be + // allowed to be changed by the methods System.setIn... + // https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4 + false + } else + readingOnlyVals(qual) + case t: Ident if !t.symbol.is(Mutable) && !t.symbol.is(Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => desugarIdent(t) match { case Some(t) => readingOnlyVals(t) case None => true } + case t: This => true // null => false, or the following fails devalify: // trait I { diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala b/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala index 8593d91ade66..5f8985535519 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala @@ -87,7 +87,8 @@ class DropNoEffects(val simplifyPhase: Simplify) extends Optimisation { keepOnlySideEffects(rec) // !name.eq(nme.TYPE_) && // Keep the .TYPE added by ClassOf, would be needed for AfterErasure - case s @ Select(qual, name) if !s.symbol.is(Mutable | Lazy | Method) => + // Without is(JavaStatic), { System.out } becomes { System }, but "Java class can't be used as value" + case s @ Select(qual, name) if !s.symbol.is(Mutable | Lazy | Method | JavaStatic) => keepOnlySideEffects(qual) case Block(List(t: DefDef), s: Closure) => diff --git a/tests/run/2772.scala b/tests/run/2772.scala new file mode 100644 index 000000000000..fcde4c363b35 --- /dev/null +++ b/tests/run/2772.scala @@ -0,0 +1,14 @@ +import java.io.OutputStream + +object Test { + def main(args: Array[String]): Unit = { + val oldErr = System.err + System.setErr(null) // setOut(null) confuses the testing framework... + val a = () => foo(oldErr) + a() + } + + def foo(err: OutputStream): Unit = { + err.write(0) + } +} From f4c19d93d1ab6843dd45cb68bc1001575fa7f279 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 19 Jun 2017 16:32:35 +0200 Subject: [PATCH 2/7] Slight Devalify refactorings --- .../dotc/transform/localopt/Devalify.scala | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala index 52c5449715b2..a4fc69e1a2e0 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala @@ -164,7 +164,17 @@ class Devalify extends Optimisation { case _ => t } - def readingOnlyVals(t: Tree)(implicit ctx: Context): Boolean = dropCasts(t) match { + def readingOnlyVals(t: Tree)(implicit ctx: Context): Boolean = { + def isGetterOfAImmutableField = t.symbol.isGetter && !t.symbol.is(Mutable) + def isCaseClassWithVar = t.symbol.info.decls.exists(_.is(Mutable)) + def isAccessingProductField = t.symbol.exists && + t.symbol.owner.derivesFrom(defn.ProductClass) && + t.symbol.owner.is(CaseClass) && + t.symbol.name.isSelectorName && + !isCaseClassWithVar // Conservatively covers case class A(var x: Int) + def isImmutableCaseAccessor = t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable) + + dropCasts(t) match { case Typed(exp, _) => readingOnlyVals(exp) case TypeApply(fun @ Select(rec, _), List(tp)) => @@ -173,29 +183,21 @@ class Devalify extends Optimisation { else false case Apply(Select(rec, _), Nil) => - def isGetterOfAImmutableField = t.symbol.isGetter && !t.symbol.is(Mutable) - def isCaseClassWithVar = t.symbol.info.decls.exists(_.is(Mutable)) - def isAccessingProductField = t.symbol.exists && - t.symbol.owner.derivesFrom(defn.ProductClass) && - t.symbol.owner.is(CaseClass) && - t.symbol.name.isSelectorName && - !isCaseClassWithVar // Conservative Covers case class A(var x: Int) - def isImmutableCaseAccessor = t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable) if (isGetterOfAImmutableField || isAccessingProductField || isImmutableCaseAccessor) readingOnlyVals(rec) else false case Select(rec, _) if t.symbol.is(Method) => - if (t.symbol.isGetter && !t.symbol.is(Mutable)) - readingOnlyVals(rec) // getter of a immutable field - else if (t.symbol.owner.derivesFrom(defn.ProductClass) && t.symbol.owner.is(CaseClass) && t.symbol.name.isSelectorName) { + if (isGetterOfAImmutableField) + readingOnlyVals(rec) // Getter of an immutable field + else if (isAccessingProductField) { def isImmutableField = { val fieldId = t.symbol.name.toString.drop(1).toInt - 1 !t.symbol.owner.caseAccessors(ctx)(fieldId).is(Mutable) } - if (isImmutableField) readingOnlyVals(rec) // accessing a field of a product + if (isImmutableField) readingOnlyVals(rec) // Accessing a field of a product else false - } else if (t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable)) + } else if (isImmutableCaseAccessor) readingOnlyVals(rec) else false @@ -208,7 +210,7 @@ class Devalify extends Optimisation { } else readingOnlyVals(qual) - case t: Ident if !t.symbol.is(Mutable) && !t.symbol.is(Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => + case t: Ident if !t.symbol.is(Mutable | Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => desugarIdent(t) match { case Some(t) => readingOnlyVals(t) case None => true @@ -228,5 +230,6 @@ class Devalify extends Optimisation { case Literal(Constant(null)) => false case t: Literal => true case _ => false + } } } From faa88642d2cbc0e052632c0339f4efb6cf380d04 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 19 Jun 2017 16:33:15 +0200 Subject: [PATCH 3/7] Indentation --- .../dotc/transform/localopt/Devalify.scala | 106 +++++++++--------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala index a4fc69e1a2e0..ff9fb6044848 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala @@ -175,61 +175,61 @@ class Devalify extends Optimisation { def isImmutableCaseAccessor = t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable) dropCasts(t) match { - case Typed(exp, _) => readingOnlyVals(exp) - - case TypeApply(fun @ Select(rec, _), List(tp)) => - if ((fun.symbol eq defn.Any_asInstanceOf) && rec.tpe.derivesFrom(tp.tpe.classSymbol)) - readingOnlyVals(rec) - else false - - case Apply(Select(rec, _), Nil) => - if (isGetterOfAImmutableField || isAccessingProductField || isImmutableCaseAccessor) - readingOnlyVals(rec) - else false - - case Select(rec, _) if t.symbol.is(Method) => - if (isGetterOfAImmutableField) - readingOnlyVals(rec) // Getter of an immutable field - else if (isAccessingProductField) { - def isImmutableField = { - val fieldId = t.symbol.name.toString.drop(1).toInt - 1 - !t.symbol.owner.caseAccessors(ctx)(fieldId).is(Mutable) - } - if (isImmutableField) readingOnlyVals(rec) // Accessing a field of a product + case Typed(exp, _) => readingOnlyVals(exp) + + case TypeApply(fun @ Select(rec, _), List(tp)) => + if ((fun.symbol eq defn.Any_asInstanceOf) && rec.tpe.derivesFrom(tp.tpe.classSymbol)) + readingOnlyVals(rec) else false - } else if (isImmutableCaseAccessor) - readingOnlyVals(rec) - else false - - case Select(qual, _) if !t.symbol.is(Mutable) => - if (t.symbol == defn.SystemModule) { - // System.in is static final fields that, for legacy reasons, must be - // allowed to be changed by the methods System.setIn... - // https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4 - false - } else - readingOnlyVals(qual) - - case t: Ident if !t.symbol.is(Mutable | Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => - desugarIdent(t) match { - case Some(t) => readingOnlyVals(t) - case None => true - } - case t: This => true - // null => false, or the following fails devalify: - // trait I { - // def foo: Any = null - // } - // object Main { - // def main = { - // val s: I = null - // s.foo - // } - // } - case Literal(Constant(null)) => false - case t: Literal => true - case _ => false + case Apply(Select(rec, _), Nil) => + if (isGetterOfAImmutableField || isAccessingProductField || isImmutableCaseAccessor) + readingOnlyVals(rec) + else false + + case Select(rec, _) if t.symbol.is(Method) => + if (isGetterOfAImmutableField) + readingOnlyVals(rec) // Getter of an immutable field + else if (isAccessingProductField) { + def isImmutableField = { + val fieldId = t.symbol.name.toString.drop(1).toInt - 1 + !t.symbol.owner.caseAccessors(ctx)(fieldId).is(Mutable) + } + if (isImmutableField) readingOnlyVals(rec) // Accessing a field of a product + else false + } else if (isImmutableCaseAccessor) + readingOnlyVals(rec) + else false + + case Select(qual, _) if !t.symbol.is(Mutable) => + if (t.symbol == defn.SystemModule) { + // System.in is static final fields that, for legacy reasons, must be + // allowed to be changed by the methods System.setIn... + // https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4 + false + } else + readingOnlyVals(qual) + + case t: Ident if !t.symbol.is(Mutable | Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => + desugarIdent(t) match { + case Some(t) => readingOnlyVals(t) + case None => true + } + + case t: This => true + // null => false, or the following fails devalify: + // trait I { + // def foo: Any = null + // } + // object Main { + // def main = { + // val s: I = null + // s.foo + // } + // } + case Literal(Constant(null)) => false + case t: Literal => true + case _ => false } } } From cd035b2e473852ee9f34f5b37fe774364e637434 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 19 Jun 2017 17:17:41 +0200 Subject: [PATCH 4/7] Factor out isMutable logic, use it in DropGoodCasts --- .../tools/dotc/transform/localopt/Devalify.scala | 12 +++--------- .../dotc/transform/localopt/DropGoodCasts.scala | 10 +++++++--- .../tools/dotc/transform/localopt/Simplify.scala | 11 +++++++++++ 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala index ff9fb6044848..961eb2b839e4 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala @@ -10,7 +10,7 @@ import core.Types._ import ast.Trees._ import scala.collection.mutable import config.Printers.simplify -import Simplify.desugarIdent +import Simplify.{desugarIdent, isMutable} import transform.SymUtils._ /** Inline vals and remove vals that are aliases to other vals @@ -201,14 +201,8 @@ class Devalify extends Optimisation { readingOnlyVals(rec) else false - case Select(qual, _) if !t.symbol.is(Mutable) => - if (t.symbol == defn.SystemModule) { - // System.in is static final fields that, for legacy reasons, must be - // allowed to be changed by the methods System.setIn... - // https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4 - false - } else - readingOnlyVals(qual) + case t @ Select(qual, _) if !isMutable(t) => + readingOnlyVals(qual) case t: Ident if !t.symbol.is(Mutable | Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => desugarIdent(t) match { diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala b/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala index c6b467dbf549..1c004438ffdf 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala @@ -10,6 +10,7 @@ import core.Types._ import core.Flags._ import ast.Trees._ import transform.SymUtils._ +import Simplify.isMutable /** Eliminated casts and equality tests whose results can be locally * determined at compile time: @@ -77,7 +78,8 @@ import transform.SymUtils._ case TypeApply(fun @ Select(x, _), List(tp)) if fun.symbol.eq(defn.Any_isInstanceOf) && - !x.symbol.is(Method | Mutable) && + !isMutable(x) && + !x.symbol.is(Method) && x.symbol.exists && !x.symbol.owner.isClass => (x.symbol, tp.tpe) :: Nil @@ -89,14 +91,16 @@ import transform.SymUtils._ def collectNullTests(t: Tree)(implicit ctx: Context): List[Symbol] = { def recur(t: Tree): List[Symbol] = t match { - case Apply(x, _) if (x.symbol == defn.Boolean_! || x.symbol == defn.Boolean_||) => Nil + case Apply(x, _) if (x.symbol == defn.Boolean_! || x.symbol == defn.Boolean_||) => + Nil case Apply(fun @ Select(x, _), y) if (fun.symbol == defn.Boolean_&&) => recur(x) ++ recur(y.head) case Apply(fun @ Select(x, _), List(tp)) if fun.symbol.eq(defn.Object_ne) && - !x.symbol.is(Method | Mutable) && + !isMutable(x) && + !x.symbol.is(Method) && x.symbol.exists && !x.symbol.owner.isClass => x.symbol :: Nil diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala index fcd3a6872b49..2ca16887dddd 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala @@ -146,4 +146,15 @@ object Simplify { case _ => None } } + + // System.in is static final fields that, for legacy reasons, must be + // allowed to be changed by the methods System.setIn. `isMutable` is true + // is the field is Mutable or if it's a field of java.lang.System. + // https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4 + def isMutable(t: Tree)(implicit ctx: Context): Boolean = t match { + case _ if t.symbol.is(Mutable) => true + case s: Symbol => (s.symbol == defn.SystemModule) + case i: Ident => desugarIdent(i).exists(isMutable) + case _ => false + } } From f824164c1923e1815c7ef70399dcef5bf9053f76 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Mon, 19 Jun 2017 17:20:23 +0200 Subject: [PATCH 5/7] Document that local optimisations assumes final fields are final --- docs/docs/reference/optimisations.md | 8 ++++++++ docs/sidebar.yml | 2 ++ 2 files changed, 10 insertions(+) create mode 100644 docs/docs/reference/optimisations.md diff --git a/docs/docs/reference/optimisations.md b/docs/docs/reference/optimisations.md new file mode 100644 index 000000000000..f9be4a6921be --- /dev/null +++ b/docs/docs/reference/optimisations.md @@ -0,0 +1,8 @@ +--- +layout: doc-page +title: "Local Optimisations" +--- + +The Dotty compiler implements several local optimisations to generate faster bytecode. These optimisations can be enabled by compiling with the `-optimise` flag. The current best source of documentation about what tree transformations are performed under `-optimise` is the Scaladoc classes in the [localopt package](https://github.com/lampepfl/dotty/tree/master/compiler/src/dotty/tools/dotc/transform/localopt). + +Local optimisations assumes no use of Java Reflection to mutate static final fields. diff --git a/docs/sidebar.yml b/docs/sidebar.yml index 8a769aafe6c8..9816b695c425 100644 --- a/docs/sidebar.yml +++ b/docs/sidebar.yml @@ -51,6 +51,8 @@ sidebar: url: docs/reference/auto-parameter-tupling.html - title: Named Type Arguments url: docs/reference/named-typeargs.html + - title: Local Optimisations + url: docs/reference/optimisations.html - title: Changed Features subsection: - title: Volatile Lazy Vals From cf540519c6d71c0dabf0e898106f74476fa706c2 Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Tue, 20 Jun 2017 08:50:16 +0200 Subject: [PATCH 6/7] Adress review --- .../tools/dotc/transform/localopt/Devalify.scala | 4 ++-- .../dotc/transform/localopt/DropGoodCasts.scala | 6 +++--- .../tools/dotc/transform/localopt/Simplify.scala | 14 +++++++------- docs/docs/reference/optimisations.md | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala index 961eb2b839e4..de58139ad531 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala @@ -10,7 +10,7 @@ import core.Types._ import ast.Trees._ import scala.collection.mutable import config.Printers.simplify -import Simplify.{desugarIdent, isMutable} +import Simplify.{desugarIdent, isEffectivelyMutable} import transform.SymUtils._ /** Inline vals and remove vals that are aliases to other vals @@ -201,7 +201,7 @@ class Devalify extends Optimisation { readingOnlyVals(rec) else false - case t @ Select(qual, _) if !isMutable(t) => + case t @ Select(qual, _) if !isEffectivelyMutable(t) => readingOnlyVals(qual) case t: Ident if !t.symbol.is(Mutable | Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala b/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala index 1c004438ffdf..72f2aefe0457 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala @@ -10,7 +10,7 @@ import core.Types._ import core.Flags._ import ast.Trees._ import transform.SymUtils._ -import Simplify.isMutable +import Simplify.isEffectivelyMutable /** Eliminated casts and equality tests whose results can be locally * determined at compile time: @@ -78,7 +78,7 @@ import Simplify.isMutable case TypeApply(fun @ Select(x, _), List(tp)) if fun.symbol.eq(defn.Any_isInstanceOf) && - !isMutable(x) && + !isEffectivelyMutable(x) && !x.symbol.is(Method) && x.symbol.exists && !x.symbol.owner.isClass => (x.symbol, tp.tpe) :: Nil @@ -99,7 +99,7 @@ import Simplify.isMutable case Apply(fun @ Select(x, _), List(tp)) if fun.symbol.eq(defn.Object_ne) && - !isMutable(x) && + !isEffectivelyMutable(x) && !x.symbol.is(Method) && x.symbol.exists && !x.symbol.owner.isClass => x.symbol :: Nil diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala index 2ca16887dddd..790978b481c6 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala @@ -147,14 +147,14 @@ object Simplify { } } - // System.in is static final fields that, for legacy reasons, must be - // allowed to be changed by the methods System.setIn. `isMutable` is true - // is the field is Mutable or if it's a field of java.lang.System. - // https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4 - def isMutable(t: Tree)(implicit ctx: Context): Boolean = t match { + /** Is this tree mutable, or java.lang.System.{in, out, err}? These three + * System members are the only static final fields that are mutable. + * See https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4 + */ + def isEffectivelyMutable(t: Tree)(implicit ctx: Context): Boolean = t match { case _ if t.symbol.is(Mutable) => true - case s: Symbol => (s.symbol == defn.SystemModule) - case i: Ident => desugarIdent(i).exists(isMutable) + case s: Select => (s.symbol == defn.SystemModule) + case i: Ident => desugarIdent(i).exists(isEffectivelyMutable) case _ => false } } diff --git a/docs/docs/reference/optimisations.md b/docs/docs/reference/optimisations.md index f9be4a6921be..2e847989a7c4 100644 --- a/docs/docs/reference/optimisations.md +++ b/docs/docs/reference/optimisations.md @@ -5,4 +5,4 @@ title: "Local Optimisations" The Dotty compiler implements several local optimisations to generate faster bytecode. These optimisations can be enabled by compiling with the `-optimise` flag. The current best source of documentation about what tree transformations are performed under `-optimise` is the Scaladoc classes in the [localopt package](https://github.com/lampepfl/dotty/tree/master/compiler/src/dotty/tools/dotc/transform/localopt). -Local optimisations assumes no use of Java Reflection to mutate static final fields. +Local optimisations assumes no use of Java Reflection to mutate final fields. From 09d4a8c10f9484550c193bc94ae2c527e1d1f2ae Mon Sep 17 00:00:00 2001 From: Olivier Blanvillain Date: Wed, 21 Jun 2017 11:20:35 +0200 Subject: [PATCH 7/7] Use symbol.owner --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 4 +++- .../src/dotty/tools/dotc/transform/localopt/Simplify.scala | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 88335d1fe91b..32b83324cd5e 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -483,7 +483,6 @@ class Definitions { lazy val BoxedFloatModule = ctx.requiredModule("java.lang.Float") lazy val BoxedDoubleModule = ctx.requiredModule("java.lang.Double") lazy val BoxedUnitModule = ctx.requiredModule("java.lang.Void") - lazy val SystemModule = ctx.requiredModule("java.lang.System") lazy val ByNameParamClass2x = enterSpecialPolyClass(tpnme.BYNAME_PARAM_CLASS, Covariant, Seq(AnyType)) lazy val EqualsPatternClass = enterSpecialPolyClass(tpnme.EQUALS_PATTERN, EmptyFlags, Seq(AnyType)) @@ -516,6 +515,9 @@ class Definitions { lazy val JavaSerializableClass = ctx.requiredClass("java.io.Serializable") lazy val ComparableClass = ctx.requiredClass("java.lang.Comparable") + lazy val SystemClass = ctx.requiredClass("java.lang.System") + lazy val SystemModule = SystemClass.linkedClass + // in scalac modified to have Any as parent lazy val SerializableType: TypeRef = ctx.requiredClassRef("scala.Serializable") diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala index 790978b481c6..5d17bf8608a5 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala @@ -153,7 +153,7 @@ object Simplify { */ def isEffectivelyMutable(t: Tree)(implicit ctx: Context): Boolean = t match { case _ if t.symbol.is(Mutable) => true - case s: Select => (s.symbol == defn.SystemModule) + case s: Select => s.symbol.owner == defn.SystemModule case i: Ident => desugarIdent(i).exists(isEffectivelyMutable) case _ => false }