Skip to content

Commit c6a359e

Browse files
committed
Fix #4811: Exclude fields when widening private[this]/protected[this]
This fix the crash because we don't need to generate a getter or setter for private[this] field anymore. The original documentation for ElimLocals says: This phase drops the Local flag from all private[this] and protected[this] members. This allows subsequent code motions where code is moved from a class to its companion object. It invalidates variance checking, which is consequently disabled when retyping. We maintain the Local flag for field since then cannot move to the companion object and it is used later in Getter.
1 parent 0db2b36 commit c6a359e

File tree

8 files changed

+69
-29
lines changed

8 files changed

+69
-29
lines changed

compiler/src/dotty/tools/dotc/Compiler.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ class Compiler {
6161
List(new FirstTransform, // Some transformations to put trees into a canonical form
6262
new CheckReentrant, // Internal use only: Check that compiled program has no data races involving global vars
6363
new ElimPackagePrefixes) :: // Eliminate references to package prefixes in Select nodes
64-
List(new CheckStatic, // Check restrictions that apply to @static members
64+
List(new ElimLocals, // Widens private[this] and protected[this] modifiers to just private/protected
65+
new CheckStatic, // Check restrictions that apply to @static members
6566
new ElimRepeated, // Rewrite vararg parameters and arguments
66-
new NormalizeFlags, // Rewrite some definition flags
6767
new ExpandSAMs, // Expand single abstract method closures to anonymous classes
6868
new ProtectedAccessors, // Add accessors for protected members
6969
new ExtensionMethods, // Expand methods of value classes with extension methods
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package dotty.tools.dotc
2+
package transform
3+
4+
import core._
5+
import DenotTransformers.SymTransformer
6+
import Phases.Phase
7+
import Contexts.Context
8+
import SymDenotations.SymDenotation
9+
import MegaPhase.MiniPhase
10+
import Flags._, Symbols._
11+
import SymUtils._
12+
13+
/** This phase drops the Local flag from all private[this] and protected[this] members.
14+
*
15+
* This allows subsequent code motions where code is moved from a class to its
16+
* companion object.
17+
* We maintain the Local flag for field since then cannot move to the companion
18+
* object and it is used in later Getter.
19+
*/
20+
class ElimLocals extends MiniPhase with SymTransformer {
21+
override def phaseName = ElimLocals.name
22+
23+
def transformSym(ref: SymDenotation)(implicit ctx: Context) =
24+
if (ref.is(Local) && !ref.isField) ref.copySymDenotation(initFlags = ref.flags &~ Local)
25+
else ref
26+
}
27+
28+
object ElimLocals {
29+
def name = "elimlocals"
30+
}

compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ class ExtensionMethods extends MiniPhase with DenotTransformer with FullParamete
4747

4848
override def runsAfter = Set(
4949
ElimRepeated.name,
50-
ProtectedAccessors.name // protected accessors cannot handle code that is moved from class to companion object
50+
ProtectedAccessors.name, // protected accessors cannot handle code that is moved from class to companion object
51+
ElimLocals.name // private[this] accessors need to be widened before
52+
// a methods is moved to its companion object
5153
)
5254

5355
override def runsAfterGroupsOf = Set(FirstTransform.name) // need companion objects to exist

compiler/src/dotty/tools/dotc/transform/Flatten.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ class Flatten extends MiniPhase with SymTransformer {
1717

1818
override def phaseName = "flatten"
1919

20+
// private[this] modifiers on classes need be widened before lifting the class
21+
override def runsAfter = Set(ElimLocals.name)
22+
2023
override def changesMembers = true // the phase removes inner classes
2124

2225
private var LiftedDefs: Store.Location[mutable.ListBuffer[Tree]] = _

compiler/src/dotty/tools/dotc/transform/Getters.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class Getters extends MiniPhase with SymTransformer {
5454
override def transformSym(d: SymDenotation)(implicit ctx: Context): SymDenotation = {
5555
def noGetterNeeded =
5656
d.is(NoGetterNeeded) ||
57-
d.initial.asInstanceOf[SymDenotation].is(PrivateLocal) && !d.owner.is(Trait) && !isDerivedValueClass(d.owner) && !d.is(Flags.Lazy) ||
57+
d.is(PrivateLocal) && !d.owner.is(Trait) && !isDerivedValueClass(d.owner) && !d.is(Flags.Lazy) ||
5858
d.is(Module) && d.isStatic ||
5959
d.hasAnnotation(defn.ScalaStaticAnnot) ||
6060
d.isSelfSym

compiler/src/dotty/tools/dotc/transform/NormalizeFlags.scala

Lines changed: 0 additions & 25 deletions
This file was deleted.

tests/pos/flatten.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
object A1 {
2+
private[this] class B
3+
4+
private[this] class C {
5+
def cons: B = new B
6+
}
7+
}
8+
9+
class A2 {
10+
private[this] class B
11+
12+
private[this] class C {
13+
def cons: B = new B
14+
}
15+
}

tests/pos/i4811.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class StringOps(val s: String) extends AnyVal {
2+
def lines: Iterator[String] = new collection.AbstractIterator[String] {
3+
private[this] var index = 0
4+
def hasNext: Boolean = false
5+
def next(): String = {
6+
index = 1
7+
""
8+
}
9+
}
10+
}
11+
12+
class FooOps(val s: String) extends AnyVal {
13+
private[this] def bar = 2
14+
def foo = bar
15+
}

0 commit comments

Comments
 (0)