Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine handling of CanThrow capabilities #13866

Merged
merged 4 commits into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import NameKinds.DefaultGetterName
import NameOps._
import SymDenotations.{NoCompleter, NoDenotation}
import Applications.unapplyArgs
import Inferencing.isFullyDefined
import transform.patmat.SpaceEngine.isIrrefutable
import config.Feature
import config.Feature.sourceVersion
Expand Down Expand Up @@ -1362,6 +1363,21 @@ trait Checking {
def checkCanThrow(tp: Type, span: Span)(using Context): Unit =
if Feature.enabled(Feature.saferExceptions) && tp.isCheckedException then
ctx.typer.implicitArgTree(defn.CanThrowClass.typeRef.appliedTo(tp), span)

/** Check that catch can generate a good CanThrow exception */
def checkCatch(pat: Tree, guard: Tree)(using Context): Unit = pat match
case Typed(_: Ident, tpt) if isFullyDefined(tpt.tpe, ForceDegree.none) && guard.isEmpty =>
// OK
case Bind(_, pat1) =>
checkCatch(pat1, guard)
case _ =>
val req =
if guard.isEmpty then "for cases of the form `ex: T` where `T` is fully defined"
else "if no pattern guard is given"
report.error(
em"""Implementation restriction: cannot generate CanThrow capability for this kind of catch.
|CanThrow capabilities can only be generated $req.""",
pat.srcPos)
}

trait ReChecking extends Checking {
Expand All @@ -1375,6 +1391,7 @@ trait ReChecking extends Checking {
override def checkMatchable(tp: Type, pos: SrcPos, pattern: Boolean)(using Context): Unit = ()
override def checkNoModuleClash(sym: Symbol)(using Context) = ()
override def checkCanThrow(tp: Type, span: Span)(using Context): Unit = ()
override def checkCatch(pat: Tree, guard: Tree)(using Context): Unit = ()
}

trait NoChecking extends ReChecking {
Expand Down
19 changes: 14 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1745,9 +1745,12 @@ class Typer extends Namer
.withSpan(expr.span)
val caps =
for
CaseDef(pat, _, _) <- cases
case CaseDef(pat, guard, _) <- cases
if Feature.enabled(Feature.saferExceptions) && pat.tpe.widen.isCheckedException
yield makeCanThrow(pat.tpe.widen)
yield
checkCatch(pat, guard)
makeCanThrow(pat.tpe.widen)

caps.foldLeft(expr)((e, g) => untpd.Block(g :: Nil, e))

def typedTry(tree: untpd.Try, pt: Type)(using Context): Try = {
Expand Down Expand Up @@ -1936,14 +1939,20 @@ class Typer extends Namer
}
var checkedArgs = preCheckKinds(args1, paramBounds)
// check that arguments conform to bounds is done in phase PostTyper
if (tpt1.symbol == defn.andType)
val tycon = tpt1.symbol
if (tycon == defn.andType)
checkedArgs = checkedArgs.mapconserve(arg =>
checkSimpleKinded(checkNoWildcard(arg)))
else if (tpt1.symbol == defn.orType)
else if (tycon == defn.orType)
checkedArgs = checkedArgs.mapconserve(arg =>
checkSimpleKinded(checkNoWildcard(arg)))
else if tycon == defn.throwsAlias
&& checkedArgs.length == 2
&& checkedArgs(1).tpe.derivesFrom(defn.RuntimeExceptionClass)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there really a gain in explicitly preventing users from declaring throwing RuntimeExceptions? And even if we disallow methods like

def foo(): Int throws ArithmeticException = ???

then

def foo()(using CanThrow[ArithmeticException]): Int = ???

will be still valid, right? So this somehow defeats the purpose IMHO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't generate a capability for it, so it's better to communicate that early. But I think ding this for throws clauses is enough. I don't want to start imposing restrictions on general implicit parameters.

then
report.error(em"throws clause cannot be defined for RuntimeException", checkedArgs(1).srcPos)
else if (ctx.isJava)
if (tpt1.symbol eq defn.ArrayClass) then
if tycon eq defn.ArrayClass then
checkedArgs match {
case List(arg) =>
val elemtp = arg.tpe.translateJavaArrayElementType
Expand Down
17 changes: 17 additions & 0 deletions tests/neg/i13846.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-- Error: tests/neg/i13846.scala:3:22 ----------------------------------------------------------------------------------
3 |def foo(): Int throws ArithmeticException = 1 / 0 // error
| ^^^^^^^^^^^^^^^^^^^
| throws clause cannot be defined for RuntimeException
-- Error: tests/neg/i13846.scala:7:9 -----------------------------------------------------------------------------------
7 | foo() // error
| ^
| The capability to throw exception ArithmeticException is missing.
| The capability can be provided by one of the following:
| - A using clause `(using CanThrow[ArithmeticException])`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't synthesize instances of CanThrow for subtypes of RuntimeException then this message is rather misleading than helpful, as propagating this capability with using clauses won't make the compilation succeed unless someone creates such an instance by themself. And X throws ArithmeticException won't even compile

| - A `throws` clause in a result type such as `X throws ArithmeticException`
| - an enclosing `try` that catches ArithmeticException
|
| The following import might fix the problem:
|
| import unsafeExceptions.canThrowAny
|
9 changes: 9 additions & 0 deletions tests/neg/i13846.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import language.experimental.saferExceptions

def foo(): Int throws ArithmeticException = 1 / 0 // error

def test(): Unit =
try
foo() // error
catch
case _: ArithmeticException => println("Caught")
5 changes: 5 additions & 0 deletions tests/neg/i13849.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Error: tests/neg/i13849.scala:16:11 ---------------------------------------------------------------------------------
16 | case _: Ex if false => println("Caught") // error
| ^^^^^
| Implementation restriction: cannot generate CanThrow capability for this kind of catch.
| CanThrow capabilities can only be generated if no pattern guard is given.
16 changes: 16 additions & 0 deletions tests/neg/i13849.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import annotation.experimental
import language.experimental.saferExceptions

@experimental
case class Ex(i: Int) extends Exception(s"Exception: $i")

@experimental
def foo(): Unit throws Ex = throw Ex(1)

@experimental
object Main:
def main(args: Array[String]): Unit =
try
foo()
catch
case _: Ex if false => println("Caught") // error
18 changes: 18 additions & 0 deletions tests/neg/i13864.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- Error: tests/neg/i13864.scala:11:9 ----------------------------------------------------------------------------------
11 | case Ex(i: Int) => println("Caught an Int") // error
| ^^^^^^^^^^
| Implementation restriction: cannot generate CanThrow capability for this kind of catch.
| CanThrow capabilities can only be generated for cases of the form `ex: T` where `T` is fully defined.
-- Error: tests/neg/i13864.scala:9:10 ----------------------------------------------------------------------------------
9 | foo(1) // error
| ^
| The capability to throw exception Ex[Int] is missing.
| The capability can be provided by one of the following:
| - A using clause `(using CanThrow[Ex[Int]])`
| - A `throws` clause in a result type such as `X throws Ex[Int]`
| - an enclosing `try` that catches Ex[Int]
|
| The following import might fix the problem:
|
| import unsafeExceptions.canThrowAny
|
11 changes: 11 additions & 0 deletions tests/neg/i13864.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import language.experimental.saferExceptions

case class Ex[A](a: A) extends Exception(s"Ex: $a")

def foo[A](a: A): Unit throws Ex[A] = throw new Ex(a)

def test(): Unit =
try
foo(1) // error
catch
case Ex(i: Int) => println("Caught an Int") // error