diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2015-11-25 14:53:50 +1000 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2015-11-25 14:54:05 +1000 |
commit | 4e4709a3b0dd869b98c1a8d854f4a54145ade2ff (patch) | |
tree | 36d48ff77e336f5e28d95eac31e2698cb665be66 | |
parent | 2890f0b767948dd9a0953b1e669e85dbd45ec0a7 (diff) | |
download | scala-4e4709a3b0dd869b98c1a8d854f4a54145ade2ff.tar.gz scala-4e4709a3b0dd869b98c1a8d854f4a54145ade2ff.tar.bz2 scala-4e4709a3b0dd869b98c1a8d854f4a54145ade2ff.zip |
SI-9567 Fix latent bugs in patmat's reasoning about mutability
Under -optimize, the pattern matcher tries to avoid local variables
in favour of directly accessing to non-var case class accessors.
However, the code that analysed the patterns failed to account
properly for repeated parameters, which could either lead to
a compiler crash (when assuming that the n-th subpattern must have
a corresponding param accessor), or could lead to a correctness
problem (when failing to eagerly the bound elements from the
sequence.)
The test case that tried to cover seems only to have been working
because of a separate bug (the primary subject of SI-9567) related
to method-local case classes: they were treated during typechecking
as extractors, rather than native case classes.
The subsequent commit will fix that problem, but first we must
pave the way with this commit that emits local vals for bound
elements of case class repeated params.
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala | 18 | ||||
-rw-r--r-- | test/files/run/t9567c.scala | 29 |
2 files changed, 44 insertions, 3 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala index 451b72d498..a2afb76b0e 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala @@ -504,14 +504,26 @@ trait MatchTranslation { */ def treeMaker(binder: Symbol, binderKnownNonNull: Boolean, pos: Position): TreeMaker = { val paramAccessors = binder.constrParamAccessors + val numParams = paramAccessors.length + def paramAccessorAt(subPatIndex: Int) = paramAccessors(math.min(subPatIndex, numParams - 1)) // binders corresponding to mutable fields should be stored (SI-5158, SI-6070) // make an exception for classes under the scala package as they should be well-behaved, // to optimize matching on List + val hasRepeated = paramAccessors.lastOption match { + case Some(x) => definitions.isRepeated(x) + case _ => false + } val mutableBinders = ( if (!binder.info.typeSymbol.hasTransOwner(ScalaPackageClass) && - (paramAccessors exists (_.isMutable))) - subPatBinders.zipWithIndex.collect{ case (binder, idx) if paramAccessors(idx).isMutable => binder } - else Nil + (paramAccessors exists (x => x.isMutable || definitions.isRepeated(x)))) { + + subPatBinders.zipWithIndex.flatMap { + case (binder, idx) => + val param = paramAccessorAt(idx) + if (param.isMutable || (definitions.isRepeated(param) && !aligner.isStar)) binder :: Nil + else Nil + } + } else Nil ) // checks binder ne null before chaining to the next extractor diff --git a/test/files/run/t9567c.scala b/test/files/run/t9567c.scala new file mode 100644 index 0000000000..560bea8821 --- /dev/null +++ b/test/files/run/t9567c.scala @@ -0,0 +1,29 @@ +case class CaseSequenceTopLevel(as: Int*) + +object Test { + def main(args: Array[String]): Unit = { + + val buffer1 = collection.mutable.Buffer(0, 0) + CaseSequenceTopLevel(buffer1: _*) match { + case CaseSequenceTopLevel(_, i) => + buffer1(1) = 1 + assert(i == 0, i) // fails in 2.11.7 -optimize + } + + case class CaseSequence(as: Int*) + val buffer2 = collection.mutable.Buffer(0, 0) + CaseSequence(buffer2: _*) match { + case CaseSequence(_, i) => + buffer2(1) = 1 + assert(i == 0, i) + } + + case class CaseSequenceWithVar(var x: Any, as: Int*) + val buffer3 = collection.mutable.Buffer(0, 0) + CaseSequenceWithVar("", buffer3: _*) match { + case CaseSequenceWithVar(_, _, i) => // crashes in 2.11.7 + buffer2(1) = 1 + assert(i == 0, i) + } + } +} |