Skip to content

Commit

Permalink
SIP 61 - add errors when unrolling apply, copy and fromProduct
Browse files Browse the repository at this point in the history
Also standardise error messages as Declaration Errors
  • Loading branch information
bishabosha committed Oct 6, 2024
1 parent b1c2ec0 commit a6b757c
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case TailrecNestedCallID //errorNumber: 199
case FinalLocalDefID // errorNumber: 200
case NonNamedArgumentInJavaAnnotationID // errorNumber: 201
case IllegalUnrollPlacementID // errorNumber: 202

def errorNumber = ordinal - 1

Expand Down
23 changes: 23 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3323,3 +3323,26 @@ class NonNamedArgumentInJavaAnnotation(using Context) extends SyntaxMsg(NonNamed
"""

end NonNamedArgumentInJavaAnnotation

class IllegalUnrollPlacement(origin: Option[Symbol])(using Context)
extends DeclarationMsg(IllegalUnrollPlacementID):
def msg(using Context) = origin match
case None => "@unroll is only allowed on a method parameter"
case Some(method) =>
val isCtor = method.isConstructor
def what = if isCtor then i"a ${if method.owner.is(Trait) then "trait" else "class"} constructor" else i"method ${method.name}"
val prefix = s"Can not unroll parameters of $what"
if method.is(Deferred) then
i"$prefix: it must not be abstract"
else if isCtor && method.owner.is(Trait) then
i"implementation restriction: $prefix"
else if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
i"$prefix: it is not final"
else if method.owner.companionClass.is(CaseClass) then
i"$prefix of a case class companion object: please annotate the class constructor instead"
else
assert(method.owner.is(CaseClass))
i"$prefix of a case class: please annotate the class constructor instead"

def explain(using Context) = ""
end IllegalUnrollPlacement
32 changes: 17 additions & 15 deletions compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>

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

def isValidUnrolledMethod(method: Symbol)(using Context): Boolean =
def isValidUnrolledMethod(method: Symbol, origin: SrcPos)(using Context): Boolean =
val seenMethods =
val local = seenUnrolledMethods
if local == null then
Expand All @@ -134,21 +134,23 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
else
local
seenMethods.getOrElseUpdate(method, {
if method.name.is(DefaultGetterName) then
val isCtor = method.isConstructor
if
method.name.is(DefaultGetterName)
then
false // not an error, but not an expandable unrolled method
else if
method.is(Deferred)
|| isCtor && method.owner.is(Trait)
|| !(isCtor || method.is(Final) || method.owner.is(ModuleClass))
|| method.owner.companionClass.is(CaseClass)
&& (method.name == nme.apply || method.name == nme.fromProduct)
|| method.owner.is(CaseClass) && method.name == nme.copy
then
report.error(IllegalUnrollPlacement(Some(method)), origin)
false
else
var res = true
if method.is(Deferred) then
report.error("Unrolled method must be final and concrete", method.srcPos)
res = false
val isCtor = method.isConstructor
if isCtor && method.owner.is(Trait) then
report.error("implementation restriction: Unrolled method cannot be a trait constructor", method.srcPos)
res = false
if !(isCtor || method.is(Final) || method.owner.is(ModuleClass)) then
report.error(s"Unrolled method ${method.name} must be final", method.srcPos)
res = false
res
true
})

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

private def registerIfUnrolledParam(sym: Symbol)(using Context): Unit =
if sym.hasAnnotation(defn.UnrollAnnot) && isValidUnrolledMethod(sym.owner) then
if sym.hasAnnotation(defn.UnrollAnnot) && isValidUnrolledMethod(sym.owner, sym.sourcePos) then
val cls = sym.enclosingClass
val classes = ctx.compilationUnit.unrolledClasses
val additions = Array(cls, cls.linkedClass).filter(_ != NoSymbol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class CrossVersionChecks extends MiniPhase:
}

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

private def checkUnrollAnnot(annotSym: Symbol, pos: SrcPos)(using Context): Unit =
if annotSym == defn.UnrollAnnot then
Expand Down
12 changes: 6 additions & 6 deletions tests/neg/unroll-abstractMethod.check
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
-- Error: tests/neg/unroll-abstractMethod.scala:6:6 --------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-abstractMethod.scala:6:41 ------------------------------------------------
6 | def foo(s: String, n: Int = 1, @unroll b: Boolean = true): String // error
| ^
| Unrolled method must be final and concrete
-- Error: tests/neg/unroll-abstractMethod.scala:10:6 -------------------------------------------------------------------
| ^
| Can not unroll parameters of method foo: it must not be abstract
-- [E202] Declaration Error: tests/neg/unroll-abstractMethod.scala:10:41 -----------------------------------------------
10 | def foo(s: String, n: Int = 1, @unroll b: Boolean = true): String // error
| ^
| Unrolled method must be final and concrete
| ^
| Can not unroll parameters of method foo: it must not be abstract
12 changes: 12 additions & 0 deletions tests/neg/unroll-duped.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E202] Declaration Error: tests/neg/unroll-duped.scala:11:45 --------------------------------------------------------
11 | final def copy(s: String = this.s, @unroll y: Boolean = this.y): UnrolledCase = // error
| ^
| Can not unroll parameters of method copy of a case class: please annotate the class constructor instead
-- [E202] Declaration Error: tests/neg/unroll-duped.scala:18:12 --------------------------------------------------------
18 | @unroll y: Boolean = true // error
| ^
|Can not unroll parameters of method apply of a case class companion object: please annotate the class constructor instead
-- [E202] Declaration Error: tests/neg/unroll-duped.scala:22:26 --------------------------------------------------------
22 | def fromProduct(@unroll p: Product = EmptyTuple): UnrolledCase = { // error
| ^
|Can not unroll parameters of method fromProduct of a case class companion object: please annotate the class constructor instead
27 changes: 27 additions & 0 deletions tests/neg/unroll-duped.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//> using options -experimental

import scala.annotation.unroll

case class UnrolledCase(
s: String,
y: Boolean = true,
) {
def foo: String = s + y

final def copy(s: String = this.s, @unroll y: Boolean = this.y): UnrolledCase = // error
new UnrolledCase(s, y)
}

object UnrolledCase {
def apply(
s: String,
@unroll y: Boolean = true // error
): UnrolledCase =
new UnrolledCase(s, y)

def fromProduct(@unroll p: Product = EmptyTuple): UnrolledCase = { // error
val s = p.productElement(0).asInstanceOf[String]
val y = p.productElement(1).asInstanceOf[Boolean]
UnrolledCase(s, y)
}
}
18 changes: 9 additions & 9 deletions tests/neg/unroll-illegal.check
Original file line number Diff line number Diff line change
@@ -1,36 +1,36 @@
-- Error: tests/neg/unroll-illegal.scala:6:6 ---------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:6:6 --------------------------------------------------------
6 |class UnrollClass // error
| ^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:9:6 ---------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:9:6 --------------------------------------------------------
9 |trait UnrollTrait // error
| ^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:12:7 --------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:12:7 -------------------------------------------------------
12 |object UnrollObject // error
| ^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:18:5 --------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:18:5 -------------------------------------------------------
18 |enum UnrollEnum { case X } // error
| ^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:21:25 -------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:21:25 ------------------------------------------------------
21 | val annotExpr: Int = 23: @unroll // error
| ^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:22:19 -------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:22:19 ------------------------------------------------------
22 | type annotType = Int @unroll // error
| ^^^^^^^^^^^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:25:6 --------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:25:6 -------------------------------------------------------
25 | val unrollVal: Int = 23 // error
| ^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:28:6 --------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:28:6 -------------------------------------------------------
28 | def unrollDef: Int = 23 // error
| ^
| @unroll is only allowed on a method parameter
-- Error: tests/neg/unroll-illegal.scala:15:5 --------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal.scala:15:5 -------------------------------------------------------
15 |type UnrollType = Int // error
| ^
| @unroll is only allowed on a method parameter
18 changes: 9 additions & 9 deletions tests/neg/unroll-illegal3.check
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
-- Error: tests/neg/unroll-illegal3.scala:7:8 --------------------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-illegal3.scala:7:31 ------------------------------------------------------
7 | def foo(s: String, @unroll y: Boolean) = s + y // error
| ^
| Unrolled method foo must be final
-- Error: tests/neg/unroll-illegal3.scala:12:6 -------------------------------------------------------------------------
| ^
| Can not unroll parameters of method foo: it is not final
-- [E202] Declaration Error: tests/neg/unroll-illegal3.scala:12:29 -----------------------------------------------------
12 | def foo(s: String, @unroll y: Boolean) = s + y // error
| ^
| Unrolled method foo must be final
-- Error: tests/neg/unroll-illegal3.scala:16:6 -------------------------------------------------------------------------
| ^
| Can not unroll parameters of method foo: it is not final
-- [E202] Declaration Error: tests/neg/unroll-illegal3.scala:16:29 -----------------------------------------------------
16 | def foo(s: String, @unroll y: Boolean): String // error
| ^
| Unrolled method must be final and concrete
| ^
| Can not unroll parameters of method foo: it must not be abstract
6 changes: 3 additions & 3 deletions tests/neg/unroll-traitConstructor.check
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-- Error: tests/neg/unroll-traitConstructor.scala:5:12 -----------------------------------------------------------------
-- [E202] Declaration Error: tests/neg/unroll-traitConstructor.scala:5:32 ----------------------------------------------
5 |trait Unroll(a: String, @unroll b: Boolean = true): // error
| ^
| implementation restriction: Unrolled method cannot be a trait constructor
| ^
| implementation restriction: Can not unroll parameters of a trait constructor

0 comments on commit a6b757c

Please sign in to comment.