Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

SI-6675 Avoid spurious warning about pattern bind arity.

In 692372c, we added a warning (under -Xlint) when binding
a `TupleN` in to a single pattern binder, which wasn't allowed
before 2.10.0, and more often than not represents a bug.

However, that warning overstretched, and warned even when
using a Tuple Pattern to bind to the elements of such a value.

This commit checks for this case, and avoids the spurious warnings.

A new test case is added for this case to go with the existing
test for SI-6675:

    $ ./tools/partest-ack 6675
    % tests-with-matching-paths      ...  3
    % tests-with-matching-code       ...  2
    # 3 tests to run.
    test/partest --show-diff --show-log                  \
      test/files/neg/t6675-old-patmat.scala              \
      test/files/neg/t6675.scala                         \
      test/files/pos/t6675.scala                         \
      ""

    Testing individual files
    testing: [...]/files/pos/t6675.scala                                  [  OK  ]

    Testing individual files
    testing: [...]/files/neg/t6675-old-patmat.scala                       [  OK  ]
    testing: [...]/files/neg/t6675.scala                                  [  OK  ]
    All of 3 tests were successful (elapsed time: 00:00:03)
  • Loading branch information...
commit c1327dcd99a6ca84d2550b8e4894ec7ee5ee2420 1 parent 4525e92
Jason Zaugg retronym authored
4 bincompat-forward.whitelist.conf
View
@@ -422,6 +422,10 @@ filter {
{
matchName="scala.reflect.internal.Types#TypeVar.setInst"
problemName=IncompatibleResultTypeProblem
+ },
+ {
+ matchName="scala.reflect.internal.TreeInfo.effectivePatternArity"
+ problemName=MissingMethodProblem
}
]
}
2  src/compiler/scala/tools/nsc/matching/Patterns.scala
View
@@ -402,7 +402,7 @@ trait Patterns extends ast.TreeDSL {
case _ => toPats(args)
}
- def resTypes = analyzer.unapplyTypeList(unfn.pos, unfn.symbol, unfn.tpe, args.length)
+ def resTypes = analyzer.unapplyTypeList(unfn.pos, unfn.symbol, unfn.tpe, args)
def resTypesString = resTypes match {
case Nil => "Boolean"
case xs => xs.mkString(", ")
2  src/compiler/scala/tools/nsc/transform/UnCurry.scala
View
@@ -623,7 +623,7 @@ abstract class UnCurry extends InfoTransform
val fn1 = withInPattern(false)(transform(fn))
val args1 = transformTrees(fn.symbol.name match {
case nme.unapply => args
- case nme.unapplySeq => transformArgs(tree.pos, fn.symbol, args, analyzer.unapplyTypeList(fn.pos, fn.symbol, fn.tpe, args.length))
+ case nme.unapplySeq => transformArgs(tree.pos, fn.symbol, args, analyzer.unapplyTypeList(fn.pos, fn.symbol, fn.tpe, args))
case _ => sys.error("internal error: UnApply node has wrong symbol")
})
treeCopy.UnApply(tree, fn1, args1)
12 src/compiler/scala/tools/nsc/typechecker/Infer.scala
View
@@ -55,6 +55,10 @@ trait Infer extends Checkable {
*
* `formals` are the formal types before expanding a potential repeated parameter (must come last in `formals`, if at all)
*
+ * @param nbSubPats The number of arguments to the extractor pattern
+ * @param effectiveNbSubPats `nbSubPats`, unless there is one sub-pattern which, after unwrapping
+ * bind patterns, is a Tuple pattern, in which case it is the number of
+ * elements. Used to issue warnings about binding a `TupleN` to a single value.
* @throws TypeError when the unapply[Seq] definition is ill-typed
* @returns (null, null) when the expected number of sub-patterns cannot be satisfied by the given extractor
*
@@ -84,7 +88,8 @@ trait Infer extends Checkable {
* `T_1, ..., T_m, U, ..., U`. Here, `U` is repeated `n-m` times.
*
*/
- def extractorFormalTypes(pos: Position, resTp: Type, nbSubPats: Int, unappSym: Symbol): (List[Type], List[Type]) = {
+ def extractorFormalTypes(pos: Position, resTp: Type, nbSubPats: Int,
+ unappSym: Symbol, effectiveNbSubPats: Int): (List[Type], List[Type]) = {
val isUnapplySeq = unappSym.name == nme.unapplySeq
val booleanExtractor = resTp.typeSymbolDirect == BooleanClass
@@ -114,8 +119,9 @@ trait Infer extends Checkable {
else if (optionArgs.nonEmpty)
if (nbSubPats == 1) {
val productArity = productArgs.size
- if (productArity > 1 && settings.lint.value)
- global.currentUnit.warning(pos, s"extractor pattern binds a single value to a Product${productArity} of type ${optionArgs.head}")
+ if (settings.lint.value && productArity > 1 && productArity != effectiveNbSubPats)
+ global.currentUnit.warning(pos,
+ s"extractor pattern binds a single value to a Product${productArity} of type ${optionArgs.head}")
optionArgs
}
// TODO: update spec to reflect we allow any ProductN, not just TupleN
6 src/compiler/scala/tools/nsc/typechecker/Typers.scala
View
@@ -3480,8 +3480,8 @@ trait Typers extends Modes with Adaptations with Tags {
else {
val resTp = fun1.tpe.finalResultType.normalize
val nbSubPats = args.length
-
- val (formals, formalsExpanded) = extractorFormalTypes(fun0.pos, resTp, nbSubPats, fun1.symbol)
+ val (formals, formalsExpanded) =
+ extractorFormalTypes(fun0.pos, resTp, nbSubPats, fun1.symbol, treeInfo.effectivePatternArity(args))
if (formals == null) duplErrorTree(WrongNumberOfArgsError(tree, fun))
else {
val args1 = typedArgs(args, mode, formals, formalsExpanded)
@@ -5297,7 +5297,7 @@ trait Typers extends Modes with Adaptations with Tags {
def typedUnApply(tree: UnApply) = {
val fun1 = typed(tree.fun)
- val tpes = formalTypes(unapplyTypeList(tree.fun.pos, tree.fun.symbol, fun1.tpe, tree.args.length), tree.args.length)
+ val tpes = formalTypes(unapplyTypeList(tree.fun.pos, tree.fun.symbol, fun1.tpe, tree.args), tree.args.length)
val args1 = map2(tree.args, tpes)(typedPattern)
treeCopy.UnApply(tree, fun1, args1) setType pt
}
5 src/compiler/scala/tools/nsc/typechecker/Unapplies.scala
View
@@ -34,12 +34,13 @@ trait Unapplies extends ast.TreeDSL
/** returns type list for return type of the extraction
* @see extractorFormalTypes
*/
- def unapplyTypeList(pos: Position, ufn: Symbol, ufntpe: Type, nbSubPats: Int) = {
+ def unapplyTypeList(pos: Position, ufn: Symbol, ufntpe: Type, args: List[Tree]) = {
assert(ufn.isMethod, ufn)
+ val nbSubPats = args.length
//Console.println("utl "+ufntpe+" "+ufntpe.typeSymbol)
ufn.name match {
case nme.unapply | nme.unapplySeq =>
- val (formals, _) = extractorFormalTypes(pos, unapplyUnwrap(ufntpe), nbSubPats, ufn)
+ val (formals, _) = extractorFormalTypes(pos, unapplyUnwrap(ufntpe), nbSubPats, ufn, treeInfo.effectivePatternArity(args))
if (formals == null) throw new TypeError(s"$ufn of type $ufntpe cannot extract $nbSubPats sub-patterns")
else formals
case _ => throw new TypeError(ufn+" is not an unapply or unapplySeq")
16 src/reflect/scala/reflect/internal/TreeInfo.scala
View
@@ -17,7 +17,7 @@ abstract class TreeInfo {
val global: SymbolTable
import global._
- import definitions.{ isVarArgsList, isCastSymbol, ThrowableClass, TupleClass, MacroContextClass, MacroContextPrefixType }
+ import definitions.{ isTupleSymbol, isVarArgsList, isCastSymbol, ThrowableClass, TupleClass, MacroContextClass, MacroContextPrefixType }
/* Does not seem to be used. Not sure what it does anyway.
def isOwnerDefinition(tree: Tree): Boolean = tree match {
@@ -515,6 +515,20 @@ abstract class TreeInfo {
case _ => false
}
+ /**
+ * {{{
+ * //------------------------ => effectivePatternArity(args)
+ * case Extractor(a) => 1
+ * case Extractor(a, b) => 2
+ * case Extractor((a, b)) => 2
+ * case Extractor(a @ (b, c)) => 2
+ * }}}
+ */
+ def effectivePatternArity(args: List[Tree]): Int = (args.map(unbind) match {
+ case Apply(fun, xs) :: Nil if isTupleSymbol(fun.symbol) => xs
+ case xs => xs
+ }).length
+
// used in the symbols for labeldefs and valdefs emitted by the pattern matcher
// tailcalls, cps,... use this flag combination to detect translated matches
1  test/files/pos/t6675.flags
View
@@ -0,0 +1 @@
+-Xfatal-warnings
20 test/files/pos/t6675.scala
View
@@ -0,0 +1,20 @@
+object LeftOrRight {
+ def unapply[A](value: Either[A, A]): Option[A] = value match {
+ case scala.Left(x) => Some(x)
+ case scala.Right(x) => Some(x)
+ }
+}
+
+object Test {
+ (Left((0, 0)): Either[(Int, Int), (Int, Int)]) match {
+ case LeftOrRight(pair @ (a, b)) => a // false -Xlint warning: "extractor pattern binds a single value to a Product2 of type (Int, Int)"
+ }
+
+ (Left((0, 0)): Either[(Int, Int), (Int, Int)]) match {
+ case LeftOrRight((a, b)) => a // false -Xlint warning: "extractor pattern binds a single value to a Product2 of type (Int, Int)"
+ }
+
+ (Left((0, 0)): Either[(Int, Int), (Int, Int)]) match {
+ case LeftOrRight(a, b) => a // false -Xlint warning: "extractor pattern binds a single value to a Product2 of type (Int, Int)"
+ }
+}
Please sign in to comment.
Something went wrong with that request. Please try again.