From 6bd882e8c2e30ab5a17eb0ed7a3f8ad96cfc8a7b Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Fri, 26 Jan 2018 13:27:42 +0100 Subject: [PATCH 1/3] Fix #3917: Properly desugar `Ident` --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 19 +++++++++------ tests/pos/i3917.scala | 27 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 tests/pos/i3917.scala diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index dd65f4fdc12c..02976046d1eb 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -826,14 +826,19 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { */ def becomes(rhs: Tree)(implicit ctx: Context): Tree = if (tree.symbol is Method) { - val setr = tree match { - case Ident(_) => - val setter = tree.symbol.setter - assert(setter.exists, tree.symbol.showLocated) - ref(tree.symbol.setter) - case Select(qual, _) => qual.select(tree.symbol.setter) + val setter = tree.symbol.setter + assert(setter.exists, tree.symbol.showLocated) + val qual = tree match { + case id: Ident => + id.tpe match { + case TermRef(prefix: TermRef, _) => + ref(prefix) + case TermRef(prefix: ThisType, _) => + This(prefix.cls) + } + case Select(qual, _) => qual } - setr.appliedTo(rhs) + qual.select(setter).appliedTo(rhs) } else Assign(tree, rhs) diff --git a/tests/pos/i3917.scala b/tests/pos/i3917.scala new file mode 100644 index 000000000000..19fd1506d69b --- /dev/null +++ b/tests/pos/i3917.scala @@ -0,0 +1,27 @@ +class A { + var a = false +} + +object B { + var b = false +} + +class C { + var c = false +} + +object C extends A { + def test = { + a = true + C.a = true + this.a = true + C.this.a = true + + import B._ + b = true + + val c0 = new C + import c0._ + c = true + } +} From 469fefebd899bf93a8889ca090a417639b809faf Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Sat, 27 Jan 2018 16:34:58 +0100 Subject: [PATCH 2/3] Factor desugarIdent functionality --- compiler/sjs/backend/sjs/JSCodeGen.scala | 15 ------------ .../backend/jvm/DottyBackendInterface.scala | 10 +------- compiler/src/dotty/tools/dotc/ast/tpd.scala | 24 +++++++++++++------ .../transform/localopt/ConstantFold.scala | 1 - .../localopt/InlineCaseIntrinsics.scala | 1 - .../dotc/transform/localopt/Simplify.scala | 10 -------- 6 files changed, 18 insertions(+), 43 deletions(-) diff --git a/compiler/sjs/backend/sjs/JSCodeGen.scala b/compiler/sjs/backend/sjs/JSCodeGen.scala index 2eb8ecadc142..5068adfd8fa6 100644 --- a/compiler/sjs/backend/sjs/JSCodeGen.scala +++ b/compiler/sjs/backend/sjs/JSCodeGen.scala @@ -850,21 +850,6 @@ class JSCodeGen()(implicit ctx: Context) { } } // end of genStatOrExpr() - // !!! DUPLICATE code with DottyBackendInterface - private def desugarIdent(i: Ident): Option[Select] = { - i.tpe match { - case TermRef(prefix: TermRef, name) => - Some(tpd.ref(prefix).select(i.symbol)) - case TermRef(prefix: ThisType, name) => - Some(tpd.This(prefix.cls).select(i.symbol)) - /*case TermRef(NoPrefix, name) => - if (i.symbol is Method) Some(This(i.symbol.topLevelClass).select(i.symbol)) // workaround #342 todo: remove after fixed - else None*/ - case _ => - None - } - } - private def qualifierOf(fun: Tree): Tree = fun match { case fun: Ident => fun.tpe match { diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index 5328fcd9afbb..4acc4f662fb1 100644 --- a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -444,15 +444,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma def desugarIdent(i: Ident): Option[tpd.Select] = { var found = desugared.get(i.tpe) if (found == null) { - i.tpe match { - case TermRef(prefix: TermRef, _) => - found = tpd.ref(prefix).select(i.symbol) - case TermRef(prefix: ThisType, _) => - found = tpd.This(prefix.cls).select(i.symbol) - case TermRef(NoPrefix, _) => - if (i.symbol is Flags.Method) found = This(i.symbol.topLevelClass).select(i.symbol) // workaround #342 todo: remove after fixed - case _ => - } + found = tpd.desugarIdent(i).orNull if (found != null) desugared.put(i.tpe, found) } if (found == null) None else Some(found) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 02976046d1eb..c68bf4123c95 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -829,13 +829,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { val setter = tree.symbol.setter assert(setter.exists, tree.symbol.showLocated) val qual = tree match { - case id: Ident => - id.tpe match { - case TermRef(prefix: TermRef, _) => - ref(prefix) - case TermRef(prefix: ThisType, _) => - This(prefix.cls) - } + case id: Ident => desugarIdentPrefix(id) case Select(qual, _) => qual } qual.select(setter).appliedTo(rhs) @@ -1017,5 +1011,21 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { val encoding = ctx.settings.encoding.value if (file != null && file.exists) new SourceFile(file, Codec(encoding)) else NoSource } + + /** Desugar identifier into a select node. Return None if not possible */ + def desugarIdent(tree: Ident)(implicit ctx: Context): Option[Select] = { + val qual = desugarIdentPrefix(tree) + if (qual.isEmpty) None + else Some(qual.select(tree.symbol)) + } + + private def desugarIdentPrefix(tree: Ident)(implicit ctx: Context): Tree = tree.tpe match { + case TermRef(prefix: TermRef, _) => + ref(prefix) + case TermRef(prefix: ThisType, _) => + This(prefix.cls) + case _ => + EmptyTree + } } diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala b/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala index 3e2ec2b3fec8..0313cd94722b 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala @@ -6,7 +6,6 @@ import core.Symbols._ import core.Types._ import typer.ConstFold import ast.Trees._ -import Simplify.desugarIdent /** Various constant folding. * diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala b/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala index e4f291679afa..3e9e1cade5da 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala @@ -10,7 +10,6 @@ import core.Flags._ import core.TypeApplications.noBounds import ast.Trees._ import transform.SymUtils._ -import Simplify.desugarIdent import dotty.tools.dotc.ast.tpd /** Inline case class specific methods using desugarings assumptions. diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala index d336c944e9e6..f61b63bec35e 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala @@ -149,16 +149,6 @@ class Simplify extends MiniPhase with IdentityDenotTransformer { object Simplify { import tpd._ - // TODO: This function is duplicated in jvm/DottyBackendInterface.scala, let's factor these out! - def desugarIdent(i: Ident)(implicit ctx: Context): Option[Select] = { - i.tpe match { - case TermRef(prefix: TermRef, _) => - Some(ref(prefix).select(i.symbol)) - case TermRef(prefix: ThisType, _) => - Some(This(prefix.cls).select(i.symbol)) - case _ => None - } - } /** Is this tree mutable, or java.lang.System.{in, out, err}? These three * System members are the only static final fields that are mutable. From 2d710304a8f6cc18d3f8b145f8d546bbec3adc9c Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Tue, 30 Jan 2018 14:43:35 +0100 Subject: [PATCH 3/3] Make `desugarIdent` return a `Tree` --- compiler/sjs/backend/sjs/JSCodeGen.scala | 6 +++--- .../backend/jvm/DottyBackendInterface.scala | 8 ++++++-- compiler/src/dotty/tools/dotc/ast/tpd.scala | 11 ++++++----- .../dotc/transform/localopt/ConstantFold.scala | 17 +++++++---------- .../dotc/transform/localopt/Devalify.scala | 2 +- .../dotc/transform/localopt/DropNoEffects.scala | 7 +++---- .../localopt/InlineCaseIntrinsics.scala | 7 +++---- .../dotc/transform/localopt/Simplify.scala | 10 +++------- 8 files changed, 32 insertions(+), 36 deletions(-) diff --git a/compiler/sjs/backend/sjs/JSCodeGen.scala b/compiler/sjs/backend/sjs/JSCodeGen.scala index 5068adfd8fa6..09f6d07b563e 100644 --- a/compiler/sjs/backend/sjs/JSCodeGen.scala +++ b/compiler/sjs/backend/sjs/JSCodeGen.scala @@ -793,7 +793,7 @@ class JSCodeGen()(implicit ctx: Context) { throw new FatalError(s"Assignment to static member ${sym.fullName} not supported") val genRhs = genExpr(rhs) val lhs = lhs0 match { - case lhs: Ident => desugarIdent(lhs).getOrElse(lhs) + case lhs: Ident => desugarIdent(lhs) case lhs => lhs } lhs match { @@ -892,7 +892,7 @@ class JSCodeGen()(implicit ctx: Context) { val sym = tree.fun.symbol val fun = tree.fun match { - case fun: Ident => desugarIdent(fun).getOrElse(fun) + case fun: Ident => desugarIdent(fun) case fun => fun } @@ -1556,7 +1556,7 @@ class JSCodeGen()(implicit ctx: Context) { implicit val pos = tree.pos val fun = tree.fun match { - case fun: Ident => desugarIdent(fun).get + case fun: Ident => desugarIdent(fun).asInstanceOf[Select] case fun: Select => fun } val receiver = fun.qualifier diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index 4acc4f662fb1..57b7c18b1ca5 100644 --- a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -444,8 +444,12 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma def desugarIdent(i: Ident): Option[tpd.Select] = { var found = desugared.get(i.tpe) if (found == null) { - found = tpd.desugarIdent(i).orNull - if (found != null) desugared.put(i.tpe, found) + tpd.desugarIdent(i) match { + case sel: tpd.Select => + desugared.put(i.tpe, sel) + found = sel + case _ => + } } if (found == null) None else Some(found) } diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index c68bf4123c95..20b55d72a0de 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -1012,14 +1012,15 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { if (file != null && file.exists) new SourceFile(file, Codec(encoding)) else NoSource } - /** Desugar identifier into a select node. Return None if not possible */ - def desugarIdent(tree: Ident)(implicit ctx: Context): Option[Select] = { + /** Desugar identifier into a select node. Return the tree itself if not possible */ + def desugarIdent(tree: Ident)(implicit ctx: Context): Tree = { val qual = desugarIdentPrefix(tree) - if (qual.isEmpty) None - else Some(qual.select(tree.symbol)) + if (qual.isEmpty) tree + else qual.select(tree.symbol) } - private def desugarIdentPrefix(tree: Ident)(implicit ctx: Context): Tree = tree.tpe match { + /** Recover identifier prefix (e.g. this) if it exists */ + def desugarIdentPrefix(tree: Ident)(implicit ctx: Context): Tree = tree.tpe match { case TermRef(prefix: TermRef, _) => ref(prefix) case TermRef(prefix: ThisType, _) => diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala b/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala index 0313cd94722b..2517c7318688 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/ConstantFold.scala @@ -181,22 +181,19 @@ import ast.Trees._ case _ => false } case t1: Ident => - desugarIdent(t1) match { - case Some(t) => - val t2i = t2 match { - case t2: Ident => desugarIdent(t2).getOrElse(t2) - case _ => t2 - } - isSimilar(t, t2i) - case None => t1.symbol eq t2.symbol + t2 match { + case t2: Ident => + t1.symbol eq t2.symbol + case _ => // Select + isSimilar(t2, t1) } case t1: Select => t2 match { case t2: Select => (t1.symbol eq t2.symbol) && isSimilar(t1.qualifier, t2.qualifier) case t2: Ident => desugarIdent(t2) match { - case Some(t2) => isSimilar(t1, t2) - case None => false + case t2: Select => isSimilar(t1, t2) + case _ => false } case _ => false } diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala index 43cb0feeda4f..419c94765b76 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Devalify.scala @@ -194,7 +194,7 @@ class Devalify extends Optimisation { readingOnlyVals(qual) case t: Ident if !t.symbol.is(Mutable | Method) && !t.symbol.info.dealias.isInstanceOf[ExprType] => - desugarIdent(t).forall(readingOnlyVals) + desugarIdent(t) match { case s: Select => readingOnlyVals(s); case _ => true } case t: This => true // null => false, or the following fails devalify: diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala b/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala index 598d0ea37d37..f84ec732e25c 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/DropNoEffects.scala @@ -100,10 +100,9 @@ class DropNoEffects(val simplifyPhase: Simplify) extends Optimisation { cpy.Block(bl)(stats2, expr2) case t: Ident if !t.symbol.is(Method | Lazy) && !t.symbol.info.isInstanceOf[ExprType] || effectsDontEscape(t) => - desugarIdent(t) match { - case Some(t) if !(t.qualifier.symbol.is(JavaDefined) && t.qualifier.symbol.is(Package)) => t - case _ => EmptyTree - } + val prefix = desugarIdentPrefix(t) + if (!prefix.isEmpty && !prefix.symbol.is(allOf(JavaDefined, Package))) t + else EmptyTree case app: Apply if app.fun.symbol.is(Label) && !app.tpe.finalResultType.derivesFrom(defn.UnitClass) => // This is "the scary hack". It changes the return type to Unit, then diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala b/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala index 3e9e1cade5da..cdb4d3e74066 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/InlineCaseIntrinsics.scala @@ -108,10 +108,9 @@ class InlineCaseIntrinsics(val simplifyPhase: Simplify) extends Optimisation { def receiver(t: Tree): Type = t match { case t: Apply => receiver(t.fun) case t: TypeApply => receiver(t.fun) - case t: Ident => desugarIdent(t) match { - case Some(t) => receiver(t) - case _ => NoType - } + case t: Ident => + val prefix = desugarIdentPrefix(t) + prefix.tpe.widenDealias case t: Select => t.qualifier.tpe.widenDealias } diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala index f61b63bec35e..fe63be6deefb 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala @@ -154,14 +154,10 @@ object Simplify { * 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 */ - @tailrec def isEffectivelyMutable(t: Tree)(implicit ctx: Context): Boolean = t match { + def isEffectivelyMutable(t: Tree)(implicit ctx: Context): Boolean = t match { case _ if t.symbol.is(Mutable) => true - case s: Select => s.symbol.owner == defn.SystemModule - case i: Ident => - desugarIdent(i) match { - case Some(ident) => isEffectivelyMutable(ident) - case None => false - } + case _: Select | _: Ident => + t.symbol.owner == defn.SystemModule case _ => false }