From 79a52e6807d2797dee12bab1730765441a0e222d Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 27 Jan 2016 19:01:25 +1000 Subject: SI-9630 Fix spurious warning related to same-named case accessors Hash consing of trees within pattern match analysis was broken, and considered `x1.foo#1` to be the same tree as `x1.foo#2`, even though the two `foo`-s referred to different symbols. The hash consing was based on `Tree#correspondsStructure`, but the predicate in that function cannot veto correspondance, it can only supplement the default structural comparison. I've instead created a custom tree comparison method for use in the pattern matcher that handles the tree shapes that we use. --- .../scala/tools/nsc/transform/patmat/Logic.scala | 2 +- .../tools/nsc/transform/patmat/MatchAnalysis.scala | 16 ++++++---- test/files/pos/t9399.flags | 1 + test/files/pos/t9399.scala | 17 ++++++++++ test/files/pos/t9411a.flags | 1 + test/files/pos/t9411a.scala | 27 ++++++++++++++++ test/files/pos/t9411b.flags | 1 + test/files/pos/t9411b.scala | 36 ++++++++++++++++++++++ test/files/pos/t9630.flags | 1 + test/files/pos/t9630/t9630a.scala | 9 ++++++ test/files/pos/t9630/t9630b.scala | 8 +++++ 11 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 test/files/pos/t9399.flags create mode 100644 test/files/pos/t9399.scala create mode 100644 test/files/pos/t9411a.flags create mode 100644 test/files/pos/t9411a.scala create mode 100644 test/files/pos/t9411b.flags create mode 100644 test/files/pos/t9411b.scala create mode 100644 test/files/pos/t9630.flags create mode 100644 test/files/pos/t9630/t9630a.scala create mode 100644 test/files/pos/t9630/t9630b.scala diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala index 1a4df9973e..8b2e09495f 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala @@ -691,7 +691,7 @@ trait ScalaLogic extends Interface with Logic with TreeAndTypeAnalysis { // if X is mutable. freshExistentialSubtype(t.tpe) } - else trees find (a => a.correspondsStructure(t)(sameValue)) match { + else trees find (a => equivalentTree(a, t)) match { case Some(orig) => debug.patmat("unique tp for tree: " + ((orig, orig.tpe))) orig.tpe diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index affbcf9ec8..4286f69c96 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -84,11 +84,15 @@ trait TreeAndTypeAnalysis extends Debugging { tp <:< tpImpliedNormalizedToAny } - // TODO: improve, e.g., for constants - def sameValue(a: Tree, b: Tree): Boolean = (a eq b) || ((a, b) match { - case (_ : Ident, _ : Ident) => a.symbol eq b.symbol - case _ => false - }) + def equivalentTree(a: Tree, b: Tree): Boolean = (a, b) match { + case (Select(qual1, _), Select(qual2, _)) => equivalentTree(qual1, qual2) && a.symbol == b.symbol + case (Ident(_), Ident(_)) => a.symbol == b.symbol + case (Literal(c1), Literal(c2)) => c1 == c2 + case (This(_), This(_)) => a.symbol == b.symbol + case (Apply(fun1, args1), Apply(fun2, args2)) => equivalentTree(fun1, fun2) && args1.corresponds(args2)(equivalentTree) + // Those are the only cases we need to handle in the pattern matcher + case _ => false + } trait CheckableTreeAndTypeAnalysis { val typer: Typer @@ -276,7 +280,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT // hashconsing trees (modulo value-equality) def unique(t: Tree, tpOverride: Type = NoType): Tree = - trees find (a => a.correspondsStructure(t)(sameValue)) match { + trees find (a => equivalentTree(a, t)) match { case Some(orig) => // debug.patmat("unique: "+ (t eq orig, orig)) orig diff --git a/test/files/pos/t9399.flags b/test/files/pos/t9399.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/t9399.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/t9399.scala b/test/files/pos/t9399.scala new file mode 100644 index 0000000000..e8a8720f94 --- /dev/null +++ b/test/files/pos/t9399.scala @@ -0,0 +1,17 @@ +sealed abstract class TA +sealed abstract class TB extends TA +case object A extends TA +case object B extends TB + +sealed trait C +case class CTA(id: Int, da: TA) extends C +case class CTB(id: Int, da: TB) extends C + +class Test { + def test(c: C): Unit = c match { + case CTA(_, A) => + case CTA(_, B) => + case CTB(_, B) => + } +} + diff --git a/test/files/pos/t9411a.flags b/test/files/pos/t9411a.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/t9411a.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/t9411a.scala b/test/files/pos/t9411a.scala new file mode 100644 index 0000000000..d5264663ec --- /dev/null +++ b/test/files/pos/t9411a.scala @@ -0,0 +1,27 @@ +object OhNoes { + + sealed trait F + sealed abstract class FA extends F + sealed abstract class FB extends F + + case object FA1 extends FA + case object FB1 extends FB + case object FB2 extends FB + + sealed trait G + case object G1 extends G + case object G2 extends G + + sealed trait H + case class H1(a: FB, b: G) extends H + case class H2(a: F) extends H + + val demo: H => Unit = { + case H1(FB1, G1) => + case H1(FB2, G2) => + case H2(_: FB) => + case H2(_: FA) => + case H1(FB1, G2) => + case H1(FB2, G1) => + } +} diff --git a/test/files/pos/t9411b.flags b/test/files/pos/t9411b.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/t9411b.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/t9411b.scala b/test/files/pos/t9411b.scala new file mode 100644 index 0000000000..6888ba9382 --- /dev/null +++ b/test/files/pos/t9411b.scala @@ -0,0 +1,36 @@ +object OhNoes { + + sealed trait F + sealed abstract class FA extends F + sealed abstract class FB extends F + + case object FA1 extends FA + case object FB1 extends FB + case object FB2 extends FB + + sealed trait G + case object G1 extends G + case object G2 extends G + + sealed trait H + case class H1(a: FB, b: G) extends H + case class H2(b: F) extends H + + val demo: H => Unit = { + case H1(FB1, G1) => + case H1(FB2, G2) => + case H2(_: FB) => + case H2(_: FA) => + case H1(FB1, G2) => + case H1(FB2, G1) => + } + + val demo2: H => Unit = { + case H2(_: FA) => + case H2(_: FB) => + case H1(FB1, G1) => + case H1(FB2, G1) => + case H1(FB1, G2) => + case H1(FB2, G2) => + } +} diff --git a/test/files/pos/t9630.flags b/test/files/pos/t9630.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/t9630.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/t9630/t9630a.scala b/test/files/pos/t9630/t9630a.scala new file mode 100644 index 0000000000..c76ecd2ff2 --- /dev/null +++ b/test/files/pos/t9630/t9630a.scala @@ -0,0 +1,9 @@ + +sealed trait Base +final case class Base_1(sameName: Some[Any]) extends Base +final case class Base_2(sameName: Nested) extends Base + +sealed trait Nested +final case class Nested_1(x: Any) extends Nested +final case class Nested_2(y: Any) extends Nested + diff --git a/test/files/pos/t9630/t9630b.scala b/test/files/pos/t9630/t9630b.scala new file mode 100644 index 0000000000..3e1787ec52 --- /dev/null +++ b/test/files/pos/t9630/t9630b.scala @@ -0,0 +1,8 @@ + +class Test { + def test(b: Base): Unit = b match { + case Base_1(Some(_)) => + case Base_2(Nested_1(_)) => + case Base_2(Nested_2(_)) => + } +} -- cgit v1.2.3