Merge pull request #1801 from dotty-staging/fix-#1790

Fix #1790: Change by-name pattern matching.

6 files changed, 94 insertions, 56 deletions

diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
index 15cb0b665..11f8b81eb 100644
--- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala
+++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -24,7 +24,6 @@ object desugar {
 
   /** Names of methods that are added unconditionally to case classes */
   def isDesugaredCaseClassMethodName(name: Name)(implicit ctx: Context): Boolean =
-    name == nme.isDefined ||
     name == nme.copy ||
     name == nme.productArity ||
     name.isSelectorName
@@ -343,7 +342,6 @@ object desugar {
       if (isCaseClass) {
         def syntheticProperty(name: TermName, rhs: Tree) =
           DefDef(name, Nil, Nil, TypeTree(), rhs).withMods(synthetic)
-        val isDefinedMeth = syntheticProperty(nme.isDefined, Literal(Constant(true)))
         val caseParams = constrVparamss.head.toArray
         val productElemMeths = for (i <- 0 until arity) yield
           syntheticProperty(nme.selectorName(i), Select(This(EmptyTypeIdent), caseParams(i).name))
@@ -369,7 +367,7 @@ object desugar {
             DefDef(nme.copy, derivedTparams, copyFirstParams :: copyRestParamss, TypeTree(), creatorExpr)
               .withMods(synthetic) :: Nil
           }
-        copyMeths ::: isDefinedMeth :: productElemMeths.toList
+        copyMeths ::: productElemMeths.toList
       }
       else Nil
 
diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala
index e4e5761b2..9759e39fc 100644
--- a/compiler/src/dotty/tools/dotc/core/Definitions.scala
+++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala
@@ -675,7 +675,9 @@ class Definitions {
 
   private def isVarArityClass(cls: Symbol, prefix: Name) = {
     val name = scalaClassName(cls)
-    name.startsWith(prefix) && name.drop(prefix.length).forall(_.isDigit)
+    name.startsWith(prefix) &&
+    name.length > prefix.length &&
+    name.drop(prefix.length).forall(_.isDigit)
   }
 
   def isBottomClass(cls: Symbol) =
@@ -735,6 +737,14 @@ class Definitions {
   def isProductSubType(tp: Type)(implicit ctx: Context) =
     (tp derivesFrom ProductType.symbol) && tp.baseClasses.exists(isProductClass)
 
+  def productArity(tp: Type)(implicit ctx: Context) =
+    if (tp derivesFrom ProductType.symbol)
+      tp.baseClasses.find(isProductClass) match {
+        case Some(prod) => prod.typeParams.length
+        case None => -1
+      }
+    else -1
+
   def isFunctionType(tp: Type)(implicit ctx: Context) =
     isFunctionClass(tp.dealias.typeSymbol) && {
       val arity = functionArity(tp)
diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala
index 741ff8b1f..e71893c1e 100644
--- a/compiler/src/dotty/tools/dotc/core/StdNames.scala
+++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala
@@ -424,7 +424,6 @@ object StdNames {
     val info: N                 = "info"
     val inlinedEquals: N        = "inlinedEquals"
     val isArray: N              = "isArray"
-    val isDefined: N            = "isDefined"
     val isDefinedAt: N          = "isDefinedAt"
     val isDefinedAtImpl: N      = "$isDefinedAt"
     val isEmpty: N              = "isEmpty"
diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
index 3e25cf82e..181dfccd9 100644
--- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
+++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
@@ -235,14 +235,21 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {
         // next: MatchMonad[U]
         // returns MatchMonad[U]
         def flatMap(prev: Tree, b: Symbol, next: Tree): Tree = {
-
-          val getTp = extractorMemberType(prev.tpe, nme.get)
-          val isDefined = extractorMemberType(prev.tpe, nme.isDefined)
-
-          if ((isDefined isRef defn.BooleanClass) && getTp.exists) {
-            // isDefined and get may be overloaded
-            val getDenot = prev.tpe.member(nme.get).suchThat(_.info.isParameterless)
-            val isDefinedDenot = prev.tpe.member(nme.isDefined).suchThat(_.info.isParameterless)
+          val resultArity = defn.productArity(b.info)
+          if (isProductMatch(prev.tpe, resultArity)) {
+            val nullCheck: Tree = prev.select(defn.Object_ne).appliedTo(Literal(Constant(null)))
+            ifThenElseZero(
+              nullCheck,
+              Block(
+                List(ValDef(b.asTerm, prev)),
+                next //Substitution(b, ref(prevSym))(next)
+              )
+            )
+          }
+          else {
+            val getDenot = extractorMember(prev.tpe, nme.get)
+            val isEmptyDenot = extractorMember(prev.tpe, nme.isEmpty)
+            assert(getDenot.exists && isEmptyDenot.exists, i"${prev.tpe}")
 
             val tmpSym = freshSym(prev.pos, prev.tpe, "o")
             val prevValue = ref(tmpSym).select(getDenot.symbol).ensureApplied
@@ -251,20 +258,10 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {
               List(ValDef(tmpSym, prev)),
               // must be isEmpty and get as we don't control the target of the call (prev is an extractor call)
               ifThenElseZero(
-                ref(tmpSym).select(isDefinedDenot.symbol),
+                ref(tmpSym).select(isEmptyDenot.symbol).select(defn.Boolean_!),
                 Block(List(ValDef(b.asTerm, prevValue)), next)
               )
             )
-          } else {
-            assert(defn.isProductSubType(prev.tpe))
-            val nullCheck: Tree = prev.select(defn.Object_ne).appliedTo(Literal(Constant(null)))
-            ifThenElseZero(
-              nullCheck,
-              Block(
-                List(ValDef(b.asTerm, prev)),
-                next //Substitution(b, ref(prevSym))(next)
-              )
-            )
           }
         }
 
@@ -1431,12 +1428,12 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {
         case _        => or
       }
 
-      def resultInMonad  = if (aligner.isBool) defn.UnitType else {
-        val getTp = extractorMemberType(resultType, nme.get)
-        if ((extractorMemberType(resultType, nme.isDefined) isRef defn.BooleanClass) && getTp.exists)
-          getTp
+      def resultInMonad  =
+        if (aligner.isBool) defn.UnitType
+        else if (isProductMatch(resultType, aligner.prodArity)) resultType
+        else if (isGetMatch(resultType)) extractorMemberType(resultType, nme.get)
         else resultType
-      }
+
       def resultType: Type
 
       /** Create the TreeMaker that embodies this extractor call
@@ -1632,13 +1629,12 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {
           //val spr = subPatRefs(binder)
           assert(go && go1)
           ref(binder) :: Nil
-        } else {
-          lazy val getTp = extractorMemberType(binderTypeTested, nme.get)
-          if ((aligner.isSingle && aligner.extractor.prodArity == 1) && ((extractorMemberType(binderTypeTested, nme.isDefined) isRef defn.BooleanClass) && getTp.exists))
-            List(ref(binder))
-          else
-            subPatRefs(binder)
         }
+        else if ((aligner.isSingle && aligner.extractor.prodArity == 1) &&
+                 !isProductMatch(binderTypeTested, aligner.prodArity) && isGetMatch(binderTypeTested))
+          List(ref(binder))
+        else
+          subPatRefs(binder)
       }
 
       /*protected def spliceApply(binder: Symbol): Tree = {
@@ -1890,9 +1886,8 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {
           else if (result.classSymbol is Flags.CaseClass) result.decls.filter(x => x.is(Flags.CaseAccessor) && x.is(Flags.Method)).map(_.info).toList
           else result.select(nme.get) :: Nil
           )*/
-          if ((extractorMemberType(resultType, nme.isDefined) isRef defn.BooleanClass) && resultOfGet.exists)
-            getUnapplySelectors(resultOfGet, args)
-          else if (defn.isProductSubType(resultType)) productSelectorTypes(resultType)
+          if (isProductMatch(resultType, args.length)) productSelectorTypes(resultType)
+          else if (isGetMatch(resultType)) getUnapplySelectors(resultOfGet, args)
           else if (resultType isRef defn.BooleanClass) Nil
           else {
             ctx.error(i"invalid return type in Unapply node: $resultType")
diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala
index a3b4cb311..4203ab9b2 100644
--- a/compiler/src/dotty/tools/dotc/typer/Applications.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala
@@ -32,16 +32,37 @@ import reporting.diagnostic.Message
 object Applications {
   import tpd._
 
+  def extractorMember(tp: Type, name: Name)(implicit ctx: Context) = {
+    def isPossibleExtractorType(tp: Type) = tp match {
+      case _: MethodType | _: PolyType => false
+      case _ => true
+    }
+    tp.member(name).suchThat(d => isPossibleExtractorType(d.info))
+  }
+
   def extractorMemberType(tp: Type, name: Name, errorPos: Position = NoPosition)(implicit ctx: Context) = {
-    val ref = tp.member(name).suchThat(_.info.isParameterless)
+    val ref = extractorMember(tp, name)
     if (ref.isOverloaded)
       errorType(i"Overloaded reference to $ref is not allowed in extractor", errorPos)
-    else if (ref.info.isInstanceOf[PolyType])
-      errorType(i"Reference to polymorphic $ref: ${ref.info} is not allowed in extractor", errorPos)
-    else
-      ref.info.widenExpr.dealias
+    ref.info.widenExpr.dealias
   }
 
+  /** Does `tp` fit the "product match" conditions as an unapply result type
+   *  for a pattern with `numArgs` subpatterns>
+   *  This is the case of `tp` is a subtype of the Product<numArgs> class.
+   */
+  def isProductMatch(tp: Type, numArgs: Int)(implicit ctx: Context) =
+    0 <= numArgs && numArgs <= Definitions.MaxTupleArity &&
+    tp.derivesFrom(defn.ProductNType(numArgs).typeSymbol)
+
+  /** Does `tp` fit the "get match" conditions as an unapply result type?
+   *  This is the case of `tp` has a `get` member as well as a
+   *  parameterless `isDefined` member of result type `Boolean`.
+   */
+  def isGetMatch(tp: Type, errorPos: Position = NoPosition)(implicit ctx: Context) =
+    extractorMemberType(tp, nme.isEmpty, errorPos).isRef(defn.BooleanClass) &&
+    extractorMemberType(tp, nme.get, errorPos).exists
+
   def productSelectorTypes(tp: Type, errorPos: Position = NoPosition)(implicit ctx: Context): List[Type] = {
     val sels = for (n <- Iterator.from(0)) yield extractorMemberType(tp, nme.selectorName(n), errorPos)
     sels.takeWhile(_.exists).toList
@@ -61,24 +82,37 @@ object Applications {
 
   def unapplyArgs(unapplyResult: Type, unapplyFn: Tree, args: List[untpd.Tree], pos: Position = NoPosition)(implicit ctx: Context): List[Type] = {
 
+    val unapplyName = unapplyFn.symbol.name
     def seqSelector = defn.RepeatedParamType.appliedTo(unapplyResult.elemType :: Nil)
     def getTp = extractorMemberType(unapplyResult, nme.get, pos)
 
-    // println(s"unapply $unapplyResult ${extractorMemberType(unapplyResult, nme.isDefined)}")
-    if (extractorMemberType(unapplyResult, nme.isDefined, pos) isRef defn.BooleanClass) {
-      if (getTp.exists)
-        if (unapplyFn.symbol.name == nme.unapplySeq) {
-          val seqArg = boundsToHi(getTp.elemType)
-          if (seqArg.exists) return args map Function.const(seqArg)
-        }
-        else return getUnapplySelectors(getTp, args, pos)
-      else if (defn.isProductSubType(unapplyResult)) return productSelectorTypes(unapplyResult, pos)
+    def fail = {
+      ctx.error(i"$unapplyResult is not a valid result type of an $unapplyName method of an extractor", pos)
+      Nil
+    }
+
+    if (unapplyName == nme.unapplySeq) {
+      if (unapplyResult derivesFrom defn.SeqClass) seqSelector :: Nil
+      else if (isGetMatch(unapplyResult, pos)) {
+        val seqArg = boundsToHi(getTp.elemType)
+        if (seqArg.exists) args.map(Function.const(seqArg))
+        else fail
+      }
+      else fail
     }
-    if (unapplyResult derivesFrom defn.SeqClass) seqSelector :: Nil
-    else if (unapplyResult isRef defn.BooleanClass) Nil
     else {
-      ctx.error(i"$unapplyResult is not a valid result type of an unapply method of an extractor", pos)
-      Nil
+      assert(unapplyName == nme.unapply)
+      if (isProductMatch(unapplyResult, args.length))
+        productSelectorTypes(unapplyResult)
+      else if (isGetMatch(unapplyResult, pos))
+        getUnapplySelectors(getTp, args, pos)
+      else if (unapplyResult isRef defn.BooleanClass)
+        Nil
+      else if (defn.isProductSubType(unapplyResult))
+        productSelectorTypes(unapplyResult)
+          // this will cause a "wrong number of arguments in pattern" error later on,
+          // which is better than the message in `fail`.
+      else fail
     }
   }
 
diff --git a/compiler/test/dotc/scala-collections.whitelist b/compiler/test/dotc/scala-collections.whitelist
index bb62b260a..e984af6c6 100644
--- a/compiler/test/dotc/scala-collections.whitelist
+++ b/compiler/test/dotc/scala-collections.whitelist
@@ -280,3 +280,5 @@
 ../scala-scala/src/library/scala/collection/generic/Subtractable.scala
 ../scala-scala/src/library/scala/collection/generic/TraversableFactory.scala
 ../scala-scala/src/library/scala/collection/generic/package.scala
+
+../scala-scala/src/library/scala/util/Try.scala
\ No newline at end of file