Skip to content

Commit 6d1fffa

Browse files
bishaboshabishabosha
committed
SIP 61 - add errors when unrolling apply, copy and fromProduct
Also standardise error messages as Declaration Errors
1 parent aec0618 commit 6d1fffa

10 files changed

+111
-46
lines changed

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

+1
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
215215
case TailrecNestedCallID //errorNumber: 199
216216
case FinalLocalDefID // errorNumber: 200
217217
case NonNamedArgumentInJavaAnnotationID // errorNumber: 201
218+
case IllegalUnrollPlacementID // errorNumber: 202
218219

219220
def errorNumber = ordinal - 1
220221

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

+26-3
Original file line numberDiff line numberDiff line change
@@ -1829,12 +1829,12 @@ class NotAPath(tp: Type, usage: String)(using Context) extends TypeMsg(NotAPathI
18291829
if sym.isAllOf(Flags.InlineParam) then
18301830
i"""
18311831
|Inline parameters are not considered immutable paths and cannot be used as
1832-
|singleton types.
1833-
|
1832+
|singleton types.
1833+
|
18341834
|Hint: Removing the `inline` qualifier from the `${sym.name}` parameter
18351835
|may help resolve this issue."""
18361836
else ""
1837-
1837+
18381838

18391839
class WrongNumberOfParameters(tree: untpd.Tree, foundCount: Int, pt: Type, expectedCount: Int)(using Context)
18401840
extends SyntaxMsg(WrongNumberOfParametersID) {
@@ -3324,3 +3324,26 @@ class NonNamedArgumentInJavaAnnotation(using Context) extends SyntaxMsg(NonNamed
33243324
"""
33253325

33263326
end NonNamedArgumentInJavaAnnotation
3327+
3328+
class IllegalUnrollPlacement(origin: Option[Symbol])(using Context)
3329+
extends DeclarationMsg(IllegalUnrollPlacementID):
3330+
def msg(using Context) = origin match
3331+
case None => "@unroll is only allowed on a method parameter"
3332+
case Some(method) =>
3333+
val isCtor = method.isConstructor
3334+
def what = if isCtor then i"a ${if method.owner.is(Trait) then "trait" else "class"} constructor" else i"method ${method.name}"
3335+
val prefix = s"Can not unroll parameters of $what"
3336+
if method.is(Deferred) then
3337+
i"$prefix: it must not be abstract"
3338+
else if isCtor && method.owner.is(Trait) then
3339+
i"implementation restriction: $prefix"
3340+
else if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
3341+
i"$prefix: it is not final"
3342+
else if method.owner.companionClass.is(CaseClass) then
3343+
i"$prefix of a case class companion object: please annotate the class constructor instead"
3344+
else
3345+
assert(method.owner.is(CaseClass))
3346+
i"$prefix of a case class: please annotate the class constructor instead"
3347+
3348+
def explain(using Context) = ""
3349+
end IllegalUnrollPlacement

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

+17-15
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

+1-1
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

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
-- Error: tests/neg/unroll-abstractMethod.scala:6:6 --------------------------------------------------------------------
1+
-- [E202] 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+
-- [E202] 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

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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

+27
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

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,36 @@
1-
-- Error: tests/neg/unroll-illegal.scala:6:6 ---------------------------------------------------------------------------
1+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
-- Error: tests/neg/unroll-illegal3.scala:7:8 --------------------------------------------------------------------------
1+
-- [E202] 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+
-- [E202] 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+
-- [E202] 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

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Error: tests/neg/unroll-traitConstructor.scala:5:12 -----------------------------------------------------------------
1+
-- [E202] 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)