Merge pull request #3925 from dotty-staging/fix-3917

Fix #3917: Properly desugar `Ident`

Author: allanrenucci

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 {
@@ -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 {
@@ -907,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
     }
 
@@ -1571,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

compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala

@@ -444,16 +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) {
-      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
+      tpd.desugarIdent(i) match {
+        case sel: tpd.Select =>
+          desugared.put(i.tpe, sel)
+          found = sel
         case _ =>
       }
-      if (found != null) desugared.put(i.tpe, found)
     }
     if (found == null) None else Some(found)
   }

compiler/src/dotty/tools/dotc/ast/tpd.scala

@@ -826,14 +826,13 @@ 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 => desugarIdentPrefix(id)
+          case Select(qual, _) => qual
         }
-        setr.appliedTo(rhs)
+        qual.select(setter).appliedTo(rhs)
       }
       else Assign(tree, rhs)
 
@@ -1012,5 +1011,22 @@ 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 the tree itself if not possible */
+  def desugarIdent(tree: Ident)(implicit ctx: Context): Tree = {
+    val qual = desugarIdentPrefix(tree)
+    if (qual.isEmpty) tree
+    else qual.select(tree.symbol)
+  }
+
+  /** 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, _) =>
+      This(prefix.cls)
+    case _ =>
+      EmptyTree
+  }
 }
 

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.
  *
@@ -182,22 +181,19 @@ import Simplify.desugarIdent
         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
     }

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:

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

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.
@@ -109,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
       }
 

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

@@ -149,29 +149,15 @@ 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.
    *  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
   }
 

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