From 2678d349b2b2738d9db38d890199f32aa39d8c3e Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Wed, 22 Jul 2015 20:51:05 +0200 Subject: SI-9403 fix ICodeReader for negative BIPUSH / SIPUSH values The byte value of a BIPUSH instruction and the (byte1 << 8) | byte2 value of a SIPUSH instruction are signed, see [1] and [2]. Similar for the increment value of IINC [3]. [1] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.bipush [2] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.sipush [3] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.iinc --- test/files/run/t9403.flags | 1 + test/files/run/t9403/C_1.scala | 5 +++++ test/files/run/t9403/Test_2.scala | 29 +++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 test/files/run/t9403.flags create mode 100644 test/files/run/t9403/C_1.scala create mode 100644 test/files/run/t9403/Test_2.scala (limited to 'test') diff --git a/test/files/run/t9403.flags b/test/files/run/t9403.flags new file mode 100644 index 0000000000..307668060c --- /dev/null +++ b/test/files/run/t9403.flags @@ -0,0 +1 @@ +-Ybackend:GenASM -optimize diff --git a/test/files/run/t9403/C_1.scala b/test/files/run/t9403/C_1.scala new file mode 100644 index 0000000000..439af1a386 --- /dev/null +++ b/test/files/run/t9403/C_1.scala @@ -0,0 +1,5 @@ +package p +class C { + @inline final def f(x: Int): Long = 10L / (if (x < 0) -2 else 2) + @inline final def g(x: Int): Long = 3000L / (if (x < 0) -300 else 300) +} diff --git a/test/files/run/t9403/Test_2.scala b/test/files/run/t9403/Test_2.scala new file mode 100644 index 0000000000..fb2777b9a8 --- /dev/null +++ b/test/files/run/t9403/Test_2.scala @@ -0,0 +1,29 @@ +import p.C +import scala.tools.asm.Opcodes +import scala.tools.partest.BytecodeTest +import scala.tools.partest.ASMConverters._ + + +object Test extends BytecodeTest { + def foo(c: C, x: Int) = c.f(x) + def goo(c: C, x: Int) = c.g(x) + + def has(i: Instruction, c: String, m: String) = { + val cls = loadClassNode(c) + val mth = convertMethod(getMethod(cls, m)) + assert(mth.instructions.contains(i)) + } + + def show(): Unit = { + assert(foo(new C, -2) == -5L) + assert(goo(new C, -2) == -10L) + + val bipush2 = IntOp(Opcodes.BIPUSH, -2) + has(bipush2, "p.C", "f") + has(bipush2, "Test$", "foo") + + val sipush300 = IntOp(Opcodes.SIPUSH, -300) + has(sipush300, "p.C", "g") + has(sipush300, "Test$", "goo") + } +} -- cgit v1.2.3 From 0c99742c51706ee4b60a56a8f5babb13682f9b10 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 27 Jul 2015 15:33:30 +1000 Subject: SI-9365 Don't null out dependencies of transient lazy vals As per Iulian's analysis: > When lazy vals are transient, the optimization in SI-720 is invalid, > leading to NPEs. These NPEs appear when recomputing the lazy val when deserializaing the object. This commit disables the field nulling if the lazy val is marked transient. The post-mixin tree in the enclosed test changes as follows: ``` --- sandbox/old.log 2015-07-27 15:48:03.000000000 +1000 +++ sandbox/new.log 2015-07-27 15:47:56.000000000 +1000 @@ -1,61 +1,54 @@ [[syntax trees at end of mixin]] // t9365.scala package { class Test extends Object with Serializable { @transient @volatile private[this] var bitmap$trans$0: Boolean = false; private def foo$lzycompute(): Object = { { Test.this.synchronized({ if (Test.this.bitmap$trans$0.unary_!()) { Test.this.foo = Test.this.x.apply(); Test.this.bitmap$trans$0 = true; () }; scala.runtime.BoxedUnit.UNIT }); - Test.this.x = null + () }; Test.this.foo }; ``` In addition to the test from the ticket, I've added a reflection based test that directly tests the nulling. This complements the test added in 449f2a7473, the fix for SI-720, which passes by virtue of not exhausting the heap. --- src/compiler/scala/tools/nsc/transform/Mixin.scala | 2 +- test/files/run/t720.scala | 48 ++++++++++++++++++++++ test/files/run/t9365.check | 2 + test/files/run/t9365.scala | 18 ++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t720.scala create mode 100644 test/files/run/t9365.check create mode 100644 test/files/run/t9365.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala index 25d45cc819..a079a76ce7 100644 --- a/src/compiler/scala/tools/nsc/transform/Mixin.scala +++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala @@ -1122,7 +1122,7 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL { if (scope exists (_.isLazy)) { val map = mutable.Map[Symbol, Set[Symbol]]() withDefaultValue Set() // check what fields can be nulled for - for ((field, users) <- singleUseFields(templ); lazyFld <- users) + for ((field, users) <- singleUseFields(templ); lazyFld <- users if !lazyFld.accessed.hasAnnotation(TransientAttr)) map(lazyFld) += field map.toMap diff --git a/test/files/run/t720.scala b/test/files/run/t720.scala new file mode 100644 index 0000000000..a5cb2495cf --- /dev/null +++ b/test/files/run/t720.scala @@ -0,0 +1,48 @@ +class Lazy(f: => Int) { + lazy val get: Int = f +} + +class UsedLater(f: => Int) { + lazy val get: Int = f + def other = f +} + +class TransientLazy(f: => Int) { + @transient + lazy val get: Int = f +} + +object Test { + def main(args: Array[String]): Unit = { + testLazy() + testUsedLater() + } + + def testLazy() { + val o = new Lazy("".length) + val f = classOf[Lazy].getDeclaredField("f") + f.setAccessible(true) + assert(f.get(o) != null) + o.get + assert(f.get(o) == null) + } + + def testUsedLater() { + val o = new UsedLater("".length) + val f = classOf[UsedLater].getDeclaredField("f") + f.setAccessible(true) + assert(f.get(o) != null) + o.get + assert(f.get(o) != null) + } + + def testTransientLazy() { + val o = new TransientLazy("".length) + val f = classOf[TransientLazy].getDeclaredField("f") + f.setAccessible(true) + assert(f.get(o) != null) + o.get + assert(f.get(o) != null) // SI-9365 + } +} + diff --git a/test/files/run/t9365.check b/test/files/run/t9365.check new file mode 100644 index 0000000000..0d55bed3a3 --- /dev/null +++ b/test/files/run/t9365.check @@ -0,0 +1,2 @@ +foo +foo diff --git a/test/files/run/t9365.scala b/test/files/run/t9365.scala new file mode 100644 index 0000000000..0c4477dda9 --- /dev/null +++ b/test/files/run/t9365.scala @@ -0,0 +1,18 @@ +class Test(x: => Object) extends Serializable { + @transient lazy val foo = x +} + +object Test { + def main(args: Array[String]): Unit = { + import java.io._ + val t = new Test("foo") + println(t.foo) + val baos = new ByteArrayOutputStream + val dos = new ObjectOutputStream(baos) + dos.writeObject(t) + dos.close() + val dis = new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray())) + val t1 = dis.readObject().asInstanceOf[Test] + println(t1.foo) // was NPE + } +} -- cgit v1.2.3 From 69c2c106fedd60f5c29a4aad696dec1de5e85200 Mon Sep 17 00:00:00 2001 From: Janek Bogucki Date: Tue, 28 Jul 2015 19:13:44 +0100 Subject: ScalaDoc fixes for library and library-aux --- src/library-aux/scala/AnyRef.scala | 4 ++-- src/library/scala/StringContext.scala | 2 +- src/library/scala/collection/GenSeqLike.scala | 2 +- src/library/scala/collection/immutable/Stream.scala | 2 +- src/library/scala/compat/Platform.scala | 2 +- src/library/scala/math/BigDecimal.scala | 2 +- src/library/scala/reflect/Manifest.scala | 4 ++-- test/scaladoc/resources/doc-root/AnyRef.scala | 4 ++-- 8 files changed, 11 insertions(+), 11 deletions(-) (limited to 'test') diff --git a/src/library-aux/scala/AnyRef.scala b/src/library-aux/scala/AnyRef.scala index 8c1862e729..7217499da7 100644 --- a/src/library-aux/scala/AnyRef.scala +++ b/src/library-aux/scala/AnyRef.scala @@ -45,7 +45,7 @@ trait AnyRef extends Any { */ def synchronized[T](body: => T): T - /** Tests whether the argument (`arg0`) is a reference to the receiver object (`this`). + /** Tests whether the argument (`that`) is a reference to the receiver object (`this`). * * The `eq` method implements an [[http://en.wikipedia.org/wiki/Equivalence_relation equivalence relation]] on * non-null instances of `AnyRef`, and has three additional properties: @@ -73,7 +73,7 @@ trait AnyRef extends Any { /** The expression `x == that` is equivalent to `if (x eq null) that eq null else x.equals(that)`. * - * @param arg0 the object to compare against this object for equality. + * @param that the object to compare against this object for equality. * @return `true` if the receiver object is equivalent to the argument; `false` otherwise. */ final def ==(that: Any): Boolean = diff --git a/src/library/scala/StringContext.scala b/src/library/scala/StringContext.scala index e60fa2f290..69533c12da 100644 --- a/src/library/scala/StringContext.scala +++ b/src/library/scala/StringContext.scala @@ -173,7 +173,7 @@ object StringContext { /** An exception that is thrown if a string contains a backslash (`\`) character * that does not start a valid escape sequence. * @param str The offending string - * @param idx The index of the offending backslash character in `str`. + * @param index The index of the offending backslash character in `str`. */ class InvalidEscapeException(str: String, @deprecatedName('idx) val index: Int) extends IllegalArgumentException( s"""invalid escape ${ diff --git a/src/library/scala/collection/GenSeqLike.scala b/src/library/scala/collection/GenSeqLike.scala index f786293822..be1da1660a 100644 --- a/src/library/scala/collection/GenSeqLike.scala +++ b/src/library/scala/collection/GenSeqLike.scala @@ -274,7 +274,7 @@ trait GenSeqLike[+A, +Repr] extends Any with GenIterableLike[A, Repr] with Equal * @tparam B the element type of the returned $coll. * @tparam That $thatinfo * @param bf $bfinfo - * @return a new $coll` which is a copy of this $coll with the element at position `index` replaced by `elem`. + * @return a new $coll which is a copy of this $coll with the element at position `index` replaced by `elem`. * @throws IndexOutOfBoundsException if `index` does not satisfy `0 <= index < length`. * * @usecase def updated(index: Int, elem: A): $Coll[A] diff --git a/src/library/scala/collection/immutable/Stream.scala b/src/library/scala/collection/immutable/Stream.scala index 17cf02cce6..d8f0559706 100644 --- a/src/library/scala/collection/immutable/Stream.scala +++ b/src/library/scala/collection/immutable/Stream.scala @@ -1190,7 +1190,7 @@ object Stream extends SeqFactory[Stream] { def #:::(prefix: Stream[A]): Stream[A] = prefix append tl } - /** A wrapper method that adds `#::` for cons and `#::: for concat as operations + /** A wrapper method that adds `#::` for cons and `#:::` for concat as operations * to streams. */ implicit def consWrapper[A](stream: => Stream[A]): ConsWrapper[A] = diff --git a/src/library/scala/compat/Platform.scala b/src/library/scala/compat/Platform.scala index 4c82d6e15b..42dfcbfdde 100644 --- a/src/library/scala/compat/Platform.scala +++ b/src/library/scala/compat/Platform.scala @@ -41,7 +41,7 @@ object Platform { * @throws java.lang.ArrayStoreException If either `src` or `dest` are not of type * [java.lang.Array]; or if the element type of `src` is not * compatible with that of `dest`. - * @throws java.lang.IndexOutOfBoundsException If either srcPos` or `destPos` are + * @throws java.lang.IndexOutOfBoundsException If either `srcPos` or `destPos` are * outside of the bounds of their respective arrays; or if `length` * is negative; or if there are less than `length` elements available * after `srcPos` or `destPos` in `src` and `dest` respectively. diff --git a/src/library/scala/math/BigDecimal.scala b/src/library/scala/math/BigDecimal.scala index 6bb35606a6..bb337e7a1d 100644 --- a/src/library/scala/math/BigDecimal.scala +++ b/src/library/scala/math/BigDecimal.scala @@ -124,7 +124,7 @@ object BigDecimal { */ def exact(s: String): BigDecimal = exact(new BigDec(s)) - /** Constructs a 'BigDecimal` that exactly represents the number + /** Constructs a `BigDecimal` that exactly represents the number * specified in base 10 in a character array. */ def exact(cs: Array[Char]): BigDecimal = exact(new BigDec(cs)) diff --git a/src/library/scala/reflect/Manifest.scala b/src/library/scala/reflect/Manifest.scala index 2f7643bccf..4ff49c44d0 100644 --- a/src/library/scala/reflect/Manifest.scala +++ b/src/library/scala/reflect/Manifest.scala @@ -248,7 +248,7 @@ object ManifestFactory { def arrayType[T](arg: Manifest[_]): Manifest[Array[T]] = arg.asInstanceOf[Manifest[T]].arrayManifest - /** Manifest for the abstract type `prefix # name'. `upperBound` is not + /** Manifest for the abstract type `prefix # name`. `upperBound` is not * strictly necessary as it could be obtained by reflection. It was * added so that erasure can be calculated without reflection. */ def abstractType[T](prefix: Manifest[_], name: String, upperBound: Predef.Class[_], args: Manifest[_]*): Manifest[T] = @@ -269,7 +269,7 @@ object ManifestFactory { (if (upperBound eq Nothing) "" else " <: "+upperBound) } - /** Manifest for the intersection type `parents_0 with ... with parents_n'. */ + /** Manifest for the intersection type `parents_0 with ... with parents_n`. */ def intersectionType[T](parents: Manifest[_]*): Manifest[T] = new Manifest[T] { def runtimeClass = parents.head.runtimeClass diff --git a/test/scaladoc/resources/doc-root/AnyRef.scala b/test/scaladoc/resources/doc-root/AnyRef.scala index 362fbcf0f5..7cdc3d1ada 100644 --- a/test/scaladoc/resources/doc-root/AnyRef.scala +++ b/test/scaladoc/resources/doc-root/AnyRef.scala @@ -45,7 +45,7 @@ trait AnyRef extends Any { */ def synchronized[T](body: => T): T - /** Tests whether the argument (`arg0`) is a reference to the receiver object (`this`). + /** Tests whether the argument (`that`) is a reference to the receiver object (`this`). * * The `eq` method implements an [[http://en.wikipedia.org/wiki/Equivalence_relation equivalence relation]] on * non-null instances of `AnyRef`, and has three additional properties: @@ -73,7 +73,7 @@ trait AnyRef extends Any { /** The expression `x == that` is equivalent to `if (x eq null) that eq null else x.equals(that)`. * - * @param arg0 the object to compare against this object for equality. + * @param that the object to compare against this object for equality. * @return `true` if the receiver object is equivalent to the argument; `false` otherwise. */ final def ==(that: AnyRef): Boolean = -- cgit v1.2.3 From ec95e534a213a6ea760aa31c507d122ce449890a Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 Jul 2015 13:29:57 +1000 Subject: SI-9422 Fix incorrect constant propagation The ConstantOptimization phase uses abstract interpretation to track what is known about values, and then to use this information to optimize away tests with a statically known result. Constant propagation was added under -optimize in Scala 2.11.0-M3, in PR #2214. For example, we might know that a variable must hold one of a set of values (`Possible`). Or, we could track that it must *not* be of of a set of value (`Impossible`). The test case in the bug report was enough to create comparison: v1 == v2 // where V1 = Possible(Set(true, false)) // V2 = Possible(Set(true, false)) This test was considered to be always true, due to a bug in `Possible#mightNotEqual`. The only time we can be sure that `Possible(p1) mightNotEquals Possible(p2)` is if `p1` and `p2` are the same singleton set. This commit changes this method to implement this logic. The starting assumption for all values is currently `Impossible(Set())`, although it would also be reasonable to represent an unknown boolean variable as `Possible(Set(true, false))`, given the finite and small domain. I tried to change the starting assumption for boolean locals in exactly this manner, and that brings the bug into sharp focus. Under this patch: https://github.com/retronym/scala/commit/e564fe522d This code: def test(a: Boolean, b: Boolean) = a == b Compiles to: public boolean test(boolean, boolean); Code: 0: iconst_1 1: ireturn Note: the enclosed test case does not list `-optimize` in a `.flags` file, I'm relying on that being passed in by the validation build. I've tested locally with that option, though. --- .../scala/tools/nsc/backend/opt/ConstantOptimization.scala | 8 +++++--- test/files/run/t9422.scala | 11 +++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 test/files/run/t9422.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala b/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala index 0e6ee76eb2..fb1799e092 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala @@ -170,9 +170,11 @@ abstract class ConstantOptimization extends SubComponent { // out all the possibilities case Impossible(possible2) => (possible -- possible2).nonEmpty }) - def mightNotEqual(other: Contents): Boolean = (this ne other) && (other match { - // two Possibles might not be equal if either has possible members that the other doesn't - case Possible(possible2) => (possible -- possible2).nonEmpty || (possible2 -- possible).nonEmpty + def mightNotEqual(other: Contents): Boolean = (other match { + case Possible(possible2) => + // two Possibles must equal if each is known to be of the same, single value + val mustEqual = possible.size == 1 && possible == possible2 + !mustEqual case Impossible(_) => true }) } diff --git a/test/files/run/t9422.scala b/test/files/run/t9422.scala new file mode 100644 index 0000000000..5ca2e8daaa --- /dev/null +++ b/test/files/run/t9422.scala @@ -0,0 +1,11 @@ +class Test(val x: Long) { + def sameDirection(y: Long): Boolean = + (y == 0 || x == 0 || ((y > 0) == (x > 0))) +} + +object Test { + def main(args: Array[String]) { + val b = new Test(1L) + assert(!b.sameDirection(-1L)) + } +} -- cgit v1.2.3