Skip to content

Commit 45e4030

Browse files
smartersmarter
authored
Merge pull request #2781 from dotty-staging/final-not-final
Fix #2772: Special case Devalify for java.lang.System.*
2 parents 34ea263 + 09d4a8c commit 45e4030

File tree

8 files changed

+108
-55
lines changed

8 files changed

+108
-55
lines changed

‎compiler/src/dotty/tools/dotc/core/Definitions.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,9 @@ class Definitions {
514514
lazy val JavaSerializableClass = ctx.requiredClass("java.io.Serializable")
515515
lazy val ComparableClass = ctx.requiredClass("java.lang.Comparable")
516516

517+
lazy val SystemClass = ctx.requiredClass("java.lang.System")
518+
lazy val SystemModule = SystemClass.linkedClass
519+
517520
// in scalac modified to have Any as parent
518521

519522
lazy val SerializableType: TypeRef = ctx.requiredClassRef("scala.Serializable")

‎compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala

Lines changed: 61 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import core.Types._
1010
import ast.Trees._
1111
import scala.collection.mutable
1212
import config.Printers.simplify
13-
import Simplify.desugarIdent
13+
import Simplify.{desugarIdent, isEffectivelyMutable}
1414
import transform.SymUtils._
1515

1616
/** Inline vals and remove vals that are aliases to other vals
@@ -164,56 +164,66 @@ class Devalify extends Optimisation {
164164
case _ => t
165165
}
166166

167-
def readingOnlyVals(t: Tree)(implicit ctx: Context): Boolean = dropCasts(t) match {
168-
case Typed(exp, _) => readingOnlyVals(exp)
169-
case TypeApply(fun @ Select(rec, _), List(tp)) =>
170-
if ((fun.symbol eq defn.Any_asInstanceOf) && rec.tpe.derivesFrom(tp.tpe.classSymbol))
171-
readingOnlyVals(rec)
172-
else false
173-
case Apply(Select(rec, _), Nil) =>
174-
def isGetterOfAImmutableField = t.symbol.isGetter && !t.symbol.is(Mutable)
175-
def isCaseClassWithVar = t.symbol.info.decls.exists(_.is(Mutable))
176-
def isAccessingProductField = t.symbol.exists &&
177-
t.symbol.owner.derivesFrom(defn.ProductClass) &&
178-
t.symbol.owner.is(CaseClass) &&
179-
t.symbol.name.isSelectorName &&
180-
!isCaseClassWithVar // Conservative Covers case class A(var x: Int)
181-
def isImmutableCaseAccessor = t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable)
182-
if (isGetterOfAImmutableField || isAccessingProductField || isImmutableCaseAccessor)
183-
readingOnlyVals(rec)
184-
else false
185-
case Select(rec, _) if t.symbol.is(Method) =>
186-
if (t.symbol.isGetter && !t.symbol.is(Mutable)) readingOnlyVals(rec) // getter of a immutable field
187-
else if (t.symbol.owner.derivesFrom(defn.ProductClass) && t.symbol.owner.is(CaseClass) && t.symbol.name.isSelectorName) {
188-
def isImmutableField = {
189-
val fieldId = t.symbol.name.toString.drop(1).toInt - 1
190-
!t.symbol.owner.caseAccessors(ctx)(fieldId).is(Mutable)
191-
}
192-
if (isImmutableField) readingOnlyVals(rec) // accessing a field of a product
167+
def readingOnlyVals(t: Tree)(implicit ctx: Context): Boolean = {
168+
def isGetterOfAImmutableField = t.symbol.isGetter && !t.symbol.is(Mutable)
169+
def isCaseClassWithVar = t.symbol.info.decls.exists(_.is(Mutable))
170+
def isAccessingProductField = t.symbol.exists &&
171+
t.symbol.owner.derivesFrom(defn.ProductClass) &&
172+
t.symbol.owner.is(CaseClass) &&
173+
t.symbol.name.isSelectorName &&
174+
!isCaseClassWithVar // Conservatively covers case class A(var x: Int)
175+
def isImmutableCaseAccessor = t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable)
176+
177+
dropCasts(t) match {
178+
case Typed(exp, _) => readingOnlyVals(exp)
179+
180+
case TypeApply(fun @ Select(rec, _), List(tp)) =>
181+
if ((fun.symbol eq defn.Any_asInstanceOf) && rec.tpe.derivesFrom(tp.tpe.classSymbol))
182+
readingOnlyVals(rec)
193183
else false
194-
} else if (t.symbol.is(CaseAccessor) && !t.symbol.is(Mutable))
195-
readingOnlyVals(rec)
196-
else false
197-
case Select(qual, _) if !t.symbol.is(Mutable) =>
198-
readingOnlyVals(qual)
199-
case t: Ident if !t.symbol.is(Mutable) && !t.symbol.is(Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] =>
200-
desugarIdent(t) match {
201-
case Some(t) => readingOnlyVals(t)
202-
case None => true
203-
}
204-
case t: This => true
205-
// null => false, or the following fails devalify:
206-
// trait I {
207-
// def foo: Any = null
208-
// }
209-
// object Main {
210-
// def main = {
211-
// val s: I = null
212-
// s.foo
213-
// }
214-
// }
215-
case Literal(Constant(null)) => false
216-
case t: Literal => true
217-
case _ => false
184+
185+
case Apply(Select(rec, _), Nil) =>
186+
if (isGetterOfAImmutableField || isAccessingProductField || isImmutableCaseAccessor)
187+
readingOnlyVals(rec)
188+
else false
189+
190+
case Select(rec, _) if t.symbol.is(Method) =>
191+
if (isGetterOfAImmutableField)
192+
readingOnlyVals(rec) // Getter of an immutable field
193+
else if (isAccessingProductField) {
194+
def isImmutableField = {
195+
val fieldId = t.symbol.name.toString.drop(1).toInt - 1
196+
!t.symbol.owner.caseAccessors(ctx)(fieldId).is(Mutable)
197+
}
198+
if (isImmutableField) readingOnlyVals(rec) // Accessing a field of a product
199+
else false
200+
} else if (isImmutableCaseAccessor)
201+
readingOnlyVals(rec)
202+
else false
203+
204+
case t @ Select(qual, _) if !isEffectivelyMutable(t) =>
205+
readingOnlyVals(qual)
206+
207+
case t: Ident if !t.symbol.is(Mutable | Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] =>
208+
desugarIdent(t) match {
209+
case Some(t) => readingOnlyVals(t)
210+
case None => true
211+
}
212+
213+
case t: This => true
214+
// null => false, or the following fails devalify:
215+
// trait I {
216+
// def foo: Any = null
217+
// }
218+
// object Main {
219+
// def main = {
220+
// val s: I = null
221+
// s.foo
222+
// }
223+
// }
224+
case Literal(Constant(null)) => false
225+
case t: Literal => true
226+
case _ => false
227+
}
218228
}
219229
}

‎compiler/src/dotty/tools/dotc/transform/localopt/DropGoodCasts.scala

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import core.Types._
1010
import core.Flags._
1111
import ast.Trees._
1212
import transform.SymUtils._
13+
import Simplify.isEffectivelyMutable
1314

1415
/** Eliminated casts and equality tests whose results can be locally
1516
* determined at compile time:
@@ -77,7 +78,8 @@ import transform.SymUtils._
7778

7879
case TypeApply(fun @ Select(x, _), List(tp))
7980
if fun.symbol.eq(defn.Any_isInstanceOf) &&
80-
!x.symbol.is(Method | Mutable) &&
81+
!isEffectivelyMutable(x) &&
82+
!x.symbol.is(Method) &&
8183
x.symbol.exists && !x.symbol.owner.isClass =>
8284
(x.symbol, tp.tpe) :: Nil
8385

@@ -89,14 +91,16 @@ import transform.SymUtils._
8991
def collectNullTests(t: Tree)(implicit ctx: Context): List[Symbol] = {
9092
def recur(t: Tree): List[Symbol] =
9193
t match {
92-
case Apply(x, _) if (x.symbol == defn.Boolean_! || x.symbol == defn.Boolean_||) => Nil
94+
case Apply(x, _) if (x.symbol == defn.Boolean_! || x.symbol == defn.Boolean_||) =>
95+
Nil
9396

9497
case Apply(fun @ Select(x, _), y) if (fun.symbol == defn.Boolean_&&) =>
9598
recur(x) ++ recur(y.head)
9699

97100
case Apply(fun @ Select(x, _), List(tp))
98101
if fun.symbol.eq(defn.Object_ne) &&
99-
!x.symbol.is(Method | Mutable) &&
102+
!isEffectivelyMutable(x) &&
103+
!x.symbol.is(Method) &&
100104
x.symbol.exists && !x.symbol.owner.isClass =>
101105
x.symbol :: Nil
102106

‎compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ class DropNoEffects(val simplifyPhase: Simplify) extends Optimisation {
8787
keepOnlySideEffects(rec)
8888

8989
// !name.eq(nme.TYPE_) && // Keep the .TYPE added by ClassOf, would be needed for AfterErasure
90-
case s @ Select(qual, name) if !s.symbol.is(Mutable | Lazy | Method) =>
90+
// Without is(JavaStatic), { System.out } becomes { System }, but "Java class can't be used as value"
91+
case s @ Select(qual, name) if !s.symbol.is(Mutable | Lazy | Method | JavaStatic) =>
9192
keepOnlySideEffects(qual)
9293

9394
case Block(List(t: DefDef), s: Closure) =>

‎compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,4 +146,15 @@ object Simplify {
146146
case _ => None
147147
}
148148
}
149+
150+
/** Is this tree mutable, or java.lang.System.{in, out, err}? These three
151+
* System members are the only static final fields that are mutable.
152+
* See https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.4
153+
*/
154+
def isEffectivelyMutable(t: Tree)(implicit ctx: Context): Boolean = t match {
155+
case _ if t.symbol.is(Mutable) => true
156+
case s: Select => s.symbol.owner == defn.SystemModule
157+
case i: Ident => desugarIdent(i).exists(isEffectivelyMutable)
158+
case _ => false
159+
}
149160
}

‎docs/docs/reference/optimisations.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
layout: doc-page
3+
title: "Local Optimisations"
4+
---
5+
6+
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).
7+
8+
Local optimisations assumes no use of Java Reflection to mutate final fields.

‎docs/sidebar.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ sidebar:
5151
url: docs/reference/auto-parameter-tupling.html
5252
- title: Named Type Arguments
5353
url: docs/reference/named-typeargs.html
54+
- title: Local Optimisations
55+
url: docs/reference/optimisations.html
5456
- title: Changed Features
5557
subsection:
5658
- title: Volatile Lazy Vals

‎tests/run/2772.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import java.io.OutputStream
2+
3+
object Test {
4+
def main(args: Array[String]): Unit = {
5+
val oldErr = System.err
6+
System.setErr(null) // setOut(null) confuses the testing framework...
7+
val a = () => foo(oldErr)
8+
a()
9+
}
10+
11+
def foo(err: OutputStream): Unit = {
12+
err.write(0)
13+
}
14+
}

0 commit comments

Comments
 (0)