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

Teach typedConstructorPattern to ignore non-redefining unapplies #9462

Merged
merged 2 commits into from Feb 3, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
7 changes: 7 additions & 0 deletions test/files/neg/case-class-23-unrelated-unapply.check
@@ -0,0 +1,7 @@
case-class-23-unrelated-unapply.scala:42: error: too many arguments for unapply pattern, maximum = 22
val TwentyThree(a, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b) = x
^
case-class-23-unrelated-unapply.scala:45: error: too many arguments for unapply pattern, maximum = 22
val TwentyThree(a2, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b2) = arr
^
2 errors
47 changes: 47 additions & 0 deletions test/files/neg/case-class-23-unrelated-unapply.scala
@@ -0,0 +1,47 @@
case class TwentyThree(
_1: Int,
_2: Int,
_3: Int,
_4: Int,
_5: Int,
_6: Int,
_7: Int,
_8: Int,
_9: Int,
_10: Int,
_11: Int,
_12: Int,
_13: Int,
_14: Int,
_15: Int,
_16: Int,
_17: Int,
_18: Int,
_19: Int,
_20: Int,
_21: Int,
_22: Int,
_23: Int
)

object TwentyThree {
def unapply(a: Array[Int]): Option[TwentyThree] = {
Option.when(a.length == 23) {
TwentyThree(
a(0), a(1), a(2), a(3), a(4), a(5), a(6), a(7), a(8), a(9),
a(10), a(11), a(12), a(13), a(14), a(15), a(16), a(17), a(18), a(19),
a(20), a(21), a(22),
)
}
}
}

// This is borked.. but I'm fairly certain it's borked since before I started meddling with it..
object Test extends App {
val x = new TwentyThree(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23)
val TwentyThree(a, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b) = x
println((a, b))
val arr = Array(0 , 1 , 2 , 3 , 4 , 5 , 6 , 7 , 8 , 9 , 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22)
val TwentyThree(a2, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b2) = arr
println((a2, b2))
}
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 }
}
1 change: 1 addition & 0 deletions test/files/run/case-class-23-redefined-unapply.check
@@ -0,0 +1 @@
(1,3)
35 changes: 35 additions & 0 deletions test/files/run/case-class-23-redefined-unapply.scala
@@ -0,0 +1,35 @@
case class TwentyThree(
_1: Int,
_2: Int,
_3: Int,
_4: Int,
_5: Int,
_6: Int,
_7: Int,
_8: Int,
_9: Int,
_10: Int,
_11: Int,
_12: Int,
_13: Int,
_14: Int,
_15: Int,
_16: Int,
_17: Int,
_18: Int,
_19: Int,
_20: Int,
_21: Int,
_22: Int,
_23: Int
)

object TwentyThree {
def unapply(x: TwentyThree): Some[(Int, Int, Int)] = Some(x._1, x._2, x._3)
}

object Test extends App {
val x = new TwentyThree(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23)
val TwentyThree(a, 2, b) = x
println((a, b))
}