Skip to content

Commit c5d05d7

Browse files
bishaboshabishabosha
committed
address review: part 2
1 parent 7b1970c commit c5d05d7

File tree

3 files changed

+30
-33
lines changed

3 files changed

+30
-33
lines changed

‎compiler/src/dotty/tools/dotc/transform/UnrollDefinitions.scala‎

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,14 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
6161
def computeIndices(annotated: Symbol)(using Context): ComputedIndices =
6262
unrolledDefs.getOrElseUpdate(annotated, {
6363
if annotated.name.is(DefaultGetterName) then
64-
Nil // happens in curried methods where more than one parameter list has @unroll
64+
// happens in curried methods, where default argument occurs in parameter list
65+
// after the unrolled parameter list.
66+
// example:
67+
// `final def foo(@unroll y: String = "")(x: Int = 23) = x`
68+
// yields:
69+
// `def foo$default$2(@unroll y: String): Int @uncheckedVariance = 23`
70+
// Perhaps annotations should be preprocessed before they are copied?
71+
Nil
6572
else
6673
val indices = annotated
6774
.paramSymss
@@ -114,15 +121,13 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
114121
* @param paramIndex index of the unrolled parameter (in the parameter list) that we stop at
115122
* @param paramCount number of parameters in the annotated parameter list
116123
* @param nextParamIndex index of next unrolled parameter - to fetch default argument
117-
* @param nextSpan span of next forwarder - used to ensure the span is not identical by shifting (TODO remove)
118124
* @param annotatedParamListIndex index of the parameter list that contains unrolled parameters
119125
* @param isCaseApply if `defdef` is a case class apply/constructor - used for selection of default arguments
120126
*/
121127
private def generateSingleForwarder(defdef: DefDef,
122128
paramIndex: Int,
123129
paramCount: Int,
124130
nextParamIndex: Int,
125-
nextSpan: Span,
126131
annotatedParamListIndex: Int,
127132
isCaseApply: Boolean)(using Context): DefDef = {
128133

@@ -133,7 +138,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
133138
defdef.symbol.flags &~ HasDefaultParams |
134139
Invisible | Synthetic,
135140
NoType, // fill in later
136-
coord = nextSpan.shift(1) // shift by 1 to avoid "secondary constructor must call preceding" error
141+
coord = defdef.span
137142
).entered
138143

139144
val newParamSymMappings = extractParamSymss(copyParamSym(_, forwarderDefSymbol0))
@@ -196,9 +201,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
196201
newParamSymLists
197202
.take(annotatedParamListIndex)
198203
.map(_.map(ref))
199-
.foldLeft(inner): (lhs, newParams) =>
200-
if (newParams.headOption.exists(_.isInstanceOf[TypeTree])) TypeApply(lhs, newParams)
201-
else Apply(lhs, newParams)
204+
.foldLeft(inner)(_.appliedToArgs(_))
202205
)
203206

204207
val forwarderInner: Tree =
@@ -210,11 +213,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
210213
else ps.map(ref)
211214
}
212215

213-
val forwarderCall0 = forwarderCallArgs.foldLeft[Tree](forwarderInner){
214-
case (lhs: Tree, newParams) =>
215-
if (newParams.headOption.exists(_.isInstanceOf[TypeTree])) TypeApply(lhs, newParams)
216-
else Apply(lhs, newParams)
217-
}
216+
val forwarderCall0 = forwarderCallArgs.foldLeft(forwarderInner)(_.appliedToArgs(_))
218217

219218
val forwarderCall =
220219
if (!defdef.symbol.isConstructor) forwarderCall0
@@ -224,9 +223,9 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
224223
}
225224

226225
val forwarderDef =
227-
tpd.DefDef(forwarderDefSymbol, rhs = forwarderRhs())
226+
tpd.DefDef(forwarderDefSymbol, rhs = forwarderRhs()).withSpan(defdef.span)
228227

229-
forwarderDef.withSpan(nextSpan.shift(1))
228+
forwarderDef
230229
}
231230

