Skip to content

Fix #4811: Widen private[this]/protected[this] just before Flatten #4814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ class Compiler {
new ElimPackagePrefixes) :: // Eliminate references to package prefixes in Select nodes
List(new CheckStatic, // Check restrictions that apply to @static members
new ElimRepeated, // Rewrite vararg parameters and arguments
new NormalizeFlags, // Rewrite some definition flags
new ExpandSAMs, // Expand single abstract method closures to anonymous classes
new ProtectedAccessors, // Add accessors for protected members
new ExtensionMethods, // Expand methods of value classes with extension methods
Expand Down
20 changes: 14 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ import SymUtils._
*
* Finally, if the constructor of a value class is private pr protected
* it is widened to public.
*
* Also, drop the Local flag from all private[this] and protected[this] members
* that will be moved to the companion object.
*/
class ExtensionMethods extends MiniPhase with DenotTransformer with FullParameterization { thisPhase =>

Expand All @@ -46,7 +49,7 @@ class ExtensionMethods extends MiniPhase with DenotTransformer with FullParamete

override def runsAfter = Set(
ElimRepeated.name,
ProtectedAccessors.name // protected accessors cannot handle code that is moved from class to companion object
ProtectedAccessors.name, // protected accessors cannot handle code that is moved from class to companion object
)

override def runsAfterGroupsOf = Set(FirstTransform.name) // need companion objects to exist
Expand Down Expand Up @@ -97,17 +100,22 @@ class ExtensionMethods extends MiniPhase with DenotTransformer with FullParamete
moduleClassSym
}
case ref: SymDenotation =>
var ref1 = ref
if (isMethodWithExtension(ref.symbol) && ref.hasAnnotation(defn.TailrecAnnot)) {
val ref1 = ref.copySymDenotation()
ref1 = ref.copySymDenotation()
ref1.removeAnnotation(defn.TailrecAnnot)
ref1
}
else if (ref.isConstructor && isDerivedValueClass(ref.owner) && ref.is(AccessFlags)) {
val ref1 = ref.copySymDenotation()
ref1 = ref.copySymDenotation()
ref1.resetFlag(AccessFlags)
ref1
}
else ref
// Drop the Local flag from all private[this] and protected[this] members
// that will be moved to the companion object.
if (ref.is(Local) && isDerivedValueClass(ref.owner)) {
if (ref1 ne ref) ref1.resetFlag(Local)
else ref1 = ref1.copySymDenotation(initFlags = ref1.flags &~ Local)
}
ref1
case _ =>
ref
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/Flatten.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class Flatten extends MiniPhase with SymTransformer {

override def phaseName = "flatten"

// private[this] and protected[this] modifiers must be dropped
// before classes are lifted. Getters drop these modifiers.
override def runsAfter = Set(Getters.name)

override def changesMembers = true // the phase removes inner classes

private var LiftedDefs: Store.Location[mutable.ListBuffer[Tree]] = _
Expand Down
32 changes: 24 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/Getters.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,26 +45,38 @@ import ValueClasses._
* --> p.x_=(e)
*
* No fields are generated yet. This is done later in phase Memoize.
*
* Also, drop the Local flag from all private[this] and protected[this] members.
* This allows subsequent code motions in Flatten.
*/
class Getters extends MiniPhase with SymTransformer {
import ast.tpd._

override def phaseName = "getters"
override def phaseName = Getters.name

override def transformSym(d: SymDenotation)(implicit ctx: Context): SymDenotation = {
def noGetterNeeded =
d.is(NoGetterNeeded) ||
d.initial.asInstanceOf[SymDenotation].is(PrivateLocal) && !d.owner.is(Trait) && !isDerivedValueClass(d.owner) && !d.is(Flags.Lazy) ||
d.is(PrivateLocal) && !d.owner.is(Trait) && !isDerivedValueClass(d.owner) && !d.is(Flags.Lazy) ||
d.is(Module) && d.isStatic ||
d.hasAnnotation(defn.ScalaStaticAnnot) ||
d.isSelfSym
if (d.isTerm && (d.is(Lazy) || d.owner.isClass) && d.info.isValueType && !noGetterNeeded) {
val maybeStable = if (d.isStable) Stable else EmptyFlags
d.copySymDenotation(
initFlags = d.flags | maybeStable | AccessorCreationFlags,
info = ExprType(d.info))

var d1 =
if (d.isTerm && (d.is(Lazy) || d.owner.isClass) && d.info.isValueType && !noGetterNeeded) {
val maybeStable = if (d.isStable) Stable else EmptyFlags
d.copySymDenotation(
initFlags = d.flags | maybeStable | AccessorCreationFlags,
info = ExprType(d.info))
}
else d

// Drop the Local flag from all private[this] and protected[this] members.
if (d1.is(Local)) {
if (d1 ne d) d1.resetFlag(Local)
else d1 = d1.copySymDenotation(initFlags = d1.flags &~ Local)
}
else d
d1
}
private val NoGetterNeeded = Method | Param | JavaDefined | JavaStatic

Expand All @@ -74,3 +86,7 @@ class Getters extends MiniPhase with SymTransformer {
override def transformAssign(tree: Assign)(implicit ctx: Context): Tree =
if (tree.lhs.symbol is Method) tree.lhs.becomes(tree.rhs).withPos(tree.pos) else tree
}

object Getters {
val name = "getters"
}
25 changes: 0 additions & 25 deletions compiler/src/dotty/tools/dotc/transform/NormalizeFlags.scala

This file was deleted.

15 changes: 15 additions & 0 deletions tests/pos/flatten.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
object A1 {
private[this] class B

private[this] class C {
def cons: B = new B
}
}

class A2 {
private[this] class B

private[this] class C {
def cons: B = new B
}
}
15 changes: 15 additions & 0 deletions tests/pos/i4811.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class StringOps(val s: String) extends AnyVal {
def lines: Iterator[String] = new collection.AbstractIterator[String] {
private[this] var index = 0
def hasNext: Boolean = false
def next(): String = {
index = 1
""
}
}
}

class FooOps(val s: String) extends AnyVal {
private[this] def bar = 2
def foo = bar
}