Skip to content

Commit

Permalink
Better error diagnostics for illegal match cases (#20905)
Browse files Browse the repository at this point in the history
  • Loading branch information
@odersky
odersky authored Jul 1, 2024
2 parents 4828244 + 9e498fa commit e725743
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 38 deletions.
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/core/MatchTypeTrace.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,10 @@ object MatchTypeTrace:
| ${casesText(cases)}"""

def illegalPatternText(scrut: Type, cas: MatchTypeCaseSpec.LegacyPatMat)(using Context): String =
val explanation =
if cas.err == null then "" else s"The pattern contains ${cas.err.explanation}.\n"
i"""The match type contains an illegal case:
| ${caseText(cas)}
|(this error can be ignored for now with `-source:3.3`)"""
|$explanation(this error can be ignored for now with `-source:3.3`)"""

end MatchTypeTrace
77 changes: 46 additions & 31 deletions compiler/src/dotty/tools/dotc/core/Types.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5307,10 +5307,25 @@ object Types extends TypeUtils {
case _ => true
end MatchTypeCasePattern

enum MatchTypeCaseError:
case Alias(sym: Symbol)
case RefiningBounds(name: TypeName)
case StructuralType(name: TypeName)
case UnaccountedTypeParam(name: TypeName)

def explanation(using Context) = this match
case Alias(sym) => i"a type alias `${sym.name}`"
case RefiningBounds(name) => i"an abstract type member `$name` with bounds that need verification"
case StructuralType(name) => i"an abstract type member `$name` that does not refine a member in its parent"
case UnaccountedTypeParam(name) => i"an unaccounted type parameter `$name`"
end MatchTypeCaseError

type MatchTypeCaseResult = MatchTypeCasePattern | MatchTypeCaseError

enum MatchTypeCaseSpec:
case SubTypeTest(origMatchCase: Type, pattern: Type, body: Type)
case SpeccedPatMat(origMatchCase: HKTypeLambda, captureCount: Int, pattern: MatchTypeCasePattern, body: Type)
case LegacyPatMat(origMatchCase: HKTypeLambda)
case LegacyPatMat(origMatchCase: HKTypeLambda, err: MatchTypeCaseError | Null)
case MissingCaptures(origMatchCase: HKTypeLambda, missing: collection.BitSet)

def origMatchCase: Type
Expand All @@ -5321,18 +5336,18 @@ object Types extends TypeUtils {
cas match
case cas: HKTypeLambda if !sourceVersion.isAtLeast(SourceVersion.`3.4`) =>
// Always apply the legacy algorithm under -source:3.3 and below
LegacyPatMat(cas)
LegacyPatMat(cas, null)
case cas: HKTypeLambda =>
val defn.MatchCase(pat, body) = cas.resultType: @unchecked
val missing = checkCapturesPresent(cas, pat)
if !missing.isEmpty then
MissingCaptures(cas, missing)
else
val specPattern = tryConvertToSpecPattern(cas, pat)
if specPattern != null then
SpeccedPatMat(cas, cas.paramNames.size, specPattern, body)
else
LegacyPatMat(cas)
tryConvertToSpecPattern(cas, pat) match
case specPattern: MatchTypeCasePattern =>
SpeccedPatMat(cas, cas.paramNames.size, specPattern, body)
case err: MatchTypeCaseError =>
LegacyPatMat(cas, err)
case _ =>
val defn.MatchCase(pat, body) = cas: @unchecked
SubTypeTest(cas, pat, body)
Expand Down Expand Up @@ -5370,15 +5385,15 @@ object Types extends TypeUtils {
* It must adhere to the specification of legal patterns defined at
* https://docs.scala-lang.org/sips/match-types-spec.html#legal-patterns
*
* Returns `null` if the pattern in `caseLambda` is a not a legal pattern.
* Returns a MatchTypeCaseError if the pattern in `caseLambda` is a not a legal pattern.
*/
private def tryConvertToSpecPattern(caseLambda: HKTypeLambda, pat: Type)(using Context): MatchTypeCasePattern | Null =
var typeParamRefsAccountedFor: Int = 0
private def tryConvertToSpecPattern(caseLambda: HKTypeLambda, pat: Type)(using Context): MatchTypeCaseResult =
var typeParamRefsUnaccountedFor = (0 until caseLambda.paramNames.length).to(mutable.BitSet)

def rec(pat: Type, variance: Int): MatchTypeCasePattern | Null =
def rec(pat: Type, variance: Int): MatchTypeCaseResult =
pat match
case pat @ TypeParamRef(binder, num) if binder eq caseLambda =>
typeParamRefsAccountedFor += 1
typeParamRefsUnaccountedFor -= num
MatchTypeCasePattern.Capture(num, isWildcard = pat.paramName.is(WildcardParamName))

case pat @ AppliedType(tycon: TypeRef, args) if variance == 1 =>
Expand All @@ -5394,13 +5409,13 @@ object Types extends TypeUtils {
MatchTypeCasePattern.BaseTypeTest(tycon, argPatterns, needsConcreteScrut)
}
else if defn.isCompiletime_S(tyconSym) && args.sizeIs == 1 then
val argPattern = rec(args.head, variance)
if argPattern == null then
null
else if argPattern.isTypeTest then
MatchTypeCasePattern.TypeTest(pat)
else
MatchTypeCasePattern.CompileTimeS(argPattern)
rec(args.head, variance) match
case err: MatchTypeCaseError =>
err
case argPattern: MatchTypeCasePattern =>
if argPattern.isTypeTest
then MatchTypeCasePattern.TypeTest(pat)
else MatchTypeCasePattern.CompileTimeS(argPattern)
else
tycon.info match
case _: RealTypeBounds =>
Expand All @@ -5416,7 +5431,7 @@ object Types extends TypeUtils {
*/
rec(pat.superType, variance)
case _ =>
null
MatchTypeCaseError.Alias(tyconSym)

case pat @ AppliedType(tycon: TypeParamRef, _) if variance == 1 =>
recAbstractTypeConstructor(pat)
Expand All @@ -5437,40 +5452,40 @@ object Types extends TypeUtils {
MatchTypeCasePattern.TypeMemberExtractor(refinedName, capture)
else
// Otherwise, a type-test + capture combo might be necessary, and we are out of spec
null
MatchTypeCaseError.RefiningBounds(refinedName)
case _ =>
// If the member does not refine a member of the `parent`, we are out of spec
null
MatchTypeCaseError.StructuralType(refinedName)

case _ =>
MatchTypeCasePattern.TypeTest(pat)
end rec

def recAbstractTypeConstructor(pat: AppliedType): MatchTypeCasePattern | Null =
def recAbstractTypeConstructor(pat: AppliedType): MatchTypeCaseResult =
recArgPatterns(pat) { argPatterns =>
MatchTypeCasePattern.AbstractTypeConstructor(pat.tycon, argPatterns)
}
end recAbstractTypeConstructor

def recArgPatterns(pat: AppliedType)(whenNotTypeTest: List[MatchTypeCasePattern] => MatchTypeCasePattern | Null): MatchTypeCasePattern | Null =
def recArgPatterns(pat: AppliedType)(whenNotTypeTest: List[MatchTypeCasePattern] => MatchTypeCaseResult): MatchTypeCaseResult =
val AppliedType(tycon, args) = pat
val tparams = tycon.typeParams
val argPatterns = args.zip(tparams).map { (arg, tparam) =>
rec(arg, tparam.paramVarianceSign)
}
if argPatterns.exists(_ == null) then
null
else
val argPatterns1 = argPatterns.asInstanceOf[List[MatchTypeCasePattern]] // they are not null
argPatterns.find(_.isInstanceOf[MatchTypeCaseError]).getOrElse:
val argPatterns1 = argPatterns.asInstanceOf[List[MatchTypeCasePattern]] // they are not errors
if argPatterns1.forall(_.isTypeTest) then
MatchTypeCasePattern.TypeTest(pat)
else
whenNotTypeTest(argPatterns1)
end recArgPatterns

val result = rec(pat, variance = 1)
if typeParamRefsAccountedFor == caseLambda.paramNames.size then result
else null
rec(pat, variance = 1) match
case err: MatchTypeCaseError => err
case ok if typeParamRefsUnaccountedFor.isEmpty => ok
case _ =>
MatchTypeCaseError.UnaccountedTypeParam(caseLambda.paramNames(typeParamRefsUnaccountedFor.head))
end tryConvertToSpecPattern
end MatchTypeCaseSpec

Expand Down
4 changes: 4 additions & 0 deletions tests/neg/i17121.check
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,26 @@
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| The match type contains an illegal case:
| case Consumer[List[t]] => t
| The pattern contains an unaccounted type parameter `t`.
| (this error can be ignored for now with `-source:3.3`)
-- [E191] Type Error: tests/neg/i17121.scala:15:17 ---------------------------------------------------------------------
15 | type G2[X] = X match { case Consumer[Consumer[t]] => t } // error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| The match type contains an illegal case:
| case Consumer[Consumer[t]] => t
| The pattern contains an unaccounted type parameter `t`.
| (this error can be ignored for now with `-source:3.3`)
-- [E191] Type Error: tests/neg/i17121.scala:17:17 ---------------------------------------------------------------------
17 | type G3[X] = X match { case Consumer[Consumer[Consumer[t]]] => t } // error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| The match type contains an illegal case:
| case Consumer[Consumer[Consumer[t]]] => t
| The pattern contains an unaccounted type parameter `t`.
| (this error can be ignored for now with `-source:3.3`)
-- [E191] Type Error: tests/neg/i17121.scala:19:17 ---------------------------------------------------------------------
19 | type G4[X] = X match { case Consumer[List[Consumer[t]]] => t } // error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| The match type contains an illegal case:
| case Consumer[List[Consumer[t]]] => t
| The pattern contains an unaccounted type parameter `t`.
| (this error can be ignored for now with `-source:3.3`)
18 changes: 12 additions & 6 deletions tests/neg/illegal-match-types.check
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,46 @@
| ^
| The match type contains an illegal case:
| case Inv[Cov[t]] => t
| The pattern contains an unaccounted type parameter `t`.
| (this error can be ignored for now with `-source:3.3`)
8 | case Inv[Cov[t]] => t
-- [E191] Type Error: tests/neg/illegal-match-types.scala:10:26 --------------------------------------------------------
10 |type ContraNesting[X] = X match // error
| ^
| The match type contains an illegal case:
| case Contra[Cov[t]] => t
| The pattern contains an unaccounted type parameter `t`.
| (this error can be ignored for now with `-source:3.3`)
11 | case Contra[Cov[t]] => t
-- [E191] Type Error: tests/neg/illegal-match-types.scala:15:22 --------------------------------------------------------
15 |type AndTypeMT[X] = X match // error
| ^
| The match type contains an illegal case:
| case t & Seq[Any] => t
| The pattern contains an unaccounted type parameter `t`.
| (this error can be ignored for now with `-source:3.3`)
16 | case t & Seq[Any] => t
-- [E191] Type Error: tests/neg/illegal-match-types.scala:22:33 --------------------------------------------------------
22 |type TypeAliasWithBoundMT[X] = X match // error
| ^
| The match type contains an illegal case:
| case IsSeq[t] => t
| The pattern contains a type alias `IsSeq`.
| (this error can be ignored for now with `-source:3.3`)
23 | case IsSeq[t] => t
-- [E191] Type Error: tests/neg/illegal-match-types.scala:29:34 --------------------------------------------------------
29 |type TypeMemberExtractorMT[X] = X match // error
| ^
| The match type contains an illegal case:
| case TypeMemberAux[t] => t
| (this error can be ignored for now with `-source:3.3`)
| The match type contains an illegal case:
| case TypeMemberAux[t] => t
| The pattern contains an abstract type member `TypeMember` that does not refine a member in its parent.
| (this error can be ignored for now with `-source:3.3`)
30 | case TypeMemberAux[t] => t
-- [E191] Type Error: tests/neg/illegal-match-types.scala:40:35 --------------------------------------------------------
40 |type TypeMemberExtractorMT2[X] = X match // error
| ^
| The match type contains an illegal case:
| case TypeMemberAux2[t] => t
| (this error can be ignored for now with `-source:3.3`)
| The match type contains an illegal case:
| case TypeMemberAux2[t] => t
| The pattern contains an abstract type member `TypeMember` with bounds that need verification.
| (this error can be ignored for now with `-source:3.3`)
41 | case TypeMemberAux2[t] => t

0 comments on commit e725743

Please sign in to comment.