Skip to content

Commit 4fe49cc

Browse files
bishaboshabishabosha
committed
SIP 61 - add errors when unrolling apply, copy and fromProduct
Also standardise error messages as Declaration Errors
1 parent 1722b69 commit 4fe49cc

File tree

10 files changed

+112
-47
lines changed

10 files changed

+112
-47
lines changed

‎compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
216216
case FinalLocalDefID // errorNumber: 200
217217
case NonNamedArgumentInJavaAnnotationID // errorNumber: 201
218218
case QuotedTypeMissingID // errorNumber: 202
219+
case IllegalUnrollPlacementID // errorNumber: 203
219220

220221
def errorNumber = ordinal - 1
221222

‎compiler/src/dotty/tools/dotc/reporting/messages.scala‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3331,15 +3331,38 @@ final class QuotedTypeMissing(tpe: Type)(using Context) extends StagingMessage(Q
33313331

33323332
private def witness = defn.QuotedTypeClass.typeRef.appliedTo(tpe)
33333333

3334-
override protected def msg(using Context): String =
3334+
override protected def msg(using Context): String =
33353335
i"Reference to $tpe within quotes requires a given ${witness} in scope"
33363336

33373337
override protected def explain(using Context): String =
3338-
i"""Referencing `$tpe` inside a quoted expression requires a `${witness}` to be in scope.
3338+
i"""Referencing `$tpe` inside a quoted expression requires a `${witness}` to be in scope.
33393339
|Since Scala is subject to erasure at runtime, the type information will be missing during the execution of the code.
3340-
|`${witness}` is therefore needed to carry `$tpe`'s type information into the quoted code.
3341-
|Without an implicit `${witness}`, the type `$tpe` cannot be properly referenced within the expression.
3340+
|`${witness}` is therefore needed to carry `$tpe`'s type information into the quoted code.
3341+
|Without an implicit `${witness}`, the type `$tpe` cannot be properly referenced within the expression.
33423342
|To resolve this, ensure that a `${witness}` is available, either through a context-bound or explicitly.
33433343
|"""
33443344

33453345
end QuotedTypeMissing
3346+
3347+
class IllegalUnrollPlacement(origin: Option[Symbol])(using Context)
3348+
extends DeclarationMsg(IllegalUnrollPlacementID):
3349+
def msg(using Context) = origin match
3350+
case None => "@unroll is only allowed on a method parameter"
3351+
case Some(method) =>
3352+
val isCtor = method.isConstructor
3353+
def what = if isCtor then i"a ${if method.owner.is(Trait) then "trait" else "class"} constructor" else i"method ${method.name}"
3354+
val prefix = s"Can not unroll parameters of $what"
3355+
if method.is(Deferred) then
3356+
i"$prefix: it must not be abstract"
3357+
else if isCtor && method.owner.is(Trait) then
3358+
i"implementation restriction: $prefix"
3359+
else if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
3360+
i"$prefix: it is not final"
3361+
else if method.owner.companionClass.is(CaseClass) then
3362+
i"$prefix of a case class companion object: please annotate the class constructor instead"
3363+
else
3364+
assert(method.owner.is(CaseClass))
3365+
i"$prefix of a case class: please annotate the class constructor instead"
3366+
3367+
def explain(using Context) = ""
3368+
end IllegalUnrollPlacement

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

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
124124

125125
private var noCheckNews: Set[New] = Set()
126126

127-
def isValidUnrolledMethod(method: Symbol)(using Context): Boolean =
127+
def isValidUnrolledMethod(method: Symbol, origin: SrcPos)(using Context): Boolean =
128128
val seenMethods =
129129
val local = seenUnrolledMethods
130130
if local == null then
@@ -134,21 +134,23 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
134134
else
135135
local
136136
seenMethods.getOrElseUpdate(method, {
137-
if method.name.is(DefaultGetterName) then
137+
val isCtor = method.isConstructor
138+
if
139+
method.name.is(DefaultGetterName)
140+
then
138141
false // not an error, but not an expandable unrolled method
142+
else if
143+
method.is(Deferred)
144+
|| isCtor && method.owner.is(Trait)
145+
|| !(isCtor || method.is(Final) || method.owner.is(ModuleClass))
146+
|| method.owner.companionClass.is(CaseClass)
147+
&& (method.name == nme.apply || method.name == nme.fromProduct)
148+
|| method.owner.is(CaseClass) && method.name == nme.copy
149+
then
150+
report.error(IllegalUnrollPlacement(Some(method)), origin)
151+
false
139152
else
140-
var res = true
141-
if method.is(Deferred) then
142-
report.error("Unrolled method must be final and concrete", method.srcPos)
143-
res = false
144-
val isCtor = method.isConstructor
145-
if isCtor && method.owner.is(Trait) then
146-
report.error("implementation restriction: Unrolled method cannot be a trait constructor", method.srcPos)
147-
res = false
148-
if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
149-
report.error(s"Unrolled method ${method.name} must be final", method.srcPos)
150-
res = false
151-
res
153+
true
152154
})
153155

154156
def withNoCheckNews[T](ts: List[New])(op: => T): T = {
@@ -204,7 +206,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
204206
}
205207

206208
private def registerIfUnrolledParam(sym: Symbol)(using Context): Unit =
207-
if sym.hasAnnotation(defn.UnrollAnnot) && isValidUnrolledMethod(sym.owner) then
209+
if sym.hasAnnotation(defn.UnrollAnnot) && isValidUnrolledMethod(sym.owner, sym.sourcePos) then
208210
val cls = sym.enclosingClass
209211
val classes = ctx.compilationUnit.unrolledClasses
210212
val additions = Array(cls, cls.linkedClass).filter(_ != NoSymbol)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class CrossVersionChecks extends MiniPhase:
8282
}
8383

8484
private def unrollError(pos: SrcPos)(using Context): Unit =
85-
report.error("@unroll is only allowed on a method parameter", pos)
85+
report.error(IllegalUnrollPlacement(None), pos)
8686

8787
private def checkUnrollAnnot(annotSym: Symbol, pos: SrcPos)(using Context): Unit =
8888
if annotSym == defn.UnrollAnnot then

‎tests/neg/unroll-abstractMethod.check‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
-- Error: tests/neg/unroll-abstractMethod.scala:6:6 --------------------------------------------------------------------
1+
-- [E203] Declaration Error: tests/neg/unroll-abstractMethod.scala:6:41 ------------------------------------------------
22
6 | def foo(s: String, n: Int = 1, @unroll b: Boolean = true): String // error
3-
| ^
4-
| Unrolled method must be final and concrete
5-
-- Error: tests/neg/unroll-abstractMethod.scala:10:6 -------------------------------------------------------------------
3+
| ^
4+
| Can not unroll parameters of method foo: it must not be abstract
5+
-- [E203] Declaration Error: tests/neg/unroll-abstractMethod.scala:10:41 -----------------------------------------------
66
10 | def foo(s: String, n: Int = 1, @unroll b: Boolean = true): String // error
7-
| ^
8-
| Unrolled method must be final and concrete
7+
| ^
8+
| Can not unroll parameters of method foo: it must not be abstract

‎tests/neg/unroll-duped.check‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
-- [E203] Declaration Error: tests/neg/unroll-duped.scala:11:45 --------------------------------------------------------
2+
11 | final def copy(s: String = this.s, @unroll y: Boolean = this.y): UnrolledCase = // error
3+
| ^
4+
| Can not unroll parameters of method copy of a case class: please annotate the class constructor instead
5+
-- [E203] Declaration Error: tests/neg/unroll-duped.scala:18:12 --------------------------------------------------------
6+
18 | @unroll y: Boolean = true // error
7+
| ^
8+
|Can not unroll parameters of method apply of a case class companion object: please annotate the class constructor instead
9+
-- [E203] Declaration Error: tests/neg/unroll-duped.scala:22:26 --------------------------------------------------------
10+
22 | def fromProduct(@unroll p: Product = EmptyTuple): UnrolledCase = { // error
11+
| ^
12+
|Can not unroll parameters of method fromProduct of a case class companion object: please annotate the class constructor instead

‎tests/neg/unroll-duped.scala‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//> using options -experimental
2+
3+
import scala.annotation.unroll
4+
5+
case class UnrolledCase(
6+
s: String,
7+
y: Boolean = true,
8+
) {
9+
def foo: String = s + y
10+
11+
final def copy(s: String = this.s, @unroll y: Boolean = this.y): UnrolledCase = // error
12+
new UnrolledCase(s, y)
13+
}
14+
15+
object UnrolledCase {
16+
def apply(
17+
s: String,
18+
@unroll y: Boolean = true // error
19+
): UnrolledCase =
20+
new UnrolledCase(s, y)
21+
22+
def fromProduct(@unroll p: Product = EmptyTuple): UnrolledCase = { // error
23+
val s = p.productElement(0).asInstanceOf[String]
24+
val y = p.productElement(1).asInstanceOf[Boolean]
25+
UnrolledCase(s, y)
26+
}
27+
}

‎tests/neg/unroll-illegal.check‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,36 @@
1-
-- Error: tests/neg/unroll-illegal.scala:6:6 ---------------------------------------------------------------------------
1+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:6:6 --------------------------------------------------------
22
6 |class UnrollClass // error
33
| ^
44
| @unroll is only allowed on a method parameter
5-
-- Error: tests/neg/unroll-illegal.scala:9:6 ---------------------------------------------------------------------------
5+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:9:6 --------------------------------------------------------
66
9 |trait UnrollTrait // error
77
| ^
88
| @unroll is only allowed on a method parameter
9-
-- Error: tests/neg/unroll-illegal.scala:12:7 --------------------------------------------------------------------------
9+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:12:7 -------------------------------------------------------
1010
12 |object UnrollObject // error
1111
| ^
1212
| @unroll is only allowed on a method parameter
13-
-- Error: tests/neg/unroll-illegal.scala:18:5 --------------------------------------------------------------------------
13+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:18:5 -------------------------------------------------------
1414
18 |enum UnrollEnum { case X } // error
1515
| ^
1616
| @unroll is only allowed on a method parameter
17-
-- Error: tests/neg/unroll-illegal.scala:21:25 -------------------------------------------------------------------------
17+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:21:25 ------------------------------------------------------
1818
21 | val annotExpr: Int = 23: @unroll // error
1919
| ^
2020
| @unroll is only allowed on a method parameter
21-
-- Error: tests/neg/unroll-illegal.scala:22:19 -------------------------------------------------------------------------
21+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:22:19 ------------------------------------------------------
2222
22 | type annotType = Int @unroll // error
2323
| ^^^^^^^^^^^
2424
| @unroll is only allowed on a method parameter
25-
-- Error: tests/neg/unroll-illegal.scala:25:6 --------------------------------------------------------------------------
25+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:25:6 -------------------------------------------------------
2626
25 | val unrollVal: Int = 23 // error
2727
| ^
2828
| @unroll is only allowed on a method parameter
29-
-- Error: tests/neg/unroll-illegal.scala:28:6 --------------------------------------------------------------------------
29+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:28:6 -------------------------------------------------------
3030
28 | def unrollDef: Int = 23 // error
3131
| ^
3232
| @unroll is only allowed on a method parameter
33-
-- Error: tests/neg/unroll-illegal.scala:15:5 --------------------------------------------------------------------------
33+
-- [E203] Declaration Error: tests/neg/unroll-illegal.scala:15:5 -------------------------------------------------------
3434
15 |type UnrollType = Int // error
3535
| ^
3636
| @unroll is only allowed on a method parameter

‎tests/neg/unroll-illegal3.check‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
-- Error: tests/neg/unroll-illegal3.scala:7:8 --------------------------------------------------------------------------
1+
-- [E203] Declaration Error: tests/neg/unroll-illegal3.scala:7:31 ------------------------------------------------------
22
7 | def foo(s: String, @unroll y: Boolean) = s + y // error
3-
| ^
4-
| Unrolled method foo must be final
5-
-- Error: tests/neg/unroll-illegal3.scala:12:6 -------------------------------------------------------------------------
3+
| ^
4+
| Can not unroll parameters of method foo: it is not final
5+
-- [E203] Declaration Error: tests/neg/unroll-illegal3.scala:12:29 -----------------------------------------------------
66
12 | def foo(s: String, @unroll y: Boolean) = s + y // error
7-
| ^
8-
| Unrolled method foo must be final
9-
-- Error: tests/neg/unroll-illegal3.scala:16:6 -------------------------------------------------------------------------
7+
| ^
8+
| Can not unroll parameters of method foo: it is not final
9+
-- [E203] Declaration Error: tests/neg/unroll-illegal3.scala:16:29 -----------------------------------------------------
1010
16 | def foo(s: String, @unroll y: Boolean): String // error
11-
| ^
12-
| Unrolled method must be final and concrete
11+
| ^
12+
| Can not unroll parameters of method foo: it must not be abstract

‎tests/neg/unroll-traitConstructor.check‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Error: tests/neg/unroll-traitConstructor.scala:5:12 -----------------------------------------------------------------
1+
-- [E203] Declaration Error: tests/neg/unroll-traitConstructor.scala:5:32 ----------------------------------------------
22
5 |trait Unroll(a: String, @unroll b: Boolean = true): // error
3-
| ^
4-
| implementation restriction: Unrolled method cannot be a trait constructor
3+
| ^
4+
| implementation restriction: Can not unroll parameters of a trait constructor

0 commit comments

Comments
 (0)