From e101f5aa0894d856e582c0bfc362bee3f93382b0 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 23 May 2018 10:43:51 +0200
Subject: [PATCH 1/5] Check protected accessors after pickling

New phase to check protected accessors.

We now create accessors also for Scala symbols. Previously this was not done
as it was assumed that such symbols will always be mapped to public.

We create the accessors anyway in order to better test the functionality, so
that we can at some point avoid the access widening from protected to public.
---
 compiler/src/dotty/tools/dotc/Compiler.scala  |  1 +
 .../tools/dotc/core/SymDenotations.scala      |  5 ++
 .../tools/dotc/transform/AccessProxies.scala  | 31 ++++++--
 .../dotty/tools/dotc/transform/Erasure.scala  | 26 +------
 .../dotc/transform/ExtensionMethods.scala     |  5 +-
 .../dotc/transform/ProtectedAccessors.scala   | 74 +++++++++++++++++++
 tests/neg/protectedacc.scala                  | 37 ++++++++++
 tests/run/protectedacc.scala                  |  2 +-
 8 files changed, 148 insertions(+), 33 deletions(-)
 create mode 100644 compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
 create mode 100644 tests/neg/protectedacc.scala

diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala
index 596b1d6bf45f..feaf5d134a09 100644
--- a/compiler/src/dotty/tools/dotc/Compiler.scala
+++ b/compiler/src/dotty/tools/dotc/Compiler.scala
@@ -60,6 +60,7 @@ class Compiler {
   protected def transformPhases: List[List[Phase]] =
     List(new FirstTransform,         // Some transformations to put trees into a canonical form
          new CheckReentrant,         // Internal use only: Check that compiled program has no data races involving global vars
+         new ProtectedAccessors,     // Add accessors for protected members
          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
diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
index 32c6b4038d82..a0a304968e9b 100644
--- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -711,6 +711,11 @@ object SymDenotations {
             i"""
                | Access to protected $this not permitted because enclosing ${ctx.owner.enclosingClass.showLocated}
                | is not a subclass of ${owner.showLocated} where target is defined""")
+        else if (cls.is(Trait) && !this.owner.is(Trait))
+          fail(
+            i"""
+               | Access to protected $this not permitted from $cls,
+               | since traits cannot access protected members of superclasses""")
         else if (
           !(  isType // allow accesses to types from arbitrary subclasses fixes #4737
            || pre.derivesFrom(cls)
diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
index 46352f0143d2..d6fb1423b52a 100644
--- a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
+++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
@@ -14,6 +14,7 @@ import NameKinds.ClassifiedNameKind
 import ast.Trees._
 import util.Property
 import util.Positions.Position
+import config.Printers.transforms
 
 /** A utility class for generating access proxies. Currently used for
  *  inline accessors and protected accessors.
@@ -56,9 +57,11 @@ abstract class AccessProxies {
     def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean
 
     /** A fresh accessor symbol */
-    def newAccessorSymbol(accessed: Symbol, name: TermName, info: Type)(implicit ctx: Context): TermSymbol =
-      ctx.newSymbol(accessed.owner.enclosingSubClass, name, Synthetic | Method,
-                    info, coord = accessed.pos).entered
+    def newAccessorSymbol(owner: Symbol, name: TermName, info: Type, pos: Position)(implicit ctx: Context): TermSymbol = {
+      val sym = ctx.newSymbol(owner, name, Synthetic | Method, info, coord = pos).entered
+      if (sym.allOverriddenSymbols.exists(!_.is(Deferred))) sym.setFlag(Override)
+      sym
+    }
 
     /** Create an accessor unless one exists already, and replace the original
       *  access with a reference to the accessor.
@@ -73,14 +76,30 @@ abstract class AccessProxies {
 
       def refersToAccessed(sym: Symbol) = accessedBy.get(sym) == Some(accessed)
 
-      val accessorInfo =
+      val accessorClass = {
+        def owningClass(start: Symbol) =
+          start.ownersIterator.findSymbol(_.derivesFrom(accessed.owner))
+        val curCls = ctx.owner.enclosingClass
+        var owner = owningClass(curCls) //`orElse` owningClass(curCls.linkedClass)
+        if (!owner.exists) {
+          transforms.println(i"${curCls.ownersIterator.toList}%, %")
+          ctx.error(i"illegal access to protected ${accessed.showLocated} from $curCls", reference.pos)
+          owner = curCls
+        }
+        owner
+      }
+
+      val accessorRawInfo =
         if (onLHS) MethodType(accessed.info :: Nil, defn.UnitType)
         else accessed.info.ensureMethodic
+      val accessorInfo =
+        accessorRawInfo.asSeenFrom(accessorClass.thisType, accessed.owner)
       val accessorName = nameKind(accessed.name)
+
       val accessorSymbol =
-        accessed.owner.info.decl(accessorName).suchThat(refersToAccessed).symbol
+        accessorClass.info.decl(accessorName).suchThat(refersToAccessed).symbol
           .orElse {
-            val acc = newAccessorSymbol(accessed, accessorName, accessorInfo)
+            val acc = newAccessorSymbol(accessorClass, accessorName, accessorInfo, accessed.pos)
             accessedBy(acc) = accessed
             acc
           }
diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
index 48080e8718a8..ba58e2f15515 100644
--- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala
+++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
@@ -20,7 +20,6 @@ import typer.ProtoTypes._
 import typer.ErrorReporting._
 import core.TypeErasure._
 import core.Decorators._
-import core.NameKinds._
 import dotty.tools.dotc.ast.{Trees, tpd, untpd}
 import ast.Trees._
 import scala.collection.mutable.ListBuffer
@@ -327,23 +326,6 @@ object Erasure {
       if (tree.isTerm) erasedRef(tp) else valueErasure(tp)
     }
 
-    object ProtectedAccessors extends AccessProxies {
-      def getterName = ProtectedAccessorName
-      def setterName = ProtectedSetterName
-
-      val insert = new Insert {
-        def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean =
-          false &&
-          sym.isTerm && sym.is(Flags.Protected) &&
-          ctx.owner.enclosingPackageClass != sym.enclosingPackageClass &&
-          !ctx.owner.enclosingClass.derivesFrom(sym.owner) &&
-            { println(i"need protected acc $sym accessed from ${ctx.owner}"); assert(false); false }
-        }
-    }
-
-    override def addAccessorDefs(cls: Symbol, body: List[Tree])(implicit ctx: Context): List[Tree] =
-      ProtectedAccessors.addAccessorDefs(cls, body)
-
     override def promote(tree: untpd.Tree)(implicit ctx: Context): tree.ThisTree[Type] = {
       assert(tree.hasType)
       val erasedTp = erasedType(tree)
@@ -378,9 +360,6 @@ object Erasure {
       else
         super.typedLiteral(tree)
 
-    override def typedIdent(tree: untpd.Ident, pt: Type)(implicit ctx: Context): Tree =
-      ProtectedAccessors.insert.accessorIfNeeded(super.typedIdent(tree, pt))
-
     /** Type check select nodes, applying the following rewritings exhaustively
      *  on selections `e.m`, where `OT` is the type of the owner of `m` and `ET`
      *  is the erased type of the selection's original qualifier expression.
@@ -461,12 +440,9 @@ object Erasure {
         }
       }
 
-      ProtectedAccessors.insert.accessorIfNeeded(recur(typed(tree.qualifier, AnySelectionProto)))
+      recur(typed(tree.qualifier, AnySelectionProto))
     }
 
-    override def typedAssign(tree: untpd.Assign, pt: Type)(implicit ctx: Context): Tree =
-      ProtectedAccessors.insert.accessorIfNeeded(super.typedAssign(tree, pt))
-
     override def typedThis(tree: untpd.This)(implicit ctx: Context): Tree =
       if (tree.symbol == ctx.owner.lexicallyEnclosingClass || tree.symbol.isStaticOwner) promote(tree)
       else {
diff --git a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala
index d5b1ec9cc72f..b617dbd1e854 100644
--- a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala
+++ b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala
@@ -45,7 +45,10 @@ class ExtensionMethods extends MiniPhase with DenotTransformer with FullParamete
   /** the following two members override abstract members in Transform */
   override def phaseName: String = ExtensionMethods.name
 
-  override def runsAfter = Set(ElimRepeated.name)
+  override def runsAfter = Set(
+    ElimRepeated.name,
+    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
 
diff --git a/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
new file mode 100644
index 000000000000..fb3c53c78208
--- /dev/null
+++ b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
@@ -0,0 +1,74 @@
+package dotty.tools.dotc
+package transform
+
+import core.Contexts.Context
+import core.NameKinds._
+import core.Symbols._
+import core.Flags._
+import core.Decorators._
+import MegaPhase.MiniPhase
+import ast.Trees._
+
+/** Add accessors for all protected accesses. An accessor is needed if
+ *  according to the rules of the JVM a protected class member is not accesissible
+ *  from the point of access, but is accessible if the access is from an enclosing
+ *  class. In this point a public access method is placed in that enclosing class.
+ */
+class ProtectedAccessors extends MiniPhase {
+  import ast.tpd._
+
+  override def phaseName = ProtectedAccessors.name
+
+  object Accessors extends AccessProxies {
+    def getterName = ProtectedAccessorName
+    def setterName = ProtectedSetterName
+
+    val insert = new Insert {
+      def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean = {
+        def insideBoundary = {
+          if (sym.is(JavaDefined)) {
+            sym.is(JavaStatic) ||  // Java's static protected definitions are treated as public
+            ctx.owner.enclosingPackageClass == sym.enclosingPackageClass
+          }
+          else {
+            // For Scala-defined symbols we currently allow private and protected accesses
+            // from inner packages, and compensate by widening accessibility of such symbols to public.
+            // It would be good if we could revisit this at some point.
+            val boundary = sym.accessBoundary(sym.enclosingPackageClass)
+            ctx.owner.isContainedIn(boundary) || ctx.owner.isContainedIn(boundary.linkedClass)
+          }
+        }
+        sym.isTerm && sym.is(Protected) &&
+        !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
+        !insideBoundary &&
+        !ctx.owner.enclosingClass.derivesFrom(sym.owner)
+      }
+    }
+  }
+
+  override def prepareForAssign(tree: Assign)(implicit ctx: Context) = tree.lhs match {
+    case tree: RefTree if Accessors.insert.needsAccessor(tree.symbol) =>
+      ctx.fresh.setTree(tree)
+    case _ =>
+      ctx
+  }
+
+  def isLHS(tree: RefTree)(implicit ctx: Context) = ctx.tree match {
+    case Assign(lhs, _) => tree.symbol == lhs.symbol
+    case _ => false
+  }
+
+  override def transformIdent(tree: Ident)(implicit ctx: Context): Tree =
+    if (isLHS(tree)) tree else Accessors.insert.accessorIfNeeded(tree)
+
+  override def transformSelect(tree: Select)(implicit ctx: Context): Tree =
+    if (isLHS(tree)) tree else Accessors.insert.accessorIfNeeded(tree)
+
+  override def transformAssign(tree: Assign)(implicit ctx: Context): Tree =
+    Accessors.insert.accessorIfNeeded(tree)
+
+  override def transformTemplate(tree: Template)(implicit ctx: Context): Tree =
+    cpy.Template(tree)(body = Accessors.addAccessorDefs(tree.symbol.owner, tree.body))
+}
+object ProtectedAccessors { val name = "protectedAccessors" }
+
diff --git a/tests/neg/protectedacc.scala b/tests/neg/protectedacc.scala
new file mode 100644
index 000000000000..dc0319f9b2d2
--- /dev/null
+++ b/tests/neg/protectedacc.scala
@@ -0,0 +1,37 @@
+package p {
+  package a {
+
+    /** Test type parameters */
+    abstract class PolyA[a] {
+      protected def m(x: a): Unit;
+
+      class B {
+
+        trait Node {
+          def s: String = "";
+        }
+        protected def tie(x: Node): Unit = { x.s; () }
+      }
+    }
+  }
+
+  package b {
+    import a._;
+
+    abstract class X[T] extends PolyA[T] {
+
+      trait Inner extends B {
+        def self: T;
+        def self2: Node;
+        def getB: Inner;
+
+        m(self)
+
+        trait InnerInner {
+          val g = getB
+          g.tie(self2.asInstanceOf[g.Node]) // error: acess not permitted
+        }
+      }
+    }
+  }
+}
diff --git a/tests/run/protectedacc.scala b/tests/run/protectedacc.scala
index 43d218fa89fd..3110032965b1 100644
--- a/tests/run/protectedacc.scala
+++ b/tests/run/protectedacc.scala
@@ -143,7 +143,7 @@ package p {
 
         trait InnerInner {
           val g = getB
-          g.tie(self2.asInstanceOf[g.Node])
+          //g.tie(self2.asInstanceOf[g.Node]) -- this would be a static error
         }
       }
     }

From b770c916ec357e97eec1afa035143580b55ebce0 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 23 May 2018 10:54:44 +0200
Subject: [PATCH 2/5] Adapt name tags for protected

Protected accessor names are no longer pickled, but inline accessors are.
Adapt name tags accordingly. Bump major Tasty version.
---
 .../src/dotty/tools/dotc/core/NameKinds.scala  | 11 +++++++----
 .../src/dotty/tools/dotc/core/NameTags.scala   |  8 +++++++-
 .../tools/dotc/core/tasty/TastyFormat.scala    | 18 ++++++------------
 .../dotc/transform/ProtectedAccessors.scala    |  2 +-
 .../tools/dotc/transform/SuperAccessors.scala  |  8 ++++----
 5 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala
index ffe221c02b63..5b38961946e1 100644
--- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala
+++ b/compiler/src/dotty/tools/dotc/core/NameKinds.scala
@@ -353,8 +353,8 @@ object NameKinds {
 
   val SuperAccessorName = new PrefixNameKind(SUPERACCESSOR, "super$")
   val InitializerName = new PrefixNameKind(INITIALIZER, "initial$")
-  val ProtectedAccessorName = new PrefixNameKind(PROTECTEDACCESSOR, "protected$")
-  val ProtectedSetterName = new PrefixNameKind(PROTECTEDSETTER, "protected$set") // dubious encoding, kept for Scala2 compatibility
+  val ProtectedGetterName = new PrefixNameKind(PROTECTEDGETTER, "protected_get$")
+  val ProtectedSetterName = new PrefixNameKind(PROTECTEDSETTER, "protected_set$")
   val InlineGetterName = new PrefixNameKind(INLINEGETTER, "inline_get$")
   val InlineSetterName = new PrefixNameKind(INLINESETTER, "inline_set$")
 
@@ -388,9 +388,12 @@ object NameKinds {
     def infoString: String = "Signed"
   }
 
-  /** Possible name kinds of a method that comes from Scala2 pickling info. */
+  /** Possible name kinds of a method that comes from Scala2 pickling info.
+   *  and that need to be unmangled. Note: Scala2 protected accessors and setters
+   *  can be left mangled, so they are not included in thus list.
+   */
   val Scala2MethodNameKinds: List[NameKind] =
-    List(DefaultGetterName, ExtMethName, UniqueExtMethName, ProtectedAccessorName, ProtectedSetterName)
+    List(DefaultGetterName, ExtMethName, UniqueExtMethName)
 
   def simpleNameKindOfTag      : collection.Map[Int, ClassifiedNameKind] = simpleNameKinds
   def qualifiedNameKindOfTag   : collection.Map[Int, QualifiedNameKind]  = qualifiedNameKinds
diff --git a/compiler/src/dotty/tools/dotc/core/NameTags.scala b/compiler/src/dotty/tools/dotc/core/NameTags.scala
index fd65c5243fab..0c3c30f73f9c 100644
--- a/compiler/src/dotty/tools/dotc/core/NameTags.scala
+++ b/compiler/src/dotty/tools/dotc/core/NameTags.scala
@@ -15,6 +15,10 @@ object NameTags extends TastyFormat.NameTags {
                                 // outer accessor that will be filled in by ExplicitOuter.
                                 // <num> indicates the number of hops needed to select the outer field.
 
+  final val PROTECTEDGETTER = 24 // The name of a protected getter `protected_get$<name>` created by ProtectedAccessors.
+
+  final val PROTECTEDSETTER = 25 // The name of a protected setter `protected_set$<name>` created by ProtectedAccessors.
+
   final val INITIALIZER = 26    // A mixin initializer method
 
   final val AVOIDCLASH = 27     // Adds a suffix to avoid a name clash;
@@ -47,7 +51,9 @@ object NameTags extends TastyFormat.NameTags {
     case OUTERSELECT => "OUTERSELECT"
 
     case SUPERACCESSOR => "SUPERACCESSOR"
-    case PROTECTEDACCESSOR => "PROTECTEDACCESSOR"
+    case INLINEGETTER => "INLINEGETTER"
+    case INLINESETTER => "INLINESETTER"
+    case PROTECTEDGETTER => "PROTECTEDGETTER"
     case PROTECTEDSETTER => "PROTECTEDSETTER"
     case INITIALIZER => "INITIALIZER"
     case AVOIDCLASH => "AVOIDCLASH"
diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala
index 962acf20b759..10fe328312d6 100644
--- a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala
+++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala
@@ -39,8 +39,8 @@ Macro-format:
                   VARIANT           Length underlying_NameRef variance_Nat      // 0: Contravariant, 1: Covariant
 
                   SUPERACCESSOR     Length underlying_NameRef
-                  PROTECTEDACCESSOR Length underlying_NameRef
-                  PROTECTEDSETTER   Length underlying_NameRef
+                  INLINEGETTER      Length underlying_NameRef
+                  INLINESETTER      Length underlying_NameRef
                   OBJECTCLASS       Length underlying_NameRef
 
                   SIGNED            Length original_NameRef resultSig_NameRef paramSig_NameRef*
@@ -226,8 +226,8 @@ Standard Section: "Positions" Assoc*
 object TastyFormat {
 
   final val header = Array(0x5C, 0xA1, 0xAB, 0x1F)
-  val MajorVersion = 7
-  val MinorVersion = 1
+  val MajorVersion = 8
+  val MinorVersion = 0
 
   /** Tags used to serialize names */
   class NameTags {
@@ -252,18 +252,12 @@ object TastyFormat {
 
     final val SUPERACCESSOR = 20     // The name of a super accessor `super$name` created by SuperAccesors.
 
-    final val PROTECTEDACCESSOR = 21 // The name of a protected accessor `protected$<name>` created by SuperAccesors.
+    final val INLINEGETTER = 21    // The name of an inline getter `inline_get$name`
 
-    final val PROTECTEDSETTER = 22   // The name of a protected setter `protected$set<name>` created by SuperAccesors.
-                                     // This is a dubious encoding for its risk for ambiguity.
-                                     // It is kept for Scala-2 compatibility.
+    final val INLINESETTER = 22     // The name of an inline setter `inline_set$name`
 
     final val OBJECTCLASS = 23       // The name of an object class (or: module class) `<name>$`.
 
-    final val INLINEGETTER = 24     // The name of an inline getter `inline_get$name`
-
-    final val INLINESETTER = 25     // The name of an inline setter `inline_set$name`
-
     final val SIGNED = 63            // A pair of a name and a signature, used to idenitfy
                                      // possibly overloaded methods.
   }
diff --git a/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
index fb3c53c78208..4946d1eccc63 100644
--- a/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
+++ b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
@@ -20,7 +20,7 @@ class ProtectedAccessors extends MiniPhase {
   override def phaseName = ProtectedAccessors.name
 
   object Accessors extends AccessProxies {
-    def getterName = ProtectedAccessorName
+    def getterName = ProtectedGetterName
     def setterName = ProtectedSetterName
 
     val insert = new Insert {
diff --git a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala
index 1435a5aaf099..5617992908ea 100644
--- a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala
+++ b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala
@@ -11,7 +11,7 @@ import Types._, Contexts._, Constants._, Names._, NameOps._, Flags._, DenotTrans
 import SymDenotations._, Symbols._, StdNames._, Annotations._, Trees._, Scopes._, Denotations._
 import util.Positions._
 import Decorators._
-import NameKinds.{ProtectedAccessorName, ProtectedSetterName, OuterSelectName, SuperAccessorName}
+import NameKinds.{ProtectedGetterName, ProtectedSetterName, OuterSelectName, SuperAccessorName}
 import Symbols._, TypeUtils._, SymUtils._
 
 /** This class performs the following functions:
@@ -166,7 +166,7 @@ class SuperAccessors(thisPhase: DenotTransformer) {
       assert(clazz.exists, sym)
       ctx.debuglog("Decided for host class: " + clazz)
 
-      val accName = ProtectedAccessorName(sym.name)
+      val accName = ProtectedGetterName(sym.name)
 
       // if the result type depends on the this type of an enclosing class, the accessor
       // has to take an object of exactly this type, otherwise it's more general
@@ -206,7 +206,7 @@ class SuperAccessors(thisPhase: DenotTransformer) {
 
     def isProtectedAccessor(tree: Tree)(implicit ctx: Context): Boolean = tree match {
       case Apply(TypeApply(Select(_, name), _), qual :: Nil) =>
-        name.is(ProtectedAccessorName) || name.is(ProtectedSetterName)
+        name.is(ProtectedGetterName) || name.is(ProtectedSetterName)
       case _ => false
     }
 
@@ -221,7 +221,7 @@ class SuperAccessors(thisPhase: DenotTransformer) {
       assert(clazz.exists, sym)
       ctx.debuglog("Decided for host class: " + clazz)
 
-      val accName = ProtectedAccessorName(sym.name)
+      val accName = ProtectedGetterName(sym.name)
 
       // if the result type depends on the this type of an enclosing class, the accessor
       // has to take an object of exactly this type, otherwise it's more general

From 86de7ec03bb0a7c99e95fc3c41e5b00cb9842bd9 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 23 May 2018 11:05:38 +0200
Subject: [PATCH 3/5] Drop requirement that traits cannot access protected
 members of superclasses

Superaccessors handles this already correctly by generating a super accessor in
this case.
---
 .../tools/dotc/core/SymDenotations.scala      |  5 ---
 tests/neg/protectedacc.scala                  | 37 -------------------
 tests/run/protectedacc.scala                  |  2 +-
 3 files changed, 1 insertion(+), 43 deletions(-)
 delete mode 100644 tests/neg/protectedacc.scala

diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
index a0a304968e9b..32c6b4038d82 100644
--- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -711,11 +711,6 @@ object SymDenotations {
             i"""
                | Access to protected $this not permitted because enclosing ${ctx.owner.enclosingClass.showLocated}
                | is not a subclass of ${owner.showLocated} where target is defined""")
-        else if (cls.is(Trait) && !this.owner.is(Trait))
-          fail(
-            i"""
-               | Access to protected $this not permitted from $cls,
-               | since traits cannot access protected members of superclasses""")
         else if (
           !(  isType // allow accesses to types from arbitrary subclasses fixes #4737
            || pre.derivesFrom(cls)
diff --git a/tests/neg/protectedacc.scala b/tests/neg/protectedacc.scala
deleted file mode 100644
index dc0319f9b2d2..000000000000
--- a/tests/neg/protectedacc.scala
+++ /dev/null
@@ -1,37 +0,0 @@
-package p {
-  package a {
-
-    /** Test type parameters */
-    abstract class PolyA[a] {
-      protected def m(x: a): Unit;
-
-      class B {
-
-        trait Node {
-          def s: String = "";
-        }
-        protected def tie(x: Node): Unit = { x.s; () }
-      }
-    }
-  }
-
-  package b {
-    import a._;
-
-    abstract class X[T] extends PolyA[T] {
-
-      trait Inner extends B {
-        def self: T;
-        def self2: Node;
-        def getB: Inner;
-
-        m(self)
-
-        trait InnerInner {
-          val g = getB
-          g.tie(self2.asInstanceOf[g.Node]) // error: acess not permitted
-        }
-      }
-    }
-  }
-}
diff --git a/tests/run/protectedacc.scala b/tests/run/protectedacc.scala
index 3110032965b1..43d218fa89fd 100644
--- a/tests/run/protectedacc.scala
+++ b/tests/run/protectedacc.scala
@@ -143,7 +143,7 @@ package p {
 
         trait InnerInner {
           val g = getB
-          //g.tie(self2.asInstanceOf[g.Node]) -- this would be a static error
+          g.tie(self2.asInstanceOf[g.Node])
         }
       }
     }

From f6d2f3dfbc42b9cda49a23aba0ec470f48797641 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 23 May 2018 11:15:14 +0200
Subject: [PATCH 4/5] Disable generation of protected accessors in
 SuperAccessors phase

Fix remaining problems, mostly having to do with choosing between
super accessors and protected accessors.
---
 .../tools/dotc/transform/AccessProxies.scala  |  21 +-
 .../tools/dotc/transform/PostTyper.scala      |   4 +-
 .../dotc/transform/ProtectedAccessors.scala   |  74 +++--
 .../tools/dotc/transform/SuperAccessors.scala | 296 ++----------------
 tests/run/protectedSuper.scala                |  36 +++
 5 files changed, 120 insertions(+), 311 deletions(-)
 create mode 100644 tests/run/protectedSuper.scala

diff --git a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
index d6fb1423b52a..c596daee5e2d 100644
--- a/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
+++ b/compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
@@ -21,6 +21,7 @@ import config.Printers.transforms
  */
 abstract class AccessProxies {
   import ast.tpd._
+  import AccessProxies._
 
   def getterName: ClassifiedNameKind
   def setterName: ClassifiedNameKind
@@ -76,17 +77,13 @@ abstract class AccessProxies {
 
       def refersToAccessed(sym: Symbol) = accessedBy.get(sym) == Some(accessed)
 
-      val accessorClass = {
-        def owningClass(start: Symbol) =
-          start.ownersIterator.findSymbol(_.derivesFrom(accessed.owner))
+      var accessorClass = hostForAccessorOf(accessed: Symbol)
+      if (!accessorClass.exists) {
         val curCls = ctx.owner.enclosingClass
-        var owner = owningClass(curCls) //`orElse` owningClass(curCls.linkedClass)
-        if (!owner.exists) {
-          transforms.println(i"${curCls.ownersIterator.toList}%, %")
-          ctx.error(i"illegal access to protected ${accessed.showLocated} from $curCls", reference.pos)
-          owner = curCls
-        }
-        owner
+        transforms.println(i"${curCls.ownersIterator.toList}%, %")
+        ctx.error(i"illegal access to protected ${accessed.showLocated} from $curCls",
+          reference.pos)
+        accessorClass = curCls
       }
 
       val accessorRawInfo =
@@ -125,4 +122,8 @@ abstract class AccessProxies {
         tree
     }
   }
+}
+object AccessProxies {
+  def hostForAccessorOf(accessed: Symbol)(implicit ctx: Context): Symbol =
+    ctx.owner.ownersIterator.findSymbol(_.derivesFrom(accessed.owner))
 }
\ No newline at end of file
diff --git a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala
index ad4885f14669..19d63314644f 100644
--- a/compiler/src/dotty/tools/dotc/transform/PostTyper.scala
+++ b/compiler/src/dotty/tools/dotc/transform/PostTyper.scala
@@ -216,12 +216,10 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
             case sel: Select =>
               val args1 = transform(args)
               val sel1 = transformSelect(sel, args1)
-              if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy.TypeApply(tree1)(sel1, args1)
+              cpy.TypeApply(tree1)(sel1, args1)
             case _ =>
               super.transform(tree1)
           }
-        case tree @ Assign(sel: Select, _) =>
-          super.transform(superAcc.transformAssign(tree))
         case Inlined(call, bindings, expansion) =>
           // Leave only a call trace consisting of
           //  - a reference to the top-level class from which the call was inlined,
diff --git a/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
index 4946d1eccc63..84d3db353b49 100644
--- a/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
+++ b/compiler/src/dotty/tools/dotc/transform/ProtectedAccessors.scala
@@ -8,14 +8,50 @@ import core.Flags._
 import core.Decorators._
 import MegaPhase.MiniPhase
 import ast.Trees._
+import util.Property
 
 /** Add accessors for all protected accesses. An accessor is needed if
  *  according to the rules of the JVM a protected class member is not accesissible
  *  from the point of access, but is accessible if the access is from an enclosing
  *  class. In this point a public access method is placed in that enclosing class.
  */
+object ProtectedAccessors {
+  val name = "protectedAccessors"
+
+  private val LHS = new Property.StickyKey[Unit]
+
+  /** Is the current context's owner inside the access boundary established by `sym`? */
+  def insideBoundaryOf(sym: Symbol)(implicit ctx: Context): Boolean = {
+    if (sym.is(JavaDefined)) {
+      sym.is(JavaStatic) ||  // Java's static protected definitions are treated as public
+      ctx.owner.enclosingPackageClass == sym.enclosingPackageClass
+    }
+    else {
+      // For Scala-defined symbols we currently allow private and protected accesses
+      // from inner packages, and compensate by widening accessibility of such symbols to public.
+      // It would be good if we could revisit this at some point.
+      val boundary = sym.accessBoundary(sym.enclosingPackageClass)
+      ctx.owner.isContainedIn(boundary) || ctx.owner.isContainedIn(boundary.linkedClass)
+    }
+  }
+
+  /** Do we need a protected accessor if the current context's owner
+   *  is not in a subclass or subtrait of `sym`?
+   */
+  def needsAccessorIfNotInSubclass(sym: Symbol)(implicit ctx: Context): Boolean =
+    sym.isTerm && sym.is(Protected) &&
+    !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
+    !insideBoundaryOf(sym)
+
+   /** Do we need a protected accessor for accessing sym from the current context's owner? */
+   def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean =
+    needsAccessorIfNotInSubclass(sym) &&
+    !ctx.owner.enclosingClass.derivesFrom(sym.owner)
+}
+
 class ProtectedAccessors extends MiniPhase {
   import ast.tpd._
+  import ProtectedAccessors._
 
   override def phaseName = ProtectedAccessors.name
 
@@ -24,39 +60,19 @@ class ProtectedAccessors extends MiniPhase {
     def setterName = ProtectedSetterName
 
     val insert = new Insert {
-      def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean = {
-        def insideBoundary = {
-          if (sym.is(JavaDefined)) {
-            sym.is(JavaStatic) ||  // Java's static protected definitions are treated as public
-            ctx.owner.enclosingPackageClass == sym.enclosingPackageClass
-          }
-          else {
-            // For Scala-defined symbols we currently allow private and protected accesses
-            // from inner packages, and compensate by widening accessibility of such symbols to public.
-            // It would be good if we could revisit this at some point.
-            val boundary = sym.accessBoundary(sym.enclosingPackageClass)
-            ctx.owner.isContainedIn(boundary) || ctx.owner.isContainedIn(boundary.linkedClass)
-          }
-        }
-        sym.isTerm && sym.is(Protected) &&
-        !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
-        !insideBoundary &&
-        !ctx.owner.enclosingClass.derivesFrom(sym.owner)
-      }
+      def needsAccessor(sym: Symbol)(implicit ctx: Context) = ProtectedAccessors.needsAccessor(sym)
     }
   }
 
-  override def prepareForAssign(tree: Assign)(implicit ctx: Context) = tree.lhs match {
-    case tree: RefTree if Accessors.insert.needsAccessor(tree.symbol) =>
-      ctx.fresh.setTree(tree)
-    case _ =>
-      ctx
+  override def prepareForAssign(tree: Assign)(implicit ctx: Context) = {
+    tree.lhs match {
+      case lhs: RefTree if needsAccessor(lhs.symbol) => lhs.putAttachment(LHS, ())
+      case _ =>
+    }
+    ctx
   }
 
-  def isLHS(tree: RefTree)(implicit ctx: Context) = ctx.tree match {
-    case Assign(lhs, _) => tree.symbol == lhs.symbol
-    case _ => false
-  }
+  private def isLHS(tree: RefTree) = tree.removeAttachment(LHS).isDefined
 
   override def transformIdent(tree: Ident)(implicit ctx: Context): Tree =
     if (isLHS(tree)) tree else Accessors.insert.accessorIfNeeded(tree)
@@ -70,5 +86,3 @@ class ProtectedAccessors extends MiniPhase {
   override def transformTemplate(tree: Template)(implicit ctx: Context): Tree =
     cpy.Template(tree)(body = Accessors.addAccessorDefs(tree.symbol.owner, tree.body))
 }
-object ProtectedAccessors { val name = "protectedAccessors" }
-
diff --git a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala
index 5617992908ea..2a04d5b4b1b0 100644
--- a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala
+++ b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala
@@ -1,27 +1,21 @@
 package dotty.tools.dotc
 package transform
 
-import dotty.tools.dotc.transform.MegaPhase._
 import dotty.tools.dotc.ast.{Trees, tpd}
-import scala.collection.{ mutable, immutable }
-import ValueClasses._
-import scala.annotation.tailrec
+import scala.collection.mutable
+import ValueClasses.isMethodWithExtension
 import core._
-import Types._, Contexts._, Constants._, Names._, NameOps._, Flags._, DenotTransformers._
-import SymDenotations._, Symbols._, StdNames._, Annotations._, Trees._, Scopes._, Denotations._
+import Types._, Contexts._, Names._, Flags._, Symbols._, NameOps._, Trees._
+import TypeUtils._, SymUtils._
+import DenotTransformers.DenotTransformer
+import Symbols._
 import util.Positions._
 import Decorators._
-import NameKinds.{ProtectedGetterName, ProtectedSetterName, OuterSelectName, SuperAccessorName}
-import Symbols._, TypeUtils._, SymUtils._
+import NameKinds.SuperAccessorName
 
-/** This class performs the following functions:
- *
- *  (1) Adds super accessors for all super calls that either
+/** This class adds super accessors for all super calls that either
  *  appear in a trait or have as a target a member of some outer class.
  *
- *  (2) Adds protected accessors if the access to the protected member happens
- *  in a class which is not a subclass of the member's owner.
- *
  *  It also checks that:
  *
  *  (1) Symbols accessed from super are not abstract, or are overridden by
@@ -144,273 +138,39 @@ class SuperAccessors(thisPhase: DenotTransformer) {
       (sym eq defn.Any_!=) ||
       (sym eq defn.Any_##)
 
-    /** Replace `sel` (or `sel[targs]` if `targs` is nonempty) with a protected accessor
-     *  call, if necessary.
-     */
-    private def ensureProtectedAccessOK(sel: Select, targs: List[Tree])(implicit ctx: Context) = {
-      val sym = sel.symbol
-      if (sym.isTerm && !sel.name.is(OuterSelectName) && needsProtectedAccessor(sym, sel.pos)) {
-        ctx.debuglog("Adding protected accessor for " + sel)
-        protectedAccessorCall(sel, targs)
-      } else sel
-    }
-
-    /** Add a protected accessor, if needed, and return a tree that calls
-     *  the accessor and returns the same member. The result is already
-     *  typed.
-     */
-    private def protectedAccessorCall(sel: Select, targs: List[Tree])(implicit ctx: Context): Tree = {
-      val Select(qual, _) = sel
-      val sym = sel.symbol.asTerm
-      val clazz = hostForAccessorOf(sym, currentClass)
-      assert(clazz.exists, sym)
-      ctx.debuglog("Decided for host class: " + clazz)
-
-      val accName = ProtectedGetterName(sym.name)
-
-      // if the result type depends on the this type of an enclosing class, the accessor
-      // has to take an object of exactly this type, otherwise it's more general
-      val receiverType =
-        if (isThisType(sym.info.finalResultType)) clazz.thisType
-        else clazz.classInfo.selfType
-      val accType = {
-        def accTypeOf(tpe: Type): Type = tpe match {
-          case tpe: PolyType =>
-            tpe.derivedLambdaType(tpe.paramNames, tpe.paramInfos, accTypeOf(tpe.resultType))
-          case _ =>
-            MethodType(receiverType :: Nil)(mt => tpe.substThis(sym.owner.asClass, mt.newParamRef(0)))
-        }
-        accTypeOf(sym.info)
-      }
-      val accPos = sel.pos.focus
-      val protectedAccessor = clazz.info.decl(accName).suchThat(_.signature == accType.signature).symbol orElse {
-        val newAcc = ctx.newSymbol(
-          clazz, accName, Artifact | Method, accType, coord = accPos).enteredAfter(thisPhase)
-        val code = polyDefDef(newAcc, trefs => vrefss => {
-          val (receiver :: _) :: tail = vrefss
-          val base = receiver.select(sym).appliedToTypes(trefs)
-          (base /: tail)(Apply(_, _)).withPos(accPos)
-        })
-        ctx.debuglog("created protected accessor: " + code)
-        accDefs(clazz) += code
-        newAcc
-      }
-      val res = This(clazz)
-        .select(protectedAccessor)
-        .appliedToTypeTrees(targs)
-        .appliedTo(qual)
-        .withPos(sel.pos)
-      ctx.debuglog(s"Replaced $sel with $res")
-      res
-    }
-
-    def isProtectedAccessor(tree: Tree)(implicit ctx: Context): Boolean = tree match {
-      case Apply(TypeApply(Select(_, name), _), qual :: Nil) =>
-        name.is(ProtectedGetterName) || name.is(ProtectedSetterName)
-      case _ => false
-    }
-
-    /** Add a protected accessor, if needed, and return a tree that calls
-     *  the accessor and returns the same member. The result is already
-     *  typed.
-     */
-    private def protectedAccessor(tree: Select, targs: List[Tree])(implicit ctx: Context): Tree = {
-      val Select(qual, _) = tree
-      val sym = tree.symbol.asTerm
-      val clazz = hostForAccessorOf(sym, currentClass)
-      assert(clazz.exists, sym)
-      ctx.debuglog("Decided for host class: " + clazz)
-
-      val accName = ProtectedGetterName(sym.name)
-
-      // if the result type depends on the this type of an enclosing class, the accessor
-      // has to take an object of exactly this type, otherwise it's more general
-      val receiverType =
-        if (isThisType(sym.info.finalResultType)) clazz.thisType
-        else clazz.classInfo.selfType
-      def accTypeOf(tpe: Type): Type = tpe match {
-        case tpe: PolyType =>
-          tpe.derivedLambdaType(tpe.paramNames, tpe.paramInfos, accTypeOf(tpe.resultType))
-        case _ =>
-          MethodType(receiverType :: Nil)(mt => tpe.substThis(sym.owner.asClass, mt.newParamRef(0)))
-      }
-      val accType = accTypeOf(sym.info)
-      val accPos = tree.pos.focus
-      val protectedAccessor = clazz.info.decl(accName).suchThat(_.signature == accType.signature).symbol orElse {
-        val newAcc = ctx.newSymbol(
-            clazz, accName, Artifact, accType, coord = accPos).enteredAfter(thisPhase)
-        val code = polyDefDef(newAcc, trefs => vrefss => {
-          val (receiver :: _) :: tail = vrefss
-          val base = receiver.select(sym).appliedToTypes(trefs)
-          (base /: vrefss)(Apply(_, _)).withPos(accPos)
-        })
-        ctx.debuglog("created protected accessor: " + code)
-        accDefs(clazz) += code
-        newAcc
-      }
-      val res = This(clazz)
-        .select(protectedAccessor)
-        .appliedToTypeTrees(targs)
-        .appliedTo(qual)
-        .withPos(tree.pos)
-      ctx.debuglog(s"Replaced $tree with $res")
-      res
-    }
-
-    /** Add an accessor for field, if needed, and return a selection tree for it .
-     *  The result is not typed.
-     */
-    private def protectedSetter(tree: Select)(implicit ctx: Context): Tree = {
-      val field = tree.symbol.asTerm
-      val clazz = hostForAccessorOf(field, currentClass)
-      assert(clazz.exists, field)
-      ctx.debuglog("Decided for host class: " + clazz)
-
-      val accName = ProtectedSetterName(field.name)
-      val accType = MethodType(clazz.classInfo.selfType :: field.info :: Nil, defn.UnitType)
-      val accPos = tree.pos.focus
-      val protectedAccessor = clazz.info.decl(accName).symbol orElse {
-        val newAcc = ctx.newSymbol(
-            clazz, accName, Artifact | Method, accType, coord = accPos).enteredAfter(thisPhase)
-        val code = DefDef(newAcc, vrefss => {
-          val (receiver :: value :: Nil) :: Nil = vrefss
-          Assign(receiver.select(field), value).withPos(accPos)
-        })
-        ctx.debuglog("created protected setter: " + code)
-        accDefs(clazz) += code
-        newAcc
-      }
-      This(clazz).select(protectedAccessor).withPos(tree.pos)
-    }
-
-    /** Does `sym` need an accessor when accessed from `currentClass`?
-     *  A special case arises for classes with explicit self-types. If the
-     *  self type is a Java class, and a protected accessor is needed, we issue
-     *  an error. If the self type is a Scala class, we don't add an accessor.
-     *  An accessor is not needed if the access boundary is larger than the
-     *  enclosing package, since that translates to 'public' on the host sys.
-     *  (as Java has no real package nesting).
-     *
-     * If the access happens inside a 'trait', access is more problematic since
-     * the implementation code is moved to an '$class' class which does not
-     * inherit anything. Since we can't (yet) add accessors for 'required'
-     * classes, this has to be signaled as error.
-     * FIXME Need to better understand this logic
-     */
-    private def needsProtectedAccessor(sym: Symbol, pos: Position)(implicit ctx: Context): Boolean = {
-      val clazz = currentClass
-      def accessibleThroughSubclassing =
-        validCurrentClass && clazz.classInfo.selfType.derivesFrom(sym.owner) && !clazz.is(Trait)
-
-      val isCandidate = (
-           sym.is(Protected)
-        && sym.is(JavaDefined)
-        && !sym.effectiveOwner.is(Package)
-        && !accessibleThroughSubclassing
-        && (sym.enclosingPackageClass != currentClass.enclosingPackageClass)
-        && (sym.enclosingPackageClass == sym.accessBoundary(sym.enclosingPackageClass))
-      )
-
-      if (!isCandidate)
-        return false
-
-      val host = hostForAccessorOf(sym, clazz)
-      val hostSelfType = host.classInfo.selfType
-
-      def isSelfType = !(host.appliedRef <:< hostSelfType) && {
-        if (hostSelfType.typeSymbol.is(JavaDefined))
-          ctx.restrictionError(
-            s"cannot accesses protected $sym from within $clazz with host self type $hostSelfType", pos)
-        true
-      }
-      def isJavaProtected = host.is(Trait) && sym.is(JavaDefined) && {
-        ctx.restrictionError(
-          s"""$clazz accesses protected $sym inside a concrete trait method.
-             |Add an accessor in a class extending ${sym.enclosingClass} as a workaround.""".stripMargin,
-          pos
-        )
-        true
-      }
-      !host.is(Package) && !isSelfType && !isJavaProtected
-    }
-
-    /** Return the innermost enclosing class C of referencingClass for which either
-     *  of the following holds:
-     *     - C is a subclass of sym.owner or
-     *     - C is declared in the same package as sym's owner
-     */
-    private def hostForAccessorOf(sym: Symbol, referencingClass: ClassSymbol)(implicit ctx: Context): ClassSymbol =
-      if (referencingClass.derivesFrom(sym.owner)
-          || referencingClass.classInfo.selfType <:< sym.owner.appliedRef
-          || referencingClass.enclosingPackageClass == sym.owner.enclosingPackageClass) {
-        assert(referencingClass.isClass, referencingClass)
-        referencingClass
-      }
-      else if (referencingClass.owner.enclosingClass.exists)
-        hostForAccessorOf(sym, referencingClass.owner.enclosingClass.asClass)
-      else
-        referencingClass
-
-    /** Is 'tpe' a ThisType, or a type proxy with a ThisType as transitively underlying type? */
-    private def isThisType(tpe: Type)(implicit ctx: Context): Boolean = tpe match {
-      case tpe: ThisType => !tpe.cls.is(PackageClass)
-      case tpe: TypeProxy => isThisType(tpe.underlying)
-      case _ => false
-    }
-
     /** Transform select node, adding super and protected accessors as needed */
     def transformSelect(tree: Tree, targs: List[Tree])(implicit ctx: Context) = {
       val sel @ Select(qual, name) = tree
       val sym = sel.symbol
+
+      /** If an accesses to protected member of a class comes from a trait,
+       *  or would need a protected accessor placed in a trait, we cannot
+       *  perform the access to the protected member directly since jvm access
+       *  restrictions require the call site to be in an actual subclass and
+       *  traits don't count as subclasses in this respect. In this case
+       *  we generate a super accessor instead. See SI-2296.
+       */
+      def needsSuperAccessor =
+        ProtectedAccessors.needsAccessorIfNotInSubclass(sym) &&
+        AccessProxies.hostForAccessorOf(sym).is(Trait)
       qual match {
-        case _: This =>
+        case _: This if needsSuperAccessor =>
+          println(i"trans super $sel in $currentClass")
           /*
            * A trait which extends a class and accesses a protected member
            *  of that class cannot implement the necessary accessor method
-           *  because its implementation is in an implementation class (e.g.
-           *  Foo$class) which inherits nothing, and jvm access restrictions
-           *  require the call site to be in an actual subclass. So non-trait
-           *  classes inspect their ancestors for any such situations and
-           *  generate the accessors.  See SI-2296.
+           *  because jvm access restrictions require the call site to be in
+           *  an actual subclass and traits don't count as subclasses in this
+           *  respect. We generate a super accessor itself, which will be fixed
+           *  by the implementing class.  See SI-2296.
            */
-          // FIXME (from scalac's SuperAccessors)
-          // - this should be unified with needsProtectedAccessor, but some
-          // subtlety which presently eludes me is foiling my attempts.
-          val shouldEnsureAccessor = (
-            (currentClass is Trait)
-            && (sym is Protected)
-            && sym.enclosingClass != currentClass
-            && !(sym.owner is PackageClass) // SI-7091 no accessor needed package owned (ie, top level) symbols
-            && !(sym.owner is Trait)
-            && sym.owner.enclosingPackageClass != currentClass.enclosingPackageClass
-            && qual.symbol.info.member(sym.name).exists
-            && !needsProtectedAccessor(sym, sel.pos))
-          if (shouldEnsureAccessor) {
-            ctx.log("Ensuring accessor for call to protected " + sym.showLocated + " from " + currentClass)
-            superAccessorCall(sel)
-          } else
-            ensureProtectedAccessOK(sel, targs)
-
+          superAccessorCall(sel)
+            .reporting(res => i"trans super $sel in $currentClass = $res")
         case Super(_, mix) =>
           transformSuperSelect(sel)
-
         case _ =>
-          ensureProtectedAccessOK(sel, targs)
-      }
-    }
-
-    /** Transform assignment, adding a protected setter if needed */
-    def transformAssign(tree: Tree)(implicit ctx: Context) = {
-      val Assign(lhs @ Select(qual, name), rhs) = tree
-      if ((lhs.symbol is Mutable) &&
-        (lhs.symbol is JavaDefined) &&
-        needsProtectedAccessor(lhs.symbol, tree.pos)) {
-        ctx.debuglog("Adding protected setter for " + tree)
-        val setter = protectedSetter(lhs)
-        ctx.debuglog("Replaced " + tree + " with " + setter)
-        setter.appliedTo(qual, rhs)
+          sel
       }
-      else tree
     }
 
     /** Wrap template to template transform `op` with needed initialization and finalization */
diff --git a/tests/run/protectedSuper.scala b/tests/run/protectedSuper.scala
new file mode 100644
index 000000000000..df21c9eb4541
--- /dev/null
+++ b/tests/run/protectedSuper.scala
@@ -0,0 +1,36 @@
+package p {
+  class A {
+    protected def foo(): Int = 1
+    protected def fuzz(): Int = 2
+  }
+}
+package q {
+  class B extends p.A {  // protected accessor for foo
+    trait BInner {
+      def bar() = foo()
+    }
+  }
+  trait Inner extends p.A {
+    def bar() = foo()   // shared super accessor for foo
+                        // new super accessor for fuzz
+    class InnerInner {
+      def bar() = foo()
+      def baz() = fuzz()
+    }
+  }
+}
+object Test extends App {
+  val b = new q.B
+  val bi = new b.BInner {}
+  assert(bi.bar() == 1)
+
+  class C extends p.A with q.Inner {
+    // implements super accessors for foo and fuzz
+  }
+  val c = new C
+  assert(c.bar() == 1)
+
+  val d = new c.InnerInner
+  assert(d.bar() == 1)
+  assert(d.baz() == 2)
+}

From 7777c7037d6c4e99398bd9ec5aaaef2172469fd8 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Wed, 23 May 2018 15:18:29 +0200
Subject: [PATCH 5/5] Drop misleading comment

---
 compiler/src/dotty/tools/dotc/transform/Erasure.scala | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
index ba58e2f15515..d401ce5aa3c4 100644
--- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala
+++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala
@@ -314,10 +314,6 @@ object Erasure {
       }
   }
 
-  /** The erasure typer.
-   *  Also inserts protected accessors where needed. This logic is placed here
-   *  since it is most naturally done in a macro transform.
-   */
   class Typer extends typer.ReTyper with NoChecking {
     import Boxing._