Also retain runtime-invokable inline methods when they are indirectly overriden

compiler/src/dotty/tools/dotc/core/NameKinds.scala

@@ -396,6 +396,13 @@ object NameKinds {
     case BothBounds extends AvoidNameKind(AVOIDBOTH, "(avoid)")
 
   val BodyRetainerName: SuffixNameKind = new SuffixNameKind(BODYRETAINER, "$retainedBody")
+
+  // Built on a plain string to avoid using a tag byte and messing with TASTy,
+  // since those methods are built before the pickler. 
+  val IndirectBodyRetainerSuffix: String = "$indirectRetainedBody"
+  def indirectRetainerName(name: TermName): TermName =
+    (name.toString + IndirectBodyRetainerSuffix).toTermName
+
   val FieldName: SuffixNameKind = new SuffixNameKind(FIELD, "$$local") {
       override def mkString(underlying: TermName, info: ThisInfo) = underlying.toString
   }

compiler/src/dotty/tools/dotc/inlines/Inlines.scala

@@ -289,6 +289,31 @@ object Inlines:
         ref(meth).appliedToArgss(prefss).withSpan(mdef.rhs.span.startPos))(
         using ctx.withOwner(retainer)))
     .showing(i"retainer for $meth: $result", inlining)
+
+  /** Similar to `bodyRetainer`, a method that keeps track of the body that is kept at runtime,
+   *  except synthesised for indirect overrides, and put into a downstream class, e.g. in:
+   *  
+   *    class Foo:
+   *      inline def bar = True
+   *    class Bar:
+   *      def bar
+   *      def test = bar
+   *    class Test extends Foo, Bar
+   * 
+   * `bar$indirectRetainedBody` is added to `Test`, and rewritten to `bar` in erasure.
+   */
+  def indirectBodyRetainer(sym: Symbol, impl: Symbol, cls: ClassSymbol)(using Context): DefDef =
+    val retainer = impl.asTerm.copy(
+        owner = cls,
+        name = NameKinds.indirectRetainerName(sym.name.asTermName),
+        flags = (impl.flags &~ (Inline | Macro | Override | AbsOverride)) | Private,
+        info = sym.info.asSeenFrom(cls.thisType, sym.owner),
+        coord = cls.coord
+      ).asTerm.entered
+      DefDef(retainer, prefss =>
+        inlineCall(
+          This(cls).select(impl).appliedToArgss(prefss).withSpan(cls.span))(
+            using ctx.withOwner(retainer)))
 
   /** Replace `Inlined` node by a block that contains its bindings and expansion */
   def dropInlined(inlined: Inlined)(using Context): Tree =

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

@@ -14,7 +14,7 @@ import core.Names.*
 import core.StdNames.*
 import core.NameOps.*
 import core.Periods.currentStablePeriod
-import core.NameKinds.{AdaptedClosureName, BodyRetainerName, DirectMethName}
+import core.NameKinds.{AdaptedClosureName, BodyRetainerName, DirectMethName, IndirectBodyRetainerSuffix}
 import core.Scopes.newScopeWith
 import core.Decorators.*
 import core.Constants.*