232231
private def generateFromProduct(startParamIndices: List[Int], paramCount: Int, defdef: DefDef)(using Context) = {
@@ -261,10 +260,10 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
261260

262261
private enum Gen:
263262
case Substitute(origin: Symbol, newDef: DefDef)
264-
case Forwarders(origin: Symbol, forwarders: Seq[DefDef])
263+
case Forwarders(origin: Symbol, forwarders: List[DefDef])
265264

266265
def origin: Symbol
267-
def extras: Seq[DefDef] = this match
266+
def extras: List[DefDef] = this match
268267
case Substitute(_, d) => d :: Nil
269268
case Forwarders(_, ds) => ds
270269

@@ -288,28 +287,26 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
288287

289288
compute(annotated) match {
290289
case Nil => None
291-
case Seq((paramClauseIndex, annotationIndices)) =>
290+
case (paramClauseIndex, annotationIndices) :: Nil =>
292291
val paramCount = annotated.paramSymss(paramClauseIndex).size
293292
if isCaseFromProduct then
294293
Some(Gen.Substitute(
295294
origin = defdef.symbol,
296295
newDef = generateFromProduct(annotationIndices, paramCount, defdef)
297296
))
298297
else
299-
val (generatedDefs, _) =
298+
val generatedDefs =
300299
val indices = (annotationIndices :+ paramCount).sliding(2).toList.reverse
301-
indices.foldLeft((Seq.empty[DefDef], defdef.symbol.span)):
302-
case ((defdefs, nextSpan), Seq(paramIndex, nextParamIndex)) =>
303-
val forwarder = generateSingleForwarder(
300+
indices.foldLeft(List.empty[DefDef]):
301+
case (defdefs, paramIndex :: nextParamIndex :: Nil) =>
302+
generateSingleForwarder(
304303
defdef,
305304
paramIndex,
306305
paramCount,
307306
nextParamIndex,
308-
nextSpan,
309307
paramClauseIndex,
310308
isCaseApply
311-
)
312-
(forwarder +: defdefs, forwarder.symbol.span)
309+
) :: defdefs
313310
case _ => unreachable("sliding with at least 2 elements")
314311
Some(Gen.Forwarders(origin = defdef.symbol, forwarders = generatedDefs))
315312

@@ -329,11 +326,6 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
329326
val bodySubs = generatedBody.collect({ case s: Gen.Substitute => s.origin }).toSet
330327
val otherDecls = tmpl.body.filterNot(d => d.symbol.exists && bodySubs(d.symbol))
331328

332-
/** inlined from compiler/src/dotty/tools/dotc/typer/Checking.scala */
333-
def checkClash(decl: Symbol, other: Symbol) =
334-
def staticNonStaticPair = decl.isScalaStatic != other.isScalaStatic
335-
decl.matches(other) && !staticNonStaticPair
336-
337329
if allGenerated.nonEmpty then
338330
val byName = (tmpl.constr :: otherDecls).groupMap(_.symbol.name.toString)(_.symbol)
339331
for
@@ -342,7 +334,7 @@ class UnrollDefinitions extends MacroTransform, IdentityDenotTransformer {
342334
do
343335
val replaced = dcl.symbol
344336
byName.get(dcl.name.toString).foreach { syms =>
345-
val clashes = syms.filter(checkClash(replaced, _))
337+
val clashes = syms.filter(ctx.typer.matchesSameStatic(replaced, _))
346338
for existing <- clashes do
347339
val src = syntheticDefs.origin
348340
report.error(i"""Unrolled $replaced clashes with existing declaration.

‎compiler/src/dotty/tools/dotc/typer/Checking.scala‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,6 +1226,10 @@ trait Checking {
12261226
/** A hook to exclude selected symbols from double declaration check */
12271227
def excludeFromDoubleDeclCheck(sym: Symbol)(using Context): Boolean = false
12281228

1229+
def matchesSameStatic(decl: Symbol, other: Symbol)(using Context): Boolean =
1230+
def staticNonStaticPair = decl.isScalaStatic != other.isScalaStatic
1231+
decl.matches(other) && !staticNonStaticPair
1232+
12291233
/** Check that class does not declare same symbol twice */
12301234
def checkNoDoubleDeclaration(cls: Symbol)(using Context): Unit = {
12311235
val seen = new mutable.HashMap[Name, List[Symbol]].withDefaultValue(Nil)
@@ -1237,8 +1241,7 @@ trait Checking {
12371241
def javaFieldMethodPair =
12381242
decl.is(JavaDefined) && other.is(JavaDefined) &&
12391243
decl.is(Method) != other.is(Method)
1240-
def staticNonStaticPair = decl.isScalaStatic != other.isScalaStatic
1241-
if (decl.matches(other) && !javaFieldMethodPair && !staticNonStaticPair) {
1244+
if (matchesSameStatic(decl, other) && !javaFieldMethodPair) {
12421245
def doubleDefError(decl: Symbol, other: Symbol): Unit =
12431246
if (!decl.info.isErroneous && !other.info.isErroneous)
12441247
report.error(DoubleDefinition(decl, other, cls), decl.srcPos)

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2950,7 +2950,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
29502950

29512951
def checkThisConstrCall(tree: Tree): Unit = tree match
29522952
case app: Apply if untpd.isSelfConstrCall(app) =>
2953-
if (sym.span.exists && app.symbol.span.exists && sym.span.start <= app.symbol.span.start)
2953+
if !sym.is(Synthetic)
2954+
&& sym.span.exists && app.symbol.span.exists && sym.span.start <= app.symbol.span.start
2955+
then
29542956
report.error("secondary constructor must call a preceding constructor", app.srcPos)
29552957
case Block(call :: _, _) => checkThisConstrCall(call)
29562958
case _ =>

0 commit comments

Comments
 (0)