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.

Author: OlivierBlanvillain

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

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 {

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

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