@@ -898,7 +898,8 @@ object Erasure {
      *  parameter of type `[]Object`.
      */
     override def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(using Context): Tree =
-      if sym.isEffectivelyErased || sym.name.is(BodyRetainerName) then
+      if sym.isEffectivelyErased || sym.name.is(BodyRetainerName)
+         || sym.name.toString.endsWith(IndirectBodyRetainerSuffix) then
         erasedDef(sym)
       else
         val restpe = if sym.isConstructor then defn.UnitType else sym.info.resultType
@@ -1043,14 +1044,55 @@ object Erasure {
 
     override def typedStats(stats: List[untpd.Tree], exprOwner: Symbol)(using Context): (List[Tree], Context) = {
       // discard Imports first, since Bridges will use tree's symbol
-      val stats0 = addRetainedInlineBodies(stats.filter(!_.isInstanceOf[untpd.Import]))(using preErasureCtx)
+      val stats00 = stats.filter(!_.isInstanceOf[untpd.Import])
+      val stats0 = addRetainedInlineBodies(implementIndirectlyOverriddenInlines(stats00))(using preErasureCtx)
       val stats1 =
         if (takesBridges(ctx.owner)) new Bridges(ctx.owner.asClass, erasurePhase).add(stats0)
         else stats0
       val (stats2, finalCtx) = super.typedStats(stats1, exprOwner)
       (stats2.filterConserve(!_.isEmpty), finalCtx)
     }
 
+    /** For every stat that is a private `$indirectRetainedBody`-named helper, create the actual public,
+     *  concrete override of the abstract member it corresponds to, splicing in the helper's
+     *  already-fully-resolved body. Mirrors `addRetainedInlineBodies`, except the target method
+     *  doesn't exist yet as a statement here and must be created, not just matched by name.
+     *
+     *  The helper itself is subsequently mapped to the empty tree in `typedDefDef`, same as an
+     *  ordinary `$retainedBody` retainer.
+     */
+    private def implementIndirectlyOverriddenInlines(stats: List[untpd.Tree])(using Context): List[untpd.Tree] =
+      val newOverrides = stats.collect {
+        case stat: DefDef if stat.symbol.name.toString.endsWith(IndirectBodyRetainerSuffix) =>
+          val helperSym = stat.symbol
+          val cls = helperSym.owner.asClass
+          val origName = helperSym.name.toString.stripSuffix(IndirectBodyRetainerSuffix).toTermName
+          val sym =
+            cls.baseClasses.iterator
+              .map(_.info.decl(origName).symbol)
+              .find(s => s.exists && s.is(Flags.Deferred))
+              .getOrElse(NoSymbol)
+          if !sym.exists then
+            assert(false, i"illegal state - $stat retainer doesn't have a corresponding method")
+          else
+            val newSym = helperSym.asTerm.copy(
+              owner = cls,
+              name = origName,
+              flags = (helperSym.flags &~ Flags.Private) | Flags.Override | (sym.flags & Flags.Protected),
+              info = sym.info.asSeenFrom(cls.thisType, sym.owner),
+              privateWithin = sym.privateWithin,
+              coord = cls.coord
+            ).asTerm.entered
+            Some(DefDef(newSym, prefss =>
+              val mapBody = TreeTypeMap(
+                oldOwners = helperSym :: Nil,
+                newOwners = newSym :: Nil,
+                substFrom = untpd.allParamSyms(stat),
+                substTo   = prefss.flatten.map(_.symbol))
+              mapBody.transform(stat.rhs)))
+      }.flatten
+      stats ++ newOverrides
+
     /** Finally drops all (language-) imports in erasure.
      *  Since some of the language imports change the subtyping,
      *  we cannot check the trees before erasure.

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -11,6 +11,7 @@ import util.Spans.*
 import scala.collection.{mutable, immutable}
 import ast.*
 import MegaPhase.*
+import inlines.Inlines
 import config.Printers.{checks, noPrinter, capt}
 import Decorators.*
 import OverridingPairs.isOverridingPair
@@ -282,6 +283,33 @@ object RefChecks {
     def checkInheritedTraitParameters: Boolean = true
   end OverridingPairsChecker
 
+  def implementationOf(clazz: ClassSymbol, mbr: Symbol)(using Context): PreDenotation =
+    val mbrDenot = mbr.asSeenFrom(clazz.thisType)
+    def isConcrete(sym: Symbol) = sym.exists && !sym.isOneOf(NotConcrete)
+    clazz.nonPrivateMembersNamed(mbr.name)
+      .filterWithPredicate(
+        impl => isConcrete(impl.symbol)
+          && withMode(Mode.IgnoreCaptures)(mbrDenot.matchesLoosely(impl, alwaysCompareTypes = true)))
+
+  def hasJavaErasedOverriding(clazz: ClassSymbol, sym: Symbol)(using Context): Boolean =
+    !erasurePhase.exists || // can't do the test, assume the best
+      atPhase(erasurePhase.next) {
+        clazz.info.nonPrivateMember(sym.name).hasAltWith { alt =>
+          alt.symbol.is(JavaDefined, butNot = Deferred) &&
+            !sym.owner.derivesFrom(alt.symbol.owner) &&
+            alt.matches(sym)
+        }
+    }
+
+  def ignoreDeferred(clazz: ClassSymbol, mbr: Symbol, checkJavaErasedOverriding: Boolean = true)(using Context): Boolean =
+    mbr.isType
+    || mbr.isSuperAccessor // not yet synthesized
+    || checkJavaErasedOverriding && mbr.is(JavaDefined) && hasJavaErasedOverriding(clazz, mbr)
+    || mbr.is(Tracked)
+      // Tracked members correspond to existing val parameters, so they don't
+      // count as deferred. The val parameter could not implement the tracked
+      // refinement since it usually has a wider type.
+
   /** 1. Check all members of class `clazz` for overriding conditions.
    *  That is for overriding member M and overridden member O:
    *
@@ -687,33 +715,9 @@ object RefChecks {
         else abstractErrors += msg
       }
 
-      def hasJavaErasedOverriding(sym: Symbol): Boolean =
-        !erasurePhase.exists || // can't do the test, assume the best
-          atPhase(erasurePhase.next) {
-            clazz.info.nonPrivateMember(sym.name).hasAltWith { alt =>
-              alt.symbol.is(JavaDefined, butNot = Deferred) &&
-                !sym.owner.derivesFrom(alt.symbol.owner) &&
-                alt.matches(sym)
-            }
-        }
+      def ignoreDeferred(mbr: Symbol): Boolean = RefChecks.ignoreDeferred(clazz, mbr)
 
-      def ignoreDeferred(mbr: Symbol) =
-        mbr.isType
-        || mbr.isSuperAccessor // not yet synthesized
-        || mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)
-        || mbr.is(Tracked)
-          // Tracked members correspond to existing val parameters, so they don't
-          // count as deferred. The val parameter could not implement the tracked
-          // refinement since it usually has a wider type.
-
-      def isImplemented(mbr: Symbol) =
-        val mbrDenot = mbr.asSeenFrom(clazz.thisType)
-        def isConcrete(sym: Symbol) = sym.exists && !sym.isOneOf(NotConcrete)
-        clazz.nonPrivateMembersNamed(mbr.name)
-          .filterWithPredicate(
-            impl => isConcrete(impl.symbol)
-              && withMode(Mode.IgnoreCaptures)(mbrDenot.matchesLoosely(impl, alwaysCompareTypes = true)))
-          .exists
+      def isImplemented(mbr: Symbol) = implementationOf(clazz, mbr).exists
 
       /** Filter out symbols from `syms` that are overridden by a symbol appearing later in the list.
        *  Symbols that are not overridden are kept. */
@@ -1464,7 +1468,7 @@ import RefChecks.*
  *  Unlike in Scala 2.x not-private members keep their name. It is
  *  up to the backend to find a unique expanded name for them. The
  *  rationale to do name changes that late is that they are very fragile.
-
+ *
  *  todo: But RefChecks is not done yet. It's still a somewhat dirty port from the Scala 2 version.
  *  todo: move untrivial logic to their own mini-phases
  */

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -3492,6 +3492,37 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         body ++ givenImpls
     end implementDeferredGivens
 
+    /** For an abstract member of `cls` that is only implemented indirectly, by an inline
+     *  method from an unrelated base class (i.e. that method's own hierarchy doesn't
+     *  directly override the abstract member), synthesize a private, non-inline helper
+     *  in `cls` whose body inline-expands a call to that method. When the referenced inline
+     *  method is erased, the helper will take it's place for runtime invocations.
+     */
+    def addIndirectInlineRetainers(body: List[Tree]): List[Tree] =
+      if ctx.isAfterTyper || cls.isRefinementClass then body
+      else
+        def hasMatchingInlineMethod(cls: ClassSymbol, sym: Symbol) =
+          cls.baseClasses.tail.exists(_.info.decl(sym.name).symbol.isInlineMethod)
+        def alreadyHandledByBaseClass(cls: ClassSymbol, sym: Symbol, impl: Symbol) =
+          cls.baseClasses.tail.exists(bc => bc.derivesFrom(sym.owner) && bc.derivesFrom(impl.owner))
+        val indirectBodyRetainers =
+          for
+            bc <- cls.baseClasses.tail
+            sym <- bc.info.decls.toList
+            if sym.is(DeferredTerm) && !sym.is(JavaDefined)
+              && hasMatchingInlineMethod(cls, sym)
+              && !RefChecks.ignoreDeferred(cls, sym, checkJavaErasedOverriding = false)
+            impl = RefChecks.implementationOf(cls, sym).toDenot(cls.thisType).symbol
+            if impl.exists
+              && !impl.owner.flags.is(Flags.Inline) // let's not interfere with inline traits
+              && !impl.allOverriddenSymbols.contains(sym)
+              && impl.isInlineMethod
+              && !alreadyHandledByBaseClass(cls, sym, impl)
+          yield Inlines.indirectBodyRetainer(sym, impl, cls)
+
+        body ++ indirectBodyRetainers
+    end addIndirectInlineRetainers
+
     ensureCorrectSuperClass()
     completeAnnotations(cdef, cls)
     val constr1 = typed(constr).asInstanceOf[DefDef]
@@ -3522,10 +3553,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
     else {
       val dummy = localDummy(cls, impl)
       val body1 =
-        implementDeferredGivens(
-          addParentRefinements(
-            addAccessorDefs(cls,
-              typedStats(impl.body, dummy)(using ctx.inClassContext(self1.symbol))._1)))
+        addIndirectInlineRetainers(
+          implementDeferredGivens(
+            addParentRefinements(
+              addAccessorDefs(cls,
+                typedStats(impl.body, dummy)(using ctx.inClassContext(self1.symbol))._1))))
 
       checkNoDoubleDeclaration(cls)
       val impl1 = cpy.Template(impl)(constr1, parents1, Nil, self1, body1)

compiler/test/dotty/tools/backend/jvm/InlineBytecodeTests.scala

@@ -756,4 +756,38 @@ class InlineBytecodeTests extends DottyBytecodeTest {
     }
   }
 
+  @Test def i25091_indirectInlineOverride = {
+    val source = """trait Settings:
+                   |  inline def switch: Boolean = true
+                   |
+                   |trait Inner:
+                   |  def switch: Boolean
+                   |  inline def go1: String = inline if switch then "Yes" else "No"
+                   |  def go2: String = if switch then "Yes" else "No"
+                   |
+                   |class Outer extends Inner, Settings
+                   |
+                   |class Outer2 extends Outer
+                 """.stripMargin
+
+    checkBCode(source) { dir =>
+      val cls     = lookupClass(dir, "Outer.class")
+      val clsNode = loadClassNode(cls)
+
+      val switchMethod = getMethod(clsNode, "switch")
+      val instructions  = instructionsFromMethod(switchMethod)
+      val expected      = List(Op(ICONST_1), Op(IRETURN))
+
+      assert(instructions == expected,
+        "indirectly overriding inline method `switch` was not retained with a concrete body in `Outer`\n" +
+        diffInstructions(instructions, expected))
+
+      val cls2     = lookupClass(dir, "Outer2.class")
+      val cls2Node = loadClassNode(cls2)
+      val switchInOuter2 = cls2Node.methods.asScala.find(_.name == "switch")
+
+      assert(switchInOuter2.isEmpty, "method `switch` should not have been redundantly copied to `Outer2`")
+    }
+  }
+
 }

docs/_docs/reference/metaprogramming/inline.md

@@ -191,6 +191,22 @@ Inline methods can override other non-inline methods. The rules are as follows:
     val a: A = B
     a.f         // error: cannot inline f in A.
     ```
+4. Indirectly overridden inline methods, like:
+    ```scala
+    trait Settings:
+      inline def switch: Boolean = true
+
+    trait Inner:
+      def switch: Boolean
+      def go: String = if switch then "Yes" else "No"
+
+    object Outer extends Inner, Settings
+    ```
+    have their inline expanded body copied (not in TASTy, only at the backend level) to the first class that is aware of the override (`Outer` in the example above).
+    This is because:
+    * inline defs that are not overriding any method at the point of implementation are erased.
+    * we might only be able to detect indirect overrides after the initial classlike with the inline method is already compiled (so we can't un-erase it then, and we have to include some reference to it in the classfile, copying makes the most sense). 
+
 
 ### Relationship to `@inline`
 

tests/run/25206-b.check

@@ -0,0 +1 @@
+Yes

tests/run/25206-b.scala

@@ -0,0 +1,12 @@
+trait Settings:
+  inline def nestedInline = false
+  inline def switch: Boolean = !nestedInline
+
+trait Inner:
+  def switch: Boolean
+  def go: String = if switch then "Yes" else "No"
+
+object Outer extends Inner, Settings
+
+@main def Test() =
+    println(Outer.go)

tests/run/25206-c.check

@@ -0,0 +1,4 @@
+Yes
+Yes
+true
+true

tests/run/25206-c/Main_4.scala

@@ -0,0 +1,8 @@
+@main def Test(): Unit =
+  println(new Outer().go)
+  println(new Outer2().go)
+  val outer: Inner = new Outer
+  val outer2: Inner = new Outer2
+  println(outer.switch)
+  println(outer2.switch)
+

tests/run/25206-c/Override2_3.scala

@@ -0,0 +1 @@
+class Outer2 extends Outer

tests/run/25206-c/Override_2.scala

@@ -0,0 +1 @@
+class Outer extends Inner, Settings

tests/run/25206-c/Traits_1.scala

@@ -0,0 +1,6 @@
+trait Settings:
+  inline def switch: Boolean = true
+
+trait Inner:
+  def switch: Boolean
+  def go: String = if switch then "Yes" else "No"

tests/run/25206.check

@@ -0,0 +1,3 @@
+Yes
+Yes
+true

tests/run/25206.scala

@@ -0,0 +1,17 @@
+trait Settings:
+  inline def switch: Boolean = true
+
+trait Inner:
+  def switch: Boolean
+  inline def go1: String = inline if switch then "Yes" else "No"
+  def go2: String = if switch then "Yes" else "No"
+
+object Outer extends Inner, Settings:
+  def test1 = go1
+  def test2 = go2
+
+@main def Test(): Unit =
+  println(Outer.test1)
+  println(Outer.test2)
+  val a: Inner = Outer
+  println(a.switch)