Skip to content

Commit

Permalink
SIP 61 - remove tasty hack by not "telescoping" forwarders
Browse files Browse the repository at this point in the history
Now, all forwarders directly call the unrolled method.
This means that there is no need to resolve an invisible method,
so the "hack" using TERMREFdirect is no longer needed.

This increases bytecode size but reduces stack depth.

Also removed scala.annotation.internal.UnrolledForwarder,
as it is only needed for the TASTy hack.
  • Loading branch information
bishabosha committed Oct 5, 2024
1 parent 4868397 commit aec0618
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 31 deletions.
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,6 @@ class Definitions {
@tu lazy val NowarnAnnot: ClassSymbol = requiredClass("scala.annotation.nowarn")
@tu lazy val UnusedAnnot: ClassSymbol = requiredClass("scala.annotation.unused")
@tu lazy val UnrollAnnot: ClassSymbol = requiredClass("scala.annotation.unroll")
@tu lazy val UnrollForwarderAnnot: ClassSymbol = requiredClass("scala.annotation.internal.UnrollForwarder")
@tu lazy val TransparentTraitAnnot: ClassSymbol = requiredClass("scala.annotation.transparentTrait")
@tu lazy val NativeAnnot: ClassSymbol = requiredClass("scala.native")
@tu lazy val RepeatedAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Repeated")
Expand Down
3 changes: 0 additions & 3 deletions compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -490,9 +490,6 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
writeByte(if name.isTypeName then SELECTtpt else SELECT)
pickleNameAndSig(name, sig, ename)
pickleTree(qual)
else if sym.is(Invisible) && qual.isInstanceOf[This] && sym.hasAnnotation(defn.UnrollForwarderAnnot) then
writeByte(TERMREFdirect)
pickleSymRef(sym) // SIP-61 HACK: resolution from Signature filters out Invisible symbols
else // select from owner
writeByte(SELECTin)
withLength {
Expand Down
7 changes: 1 addition & 6 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1259,12 +1259,7 @@ class TreeUnpickler(reader: TastyReader,
goto(start)
readType() match {
case path: TypeRef => TypeTree(path)
case path: TermRef =>
val sym = path.symbol
if sym.is(Invisible) && sym.hasAnnotation(defn.UnrollForwarderAnnot) then
This(sym.owner.asClass).select(sym)
else
ref(path)
case path: TermRef => ref(path)
case path: ThisType => untpd.This(untpd.EmptyTypeIdent).withType(path)
case path: ConstantType => Literal(path.value)
case path: ErrorType if isBestEffortTasty => TypeTree(path)
Expand Down
18 changes: 7 additions & 11 deletions compiler/src/dotty/tools/dotc/transform/UnrollDefinitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
def generateSingleForwarder(defdef: DefDef,
prevMethodType: Type,
paramIndex: Int,
paramCount: Int,
nextParamIndex: Int,
nextSymbol: Symbol,
annotatedParamListIndex: Int,
Expand All @@ -125,18 +126,12 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
val forwarderDefSymbol0 = Symbols.newSymbol(
defdef.symbol.owner,
defdef.name,
(defdef.symbol.flags &~
HasDefaultParams &~
(if nextParamIndex == -1 then EmptyFlags else Deferred)) |
defdef.symbol.flags &~ HasDefaultParams |
Invisible | Synthetic,
NoType, // fill in later
coord = nextSymbol.span.shift(1) // shift by 1 to avoid "secondary constructor must call preceding" error
).entered

// we need this such that when unpickling a TERMREFdirect, if we see this annotation,
// we restore the tree to a Select
forwarderDefSymbol0.addAnnotation(defn.UnrollForwarderAnnot)

val newParamSymMappings = extractParamSymss(copyParamSym(_, forwarderDefSymbol0))
val (oldParams, newParams) = newParamSymMappings.flatten.unzip

Expand Down Expand Up @@ -170,7 +165,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
.map(_.size)
.sum

val defaultCalls = Range(paramIndex, nextParamIndex).map(n =>
val defaultCalls = Range(paramIndex, paramCount).map(n =>

def makeSelect(refTree: Tree, name: TermName): Tree =
val sym = refTree.symbol
Expand Down Expand Up @@ -204,7 +199,8 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
else Apply(lhs, newParams)
)

val forwarderInner: Tree = This(defdef.symbol.owner.asClass).select(nextSymbol)
val forwarderInner: Tree =
This(defdef.symbol.owner.asClass).select(defdef.symbol)

val forwarderCallArgs =
newParamSymLists.zipWithIndex.map{case (ps, i) =>
Expand All @@ -226,8 +222,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
}

val forwarderDef =
tpd.DefDef(forwarderDefSymbol,
rhs = if nextParamIndex == -1 then EmptyTree else forwarderRhs())
tpd.DefDef(forwarderDefSymbol, rhs = forwarderRhs())

forwarderDef.withSpan(nextSymbol.span.shift(1))
}
Expand Down Expand Up @@ -297,6 +292,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
defdef,
defdef.symbol.info,
paramIndex,
paramCount,
nextParamIndex,
nextSymbol,
paramClauseIndex,
Expand Down
9 changes: 0 additions & 9 deletions library/src/scala/annotation/internal/UnrollForwarder.scala

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ val experimentalDefinitionInLibrary = Set(
"scala.Predef$.runtimeChecked", "scala.annotation.internal.RuntimeChecked",

// New feature: SIP 61 - @unroll annotation
"scala.annotation.unroll", "scala.annotation.internal.UnrollForwarder"
"scala.annotation.unroll"
)


Expand Down
72 changes: 72 additions & 0 deletions tests/run/unroll-multiple.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//> using options -experimental

import scala.annotation.unroll
import scala.deriving.Mirror

class Unrolled {
final def foo(
s: String,
@unroll y: Boolean = true,
@unroll i: Int = 0,
@unroll c: Char = '?'): String = s + y + i + c
}

class Outer {
class Unrolled {
final def bar(
s: String,
@unroll y: Boolean = true,
@unroll i: Int = 0,
@unroll c: Char = '?'): String = s + y + i + c
}

case class UnrolledCase(
s: String,
@unroll y: Boolean = true,
@unroll i: Int = 0,
@unroll c: Char = '?') {
def baz: String = s + y + i + c
}

class UnrolledSecondary {
private var msg = ""

def qux: String = msg

def this(
s: String,
@unroll y: Boolean = true,
@unroll i: Int = 0,
@unroll c: Char = '?') = {
this()
msg = s + y + i + c
}
}
}

@main def Test: Unit =
assert(Unrolled().foo("foo") == "footrue0?")
assert(Unrolled().foo("foo", false) == "foofalse0?")
assert(Unrolled().foo("foo", false, 1) == "foofalse1?")
assert(Unrolled().foo("foo", false, 1, '@') == "foofalse1@")
val outer = new Outer()
assert(new outer.Unrolled().bar("bar") == "bartrue0?")
assert(new outer.Unrolled().bar("bar", false) == "barfalse0?")
assert(new outer.Unrolled().bar("bar", false, 1) == "barfalse1?")
assert(new outer.Unrolled().bar("bar", false, 1, '@') == "barfalse1@")
assert(outer.UnrolledCase.apply("baz").baz == "baztrue0?")
assert(outer.UnrolledCase.apply("baz", false).baz == "bazfalse0?")
assert(outer.UnrolledCase.apply("baz", false, 1).baz == "bazfalse1?")
assert(outer.UnrolledCase.apply("baz", false, 1, '@').baz == "bazfalse1@")
assert((new outer.UnrolledCase("baz")).baz == "baztrue0?")
assert((new outer.UnrolledCase("baz", false)).baz == "bazfalse0?")
assert((new outer.UnrolledCase("baz", false, 1)).baz == "bazfalse1?")
assert((new outer.UnrolledCase("baz", false, 1, '@')).baz == "bazfalse1@")
assert(summon[Mirror.Of[outer.UnrolledCase]].fromProduct(Tuple("baz")).baz == "baztrue0?")
assert(summon[Mirror.Of[outer.UnrolledCase]].fromProduct(("baz", false)).baz == "bazfalse0?")
assert(summon[Mirror.Of[outer.UnrolledCase]].fromProduct(("baz", false, 1)).baz == "bazfalse1?")
assert(summon[Mirror.Of[outer.UnrolledCase]].fromProduct(("baz", false, 1, '@')).baz == "bazfalse1@")
assert(new outer.UnrolledSecondary("qux").qux == "quxtrue0?")
assert(new outer.UnrolledSecondary("qux", false).qux == "quxfalse0?")
assert(new outer.UnrolledSecondary("qux", false, 1).qux == "quxfalse1?")
assert(new outer.UnrolledSecondary("qux", false, 1, '@').qux == "quxfalse1@")

0 comments on commit aec0618

Please sign in to comment.