Skip to content

Commit

Permalink
Teach typedConstructorPattern to ignore non-redefining unapplies
Browse files Browse the repository at this point in the history
In pos/t12250.scala Foo defines an unapply which doesn't redefine the
synthetic one.  Thus unapply is overloaded.  In the pattern being typed,
the synthetic one is indeed the one seeked, so despite being overloaded,
we should convertToCaseConstructor.
  • Loading branch information
dwijnand committed Feb 3, 2021
1 parent e9b1546 commit 0b8dd2a
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 6 deletions.
22 changes: 16 additions & 6 deletions src/compiler/scala/tools/nsc/typechecker/PatternTypers.scala
Expand Up @@ -83,6 +83,21 @@ trait PatternTypers {
val member = unapplyMember(fun.tpe)
def resultType = (fun.tpe memberType member).finalResultType
def isEmptyType = resultOfIsEmpty(resultType)

def useConstructor = (
// Dueling test cases: pos/overloaded-unapply.scala, run/case-class-23.scala, pos/t5022.scala
// Use the case class constructor if (after canElide + isCase) the unapply method:
// (1) doesn't exist, e.g. case classes with 23+ params. run/case-class-23.scala
// (2) is the synthetic case class one, i.e. not user redefined. pos/t11252.scala
// (3a) is overloaded and the synthetic case class one is still present (i.e. not suppressed) pos/t12250.scala
// (3b) the scrutinee type is the case class (not a subtype). pos/overloaded-unapply.scala vs pos/t12250b.scala
canElide && caseClass.isCase && (
member == NoSymbol // (1)
|| member.isSynthetic // (2)
|| (member.alternatives.exists(_.isSynthetic) && caseClass.tpe =:= pt) // (3a)(3b)
)
)

def isOkay = (
resultType.isErroneous
|| (resultType <:< BooleanTpe)
Expand All @@ -94,12 +109,7 @@ trait PatternTypers {
// if we're already failing, no need to emit another error here
if (fun.tpe.isErroneous)
fun
// Dueling test cases: pos/overloaded-unapply.scala, run/case-class-23.scala, pos/t5022.scala
// A case class with 23+ params has no unapply method.
// A case class constructor may be overloaded with unapply methods in the companion.
// A case class maybe have its own custom unapply (so non-synthetic) scala/bug#11252
// Unapply methods aren't `isCaseApplyOrUnapply` in Scala 3 tasty/run/src-2/tastytest/TestColour.scala
else if (canElide && caseClass.isCase && !member.isOverloaded && (member == NoSymbol || member.isSynthetic))
else if (useConstructor)
logResult(s"convertToCaseConstructor($fun, $caseClass, pt=$pt)")(convertToCaseConstructor(fun, caseClass, pt))
else if (!reallyExists(member))
CaseClassConstructorError(fun, s"${fun.symbol} is not a case class, nor does it have a valid unapply/unapplySeq member")
Expand Down
11 changes: 11 additions & 0 deletions test/files/pos/t12250.scala
@@ -0,0 +1,11 @@
// scalac: -Werror
final case class Foo(value: String)

object Foo {
def unapply(str: String): Option[Foo] = Some(Foo(str))

def extract(id: Foo): String =
id match {
case Foo(a) => a
}
}
23 changes: 23 additions & 0 deletions test/files/pos/t12250b.scala
@@ -0,0 +1,23 @@
// scalac: -Werror

sealed case class Sub1(str: String)
final case class Sup1(str: String) extends Sup0

final class Sub2 extends Sub1("")
sealed trait Sup0 { def str: String }

// both of these unapplies are overloads of the synthetic unapply
// i.e. it isn't suppressed
object Sub1 { def unapply(x: Sub2): Some[String] = Some(x.str) }
object Sup1 { def unapply(x: Sup0): Some[String] = Some(x.str) }

object Test {
// these seek the original unapplies and should be converted to use their constructors
def testSub1(x: Sub1) = x match { case Sub1(str) => str }
def testSup1(x: Sup1) = x match { case Sup1(str) => str }

// these seek the user-defined alternative unapplies
// thus they shouldn't accidentally be converted to use their constructors
def testSub2(x: Sub2) = x match { case Sub1(str) => str }
def testSup0(x: Sup0) = x match { case Sup1(str) => str }
}

0 comments on commit 0b8dd2a

Please sign in to comment.