Skip to content

Commit 31210e7

Browse files
committed
Type Alternative patterns as the lub of their branches.
This came up on gitter a few weeks back, and the spec is rather ambiguous on whether this happens or not, tersely claiming that "All alternative patterns are type checked with the expected type of the pattern". As such, this is a bit of a spaghetti PR (in the "throw it at the wall to see if it sticks" sense) motivated by checking Dotty and seeing that The Future Of Scala™ duly types alternatives in this way. This will live behind a version flag because the sharper type could accidentally change a public API if there was no type ascription provided. Also here is a tweak to weakLub so that it also uses the numeric lub for singleton types of numbers. Otherwise we get strange things like scala> def ct(v: Any) = ConstantType(Constant(v)) ct: (v: Any)$r.intp.global.UniqueConstantType scala> val (one, twoL) = (ct(1), ct(2L)) one: $r.intp.global.UniqueConstantType = Int(1) twoL: $r.intp.global.UniqueConstantType = Long(2L) scala> weakLub(one :: twoL :: Nil) res0: $r.intp.global.Type = AnyVal scala> weakLub((one :: twoL :: Nil) map (_.widen)) res1: $r.intp.global.Type = Long which just cannot be right. The spec is now updated accordingly. Review by adriaanm, retronym
1 parent 6d29764 commit 31210e7

File tree

5 files changed

+56
-11
lines changed

5 files changed

+56
-11
lines changed

spec/08-pattern-matching.md

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,15 @@ shorthand for the constructor or extractor pattern $\mathit{op}(p, q_1
280280
Pattern ::= Pattern1 { ‘|’ Pattern1 }
281281
```
282282

283-
A _pattern alternative_ `$p_1$ | $\ldots$ | $p_n$`
284-
consists of a number of alternative patterns $p_i$. All alternative
285-
patterns are type checked with the expected type of the pattern. They
286-
may not bind variables other than wildcards. The alternative pattern
287-
matches a value $v$ if at least one its alternatives matches $v$.
283+
A _pattern alternative_ `$p_1$ | $\ldots$ | $p_n$` consists of a number of
284+
_alternative patterns_ $p_i$. All alternative patterns are type checked with
285+
the expected type of the pattern. The type of the pattern alternative is the
286+
[weak least upper bound](03-types.html#weak-conformance) of the types of
287+
$p_1 , \ldots , p_n$.
288+
289+
Alternative patterns may not bind variables other than wildcards.
290+
The alternative pattern matches a value $v$ if at least one its alternatives
291+
matches $v$.
288292

289293
### XML Patterns
290294

src/compiler/scala/tools/nsc/typechecker/Typers.scala

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,7 +1174,15 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
11741174
} else tree.tpe match {
11751175
case atp @ AnnotatedType(_, _) if canAdaptAnnotations(tree, this, mode, pt) => // (-1)
11761176
adaptAnnotations(tree, this, mode, pt)
1177-
case ct @ ConstantType(value) if mode.inNone(TYPEmode | FUNmode) && (ct <:< pt) && canAdaptConstantTypeToLiteral => // (0)
1177+
case ct @ ConstantType(value) if mode.inNone(TYPEmode | FUNmode | PATTERNmode) && (ct <:< pt) && canAdaptConstantTypeToLiteral => // (0)
1178+
/* We don't adapt in PATTERNmode because patmat checks for duplicated
1179+
* alternatives later on in MatchOptimizations, so that this desugars
1180+
* to a single check. Moreover, as we don't adaptConstant now, we are
1181+
* able to issue a "duplicated alternative" warning at the same time;
1182+
* see scala/bug#7290 for context. Theoretically I could warn earlier
1183+
* in typedAlternative, but there are other checks done in patmat, so
1184+
* it's easier just to wait until then.
1185+
*/
11781186
adaptConstant(value)
11791187
case OverloadedType(pre, alts) if !mode.inFunMode => // (1)
11801188
inferExprAlternative(tree, pt)
@@ -5224,9 +5232,11 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
52245232
}
52255233

52265234
def typedAlternative(alt: Alternative) = {
5227-
context withinPatAlternative (
5228-
treeCopy.Alternative(tree, alt.trees mapConserve (alt => typed(alt, mode, pt))) setType pt
5229-
)
5235+
context withinPatAlternative {
5236+
val trees1 = alt.trees mapConserve (alt => typed(alt, mode, pt))
5237+
val tpe = if (settings.isScala213) weakLub(trees1 map (_.tpe)) else pt
5238+
treeCopy.Alternative(tree, trees1) setType tpe
5239+
}
52305240
}
52315241
def typedStar(tree: Star) = {
52325242
if (!context.starPatterns && !isPastTyper)

src/reflect/scala/reflect/internal/tpe/GlbLubs.scala

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ private[internal] trait GlbLubs {
218218
def sameWeakLubAsLub(tps: List[Type]) = tps match {
219219
case Nil => true
220220
case tp :: Nil => !typeHasAnnotations(tp)
221-
case tps => !(tps exists typeHasAnnotations) && !(tps forall isNumericValueType)
221+
case tps => !(tps exists typeHasAnnotations) && !shouldUseNumericLub(tps)
222222
}
223223

224224
/** If the arguments are all numeric value types, the numeric
@@ -230,14 +230,17 @@ private[internal] trait GlbLubs {
230230
def weakLub(tps: List[Type]): Type = (
231231
if (tps.isEmpty)
232232
NothingTpe
233-
else if (tps forall isNumericValueType)
233+
else if (shouldUseNumericLub(tps))
234234
numericLub(tps)
235235
else if (tps exists typeHasAnnotations)
236236
annotationsLub(lub(tps map (_.withoutAnnotations)), tps)
237237
else
238238
lub(tps)
239239
)
240240

241+
private def shouldUseNumericLub(tps: List[Type]) =
242+
tps forall (tp => isNumericValueType(tp.widen))
243+
241244
def numericLub(ts: List[Type]) =
242245
ts reduceLeft ((t1, t2) =>
243246
if (isNumericSubType(t1, t2)) t2

test/files/pos/alternative-lubs.flags

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-Xsource:2.13

test/files/pos/alternative-lubs.scala

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
sealed trait A
2+
sealed trait B extends A
3+
case class B1(i: Int) extends B
4+
case class B2(i: Int) extends B
5+
6+
object Test {
7+
val b1: B1 = B1(0)
8+
val b2: B2 = B2(0)
9+
10+
def takesB(b: B): Int = 10
11+
def takesB1(it: b1.type): Int = 11
12+
13+
def foo0(a: A) = a match {
14+
case b @ (`b1` | `b2`) => takesB(b)
15+
case b @ (_ : `b1`.type | _ : `b2`.type) => takesB(b)
16+
case b @ (_ : `b1`.type | _ : `b1`.type) => takesB1(b)
17+
case b @ (B1(_) | B2(_)) => takesB(b)
18+
case _ => 20
19+
}
20+
21+
// weak subtyping, fun...
22+
def foo1(a: Option[Any]) = (a : @unchecked) match { case Some(x @ (1 | 1)) => x }
23+
def foo2(a: Option[Any]) = (a : @unchecked) match { case Some(x @ (1 | 1L)) => x }
24+
25+
foo1(None) : Int
26+
foo2(None) : Long
27+
}

0 commit comments

Comments
 (0)