Adress review

Author: OlivierBlanvillain

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] =>

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

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
   }
 }

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.