Merge pull request #4164 from retronym/ticket/9003

SI-9003 Eagerly capture more potentially mutable binders

4 files changed, 81 insertions, 1 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala
index d862805a07..22661d6ccf 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala
@@ -544,10 +544,17 @@ trait MatchTranslation {
         // wrong when isSeq, and resultInMonad should always be correct since it comes
         // directly from the extractor's result type
         val binder         = freshSym(pos, pureType(resultInMonad))
+        val potentiallyMutableBinders: Set[Symbol] =
+          if (extractorApply.tpe.typeSymbol.isNonBottomSubClass(OptionClass) && !aligner.isSeq)
+            Set.empty
+          else
+            // Ensures we capture unstable bound variables eagerly. These can arise under name based patmat or by indexing into mutable Seqs. See run t9003.scala
+            subPatBinders.toSet
 
         ExtractorTreeMaker(extractorApply, lengthGuard(binder), binder)(
           subPatBinders,
           subPatRefs(binder),
+          potentiallyMutableBinders,
           aligner.isBool,
           checkedLength,
           patBinderOrCasted,
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
index 3abec521df..3fd9ce76f8 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
@@ -192,13 +192,14 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
     case class ExtractorTreeMaker(extractor: Tree, extraCond: Option[Tree], nextBinder: Symbol)(
           val subPatBinders: List[Symbol],
           val subPatRefs: List[Tree],
+          val potentiallyMutableBinders: Set[Symbol],
           extractorReturnsBoolean: Boolean,
           val checkedLength: Option[Int],
           val prevBinder: Symbol,
           val ignoredSubPatBinders: Set[Symbol]
           ) extends FunTreeMaker with PreserveSubPatBinders {
 
-      def extraStoredBinders: Set[Symbol] = Set()
+      def extraStoredBinders: Set[Symbol] = potentiallyMutableBinders
 
       debug.patmat(s"""
         |ExtractorTreeMaker($extractor, $extraCond, $nextBinder) {
diff --git a/test/files/run/t9003.flags b/test/files/run/t9003.flags
new file mode 100644
index 0000000000..49d036a887
--- /dev/null
+++ b/test/files/run/t9003.flags
@@ -0,0 +1 @@
+-optimize
diff --git a/test/files/run/t9003.scala b/test/files/run/t9003.scala
new file mode 100644
index 0000000000..4f24712201
--- /dev/null
+++ b/test/files/run/t9003.scala
@@ -0,0 +1,71 @@
+object Single {
+  var i = 0
+  def isEmpty = false
+  def get = i
+  def unapply(a: Single.type) = this
+}
+
+object Product {
+  var i = 0
+  def _1: Int = i
+  def _2: String = ???
+  def productArity = 2
+  def unapply(a: Product.type) = this
+  def isEmpty = false
+  def get: this.type = this
+}
+
+object Sequence {
+  var i = 0
+  def apply(n: Int): Int = i
+  def length = 2
+  def unapplySeq(a: Sequence.type) = this
+  def isEmpty = false
+  def get = this
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    def assertZero(i: Int) = assert(i == 0)
+
+    Single match {
+      case Single(i) =>
+        Single.i = 1
+        assertZero(i) // fails under -optimize
+    }
+
+    Product match {
+      case Product(i, _) =>
+        Product.i = 1
+        assertZero(i) // fails under -optimize
+    }
+
+    Sequence match {
+      case Sequence(i, _ @ _*) =>
+        Sequence.i = 1
+        assertZero(i) // okay
+    }
+
+    Sequence.i = 0
+    Sequence match {
+      case Sequence(_, i) =>
+        Sequence.i = 1
+        assertZero(i) // okay
+    }
+
+    val buffer = collection.mutable.Buffer(0, 0)
+    buffer match {
+      case Seq(_, i) =>
+        buffer(1) = 1
+        assertZero(i) // failed
+    }
+
+    case class CaseSequence(as: Int*)
+    val buffer1 = collection.mutable.Buffer(0, 0)
+    CaseSequence(buffer1: _*) match {
+      case CaseSequence(_, i) =>
+        buffer1(1) = 1
+        assertZero(i) // failed
+    }
+  }
+}