Skip to content

Fix #10614: align unchecked pattern definition syntax with Scala 2 #10793

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

Merged
merged 7 commits into from
Dec 15, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 13 additions & 7 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1006,15 +1006,21 @@ object desugar {
* - None : sel @unchecked
* - Exhaustive : sel
* - IrrefutablePatDef,
* IrrefutableGenFrom: sel @unchecked with attachment `CheckIrrefutable -> checkMode`
* IrrefutableGenFrom: sel with attachment `CheckIrrefutable -> checkMode`
*/
def makeSelector(sel: Tree, checkMode: MatchCheck)(using Context): Tree =
if (checkMode == MatchCheck.Exhaustive) sel
else {
val sel1 = Annotated(sel, New(ref(defn.UncheckedAnnot.typeRef)))
if (checkMode != MatchCheck.None) sel1.pushAttachment(CheckIrrefutable, checkMode)
sel1
}
checkMode match
case MatchCheck.None =>
Annotated(sel, New(ref(defn.UncheckedAnnot.typeRef)))

case MatchCheck.Exhaustive =>
sel

case MatchCheck.IrrefutablePatDef | MatchCheck.IrrefutableGenFrom =>
// TODO: use `pushAttachment` and investigate duplicate attachment
sel.withAttachment(CheckIrrefutable, checkMode)
sel
end match

/** If `pat` is a variable pattern,
*
Expand Down
18 changes: 9 additions & 9 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3184,7 +3184,7 @@ object Parsers {
}

/** PatDef ::= ids [‘:’ Type] ‘=’ Expr
* | Pattern2 [‘:’ Type | Ascription] ‘=’ Expr
* | Pattern2 [‘:’ Type] ‘=’ Expr
* VarDef ::= PatDef | id {`,' id} `:' Type `=' `_'
* ValDcl ::= id {`,' id} `:' Type
* VarDcl ::= id {`,' id} `:' Type
Expand All @@ -3202,11 +3202,7 @@ object Parsers {
val tpt =
if (in.token == COLON) {
in.nextToken()
if (in.token == AT && lhs.tail.isEmpty) {
lhs = ascription(first, Location.ElseWhere) :: Nil
emptyType
}
else toplevelTyp()
toplevelTyp()
}
else emptyType
val rhs =
Expand All @@ -3229,9 +3225,13 @@ object Parsers {
case IdPattern(id, t) => t.isEmpty
case _ => false
}
if rhs.isEmpty && !isAllIds then
syntaxError(ExpectedTokenButFound(EQUALS, in.token), Span(in.lastOffset))
PatDef(mods, lhs, tpt, rhs)
val rhs2 =
if rhs.isEmpty && !isAllIds then
syntaxError(ExpectedTokenButFound(EQUALS, in.token), Span(in.lastOffset))
errorTermTree
else
rhs
PatDef(mods, lhs, tpt, rhs2)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ object ContextFunctionResults:
def contextResultCount(sym: Symbol)(using Context): Int =
sym.getAnnotation(defn.ContextResultCountAnnot) match
case Some(annot) =>
val ast.Trees.Literal(Constant(crCount: Int)) :: Nil: @unchecked = annot.arguments
val ast.Trees.Literal(Constant(crCount: Int)) :: Nil = annot.arguments: @unchecked
crCount
case none => 0

Expand All @@ -73,7 +73,7 @@ object ContextFunctionResults:
def contextParamCount(tp: Type, crCount: Int): Int =
if crCount == 0 then 0
else
val defn.ContextFunctionType(params, resTpe, isErased): @unchecked = tp
val defn.ContextFunctionType(params, resTpe, isErased) = tp: @unchecked
val rest = contextParamCount(resTpe, crCount - 1)
if isErased then rest else params.length + rest

Expand Down Expand Up @@ -106,7 +106,7 @@ object ContextFunctionResults:
def missingCR(tp: Type, crCount: Int): (Type, Int) =
if crCount == 0 then (tp, 0)
else
val defn.ContextFunctionType(formals, resTpe, isErased): @unchecked = tp
val defn.ContextFunctionType(formals, resTpe, isErased) = tp: @unchecked
val result @ (rt, nparams) = missingCR(resTpe, crCount - 1)
assert(nparams <= paramCount)
if nparams == paramCount || isErased then result
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -490,15 +490,15 @@ object Erasure {
def abstracted(args: List[Tree], tp: Type, pt: Type)(using Context): Tree =
if args.length < targetLength then
try
val defn.ContextFunctionType(argTpes, resTpe, isErased): @unchecked = tp
val defn.ContextFunctionType(argTpes, resTpe, isErased) = tp: @unchecked
if isErased then abstracted(args, resTpe, pt)
else
val anonFun = newSymbol(
ctx.owner, nme.ANON_FUN, Flags.Synthetic | Flags.Method,
MethodType(argTpes, resTpe), coord = tree.span.endPos)
anonFun.info = transformInfo(anonFun, anonFun.info)
def lambdaBody(refss: List[List[Tree]]) =
val refs :: Nil : @unchecked = refss
val refs :: Nil = refss: @unchecked
val expandedRefs = refs.map(_.withSpan(tree.span.endPos)) match
case (bunchedParam @ Ident(nme.ALLARGS)) :: Nil =>
argTpes.indices.toList.map(n =>
Expand Down
18 changes: 10 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -671,30 +671,32 @@ trait Checking {
report.error(ex"$cls cannot be instantiated since it${rstatus.msg}", pos)
}

/** Check that pattern `pat` is irrefutable for scrutinee tye `pt`.
* This means `pat` is either marked @unchecked or `pt` conforms to the
/** Check that pattern `pat` is irrefutable for scrutinee type `sel.tpe`.
* This means `sel` is either marked @unchecked or `sel.tpe` conforms to the
* pattern's type. If pattern is an UnApply, do the check recursively.
*/
def checkIrrefutable(pat: Tree, pt: Type, isPatDef: Boolean)(using Context): Boolean = {
def checkIrrefutable(sel: Tree, pat: Tree, isPatDef: Boolean)(using Context): Boolean = {
val pt = sel.tpe

def fail(pat: Tree, pt: Type): Boolean = {
var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
if (!pat.tpe.isSingleton) reportedPt = reportedPt.widen
val problem = if (pat.tpe <:< reportedPt) "is more specialized than" else "does not match"
val fix = if (isPatDef) "`: @unchecked` after" else "`case ` before"
report.errorOrMigrationWarning(
val fix = if (isPatDef) "adding `: @unchecked` after the expression" else "writing `case ` before the full pattern"
val pos = if (isPatDef) sel.srcPos else pat.srcPos
report.warning(
ex"""pattern's type ${pat.tpe} $problem the right hand side expression's type $reportedPt
|
|If the narrowing is intentional, this can be communicated by writing $fix the full pattern.${err.rewriteNotice}""",
pat.srcPos)
|If the narrowing is intentional, this can be communicated by $fix.${err.rewriteNotice}""",
pos)
false
}

def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt)

def recur(pat: Tree, pt: Type): Boolean =
!sourceVersion.isAtLeast(`3.1`) || // only for 3.1 for now since mitigations work only after this PR
pat.tpe.widen.hasAnnotation(defn.UncheckedAnnot) || {
pt.hasAnnotation(defn.UncheckedAnnot) || {
patmatch.println(i"check irrefutable $pat: ${pat.tpe} against $pt")
pat match {
case Bind(_, pat1) =>
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 @@ -1386,18 +1386,27 @@ class Typer extends Namer
}

result match {
case Match(sel, CaseDef(pat, _, _) :: _) =>
case result @ Match(sel, CaseDef(pat, _, _) :: _) =>
tree.selector.removeAttachment(desugar.CheckIrrefutable) match {
case Some(checkMode) =>
val isPatDef = checkMode == desugar.MatchCheck.IrrefutablePatDef
if (!checkIrrefutable(pat, sel.tpe, isPatDef) && sourceVersion == `3.1-migration`)
if (isPatDef) patch(Span(pat.span.end), ": @unchecked")
if (!checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `3.1-migration`)
if (isPatDef) patch(Span(tree.selector.span.end), ": @unchecked")
else patch(Span(pat.span.start), "case ")

// skip exhaustivity check in later phase
// TODO: move the check above to patternMatcher phase
val uncheckedTpe = AnnotatedType(sel.tpe.widen, Annotation(defn.UncheckedAnnot))
tpd.cpy.Match(result)(
selector = tpd.Typed(sel, tpd.TypeTree(uncheckedTpe)),
cases = result.cases
)
case _ =>
result
}
case _ =>
result
}
result
}

/** Special typing of Match tree when the expected type is a MatchType,
Expand Down Expand Up @@ -1873,7 +1882,7 @@ class Typer extends Namer
case _ =>
var name = tree.name
if (name == nme.WILDCARD && tree.mods.is(Given)) {
val Typed(_, tpt): @unchecked = tree.body
val Typed(_, tpt) = tree.body: @unchecked
name = desugar.inventGivenOrExtensionName(tpt)
}
if (name == nme.WILDCARD) body1
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class CompilationTests {
aggregateTests(
compileFilesInDir("tests/neg", defaultOptions),
compileFilesInDir("tests/neg-tailcall", defaultOptions),
compileFilesInDir("tests/neg-strict", defaultOptions.and("-source", "3.1")),
compileFilesInDir("tests/neg-strict", defaultOptions.and("-source", "3.1", "-Xfatal-warnings")),
compileFilesInDir("tests/neg-no-kind-polymorphism", defaultOptions and "-Yno-kind-polymorphism"),
compileFilesInDir("tests/neg-custom-args/deprecation", defaultOptions.and("-Xfatal-warnings", "-deprecation")),
compileFilesInDir("tests/neg-custom-args/fatal-warnings", defaultOptions.and("-Xfatal-warnings")),
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/internals/syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ Def ::= ‘val’ PatDef
| ‘type’ {nl} TypeDcl
| TmplDef
PatDef ::= ids [‘:’ Type] ‘=’ Expr
| Pattern2 [‘:’ Type | Ascription] ‘=’ Expr PatDef(_, pats, tpe?, expr)
| Pattern2 [‘:’ Type] ‘=’ Expr PatDef(_, pats, tpe?, expr)
VarDef ::= PatDef
| ids ‘:’ Type ‘=’ ‘_’
DefDef ::= DefSig [‘:’ Type] ‘=’ Expr DefDef(_, name, tparams, vparamss, tpe, expr)
Expand Down
16 changes: 7 additions & 9 deletions docs/docs/reference/changed-features/pattern-bindings.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ title: "Pattern Bindings"
In Scala 2, pattern bindings in `val` definitions and `for` expressions are
loosely typed. Potentially failing matches are still accepted at compile-time,
but may influence the program's runtime behavior.
From Scala 3.1 on, type checking rules will be tightened so that errors are reported at compile-time instead.
From Scala 3.1 on, type checking rules will be tightened so that warnings are reported at compile-time instead.

## Bindings in Pattern Definitions

Expand All @@ -15,7 +15,7 @@ val xs: List[Any] = List(1, 2, 3)
val (x: String) :: _ = xs // error: pattern's type String is more specialized
// than the right hand side expression's type Any
```
This code gives a compile-time error in Scala 3.1 (and also in Scala 3.0 under the `-source 3.1` setting) whereas it will fail at runtime with a `ClassCastException` in Scala 2. In Scala 3.1, a pattern binding is only allowed if the pattern is _irrefutable_, that is, if the right-hand side's type conforms to the pattern's type. For instance, the following is OK:
This code gives a compile-time warning in Scala 3.1 (and also in Scala 3.0 under the `-source 3.1` setting) whereas it will fail at runtime with a `ClassCastException` in Scala 2. In Scala 3.1, a pattern binding is only allowed if the pattern is _irrefutable_, that is, if the right-hand side's type conforms to the pattern's type. For instance, the following is OK:
```scala
val pair = (1, true)
val (x, y) = pair
Expand All @@ -25,9 +25,9 @@ want to decompose it like this:
```scala
val first :: rest = elems // error
```
This works in Scala 2. In fact it is a typical use case for Scala 2's rules. But in Scala 3.1 it will give a type error. One can avoid the error by marking the pattern with an `@unchecked` annotation:
This works in Scala 2. In fact it is a typical use case for Scala 2's rules. But in Scala 3.1 it will give a warning. One can avoid the warning by marking the right-hand side with an `@unchecked` annotation:
```scala
val first :: rest : @unchecked = elems // OK
val first :: rest = elems: @unchecked // OK
```
This will make the compiler accept the pattern binding. It might give an error at runtime instead, if the underlying assumption that `elems` can never be empty is wrong.

Expand All @@ -40,7 +40,7 @@ val elems: List[Any] = List((1, 2), "hello", (3, 4))
for ((x, y) <- elems) yield (y, x) // error: pattern's type (Any, Any) is more specialized
// than the right hand side expression's type Any
```
This code gives a compile-time error in Scala 3.1 whereas in Scala 2 the list `elems`
This code gives a compile-time warning in Scala 3.1 whereas in Scala 2 the list `elems`
is filtered to retain only the elements of tuple type that match the pattern `(x, y)`.
The filtering functionality can be obtained in Scala 3 by prefixing the pattern with `case`:
```scala
Expand All @@ -49,13 +49,11 @@ The filtering functionality can be obtained in Scala 3 by prefixing the pattern

## Syntax Changes

There are two syntax changes relative to Scala 2: First, pattern definitions can carry ascriptions such as `: @unchecked`. Second, generators in for expressions may be prefixed with `case`.
Generators in for expressions may be prefixed with `case`.
```
PatDef ::= ids [‘:’ Type] ‘=’ Expr
| Pattern2 [‘:’ Type | Ascription] ‘=’ Expr
Generator ::= [‘case’] Pattern1 ‘<-’ Expr
```

## Migration

The new syntax is supported in Dotty and Scala 3.0. However, to enable smooth cross compilation between Scala 2 and Scala 3, the changed behavior and additional type checks are only enabled under the `-source 3.1` setting. They will be enabled by default in version 3.1 of the language.
The new syntax is supported in Scala 3.0. However, to enable smooth cross compilation between Scala 2 and Scala 3, the changed behavior and additional type checks are only enabled under the `-source 3.1` setting. They will be enabled by default in version 3.1 of the language.
2 changes: 1 addition & 1 deletion docs/docs/reference/syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ Def ::= ‘val’ PatDef
| ‘type’ {nl} TypeDcl
| TmplDef
PatDef ::= ids [‘:’ Type] ‘=’ Expr
| Pattern2 [‘:’ Type | Ascription] ‘=’ Expr
| Pattern2 [‘:’ Type] ‘=’ Expr
VarDef ::= PatDef
| ids ‘:’ Type ‘=’ ‘_’
DefDef ::= DefSig [‘:’ Type] ‘=’ Expr
Expand Down
2 changes: 1 addition & 1 deletion tests/neg-custom-args/infix.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def test() = {
val Pair(_, _) = p
val _ Pair _ = p // error
val _ `Pair` _ = p // OK
val (_ PP _): @unchecked = p // OK
val (_ PP _) = p: @unchecked // OK

val q = Q(1, 2)
val Q(_, _) = q
Expand Down
4 changes: 2 additions & 2 deletions tests/neg-strict/filtering-fors.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
object Test {

val xs: List[Any] = ???
val xs: List[AnyRef] = ???

for (x <- xs) do () // OK
for (x: Any <- xs) do () // OK
Expand All @@ -16,7 +16,7 @@ object Test {
for (case (x: String) <- xs; (y, z) <- xs) do () // error
for ((x: String) <- xs; case (y, z) <- xs) do () // error

val pairs: List[Any] = List((1, 2), "hello", (3, 4))
val pairs: List[AnyRef] = List((1, 2), "hello", (3, 4))
for ((x, y) <- pairs) yield (y, x) // error

for (case x: String <- xs) do () // OK
Expand Down
Loading