From 731ed385dea0196305a0c527303649ea0325de63 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 15 Jan 2014 16:00:30 +0100 Subject: SI-8134 SI-5954 Fix companions in package object under separate comp. The tests cases enclosed exhibited two failures modes under separate compilation. 1. When a synthetic companion object for a case- or implicit-class defined in a package object is called for, `Namer#ensureCompanionObject` is used to check for an explicitly defined companion before decided to create a synthetic one. This lookup of an existing companion symbol by `companionObjectOf` would locate a symbol backed by a class file which was in the scope of the enclosing package class. Furthermore, because the owner of that symbol is the package object class that has now been noted as corresponding to a source file in the current run, the class-file backed module symbol is *also* deemed to be from the current run. (This logic is in `Run#compiles`.) Thinking the companion module already existed, no synthetic module was created, which would lead to a crash in extension methods, which needs to add methods to it. 2. In cases when the code explicitly contains the companion pair, we still ran into problems in the backend whereby the class-file based and source-file based symbols for the module ended up in the same scope (of the package class). This tripped an assertion in `Symbol#companionModule`. We get into these problems because of the eager manner in which class-file based package object are opened in `openPackageModule`. The members of the module are copied into the scope of the enclosing package: scala> ScalaPackage.info.member(nme.List) res0: $r#59116.intp#45094.global#28436.Symbol#29451 = value List#2462 scala> ScalaPackage.info.member(nme.PACKAGE).info.member(nme.List) res1: $r#59116.intp#45094.global#28436.Symbol#29451 = value List#2462 This seems to require a two-pronged defense: 1. When we attach a pre-existing symbol for a package object symbol to the tree of its new source, unlink the "forwarder" symbols (its decls from the enclosing package class. 2. In `Flatten`, in the spirit of `replaceSymbolInCurrentScope`, remove static member modules from the scope of the enclosing package object (aka `exitingFlatten(nestedModule.owner)`). This commit also removes the warnings about defining companions in package objects and converts those neg tests to pos (with -Xfatal-warnings to prove they are warning free.) Defining nested classes/objects in package objects still has a drawback: you can't shift a class from the package to the package object, or vice versa, in a binary compatible manner, because of the `package$` prefix on the flattened name of nested classes. For this reason, the `-Xlint` warning about this remains. This issue is tracked as SI-4344. However, if one heeds this warning and incrementatlly recompiles, we no longer need to run into a DoubleDefinition error (which was dressed up with a more specific diagnostic in SI-5760.) The neg test case for that bug has been converted to a pos. --- test/files/neg/package-ob-case.check | 10 ----- test/files/neg/package-ob-case.flags | 1 - test/files/neg/package-ob-case.scala | 5 --- test/files/neg/t5760-pkgobj-warn.check | 4 -- test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala | 11 ------ test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala | 11 ------ test/files/neg/t5954.check | 18 --------- test/files/neg/t5954.flags | 1 - test/files/neg/t5954.scala | 46 ----------------------- test/files/pos/package-ob-case.flags | 1 + test/files/pos/package-ob-case/A_1.scala | 5 +++ test/files/pos/package-ob-case/B_2.scala | 5 +++ test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala | 11 ++++++ test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala | 11 ++++++ test/files/pos/t5954a/A_1.scala | 6 +++ test/files/pos/t5954a/B_2.scala | 6 +++ test/files/pos/t5954b/A_1.scala | 6 +++ test/files/pos/t5954b/B_2.scala | 5 +++ test/files/pos/t5954c.flags | 1 + test/files/pos/t5954c/A_1.scala | 18 +++++++++ test/files/pos/t5954c/B_2.scala | 18 +++++++++ test/files/pos/t8134/A_1.scala | 4 ++ test/files/pos/t8134/B_2.scala | 4 ++ 23 files changed, 101 insertions(+), 107 deletions(-) delete mode 100644 test/files/neg/package-ob-case.check delete mode 100644 test/files/neg/package-ob-case.flags delete mode 100644 test/files/neg/package-ob-case.scala delete mode 100644 test/files/neg/t5760-pkgobj-warn.check delete mode 100644 test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala delete mode 100644 test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala delete mode 100644 test/files/neg/t5954.check delete mode 100644 test/files/neg/t5954.flags delete mode 100644 test/files/neg/t5954.scala create mode 100644 test/files/pos/package-ob-case.flags create mode 100644 test/files/pos/package-ob-case/A_1.scala create mode 100644 test/files/pos/package-ob-case/B_2.scala create mode 100644 test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala create mode 100644 test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala create mode 100644 test/files/pos/t5954a/A_1.scala create mode 100644 test/files/pos/t5954a/B_2.scala create mode 100644 test/files/pos/t5954b/A_1.scala create mode 100644 test/files/pos/t5954b/B_2.scala create mode 100644 test/files/pos/t5954c.flags create mode 100644 test/files/pos/t5954c/A_1.scala create mode 100644 test/files/pos/t5954c/B_2.scala create mode 100644 test/files/pos/t8134/A_1.scala create mode 100644 test/files/pos/t8134/B_2.scala (limited to 'test') diff --git a/test/files/neg/package-ob-case.check b/test/files/neg/package-ob-case.check deleted file mode 100644 index 9b0ede1c6d..0000000000 --- a/test/files/neg/package-ob-case.check +++ /dev/null @@ -1,10 +0,0 @@ -package-ob-case.scala:3: warning: it is not recommended to define classes/objects inside of package objects. -If possible, define class X in package foo instead. - case class X(z: Int) { } - ^ -package-ob-case.scala:3: warning: class X should be placed directly in package foo instead of package object foo. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - case class X(z: Int) { } - ^ -error: No warnings can be incurred under -Xfatal-warnings. -two warnings found -one error found diff --git a/test/files/neg/package-ob-case.flags b/test/files/neg/package-ob-case.flags deleted file mode 100644 index 6c1dd108ae..0000000000 --- a/test/files/neg/package-ob-case.flags +++ /dev/null @@ -1 +0,0 @@ --Xfatal-warnings -Xlint \ No newline at end of file diff --git a/test/files/neg/package-ob-case.scala b/test/files/neg/package-ob-case.scala deleted file mode 100644 index 91a1fb7e48..0000000000 --- a/test/files/neg/package-ob-case.scala +++ /dev/null @@ -1,5 +0,0 @@ -package foo { - package object foo { - case class X(z: Int) { } - } -} diff --git a/test/files/neg/t5760-pkgobj-warn.check b/test/files/neg/t5760-pkgobj-warn.check deleted file mode 100644 index a89398c3f7..0000000000 --- a/test/files/neg/t5760-pkgobj-warn.check +++ /dev/null @@ -1,4 +0,0 @@ -stalepkg_2.scala:6: error: Foo is already defined as class Foo in package object stalepkg - class Foo - ^ -one error found diff --git a/test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala b/test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala deleted file mode 100644 index ed4b731bb0..0000000000 --- a/test/files/neg/t5760-pkgobj-warn/stalepkg_1.scala +++ /dev/null @@ -1,11 +0,0 @@ - -package object stalepkg { - class Foo -} - -package stalepkg { - object Test { - def main(args: Array[String]) { - } - } -} diff --git a/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala b/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala deleted file mode 100644 index 9abcdbab17..0000000000 --- a/test/files/neg/t5760-pkgobj-warn/stalepkg_2.scala +++ /dev/null @@ -1,11 +0,0 @@ - -package object stalepkg { -} - -package stalepkg { - class Foo - object Test { - def main(args: Array[String]) { - } - } -} diff --git a/test/files/neg/t5954.check b/test/files/neg/t5954.check deleted file mode 100644 index 3950d14e4e..0000000000 --- a/test/files/neg/t5954.check +++ /dev/null @@ -1,18 +0,0 @@ -t5954.scala:36: warning: class D should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - case class D() - ^ -t5954.scala:35: warning: object C should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - object C - ^ -t5954.scala:34: warning: trait C should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - trait C - ^ -t5954.scala:33: warning: object B should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - object B - ^ -t5954.scala:32: warning: class B should be placed directly in package A instead of package object A. Under some circumstances companion objects and case classes in package objects can fail to recompile. See https://issues.scala-lang.org/browse/SI-5954. - class B - ^ -error: No warnings can be incurred under -Xfatal-warnings. -5 warnings found -one error found diff --git a/test/files/neg/t5954.flags b/test/files/neg/t5954.flags deleted file mode 100644 index 85d8eb2ba2..0000000000 --- a/test/files/neg/t5954.flags +++ /dev/null @@ -1 +0,0 @@ --Xfatal-warnings diff --git a/test/files/neg/t5954.scala b/test/files/neg/t5954.scala deleted file mode 100644 index 3ccb5ed3ff..0000000000 --- a/test/files/neg/t5954.scala +++ /dev/null @@ -1,46 +0,0 @@ -// if you ever think you've fixed the underlying reason for the warning -// imposed by SI-5954, then here's a test that should pass with two "succes"es -// -//import scala.tools.partest._ -// -//object Test extends DirectTest { -// def code = ??? -// -// def problemCode = """ -// package object A { -// class B -// object B -// case class C() -// } -// """ -// -// def compileProblemCode() = { -// val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") -// compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(problemCode) -// } -// -// def show() : Unit = { -// for (i <- 0 until 2) { -// compileProblemCode() -// println(s"success ${i + 1}") -// } -// } -//} - -package object A { - // these should be prevented by the implementation restriction - class B - object B - trait C - object C - case class D() - // all the rest of these should be ok - class E - object F - val g = "omg" - var h = "wtf" - def i = "lol" - type j = String - class K(val k : Int) extends AnyVal - implicit class L(val l : Int) -} diff --git a/test/files/pos/package-ob-case.flags b/test/files/pos/package-ob-case.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/package-ob-case.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/package-ob-case/A_1.scala b/test/files/pos/package-ob-case/A_1.scala new file mode 100644 index 0000000000..91a1fb7e48 --- /dev/null +++ b/test/files/pos/package-ob-case/A_1.scala @@ -0,0 +1,5 @@ +package foo { + package object foo { + case class X(z: Int) { } + } +} diff --git a/test/files/pos/package-ob-case/B_2.scala b/test/files/pos/package-ob-case/B_2.scala new file mode 100644 index 0000000000..91a1fb7e48 --- /dev/null +++ b/test/files/pos/package-ob-case/B_2.scala @@ -0,0 +1,5 @@ +package foo { + package object foo { + case class X(z: Int) { } + } +} diff --git a/test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala b/test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala new file mode 100644 index 0000000000..ed4b731bb0 --- /dev/null +++ b/test/files/pos/t5760-pkgobj-warn/stalepkg_1.scala @@ -0,0 +1,11 @@ + +package object stalepkg { + class Foo +} + +package stalepkg { + object Test { + def main(args: Array[String]) { + } + } +} diff --git a/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala b/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala new file mode 100644 index 0000000000..9abcdbab17 --- /dev/null +++ b/test/files/pos/t5760-pkgobj-warn/stalepkg_2.scala @@ -0,0 +1,11 @@ + +package object stalepkg { +} + +package stalepkg { + class Foo + object Test { + def main(args: Array[String]) { + } + } +} diff --git a/test/files/pos/t5954a/A_1.scala b/test/files/pos/t5954a/A_1.scala new file mode 100644 index 0000000000..10ead0b1ca --- /dev/null +++ b/test/files/pos/t5954a/A_1.scala @@ -0,0 +1,6 @@ +package p1 { + object `package` { + implicit class Foo(a: Any) + object Foo + } +} diff --git a/test/files/pos/t5954a/B_2.scala b/test/files/pos/t5954a/B_2.scala new file mode 100644 index 0000000000..10ead0b1ca --- /dev/null +++ b/test/files/pos/t5954a/B_2.scala @@ -0,0 +1,6 @@ +package p1 { + object `package` { + implicit class Foo(a: Any) + object Foo + } +} diff --git a/test/files/pos/t5954b/A_1.scala b/test/files/pos/t5954b/A_1.scala new file mode 100644 index 0000000000..8465e8f8c6 --- /dev/null +++ b/test/files/pos/t5954b/A_1.scala @@ -0,0 +1,6 @@ +package p { + package object base { + class B + object B + } +} diff --git a/test/files/pos/t5954b/B_2.scala b/test/files/pos/t5954b/B_2.scala new file mode 100644 index 0000000000..f7e4704b3e --- /dev/null +++ b/test/files/pos/t5954b/B_2.scala @@ -0,0 +1,5 @@ +package p { + package object base { + case class B() + } +} diff --git a/test/files/pos/t5954c.flags b/test/files/pos/t5954c.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/pos/t5954c.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/pos/t5954c/A_1.scala b/test/files/pos/t5954c/A_1.scala new file mode 100644 index 0000000000..29ad9547a2 --- /dev/null +++ b/test/files/pos/t5954c/A_1.scala @@ -0,0 +1,18 @@ +package object A { + // these used to should be prevented by the implementation restriction + // but are now allowed + class B + object B + trait C + object C + case class D() + // all the rest of these should be ok + class E + object F + val g = "omg" + var h = "wtf" + def i = "lol" + type j = String + class K(val k : Int) extends AnyVal + implicit class L(val l : Int) +} diff --git a/test/files/pos/t5954c/B_2.scala b/test/files/pos/t5954c/B_2.scala new file mode 100644 index 0000000000..29ad9547a2 --- /dev/null +++ b/test/files/pos/t5954c/B_2.scala @@ -0,0 +1,18 @@ +package object A { + // these used to should be prevented by the implementation restriction + // but are now allowed + class B + object B + trait C + object C + case class D() + // all the rest of these should be ok + class E + object F + val g = "omg" + var h = "wtf" + def i = "lol" + type j = String + class K(val k : Int) extends AnyVal + implicit class L(val l : Int) +} diff --git a/test/files/pos/t8134/A_1.scala b/test/files/pos/t8134/A_1.scala new file mode 100644 index 0000000000..32bce003fb --- /dev/null +++ b/test/files/pos/t8134/A_1.scala @@ -0,0 +1,4 @@ +// a.scala +package object pkg { + class AnyOps(val x: Any) extends AnyVal +} diff --git a/test/files/pos/t8134/B_2.scala b/test/files/pos/t8134/B_2.scala new file mode 100644 index 0000000000..32bce003fb --- /dev/null +++ b/test/files/pos/t8134/B_2.scala @@ -0,0 +1,4 @@ +// a.scala +package object pkg { + class AnyOps(val x: Any) extends AnyVal +} -- cgit v1.2.3 From 357905c9555f8081784acd1912cf0681b145ca8b Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 17 Jan 2014 09:27:55 +0100 Subject: SI-5954 Invalidate TypeRef cache when opening package object I noticed that the pos/5954d was tripping a println "assertion". This stemmed from an inconsistency between `TypeRef#{parents, baseTypeSeq}` for a package objects compiled from source that also has a class file from a previous compilation run. I've elevated the println to a devWarning, and changed `updatePosFlags`, the home of this evil symbol overwriting, to invalidate the caches in the symbols info. Yuck. I believe that this symptom is peculiar to package objects because of the way that the completer for packages calls `parents` during the Namer phase for package objects, before we switch the symbol to represent the package-object-from-source. But it seems prudent to defensively invalidate the caches for any symbol that finds its way into `updatePosFlags`. --- src/compiler/scala/tools/nsc/typechecker/Namers.scala | 7 +++++++ src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 2 +- src/reflect/scala/reflect/internal/Types.scala | 4 ++++ test/files/pos/t5954d.flags | 1 + test/files/pos/t5954d/A_1.scala | 6 ++++++ test/files/pos/t5954d/B_2.scala | 7 +++++++ 6 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/t5954d.flags create mode 100644 test/files/pos/t5954d/A_1.scala create mode 100644 test/files/pos/t5954d/B_2.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 48c2c55d7c..fd093614b5 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -148,6 +148,13 @@ trait Namers extends MethodSynthesis { def updatePosFlags(sym: Symbol, pos: Position, flags: Long): Symbol = { debuglog("[overwrite] " + sym) val newFlags = (sym.flags & LOCKED) | flags + sym.rawInfo match { + case tr: TypeRef => + // !!! needed for: pos/t5954d; the uniques type cache will happilly serve up the same TypeRef + // over this mutated symbol, and we witness a stale cache for `parents`. + tr.invalidateCaches() + case _ => + } sym reset NoType setFlag newFlags setPos pos sym.moduleClass andAlso (updatePosFlags(_, pos, moduleClassFlags(flags))) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 95f2620061..f5893172e8 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -857,7 +857,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans val baseClass = clazz.info.baseTypeSeq(i).typeSymbol seenTypes(i) match { case Nil => - println("??? base "+baseClass+" not found in basetypes of "+clazz) + devWarning(s"base $baseClass not found in basetypes of $clazz. This might indicate incorrect caching of TypeRef#parents.") case _ :: Nil => ;// OK case tp1 :: tp2 :: _ => diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 99e6ae633f..437b962abf 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -2218,6 +2218,10 @@ trait Types trivial = fromBoolean(!sym.isTypeParameter && pre.isTrivial && areTrivialTypes(args)) toBoolean(trivial) } + private[scala] def invalidateCaches(): Unit = { + parentsPeriod = NoPeriod + baseTypeSeqPeriod = NoPeriod + } private[reflect] var parentsCache: List[Type] = _ private[reflect] var parentsPeriod = NoPeriod private[reflect] var baseTypeSeqCache: BaseTypeSeq = _ diff --git a/test/files/pos/t5954d.flags b/test/files/pos/t5954d.flags new file mode 100644 index 0000000000..6ced0e7090 --- /dev/null +++ b/test/files/pos/t5954d.flags @@ -0,0 +1 @@ +-Xfatal-warnings -Xdev diff --git a/test/files/pos/t5954d/A_1.scala b/test/files/pos/t5954d/A_1.scala new file mode 100644 index 0000000000..8465e8f8c6 --- /dev/null +++ b/test/files/pos/t5954d/A_1.scala @@ -0,0 +1,6 @@ +package p { + package object base { + class B + object B + } +} diff --git a/test/files/pos/t5954d/B_2.scala b/test/files/pos/t5954d/B_2.scala new file mode 100644 index 0000000000..a4aa2eb587 --- /dev/null +++ b/test/files/pos/t5954d/B_2.scala @@ -0,0 +1,7 @@ +package p { + trait T { + class B + object B + } + package object base extends T +} -- cgit v1.2.3 From 95f21ca8a095767202e1c4d620a865c1647d7e6c Mon Sep 17 00:00:00 2001 From: Rex Kerr Date: Thu, 30 Jan 2014 12:28:14 -0800 Subject: SI-6736 Range.contains is wrong Removed once-used private method that was calculating ranges in error and corrected the contains method (plus improved performance). --- src/library/scala/collection/immutable/Range.scala | 18 +++++++++++------- test/junit/scala/collection/NumericRangeTest.scala | 7 ++++++- 2 files changed, 17 insertions(+), 8 deletions(-) (limited to 'test') diff --git a/src/library/scala/collection/immutable/Range.scala b/src/library/scala/collection/immutable/Range.scala index 786b18cd21..ba695dfbdc 100644 --- a/src/library/scala/collection/immutable/Range.scala +++ b/src/library/scala/collection/immutable/Range.scala @@ -203,12 +203,6 @@ extends scala.collection.AbstractSeq[Int] } counted } - // Tests whether a number is within the endpoints, without testing - // whether it is a member of the sequence (i.e. when step > 1.) - private def isWithinBoundaries(elem: Int) = !isEmpty && ( - (step > 0 && start <= elem && elem <= last ) || - (step < 0 && last <= elem && elem <= start) - ) // Methods like apply throw exceptions on invalid n, but methods like take/drop // are forgiving: therefore the checks are with the methods. private def locationAfterN(n: Int) = start + (step * n) @@ -256,7 +250,17 @@ extends scala.collection.AbstractSeq[Int] if (isInclusive) this else new Range.Inclusive(start, end, step) - final def contains(x: Int) = isWithinBoundaries(x) && ((x - start) % step == 0) + final def contains(x: Int) = { + if (x==end && !isInclusive) false + else if (step > 0) { + if (x < start || x > end) false + else (step == 1) || (((x - start) % step) == 0) + } + else { + if (x < end || x > start) false + else (step == -1) || (((x - start) % step) == 0) + } + } final override def sum[B >: Int](implicit num: Numeric[B]): Int = { if (num eq scala.math.Numeric.IntIsIntegral) { diff --git a/test/junit/scala/collection/NumericRangeTest.scala b/test/junit/scala/collection/NumericRangeTest.scala index 0260723b9d..f03bf1c498 100644 --- a/test/junit/scala/collection/NumericRangeTest.scala +++ b/test/junit/scala/collection/NumericRangeTest.scala @@ -6,7 +6,7 @@ import org.junit.Test import scala.math._ import scala.util._ -/* Tests various maps by making sure they all agree on the same answers. */ +/* Tests various ranges by making sure they all agree on the same answers. */ @RunWith(classOf[JUnit4]) class RangeConsistencyTest { def r2nr[T: Integral]( @@ -120,4 +120,9 @@ class RangeConsistencyTest { case _ => false } }} + + @Test + def testSI6736() { assert{ + (0 to Int.MaxValue).contains(4) && !((Int.MinValue to 0).contains(4)) + } } } -- cgit v1.2.3 From e152297090c26051b7e9a6a1740c4670d23d9d5d Mon Sep 17 00:00:00 2001 From: Rex Kerr Date: Fri, 31 Jan 2014 17:18:29 -0800 Subject: Reasonable Range operations consistently work when overfull. Operations are reasonable when they don't require indexing or conversion into a collection. These include head, tail, init, last, drop, take, dropWhile, takeWhile, dropRight, takeRight, span. Tests added also to verify the new behavior. --- src/library/scala/collection/immutable/Range.scala | 129 +++++++++++++++++---- test/files/scalacheck/range.scala | 3 +- test/junit/scala/collection/NumericRangeTest.scala | 18 ++- 3 files changed, 122 insertions(+), 28 deletions(-) (limited to 'test') diff --git a/src/library/scala/collection/immutable/Range.scala b/src/library/scala/collection/immutable/Range.scala index ba695dfbdc..26ccd09803 100644 --- a/src/library/scala/collection/immutable/Range.scala +++ b/src/library/scala/collection/immutable/Range.scala @@ -23,6 +23,15 @@ import scala.collection.parallel.immutable.ParRange * println(r2.length) // = 5 * }}} * + * Ranges that contain more than `Int.MaxValue` elements can be created, but + * these overfull ranges have only limited capabilities. Any method that + * could require a collection of over `Int.MaxValue` length to be created, or + * could be asked to index beyond `Int.MaxValue` elements will throw an + * exception. Overfull ranges can safely be reduced in size by changing + * the step size (e.g. `by 3`) or taking/dropping elements. `contains`, + * `equals`, and access to the ends of the range (`head`, `last`, `tail`, + * `init`) are also permitted on overfull ranges. + * * @param start the start of this range. * @param end the exclusive end of the range. * @param step the step for the range. @@ -77,10 +86,24 @@ extends scala.collection.AbstractSeq[Int] } } @deprecated("This method will be made private, use `last` instead.", "2.11") - final val lastElement = start + (numRangeElements - 1) * step + final val lastElement = + if (isEmpty) start - step + else step match { + case 1 => if (isInclusive) end else end-1 + case -1 => if (isInclusive) end else end+1 + case _ => + val remainder = (gap % step).toInt + if (remainder != 0) end - remainder + else if (isInclusive) end + else end - step + } + @deprecated("This method will be made private.", "2.11") - final val terminalElement = start + numRangeElements * step + final val terminalElement = lastElement + step + /** The last element of this range. This method will return the correct value + * even if there are too many elements to iterate over. + */ override def last = if (isEmpty) Nil.last else lastElement override def head = if (isEmpty) Nil.head else start @@ -149,8 +172,12 @@ extends scala.collection.AbstractSeq[Int] */ final override def take(n: Int): Range = ( if (n <= 0 || isEmpty) newEmptyRange(start) - else if (n >= numRangeElements) this - else new Range.Inclusive(start, locationAfterN(n - 1), step) + else if (n >= numRangeElements && numRangeElements >= 0) this + else { + // May have more than Int.MaxValue elements in range (numRangeElements < 0) + // but the logic is the same either way: take the first n + new Range.Inclusive(start, locationAfterN(n - 1), step) + } ) /** Creates a new range containing all the elements of this range except the first `n` elements. @@ -162,8 +189,12 @@ extends scala.collection.AbstractSeq[Int] */ final override def drop(n: Int): Range = ( if (n <= 0 || isEmpty) this - else if (n >= numRangeElements) newEmptyRange(end) - else copy(locationAfterN(n), end, step) + else if (n >= numRangeElements && numRangeElements >= 0) newEmptyRange(end) + else { + // May have more than Int.MaxValue elements (numRangeElements < 0) + // but the logic is the same either way: go forwards n steps, keep the rest + copy(locationAfterN(n), end, step) + } ) /** Creates a new range containing all the elements of this range except the last one. @@ -192,16 +223,16 @@ extends scala.collection.AbstractSeq[Int] drop(1) } - // Counts how many elements from the start meet the given test. - private def skipCount(p: Int => Boolean): Int = { - var current = start - var counted = 0 - - while (counted < numRangeElements && p(current)) { - counted += 1 - current += step + // Advance from the start while we meet the given test + private def argTakeWhile(p: Int => Boolean): Long = { + if (isEmpty) start + else { + var current = start + val stop = last + while (current != stop && p(current)) current += step + if (current != stop || !p(current)) current + else current.toLong + step } - counted } // Methods like apply throw exceptions on invalid n, but methods like take/drop // are forgiving: therefore the checks are with the methods. @@ -213,9 +244,33 @@ extends scala.collection.AbstractSeq[Int] // based on the given value. private def newEmptyRange(value: Int) = new Range(value, value, step) - final override def takeWhile(p: Int => Boolean): Range = take(skipCount(p)) - final override def dropWhile(p: Int => Boolean): Range = drop(skipCount(p)) - final override def span(p: Int => Boolean): (Range, Range) = splitAt(skipCount(p)) + final override def takeWhile(p: Int => Boolean): Range = { + val stop = argTakeWhile(p) + if (stop==start) newEmptyRange(start) + else { + val x = (stop - step).toInt + if (x == last) this + else new Range.Inclusive(start, x, step) + } + } + final override def dropWhile(p: Int => Boolean): Range = { + val stop = argTakeWhile(p) + if (stop == start) this + else { + val x = (stop - step).toInt + if (x == last) newEmptyRange(last) + else new Range.Inclusive(x + step, last, step) + } + } + final override def span(p: Int => Boolean): (Range, Range) = { + val border = argTakeWhile(p) + if (border == start) (newEmptyRange(start), this) + else { + val x = (border - step).toInt + if (x == last) (this, newEmptyRange(last)) + else (new Range.Inclusive(start, x, step), new Range.Inclusive(x+step, last, step)) + } + } /** Creates a pair of new ranges, first consisting of elements before `n`, and the second * of elements after `n`. @@ -228,13 +283,32 @@ extends scala.collection.AbstractSeq[Int] * * $doesNotUseBuilders */ - final override def takeRight(n: Int): Range = drop(numRangeElements - n) + final override def takeRight(n: Int): Range = { + if (n <= 0) newEmptyRange(start) + else if (numRangeElements >= 0) drop(numRangeElements - n) + else { + // Need to handle over-full range separately + val y = last + val x = y - step.toLong*(n-1) + if ((step > 0 && x < start) || (step < 0 && x > start)) this + else new Range.Inclusive(x.toInt, y, step) + } + } /** Creates a new range consisting of the initial `length - n` elements of the range. * * $doesNotUseBuilders */ - final override def dropRight(n: Int): Range = take(numRangeElements - n) + final override def dropRight(n: Int): Range = { + if (n <= 0) this + else if (numRangeElements >= 0) take(numRangeElements - n) + else { + // Need to handle over-full range separately + val y = last - step.toInt*n + if ((step > 0 && y < start) || (step < 0 && y > start)) newEmptyRange(start) + else new Range.Inclusive(start, y.toInt, step) + } + } /** Returns the reverse of this range. * @@ -289,9 +363,15 @@ extends scala.collection.AbstractSeq[Int] override def equals(other: Any) = other match { case x: Range => - (x canEqual this) && (length == x.length) && ( - isEmpty || // all empty sequences are equal - (start == x.start && last == x.last) // same length and same endpoints implies equality + // Note: this must succeed for overfull ranges (length > Int.MaxValue) + (x canEqual this) && ( + isEmpty || // all empty sequences are equal + (start == x.start && { // Otherwise, must have same start + val l0 = last + (l0 == x.last && ( // And same end + start == l0 || step == x.step // And either the same step, or not take any steps + )) + }) ) case _ => super.equals(other) @@ -301,7 +381,8 @@ extends scala.collection.AbstractSeq[Int] */ override def toString() = { - val endStr = if (numRangeElements > Range.MAX_PRINT) ", ... )" else ")" + val endStr = + if (numRangeElements > Range.MAX_PRINT || (!isEmpty && numRangeElements < 0)) ", ... )" else ")" take(Range.MAX_PRINT).mkString("Range(", ", ", endStr) } } diff --git a/test/files/scalacheck/range.scala b/test/files/scalacheck/range.scala index 1eb186f303..493083a51f 100644 --- a/test/files/scalacheck/range.scala +++ b/test/files/scalacheck/range.scala @@ -265,7 +265,8 @@ object TooLargeRange extends Properties("Too Large Range") { property("Too large range throws exception") = forAll(genTooLargeStart) { start => try { val r = Range.inclusive(start, Int.MaxValue, 1) - println("how here? r = " + r.toString) + val l = r.length + println("how here? length = " + l + ", r = " + r.toString) false } catch { case _: IllegalArgumentException => true } diff --git a/test/junit/scala/collection/NumericRangeTest.scala b/test/junit/scala/collection/NumericRangeTest.scala index f03bf1c498..3980c31577 100644 --- a/test/junit/scala/collection/NumericRangeTest.scala +++ b/test/junit/scala/collection/NumericRangeTest.scala @@ -122,7 +122,19 @@ class RangeConsistencyTest { }} @Test - def testSI6736() { assert{ - (0 to Int.MaxValue).contains(4) && !((Int.MinValue to 0).contains(4)) - } } + def testSI6736() { + // These operations on overfull ranges should all succeed. + assert( (0 to Int.MaxValue).contains(4) ) + assert( !((Int.MinValue to 0).contains(4)) ) + assert( (Int.MinValue to 0).last == 0 ) + assert( (Int.MinValue until 5).last == 4 ) + assert( (-7 to -99 by -4).last == -99 && (-7 until -99 by -4).last == -95 ) + assert( (Int.MinValue to 5) == (Int.MinValue until 6) ) + assert( (-3 to Int.MaxValue).drop(4).length == Int.MaxValue ) + assert( (-3 to Int.MaxValue).take(1234) == (-3 to 1230) ) + assert( (-3 to Int.MaxValue).dropRight(4).length == Int.MaxValue ) + assert( (-3 to Int.MaxValue).takeRight(1234).length == 1234 ) + assert( (-3 to Int.MaxValue).dropWhile(_ <= 0).length == Int.MaxValue ) + assert( (-3 to Int.MaxValue).span(_ <= 0) match { case (a,b) => a.length == 4 && b.length == Int.MaxValue } ) + } } -- cgit v1.2.3 From d187a0ab9973a5e4042597a9dbf4f6d48ca482fe Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 11 Feb 2014 19:57:59 -0800 Subject: SI-261 private vals in traits depend on composition order Fix for SI-7475 in #3440 took care of this one. I nixed the old (redundant) test cases and replaced by a single run/ test, which didn't compile until. --- test/files/pos/t261-ab.scala | 9 --------- test/files/pos/t261-ba.scala | 9 --------- test/files/run/t261.check | 2 ++ test/files/run/t261.scala | 11 +++++++++++ 4 files changed, 13 insertions(+), 18 deletions(-) delete mode 100644 test/files/pos/t261-ab.scala delete mode 100644 test/files/pos/t261-ba.scala create mode 100644 test/files/run/t261.check create mode 100644 test/files/run/t261.scala (limited to 'test') diff --git a/test/files/pos/t261-ab.scala b/test/files/pos/t261-ab.scala deleted file mode 100644 index df641e811a..0000000000 --- a/test/files/pos/t261-ab.scala +++ /dev/null @@ -1,9 +0,0 @@ -trait A { val foo: String = "A" } -trait B { - private val foo: String = "B" - def f = println(foo) -} -object Test extends App with B with A { - println(foo) // prints "A", as expected - f // prints "B", as expected -} diff --git a/test/files/pos/t261-ba.scala b/test/files/pos/t261-ba.scala deleted file mode 100644 index 6c9c5b10b7..0000000000 --- a/test/files/pos/t261-ba.scala +++ /dev/null @@ -1,9 +0,0 @@ -trait B { - private val foo: String = "B" - def f = println(foo) -} -trait A { val foo: String = "A" } -object Test extends App with B with A { - println(foo) // prints "A", as expected - f // prints "B", as expected -} diff --git a/test/files/run/t261.check b/test/files/run/t261.check new file mode 100644 index 0000000000..35d242ba79 --- /dev/null +++ b/test/files/run/t261.check @@ -0,0 +1,2 @@ +A +B diff --git a/test/files/run/t261.scala b/test/files/run/t261.scala new file mode 100644 index 0000000000..d8ddb28c00 --- /dev/null +++ b/test/files/run/t261.scala @@ -0,0 +1,11 @@ +trait A { val foo: String = "A" } +trait B { + private val foo: String = "B" + def f = println(foo) +} +object Test extends A with B { + def main(args: Array[String]) = { + println(foo) + f + } +} \ No newline at end of file -- cgit v1.2.3 From 2d4d2d3f26e66cc1be6dc32ff37ba889bd4c8d0a Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 12 Feb 2014 17:07:05 +0100 Subject: A test case for a name binding progression I noticed the change when adapting Slick to work with Scala 2.11 in `AbstractSourceCodeGenerator.scala`. The behaviour changed in a70c8219. This commit locks down the new, correct behaviour with a test. --- test/files/neg/name-lookup-stable.check | 11 +++++++++++ test/files/neg/name-lookup-stable.scala | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 test/files/neg/name-lookup-stable.check create mode 100644 test/files/neg/name-lookup-stable.scala (limited to 'test') diff --git a/test/files/neg/name-lookup-stable.check b/test/files/neg/name-lookup-stable.check new file mode 100644 index 0000000000..751df9505e --- /dev/null +++ b/test/files/neg/name-lookup-stable.check @@ -0,0 +1,11 @@ +name-lookup-stable.scala:15: error: reference to PrimaryKey is ambiguous; +it is both defined in class A and imported subsequently by +import ColumnOption._ + (null: Any) match { case PrimaryKey => } + ^ +name-lookup-stable.scala:17: error: reference to PrimaryKey is ambiguous; +it is both defined in class A and imported subsequently by +import ColumnOption._ + PrimaryKey // was already ambigious in 2.10.3 + ^ +two errors found diff --git a/test/files/neg/name-lookup-stable.scala b/test/files/neg/name-lookup-stable.scala new file mode 100644 index 0000000000..0d862f06e1 --- /dev/null +++ b/test/files/neg/name-lookup-stable.scala @@ -0,0 +1,20 @@ +// This used to compile under 2.10.3 but the ambiguity is now noticed +// in 2.11.x (after a70c8219). I think the new behaviour is correct; +// we shouldn't discard names based on "expected stability" before +// evaluating ambiguity. +object ColumnOption { + object PrimaryKey +} + +class A { + def PrimaryKey: Any = ??? + + { + import ColumnOption._ + + (null: Any) match { case PrimaryKey => } + + PrimaryKey // was already ambigious in 2.10.3 + } +} + -- cgit v1.2.3 From 7ea7a3b89b2a597f7aa22eefe39e3bae7244882f Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 6 Feb 2014 17:59:14 -0800 Subject: SI-8177 co-evolve more than just RefinedTypes `asSeenFrom` produced a typeref of the shape `T'#A` where `A` referred to a symbol defined in a `T` of times past. More precisely, the `TypeRef` case of `TypeMap`'s `mapOver` correctly modified a prefix from having an underlying type of `{ type A = AIn }` to `{ type A = Int }`, with a new symbol for `A` (now with info `Int`), but the symbol in the outer `TypeRef` wasn't co-evolved (so it still referred to the `A` in `{ type A = AIn }` underlying the old prefix). `coEvolveSym` used to only look at prefixes that were directly `RefinedType`s, but this bug shows they could also be `SingleType`s with an underlying `RefinedType`. --- .../scala/tools/nsc/typechecker/RefChecks.scala | 3 ++ src/reflect/scala/reflect/internal/Types.scala | 49 ++++++++++++++++------ test/files/pos/t8177.scala | 11 +++++ 3 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 test/files/pos/t8177.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 2125e281f0..2b5eed7102 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -467,6 +467,9 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans // overrideError("may not override parameterized type"); // @M: substSym def checkOverrideAlias() { + // Important: first check the pair has the same kind, since the substitution + // carries high's type parameter's bounds over to low, so that + // type equality doesn't consider potentially different bounds on low/high's type params. if (pair.sameKind && lowType.substSym(low.typeParams, high.typeParams) =:= highType) () else overrideTypeError() // (1.6) } diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index cf405ade03..0ef5d60a10 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -2017,22 +2017,45 @@ trait Types // appliedType(sym.info, typeArgs).asSeenFrom(pre, sym.owner) override def betaReduce = transform(sym.info.resultType) - // #3731: return sym1 for which holds: pre bound sym.name to sym and - // pre1 now binds sym.name to sym1, conceptually exactly the same - // symbol as sym. The selection of sym on pre must be updated to the - // selection of sym1 on pre1, since sym's info was probably updated - // by the TypeMap to yield a new symbol, sym1 with transformed info. - // @returns sym1 - override def coevolveSym(pre1: Type): Symbol = - if (pre eq pre1) sym else (pre, pre1) match { - // don't look at parents -- it would be an error to override alias types anyway - case (RefinedType(_, _), RefinedType(_, decls1)) => decls1 lookup sym.name - // TODO: is there another way a typeref's symbol can refer to a symbol defined in its pre? - case _ => sym - } + /** SI-3731, SI-8177: when prefix is changed to `newPre`, maintain consistency of prefix and sym + * (where the symbol refers to a declaration "embedded" in the prefix). + * + * @returns newSym so that `newPre` binds `sym.name` to `newSym`, + * to remain consistent with `pre` previously binding `sym.name` to `sym`. + * + * `newSym` and `sym` are conceptually the same symbols, but some change to our `prefix` + * got them out of whack. (Usually triggered by substitution or `asSeenFrom`.) + * The only kind of "binds" we consider is where `prefix` (or its underlying type) + * is a refined type that declares `sym` (since the old prefix was discarded, + * the old symbol is now stale and we should update it, like in `def rebind`, + * except this is not for overriding symbols -- a vertical move -- but a "lateral" change.) + * + * The reason for this hack is that substitution and asSeenFrom clone RefinedTypes and + * their members, without updating the potential references to those members -- here, we aim to patch + * this up, so that: when changing a TypeRef(pre, sym, args) to a TypeRef(pre', sym', args'), and pre + * embeds a symbol sym (pre is a RefinedType(_, Scope(..., sym,...)) or a SingleType with such an + * underlying type), make sure that we update sym' to compensate for the change of pre -> pre' (which may + * have created a new symbol for the one the original sym referred to) + */ + override def coevolveSym(newPre: Type): Symbol = + if ((pre ne newPre) && embeddedSymbol(pre, sym.name) == sym) { + val newSym = embeddedSymbol(newPre, sym.name) + debuglog(s"co-evolve: ${pre} -> ${newPre}, $sym : ${sym.info} -> $newSym : ${newSym.info}") + // To deal with erroneous `preNew`, fallback via `orElse sym`, in case `preNew` does not have a decl named `sym.name`. + newSym orElse sym + } else sym + override def kind = "AliasTypeRef" } + // Return the symbol named `name` that's "embedded" in tp + // This is the case if `tp` is a `T{...; type/val $name ; ...}`, + // or a singleton type with such an underlying type. + private def embeddedSymbol(tp: Type, name: Name): Symbol = tp.widen match { + case RefinedType(_, decls) => decls lookup name + case _ => NoSymbol + } + trait AbstractTypeRef extends NonClassTypeRef { require(sym.isAbstractType, sym) diff --git a/test/files/pos/t8177.scala b/test/files/pos/t8177.scala new file mode 100644 index 0000000000..9bdf80b1bb --- /dev/null +++ b/test/files/pos/t8177.scala @@ -0,0 +1,11 @@ +trait Thing { type A } +object IntThing extends Thing { type A = Int } + +// The following erroneously failed with error: method f overrides nothing. +// because asSeenFrom produced a typeref of the shape T'#A where A referred to a symbol defined in a T of times past +// More precisely, the TypeRef case of TypeMap's mapOver correctly modified prefix +// from having an underlying type of { type A = Ain } to { type A = Int }, with a new symbol for A (now with info Int), +// but the symbol in the outer type ref wasn't co-evolved (so it still referred to the { type A = AIn } underlying the old prefix) +// coEvolveSym used to only look at prefixes that were directly RefinedTypes, but they could also be SingleTypes with an underlying RefinedType +class View[AIn](val in: Thing { type A = AIn }) { def f(p: in.A): in.A = p } +class SubView extends View[Int](IntThing) { override def f(p: in.A): in.A = p } -- cgit v1.2.3 From 427b82648422e4118c68f34e81c94deca3755deb Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Mon, 10 Feb 2014 13:43:30 -0800 Subject: SI-8177 refine embeddedSymbols We look for any prefix that has a refinement class for a type symbol. This includes ThisTypes, which were not considered before. pos/t8177g.scala, neg/t0764*scala now compile, as they should Additional test cases contributed by Jason & Paul. --- .../scala/tools/nsc/typechecker/RefChecks.scala | 2 + src/reflect/scala/reflect/internal/Types.scala | 14 +++-- test/files/neg/t0764.check | 7 --- test/files/neg/t0764.scala | 14 ----- test/files/neg/t0764b.check | 63 --------------------- test/files/neg/t0764b.scala | 64 ---------------------- test/files/pos/t0764.scala | 27 +++++++++ test/files/pos/t0764b.scala | 61 +++++++++++++++++++++ test/files/pos/t8177.scala | 1 + test/files/pos/t8177a.scala | 9 +++ test/files/pos/t8177b.scala | 13 +++++ test/files/pos/t8177d.scala | 12 ++++ test/files/pos/t8177e.scala | 3 + test/files/pos/t8177g.scala | 11 ++++ test/files/run/t8177f.scala | 20 +++++++ 15 files changed, 169 insertions(+), 152 deletions(-) delete mode 100644 test/files/neg/t0764.check delete mode 100644 test/files/neg/t0764.scala delete mode 100644 test/files/neg/t0764b.check delete mode 100644 test/files/neg/t0764b.scala create mode 100644 test/files/pos/t0764.scala create mode 100644 test/files/pos/t0764b.scala create mode 100644 test/files/pos/t8177a.scala create mode 100644 test/files/pos/t8177b.scala create mode 100644 test/files/pos/t8177d.scala create mode 100644 test/files/pos/t8177e.scala create mode 100644 test/files/pos/t8177g.scala create mode 100644 test/files/run/t8177f.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 2b5eed7102..02dd63f011 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -470,6 +470,8 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans // Important: first check the pair has the same kind, since the substitution // carries high's type parameter's bounds over to low, so that // type equality doesn't consider potentially different bounds on low/high's type params. + // In b781e25afe this went from using memberInfo to memberType (now lowType/highType), tested by neg/override.scala. + // TODO: was that the right fix? it seems type alias's RHS should be checked by looking at the symbol's info if (pair.sameKind && lowType.substSym(low.typeParams, high.typeParams) =:= highType) () else overrideTypeError() // (1.6) } diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 0ef5d60a10..218ad28a03 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -2051,10 +2051,16 @@ trait Types // Return the symbol named `name` that's "embedded" in tp // This is the case if `tp` is a `T{...; type/val $name ; ...}`, // or a singleton type with such an underlying type. - private def embeddedSymbol(tp: Type, name: Name): Symbol = tp.widen match { - case RefinedType(_, decls) => decls lookup name - case _ => NoSymbol - } + private def embeddedSymbol(tp: Type, name: Name): Symbol = + // normalize to flatten nested RefinedTypes + // don't check whether tp is a RefinedType -- it may be a ThisType of one, for example + // TODO: check the resulting symbol is owned by the refinement class? likely an invariant... + if (tp.typeSymbol.isRefinementClass) tp.normalize.decls lookup name + else { + debuglog(s"no embedded symbol $name found in ${showRaw(tp)} --> ${tp.normalize.decls lookup name}") + NoSymbol + } + trait AbstractTypeRef extends NonClassTypeRef { require(sym.isAbstractType, sym) diff --git a/test/files/neg/t0764.check b/test/files/neg/t0764.check deleted file mode 100644 index 6156b52712..0000000000 --- a/test/files/neg/t0764.check +++ /dev/null @@ -1,7 +0,0 @@ -t0764.scala:13: error: type mismatch; - found : Node{type T = _1.type} where val _1: Node{type T = NextType} - required: Node{type T = Main.this.AType} - (which expands to) Node{type T = Node{type T = NextType}} - new Main[AType]( (value: AType).prepend ) - ^ -one error found diff --git a/test/files/neg/t0764.scala b/test/files/neg/t0764.scala deleted file mode 100644 index f2cc65cf7d..0000000000 --- a/test/files/neg/t0764.scala +++ /dev/null @@ -1,14 +0,0 @@ -class Top[A] { - type AType = A -} - -trait Node { outer => - type T <: Node - def prepend = new Node { type T = outer.type } -} - -class Main[NextType <: Node](value: Node { type T = NextType }) - extends Top[Node { type T = NextType }] { - - new Main[AType]( (value: AType).prepend ) -} diff --git a/test/files/neg/t0764b.check b/test/files/neg/t0764b.check deleted file mode 100644 index d74a9efbfe..0000000000 --- a/test/files/neg/t0764b.check +++ /dev/null @@ -1,63 +0,0 @@ -t0764b.scala:27: error: type mismatch; - found : p1.t0764.Node{type T = p1.t0764..type} - required: p1.t0764.NodeAlias[p1.t0764.NodeAlias[A]] - (which expands to) p1.t0764.Node{type T = p1.t0764.Node{type T = A}} - private[this] def f1 = new Main1(v.prepend) // fail - ^ -t0764b.scala:28: error: type mismatch; - found : p1.t0764.Node{type T = p1.t0764..type} - required: p1.t0764.NodeAlias[p1.t0764.NodeAlias[A]] - (which expands to) p1.t0764.Node{type T = p1.t0764.Node{type T = A}} - private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail - ^ -t0764b.scala:29: error: type mismatch; - found : p1.t0764.Node{type T = p1.t0764..type} - required: p1.t0764.NodeAlias[p1.t0764.Node{type T = A}] - (which expands to) p1.t0764.Node{type T = p1.t0764.Node{type T = A}} - private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail - ^ -t0764b.scala:34: error: type mismatch; - found : p1.t0764.Node{type T = p1.t0764..type} - required: p1.t0764.Node{type T = p1.t0764.Node{type T = A}} - private[this] def f1 = new Main2(v.prepend) // fail - ^ -t0764b.scala:35: error: type mismatch; - found : p1.t0764.Node{type T = p1.t0764..type} - required: p1.t0764.Node{type T = p1.t0764.NodeAlias[A]} - (which expands to) p1.t0764.Node{type T = p1.t0764.Node{type T = A}} - private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail - ^ -t0764b.scala:36: error: type mismatch; - found : p1.t0764.Node{type T = p1.t0764..type} - required: p1.t0764.Node{type T = p1.t0764.Node{type T = A}} - private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail - ^ -t0764b.scala:52: error: type mismatch; - found : p2.t0764.Node{type T = p2.t0764..type} - required: p2.t0764.NodeAlias[p2.t0764.NodeAlias[A]] - (which expands to) p2.t0764.Node{type T = p2.t0764.Node{type T = A}} - private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail - ^ -t0764b.scala:53: error: type mismatch; - found : p2.t0764.Node{type T = p2.t0764..type} - required: p2.t0764.NodeAlias[p2.t0764.Node{type T = A}] - (which expands to) p2.t0764.Node{type T = p2.t0764.Node{type T = A}} - private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail - ^ -t0764b.scala:58: error: type mismatch; - found : p2.t0764.Node{type T = p2.t0764..type} - required: p2.t0764.Node{type T = p2.t0764.Node{type T = A}} - private[this] def f1 = new Main2(v.prepend) // fail - ^ -t0764b.scala:59: error: type mismatch; - found : p2.t0764.Node{type T = p2.t0764..type} - required: p2.t0764.Node{type T = p2.t0764.NodeAlias[A]} - (which expands to) p2.t0764.Node{type T = p2.t0764.Node{type T = A}} - private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail - ^ -t0764b.scala:60: error: type mismatch; - found : p2.t0764.Node{type T = p2.t0764..type} - required: p2.t0764.Node{type T = p2.t0764.Node{type T = A}} - private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail - ^ -11 errors found diff --git a/test/files/neg/t0764b.scala b/test/files/neg/t0764b.scala deleted file mode 100644 index 4ad5ecdc03..0000000000 --- a/test/files/neg/t0764b.scala +++ /dev/null @@ -1,64 +0,0 @@ -/** Note that this should compile! It's a neg test to track the -behavior. If you have broken this test by making it compile, that -means you have fixed it and it should be moved to pos. -**/ - -// In all cases when calling "prepend" the receiver 'v' -// has static type NodeAlias[A] or (equivalently) Node { type T = A }. -// Since prepend explicitly returns the singleton type of the receiver, -// the return type of prepend in all cases is "v.type", and so the call -// to "new Main" can be parameterized with any of the following, in order -// of decreasing specificity with a tie for second place: -// -// new Main[v.type](v.prepend) -// new Main[NodeAlias[A]](v.prepend) -// new Main[Node { type T = A }](v.prepend) -// new Main(v.prepend) - -package p1 { - object t0764 { - type NodeAlias[A] = Node { type T = A } - trait Node { outer => - type T <: Node - def prepend: Node { type T = outer.type } = ??? - } - - class Main1[A <: Node](v: NodeAlias[A]) { - private[this] def f1 = new Main1(v.prepend) // fail - private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main1[v.type](v.prepend) // ok - } - - class Main2[A <: Node](v: Node { type T = A }) { - private[this] def f1 = new Main2(v.prepend) // fail - private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main2[v.type](v.prepend) // ok - } - } -} - -package p2 { - object t0764 { - type NodeAlias[A] = Node { type T = A } - trait Node { outer => - type T <: Node - def prepend: NodeAlias[outer.type] = ??? - } - - class Main1[A <: Node](v: NodeAlias[A]) { - private[this] def f1 = new Main1(v.prepend) // ok! <<========== WOT - private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main1[v.type](v.prepend) // ok - } - - class Main2[A <: Node](v: Node { type T = A }) { - private[this] def f1 = new Main2(v.prepend) // fail - private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main2[v.type](v.prepend) // ok - } - } -} diff --git a/test/files/pos/t0764.scala b/test/files/pos/t0764.scala new file mode 100644 index 0000000000..f1084f5ff8 --- /dev/null +++ b/test/files/pos/t0764.scala @@ -0,0 +1,27 @@ +class Top[A] { + type AType = A +} + +trait Node { outer => + type T <: Node + def prepend = new Node { type T = outer.type } +} + +class Main[NextType <: Node](value: Node { type T = NextType }) + extends Top[Node { type T = NextType }] { + + new Main[AType]( (value: AType).prepend ) +} + +/* this used to be a neg test, even though it should've compiled +SI-8177 fixed this. + +Behold the equivalent program which type checks without the fix for SI-8177. +(Expand type alias, convert type member to type param; +note the covariance to encode subtyping on type members.) + +class Node[+T <: Node[_]] { def prepend = new Node[this.type] } +class Main[NextType <: Node[_]](value: Node[NextType]) { + new Main(value.prepend) +} +*/ \ No newline at end of file diff --git a/test/files/pos/t0764b.scala b/test/files/pos/t0764b.scala new file mode 100644 index 0000000000..6ae3c105c9 --- /dev/null +++ b/test/files/pos/t0764b.scala @@ -0,0 +1,61 @@ +// In all cases when calling "prepend" the receiver 'v' +// has static type NodeAlias[A] or (equivalently) Node { type T = A }. +// Since prepend explicitly returns the singleton type of the receiver, +// the return type of prepend in all cases is "v.type", and so the call +// to "new Main" can be parameterized with any of the following, in order +// of decreasing specificity with a tie for second place: +// +// new Main[v.type](v.prepend) +// new Main[NodeAlias[A]](v.prepend) +// new Main[Node { type T = A }](v.prepend) +// new Main(v.prepend) + +// the `fail` comments below denote what didn't compile before SI-8177 fixed all of them + +package p1 { + object t0764 { + type NodeAlias[A] = Node { type T = A } + trait Node { outer => + type T <: Node + def prepend: Node { type T = outer.type } = ??? + } + + class Main1[A <: Node](v: NodeAlias[A]) { + private[this] def f1 = new Main1(v.prepend) // fail + private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main1[v.type](v.prepend) // ok + } + + class Main2[A <: Node](v: Node { type T = A }) { + private[this] def f1 = new Main2(v.prepend) // fail + private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main2[v.type](v.prepend) // ok + } + } +} + +package p2 { + object t0764 { + type NodeAlias[A] = Node { type T = A } + trait Node { outer => + type T <: Node + def prepend: NodeAlias[outer.type] = ??? + } + + class Main1[A <: Node](v: NodeAlias[A]) { + private[this] def f1 = new Main1(v.prepend) // ok! <<========== WOT + private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main1[v.type](v.prepend) // ok + } + + class Main2[A <: Node](v: Node { type T = A }) { + private[this] def f1 = new Main2(v.prepend) // fail + private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main2[v.type](v.prepend) // ok + } + } +} diff --git a/test/files/pos/t8177.scala b/test/files/pos/t8177.scala index 9bdf80b1bb..fe265f8d0a 100644 --- a/test/files/pos/t8177.scala +++ b/test/files/pos/t8177.scala @@ -1,3 +1,4 @@ +// exercise coevolveSym: SingleType with an underlying RefinedType trait Thing { type A } object IntThing extends Thing { type A = Int } diff --git a/test/files/pos/t8177a.scala b/test/files/pos/t8177a.scala new file mode 100644 index 0000000000..7e2cfb386c --- /dev/null +++ b/test/files/pos/t8177a.scala @@ -0,0 +1,9 @@ +// exercise coevolveSym +trait Thing { type A; var p: A = _ } +class AA[T](final val x: Thing { type A = T }) { + def foo: x.A = ??? +} + +class B extends AA[Int](null) { + override def foo: B.this.x.A = super.foo +} diff --git a/test/files/pos/t8177b.scala b/test/files/pos/t8177b.scala new file mode 100644 index 0000000000..b7ed9342a3 --- /dev/null +++ b/test/files/pos/t8177b.scala @@ -0,0 +1,13 @@ +// exercise coevolveSym: SingleType with an underlying RefinedType, via a type alias +trait Thing { type A } +object IntThing extends Thing { type A = Int } +object ThingHolder { type Alias[AIn] = Thing { type A = AIn } } + +// The following erroneously failed with error: method f overrides nothing. +// because asSeenFrom produced a typeref of the shape T'#A where A referred to a symbol defined in a T of times past +// More precisely, the TypeRef case of TypeMap's mapOver correctly modified prefix +// from having an underlying type of { type A = Ain } to { type A = Int }, with a new symbol for A (now with info Int), +// but the symbol in the outer type ref wasn't co-evolved (so it still referred to the { type A = AIn } underlying the old prefix) +// coEvolveSym used to only look at prefixes that were directly RefinedTypes, but they could also be SingleTypes with an underlying RefinedType +class View[AIn](val in: ThingHolder.Alias[AIn]) { def f(p: in.A): in.A = p } +class SubView extends View[Int](IntThing) { override def f(p: in.A): in.A = p } \ No newline at end of file diff --git a/test/files/pos/t8177d.scala b/test/files/pos/t8177d.scala new file mode 100644 index 0000000000..d15a05a359 --- /dev/null +++ b/test/files/pos/t8177d.scala @@ -0,0 +1,12 @@ +// exercise coevolveSym +trait HasElem { type A } +trait View[AIn] { + val tc: HasElem { type A = AIn } + def f2(p: tc.A): tc.A = p +} + +object Test { + val view: View[Int] = null + + view f2 5 // fails +} diff --git a/test/files/pos/t8177e.scala b/test/files/pos/t8177e.scala new file mode 100644 index 0000000000..cb1136ff11 --- /dev/null +++ b/test/files/pos/t8177e.scala @@ -0,0 +1,3 @@ +// exercise coevolveSym +trait T[A] { val foo: { type B = A } = ???; def bar(b: foo.B) = () } +object O extends T[Int] { bar(0) } diff --git a/test/files/pos/t8177g.scala b/test/files/pos/t8177g.scala new file mode 100644 index 0000000000..bb66d32021 --- /dev/null +++ b/test/files/pos/t8177g.scala @@ -0,0 +1,11 @@ +// exercise coevolveSym: ThisType +trait HasA { type A } +class AA[T] { + type HasAT[T] = HasA{ type A = T } + val x: HasAT[T] = ??? + def foo: x.A = ??? +} + +class B extends AA[Int] { + override def foo: B.this.x.A = super.foo +} \ No newline at end of file diff --git a/test/files/run/t8177f.scala b/test/files/run/t8177f.scala new file mode 100644 index 0000000000..f50a5d98d6 --- /dev/null +++ b/test/files/run/t8177f.scala @@ -0,0 +1,20 @@ +trait Thing { type A; var p: A = _ } +class A[T](final val x: Thing { type A = T }) { + type Q = T + + def x1: T = x.p + def x2: Q = x.p + def x3: x.A = x.p +} +// all result types should be inferred as Int +class B extends A[Int](null) { + def y1 = x1 + def y2 = x2 + val y3 = x3 // before SI-8177, this lead to a signature that erased to java.lang.Object +} + + +object Test extends App { + val methods = classOf[B].getDeclaredMethods.sortBy(_.getName) + assert(methods.forall(_.toGenericString.startsWith("public int"))) +} -- cgit v1.2.3 From d5a1ea61871ad695c67a2165d0e569f304e63662 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Mon, 10 Feb 2014 16:16:37 -0800 Subject: SI-7753 InstantiateDependentMap narrows type of unstable args [Most of this comment and the initial fix were implemented by Jason Zaugg. I just cleaned it up a bit.] After a soundness fix in SI-3873, instantiation of dependent method type results behaved differently depending on whether the argument from which we were propagating information had a stable type or not. This is particular to substitution into singleton types over the parameter in question. If the argument was stable, it was substituted into singleton types, such as the one below in the prefix in `a.type#B` (which is the longhand version of `a.B`) scala> class A { type B >: Null <: AnyRef } defined class A scala> object AA extends A { type B = String } defined object AA scala> def foo(a: A): a.B = null foo: (a: A)a.B scala> foo(AA) res0: AA.B = null But what if it isn't stable? scala> foo({def a = AA; a: A { type B <: String}}) res1: a.B = null This commit changes that to: scala> foo({def a = AA; a: A { type B <: String}}) res1: A{type B <: String}#B = null --- .../scala/reflect/internal/tpe/TypeMaps.scala | 81 ++++++++++++++-------- test/files/neg/t3873.check | 4 +- test/files/neg/t3873.scala | 2 +- test/files/pos/t7753.scala | 36 ++++++++++ test/files/pos/t8223.scala | 29 ++++++++ 5 files changed, 119 insertions(+), 33 deletions(-) create mode 100644 test/files/pos/t7753.scala create mode 100644 test/files/pos/t8223.scala (limited to 'test') diff --git a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala index f427813c01..07c9242bf3 100644 --- a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala +++ b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala @@ -863,31 +863,24 @@ private[internal] trait TypeMaps { private val existentials = new Array[Symbol](actuals.size) def existentialsNeeded: List[Symbol] = existentials.iterator.filter(_ ne null).toList - private object StableArg { - def unapply(param: Symbol) = Arg unapply param map actuals filter (tp => - tp.isStable && (tp.typeSymbol != NothingClass) - ) - } - private object Arg { - def unapply(param: Symbol) = Some(params indexOf param) filter (_ >= 0) - } - - def apply(tp: Type): Type = mapOver(tp) match { - // unsound to replace args by unstable actual #3873 - case SingleType(NoPrefix, StableArg(arg)) => arg - // (soundly) expand type alias selections on implicit arguments, - // see depmet_implicit_oopsla* test cases -- typically, `param.isImplicit` - case tp1 @ TypeRef(SingleType(NoPrefix, Arg(pid)), sym, targs) => - val arg = actuals(pid) - val res = typeRef(arg, sym, targs) - if (res.typeSymbolDirect.isAliasType) res.dealias else tp1 - // don't return the original `tp`, which may be different from `tp1`, - // due to dropping annotations - case tp1 => tp1 + private object StableArgTp { + // type of actual arg corresponding to param -- if the type is stable + def unapply(param: Symbol): Option[Type] = (params indexOf param) match { + case -1 => None + case pid => + val tp = actuals(pid) + if (tp.isStable && (tp.typeSymbol != NothingClass)) Some(tp) + else None + } } - /* Return the type symbol for referencing a parameter inside the existential quantifier. - * (Only needed if the actual is unstable.) + /** Return the type symbol for referencing a parameter that's instantiated to an unstable actual argument. + * + * To soundly abstract over an unstable value (x: T) while retaining the most type information, + * use `x.type forSome { type x.type <: T with Singleton}` + * `typeOf[T].narrowExistentially(symbolOf[x])`. + * + * See also: captureThis in AsSeenFromMap. */ private def existentialFor(pid: Int) = { if (existentials(pid) eq null) { @@ -900,6 +893,38 @@ private[internal] trait TypeMaps { existentials(pid) } + private object UnstableArgTp { + // existential quantifier and type of corresponding actual arg with unstable type + def unapply(param: Symbol): Option[(Symbol, Type)] = (params indexOf param) match { + case -1 => None + case pid => + val sym = existentialFor(pid) + Some((sym, sym.tpe_*)) // refers to an actual value, must be kind-* + } + } + + private object StabilizedArgTp { + def unapply(param: Symbol): Option[Type] = + param match { + case StableArgTp(tp) => Some(tp) // (1) + case UnstableArgTp(_, tp) => Some(tp) // (2) + case _ => None + } + } + + /** instantiate `param.type` to the (sound approximation of the) type `T` + * of the actual argument `arg` that was passed in for `param` + * + * (1) If `T` is stable, we can just use that. + * + * (2) SI-3873: it'd be unsound to instantiate `param.type` to an unstable `T`, + * so we approximate to `X forSome {type X <: T with Singleton}` -- we can't soundly say more. + */ + def apply(tp: Type): Type = tp match { + case SingleType(NoPrefix, StabilizedArgTp(tp)) => tp + case _ => mapOver(tp) + } + //AM propagate more info to annotations -- this seems a bit ad-hoc... (based on code by spoon) override def mapOver(arg: Tree, giveup: ()=>Nothing): Tree = { // TODO: this should be simplified; in the stable case, one can @@ -922,13 +947,9 @@ private[internal] trait TypeMaps { // Both examples are from run/constrained-types.scala. object treeTrans extends Transformer { override def transform(tree: Tree): Tree = tree.symbol match { - case StableArg(actual) => - gen.mkAttributedQualifier(actual, tree.symbol) - case Arg(pid) => - val sym = existentialFor(pid) - Ident(sym) copyAttrs tree setType typeRef(NoPrefix, sym, Nil) - case _ => - super.transform(tree) + case StableArgTp(tp) => gen.mkAttributedQualifier(tp, tree.symbol) + case UnstableArgTp(quant, tp) => Ident(quant) copyAttrs tree setType tp + case _ => super.transform(tree) } } treeTrans transform arg diff --git a/test/files/neg/t3873.check b/test/files/neg/t3873.check index 54d6abdf63..f9f413aeaf 100644 --- a/test/files/neg/t3873.check +++ b/test/files/neg/t3873.check @@ -1,6 +1,6 @@ t3873.scala:11: error: type mismatch; found : Test.a.B - required: a.B - wrongf(new A)(a.b) // should not compile -- TODO: improve error message? the "a" is ambiguous + required: a.B where val a: A + wrongf(new A)(a.b) // should not compile ^ one error found diff --git a/test/files/neg/t3873.scala b/test/files/neg/t3873.scala index e7815f0937..b27b4e9c9d 100644 --- a/test/files/neg/t3873.scala +++ b/test/files/neg/t3873.scala @@ -8,5 +8,5 @@ object Test { val a = new A wrongf(a)(a.b) - wrongf(new A)(a.b) // should not compile -- TODO: improve error message? the "a" is ambiguous + wrongf(new A)(a.b) // should not compile } \ No newline at end of file diff --git a/test/files/pos/t7753.scala b/test/files/pos/t7753.scala new file mode 100644 index 0000000000..93ad23f114 --- /dev/null +++ b/test/files/pos/t7753.scala @@ -0,0 +1,36 @@ +import scala.language.{ higherKinds, implicitConversions } + +trait Foo { type Out } + +trait SI { + val instance: Foo + type Out +} + +object Test { + def test { + def indirect(si: SI)(v: si.instance.Out) = v + + val foo: Foo { type Out = Int } = ??? + def conv(i: Foo): SI { type Out = i.Out; val instance: i.type } = ??? + + val converted = conv(foo) + + val v1: Int = indirect(converted)(23) // Okay (after refining the return type `instance` in the return type of `conv`) + /* + indirect(converted){(v: converted.instance.Out)converted.instance.Out}( + 23{Int(23)} + ){converted.instance.Out}; + */ + + val v2: Int = indirect(conv(foo))(23) // Used to fail as follows: + /* + indirect( + conv(foo){si.SI{type Out = foo.Out; val instance: si.Test..type}} + ){(v: si.instance.Out)si.instance.Out}( + 23{} + ){}; + */ + + } +} diff --git a/test/files/pos/t8223.scala b/test/files/pos/t8223.scala new file mode 100644 index 0000000000..52d6b0098e --- /dev/null +++ b/test/files/pos/t8223.scala @@ -0,0 +1,29 @@ +package p { + class ViewEnv[AIn] { + type A = AIn + class SubView { def has(x: A): Boolean = ??? } + def get: SubView = new SubView + } + + trait HasA { type A } + trait Indexable[R] extends HasA + class ArrayTC[AIn] extends Indexable[Array[AIn]] { type A = AIn } +} + +package object p { + implicit def arrayTypeClass[A] : ArrayTC[A] = new ArrayTC[A] + object intArrayTC extends ArrayTC[Int] + + type EnvAlias[W <: HasA] = ViewEnv[W#A] + type SubAlias[W <: HasA] = ViewEnv[W#A]#SubView + + def f0[R](xs: R)(implicit tc: Indexable[R]): ViewEnv[tc.A]#SubView = new ViewEnv[tc.A]() get + def f1[R](xs: R)(implicit tc: Indexable[R]): EnvAlias[tc.type]#SubView = new ViewEnv[tc.A]() get + def f2[R](xs: R)(implicit tc: Indexable[R]): SubAlias[tc.type] = new ViewEnv[tc.A]() get + + def g0 = f0(Array(1)) has 2 // ok + def g1 = f1(Array(1)) has 2 // ok + def g2 = f2(Array(1)) has 2 // "found: Int(2), required: tc.A" + def g3 = f2(Array(1))(new ArrayTC[Int]) has 2 // "found: Int(2), required: tc.A" + def g4 = f2(Array(1))(intArrayTC) has 2 // ok +} -- cgit v1.2.3 From f62e280825422c1e64c2427bc5958869662701ca Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 7 Feb 2014 11:52:55 +0100 Subject: SI-8244 Fix raw type regression under separate compilation In #1901, handling of raw types encountered in signatures during class file parsing was changed to work in the same manner as `classExistentialType`, by using `existentialAbstraction(cls.tparms, cls.tpe_*)` But this never creates fresh existential symbols, and just sticks the class type parameters it `quantified`: scala> trait T[A <: String] defined trait T scala> val cls = typeOf[T[_]].typeSymbol cls = trait T#101864 scala> cls.typeParams res0 = List(type A#101865) scala> cls.tpe_* res1 = T#101864[A#101865] scala> classExistentialType(cls) res3 = T#101864[_ <: String#7209] scala> val ExistentialType(quantified, result) = res3 List(type A#101865) In the enclosed test case, this class type parameter was substituted during `typeOf[X] memberType sym`, which led us unsoundly thinking that `Raw[_]` was `Raw[X]`. I've added a TODO comment to review the other usages of `classExistentialType`. Test variations include joint and separate compilation, and the corresponding Scala-only code. All fail with type errors now, as we expect. I've also added a distillation of a bootstrap error that failed when I forgot to wrap the `existentialType`. --- .../tools/nsc/symtab/classfile/ClassfileParser.scala | 7 +++++-- src/reflect/scala/reflect/internal/Definitions.scala | 9 ++++++--- test/files/neg/t8244.check | 4 ++++ test/files/neg/t8244/Raw_1.java | 4 ++++ test/files/neg/t8244/Test_2.scala | 12 ++++++++++++ test/files/neg/t8244b.check | 4 ++++ test/files/neg/t8244b.scala | 18 ++++++++++++++++++ test/files/neg/t8244c.check | 4 ++++ test/files/neg/t8244c.scala | 18 ++++++++++++++++++ test/files/neg/t8244e.check | 4 ++++ test/files/neg/t8244e/Raw.java | 4 ++++ test/files/neg/t8244e/Test.scala | 12 ++++++++++++ test/files/pos/t8244d/InodeBase_1.java | 6 ++++++ test/files/pos/t8244d/Test_2.scala | 3 +++ 14 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 test/files/neg/t8244.check create mode 100644 test/files/neg/t8244/Raw_1.java create mode 100644 test/files/neg/t8244/Test_2.scala create mode 100644 test/files/neg/t8244b.check create mode 100644 test/files/neg/t8244b.scala create mode 100644 test/files/neg/t8244c.check create mode 100644 test/files/neg/t8244c.scala create mode 100644 test/files/neg/t8244e.check create mode 100644 test/files/neg/t8244e/Raw.java create mode 100644 test/files/neg/t8244e/Test.scala create mode 100644 test/files/pos/t8244d/InodeBase_1.java create mode 100644 test/files/pos/t8244d/Test_2.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala index 664645e53e..2f9cc01c0b 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala @@ -665,8 +665,11 @@ abstract class ClassfileParser { // so have to check unsafeTypeParams.isEmpty before worrying about raw type case below, // or we'll create a boatload of needless existentials. else if (classSym.isMonomorphicType || classSym.unsafeTypeParams.isEmpty) tp - // raw type - existentially quantify all type parameters - else debuglogResult(s"raw type from $classSym")(unsafeClassExistentialType(classSym)) + else debuglogResult(s"raw type from $classSym"){ + // raw type - existentially quantify all type parameters + val eparams = typeParamsToExistentials(classSym, classSym.unsafeTypeParams) + newExistentialType(eparams, typeRef(pre, classSym, eparams.map(_.tpeHK))) + } case tp => assert(sig.charAt(index) != '<', s"sig=$sig, index=$index, tp=$tp") tp diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 78e639fdff..645d6aa4ff 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -898,12 +898,15 @@ trait Definitions extends api.StandardDefinitions { * * C[E1, ..., En] forSome { E1 >: LB1 <: UB1 ... en >: LBn <: UBn }. */ + // TODO Review the way this is used. I see two potential problems: + // 1. `existentialAbstraction` here doesn't create fresh existential type symbols, it just + // uses the class type parameter symbols directly as the list of quantified symbols. + // See SI-8244 for the trouble that this can cause. + // Compare with callers of `typeParamsToExistentials` (used in Java raw type handling) + // 2. Why don't we require a prefix? Could its omission lead to wrong results in CheckabilityChecker? def classExistentialType(clazz: Symbol): Type = existentialAbstraction(clazz.typeParams, clazz.tpe_*) - def unsafeClassExistentialType(clazz: Symbol): Type = - existentialAbstraction(clazz.unsafeTypeParams, clazz.tpe_*) - // members of class scala.Any // TODO these aren't final! They are now overriden in AnyRef/Object. Prior to the fix diff --git a/test/files/neg/t8244.check b/test/files/neg/t8244.check new file mode 100644 index 0000000000..90b2bf6f46 --- /dev/null +++ b/test/files/neg/t8244.check @@ -0,0 +1,4 @@ +Test_2.scala:9: error: value exxx is not a member of ?0 + raw.t.exxx // java.lang.ClassCastException: java.lang.String cannot be cast to X + ^ +one error found diff --git a/test/files/neg/t8244/Raw_1.java b/test/files/neg/t8244/Raw_1.java new file mode 100644 index 0000000000..0c667f1106 --- /dev/null +++ b/test/files/neg/t8244/Raw_1.java @@ -0,0 +1,4 @@ +public abstract class Raw_1{ + public Raw_1 raw() { return new Raw_1() { public String t() { return ""; } }; } + public abstract T t(); +} diff --git a/test/files/neg/t8244/Test_2.scala b/test/files/neg/t8244/Test_2.scala new file mode 100644 index 0000000000..152bb0b870 --- /dev/null +++ b/test/files/neg/t8244/Test_2.scala @@ -0,0 +1,12 @@ +class X extends Raw_1[X] { + override def t = this + def exxx = 0 +} + +object Test extends App { + def c(s: X) = { + val raw = s.raw + raw.t.exxx // java.lang.ClassCastException: java.lang.String cannot be cast to X + } + c(new X()) +} diff --git a/test/files/neg/t8244b.check b/test/files/neg/t8244b.check new file mode 100644 index 0000000000..f6cbf99eb5 --- /dev/null +++ b/test/files/neg/t8244b.check @@ -0,0 +1,4 @@ +t8244b.scala:15: error: value exxx is not a member of _$1 + raw.t.exxx + ^ +one error found diff --git a/test/files/neg/t8244b.scala b/test/files/neg/t8244b.scala new file mode 100644 index 0000000000..2fb4f451a1 --- /dev/null +++ b/test/files/neg/t8244b.scala @@ -0,0 +1,18 @@ +class Raw_1[T]{ + def raw(): Raw_1[_] = { new Raw_1[String] { def t() = "" } } + def t(): T +} + + +class X extends Raw_1[X] { + override def t = this + def exxx = 0 +} + +object Test extends App { + def c(s: X) = { + val raw = s.raw + raw.t.exxx + } + c(new X()) +} diff --git a/test/files/neg/t8244c.check b/test/files/neg/t8244c.check new file mode 100644 index 0000000000..fd58a5847c --- /dev/null +++ b/test/files/neg/t8244c.check @@ -0,0 +1,4 @@ +t8244c.scala:15: error: value exxx is not a member of _$1 + raw.t.exxx + ^ +one error found diff --git a/test/files/neg/t8244c.scala b/test/files/neg/t8244c.scala new file mode 100644 index 0000000000..2fb4f451a1 --- /dev/null +++ b/test/files/neg/t8244c.scala @@ -0,0 +1,18 @@ +class Raw_1[T]{ + def raw(): Raw_1[_] = { new Raw_1[String] { def t() = "" } } + def t(): T +} + + +class X extends Raw_1[X] { + override def t = this + def exxx = 0 +} + +object Test extends App { + def c(s: X) = { + val raw = s.raw + raw.t.exxx + } + c(new X()) +} diff --git a/test/files/neg/t8244e.check b/test/files/neg/t8244e.check new file mode 100644 index 0000000000..ebd74036e5 --- /dev/null +++ b/test/files/neg/t8244e.check @@ -0,0 +1,4 @@ +Test.scala:9: error: value exxx is not a member of ?0 + raw.t.exxx // java.lang.ClassCastException: java.lang.String cannot be cast to X + ^ +one error found diff --git a/test/files/neg/t8244e/Raw.java b/test/files/neg/t8244e/Raw.java new file mode 100644 index 0000000000..53202e319d --- /dev/null +++ b/test/files/neg/t8244e/Raw.java @@ -0,0 +1,4 @@ +public abstract class Raw{ + public Raw raw() { return new Raw() { public String t() { return ""; } }; } + public abstract T t(); +} diff --git a/test/files/neg/t8244e/Test.scala b/test/files/neg/t8244e/Test.scala new file mode 100644 index 0000000000..ca2a90583f --- /dev/null +++ b/test/files/neg/t8244e/Test.scala @@ -0,0 +1,12 @@ +class X extends Raw[X] { + override def t = this + def exxx = 0 +} + +object Test extends App { + def c(s: X) = { + val raw = s.raw + raw.t.exxx // java.lang.ClassCastException: java.lang.String cannot be cast to X + } + c(new X()) +} diff --git a/test/files/pos/t8244d/InodeBase_1.java b/test/files/pos/t8244d/InodeBase_1.java new file mode 100644 index 0000000000..36c2123418 --- /dev/null +++ b/test/files/pos/t8244d/InodeBase_1.java @@ -0,0 +1,6 @@ +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; + +abstract class INodeBase_1 { + @SuppressWarnings("rawtypes") + public static final AtomicReferenceFieldUpdater updater = null; +} diff --git a/test/files/pos/t8244d/Test_2.scala b/test/files/pos/t8244d/Test_2.scala new file mode 100644 index 0000000000..cb39c9692c --- /dev/null +++ b/test/files/pos/t8244d/Test_2.scala @@ -0,0 +1,3 @@ +class INodeX[K, V] extends INodeBase_1[K, V] { + INodeBase_1.updater.set(this, null) +} -- cgit v1.2.3 From 509bd09a994365065550bcac4c821b15b8c6dbf0 Mon Sep 17 00:00:00 2001 From: Rex Kerr Date: Thu, 16 Jan 2014 08:23:24 -0800 Subject: SI-8153 Mutation is hard, let's go shopping with an empty list Changed the implementation of iterator to be more robust to mutation of the underlying ListBuffer. Added a test to make sure bug is gone. Fixed an "unsafe" usage of ListBuffer.iterator in the compiler, and added a comment explaining the (new) policy for iterating over a changing ListBuffer. The compiler now uses a synchronized-wrapped ArrayBuffer instead of ListBuffer, as that makes the (custom) units Iterator more natural to write (and avoids O(n) lookups). --- src/compiler/scala/tools/nsc/Global.scala | 30 +++++++++++++++++----- .../scala/collection/mutable/ListBuffer.scala | 27 ++++++++++--------- test/files/run/t8153.check | 1 + test/files/run/t8153.scala | 14 ++++++++++ 4 files changed, 51 insertions(+), 21 deletions(-) create mode 100644 test/files/run/t8153.check create mode 100644 test/files/run/t8153.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 1617db7517..81e96b76ac 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -1214,7 +1214,26 @@ class Global(var currentSettings: Settings, var reporter: Reporter) /** Have we already supplemented the error message of a compiler crash? */ private[nsc] final var supplementedError = false - private val unitbuf = new mutable.ListBuffer[CompilationUnit] + private class SyncedCompilationBuffer { self => + private val underlying = new mutable.ArrayBuffer[CompilationUnit] + def size = synchronized { underlying.size } + def +=(cu: CompilationUnit): this.type = { synchronized { underlying += cu }; this } + def head: CompilationUnit = synchronized{ underlying.head } + def apply(i: Int): CompilationUnit = synchronized { underlying(i) } + def iterator: Iterator[CompilationUnit] = new collection.AbstractIterator[CompilationUnit] { + private var used = 0 + def hasNext = self.synchronized{ used < underlying.size } + def next = self.synchronized { + if (!hasNext) throw new NoSuchElementException("next on empty Iterator") + used += 1 + underlying(used-1) + } + } + def toList: List[CompilationUnit] = synchronized{ underlying.toList } + } + + private val unitbuf = new SyncedCompilationBuffer + val compiledFiles = new mutable.HashSet[String] /** A map from compiled top-level symbols to their source files */ @@ -1225,9 +1244,8 @@ class Global(var currentSettings: Settings, var reporter: Reporter) private var phasec: Int = 0 // phases completed private var unitc: Int = 0 // units completed this phase - private var _unitbufSize = 0 - def size = _unitbufSize + def size = unitbuf.size override def toString = "scalac Run for:\n " + compiledFiles.toList.sorted.mkString("\n ") // Calculate where to stop based on settings -Ystop-before or -Ystop-after. @@ -1452,7 +1470,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter) /** add unit to be compiled in this run */ private def addUnit(unit: CompilationUnit) { unitbuf += unit - _unitbufSize += 1 // counting as they're added so size is cheap compiledFiles += unit.source.file.path } private def checkDeprecatedSettings(unit: CompilationUnit) { @@ -1468,11 +1485,10 @@ class Global(var currentSettings: Settings, var reporter: Reporter) /* !!! Note: changing this to unitbuf.toList.iterator breaks a bunch of tests in tests/res. This is bad, it means the resident compiler relies on an iterator of a mutable data structure reflecting changes - made to the underlying structure (in whatever accidental way it is - currently depending upon.) + made to the underlying structure. */ def units: Iterator[CompilationUnit] = unitbuf.iterator - + def registerPickle(sym: Symbol): Unit = () /** does this run compile given class, module, or case factory? */ diff --git a/src/library/scala/collection/mutable/ListBuffer.scala b/src/library/scala/collection/mutable/ListBuffer.scala index 7f54692c8b..e76825cea9 100644 --- a/src/library/scala/collection/mutable/ListBuffer.scala +++ b/src/library/scala/collection/mutable/ListBuffer.scala @@ -381,6 +381,12 @@ final class ListBuffer[A] this } + /** Returns an iterator over this `ListBuffer`. The iterator will reflect + * changes made to the underlying `ListBuffer` beyond the next element; + * the next element's value is cached so that `hasNext` and `next` are + * guaranteed to be consistent. In particular, an empty `ListBuffer` + * will give an empty iterator even if the `ListBuffer` is later filled. + */ override def iterator: Iterator[A] = new AbstractIterator[A] { // Have to be careful iterating over mutable structures. // This used to have "(cursor ne last0)" as part of its hasNext @@ -389,22 +395,15 @@ final class ListBuffer[A] // a structure while iterating, but we should never return hasNext == true // on exhausted iterators (thus creating exceptions) merely because // values were changed in-place. - var cursor: List[A] = null - var delivered = 0 - - // Note: arguably this should not be a "dynamic test" against - // the present length of the buffer, but fixed at the size of the - // buffer when the iterator is created. At the moment such a - // change breaks tests: see comment on def units in Global.scala. - def hasNext: Boolean = delivered < ListBuffer.this.length + var cursor: List[A] = if (ListBuffer.this.isEmpty) Nil else start + + def hasNext: Boolean = cursor ne Nil def next(): A = - if (!hasNext) - throw new NoSuchElementException("next on empty Iterator") + if (!hasNext) throw new NoSuchElementException("next on empty Iterator") else { - if (cursor eq null) cursor = start - else cursor = cursor.tail - delivered += 1 - cursor.head + val ans = cursor.head + cursor = cursor.tail + ans } } diff --git a/test/files/run/t8153.check b/test/files/run/t8153.check new file mode 100644 index 0000000000..0cfbf08886 --- /dev/null +++ b/test/files/run/t8153.check @@ -0,0 +1 @@ +2 diff --git a/test/files/run/t8153.scala b/test/files/run/t8153.scala new file mode 100644 index 0000000000..f9b223f974 --- /dev/null +++ b/test/files/run/t8153.scala @@ -0,0 +1,14 @@ +object Test { + def f() = { + val lb = scala.collection.mutable.ListBuffer[Int](1, 2) + val it = lb.iterator + if (it.hasNext) it.next + val xs = lb.toList + lb += 3 + it.mkString + } + + def main(args: Array[String]) { + println(f()) + } +} -- cgit v1.2.3 From b4e1a308f81d48b72ba90b7a8594759f27e1d8f3 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 12 Feb 2014 14:05:01 +0100 Subject: SI-5900 Pending test to show that SI-7886 persists qbin/scalac test/pending/neg/t7886b.scala && qbin/scala Test java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String at Test$$anon$1.accept(t7886b.scala:15) at Test$.g(t7886b.scala:9) --- test/pending/neg/t7886b.scala | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 test/pending/neg/t7886b.scala (limited to 'test') diff --git a/test/pending/neg/t7886b.scala b/test/pending/neg/t7886b.scala new file mode 100644 index 0000000000..1db8be9821 --- /dev/null +++ b/test/pending/neg/t7886b.scala @@ -0,0 +1,23 @@ +trait Covariant[+A] +trait Contra[-A] { def accept(p: A): Unit } +trait Invariant[A] extends Covariant[A] with Contra[A] + +trait T +case class Unravel[A](m: Contra[A], msg: A) extends T + +object Test extends Covariant[Any] { + def g(m: Contra[Any]): Unit = m accept 5 + def f(x: T): Unit = x match { + case Unravel(m, msg) => g(m) + case _ => + } + def main(args: Array[String]) { + f(Unravel[String](new Contra[String] { def accept(x: String) = x.length }, "")) + } +} +// java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String +// at Test$$anon$1.accept(a.scala:18) +// at Test$.g(a.scala:13) +// at Test$.f(a.scala:15) +// at Test$.main(a.scala:18) +// at Test.main(a.scala) -- cgit v1.2.3 From c956a27c3278b99d45676c268955a9e58a1ed15c Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 12 Feb 2014 14:41:36 +0100 Subject: SI-5900 Fix pattern inference regression This commit does not close SI-5900. It only addresses a regression in 2.11 prereleases caused by SI-7886. The fix for SI-7886 was incomplete (as shown by the last commit) and incorrect (as shown by the regression in pos/t5900a.scala and the fact it ended up inferring type parameters.) I believe that the key to fixing this problem will be unifying the inference of case class constructor patterns and extractor patterns. I've explored that idea: https://gist.github.com/retronym/7704153 https://github.com/retronym/scala/compare/ticket/5900 But didn't quite get there. --- .../tools/nsc/typechecker/PatternTypers.scala | 8 +++--- test/files/neg/t4818.check | 2 +- test/files/neg/t5189.check | 2 +- test/files/neg/t6680a.flags | 1 + test/files/neg/t6829.check | 12 ++++----- test/files/neg/t7886.check | 6 ----- test/files/neg/t7886.scala | 22 ---------------- test/files/pos/pattern-typing.scala | 29 ---------------------- test/files/pos/t5900a.scala | 9 +++++++ test/pending/neg/t7886.scala | 22 ++++++++++++++++ test/pending/pos/pattern-typing.scala | 29 ++++++++++++++++++++++ 11 files changed, 74 insertions(+), 68 deletions(-) create mode 100644 test/files/neg/t6680a.flags delete mode 100644 test/files/neg/t7886.check delete mode 100644 test/files/neg/t7886.scala delete mode 100644 test/files/pos/pattern-typing.scala create mode 100644 test/files/pos/t5900a.scala create mode 100644 test/pending/neg/t7886.scala create mode 100644 test/pending/pos/pattern-typing.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternTypers.scala b/src/compiler/scala/tools/nsc/typechecker/PatternTypers.scala index 41c656f8ce..cf3f265f0c 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternTypers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternTypers.scala @@ -221,10 +221,12 @@ trait PatternTypers { * see test/files/../t5189*.scala */ private def convertToCaseConstructor(tree: Tree, caseClass: Symbol, ptIn: Type): Tree = { - def untrustworthyPt = ( + // TODO SI-7886 / SI-5900 This is well intentioned but doesn't quite hit the nail on the head. + // For now, I've put it completely behind -Xstrict-inference. + val untrustworthyPt = settings.strictInference && ( ptIn =:= AnyTpe || ptIn =:= NothingTpe - || settings.strictInference && ptIn.typeSymbol != caseClass + || ptIn.typeSymbol != caseClass ) val variantToSkolem = new VariantToSkolemMap val caseClassType = tree.tpe.prefix memberType caseClass @@ -371,4 +373,4 @@ trait PatternTypers { } } } -} \ No newline at end of file +} diff --git a/test/files/neg/t4818.check b/test/files/neg/t4818.check index 8a2c024b30..a5e15e456b 100644 --- a/test/files/neg/t4818.check +++ b/test/files/neg/t4818.check @@ -1,6 +1,6 @@ t4818.scala:4: error: type mismatch; found : Int(5) - required: A + required: Nothing def f(x: Any) = x match { case Fn(f) => f(5) } ^ one error found diff --git a/test/files/neg/t5189.check b/test/files/neg/t5189.check index aecc1d11c4..4885de99cd 100644 --- a/test/files/neg/t5189.check +++ b/test/files/neg/t5189.check @@ -1,5 +1,5 @@ t5189.scala:3: error: type mismatch; - found : T => U + found : Nothing => Any required: Any => Any def f(x: Any): Any => Any = x match { case Foo(bar) => bar } ^ diff --git a/test/files/neg/t6680a.flags b/test/files/neg/t6680a.flags new file mode 100644 index 0000000000..19243266d1 --- /dev/null +++ b/test/files/neg/t6680a.flags @@ -0,0 +1 @@ +-Xstrict-inference \ No newline at end of file diff --git a/test/files/neg/t6829.check b/test/files/neg/t6829.check index a0b43e3b52..914a1c9260 100644 --- a/test/files/neg/t6829.check +++ b/test/files/neg/t6829.check @@ -16,32 +16,32 @@ t6829.scala:49: error: not found: value nextState val (s,a,s2) = (state,actions(agent),nextState) ^ t6829.scala:50: error: type mismatch; - found : s.type (with underlying type T1) + found : s.type (with underlying type Any) required: _53.State where val _53: G val r = rewards(agent).r(s,a,s2) ^ t6829.scala:50: error: type mismatch; - found : a.type (with underlying type T2) + found : a.type (with underlying type Any) required: _53.Action where val _53: G val r = rewards(agent).r(s,a,s2) ^ t6829.scala:50: error: type mismatch; - found : s2.type (with underlying type T3) + found : s2.type (with underlying type Any) required: _53.State where val _53: G val r = rewards(agent).r(s,a,s2) ^ t6829.scala:51: error: type mismatch; - found : s.type (with underlying type T1) + found : s.type (with underlying type Any) required: _50.State agent.learn(s,a,s2,r): G#Agent ^ t6829.scala:51: error: type mismatch; - found : a.type (with underlying type T2) + found : a.type (with underlying type Any) required: _50.Action agent.learn(s,a,s2,r): G#Agent ^ t6829.scala:51: error: type mismatch; - found : s2.type (with underlying type T3) + found : s2.type (with underlying type Any) required: _50.State agent.learn(s,a,s2,r): G#Agent ^ diff --git a/test/files/neg/t7886.check b/test/files/neg/t7886.check deleted file mode 100644 index 338eee9708..0000000000 --- a/test/files/neg/t7886.check +++ /dev/null @@ -1,6 +0,0 @@ -t7886.scala:10: error: type mismatch; - found : Contra[A] - required: Contra[Any] - case Unravel(m, msg) => g(m) - ^ -one error found diff --git a/test/files/neg/t7886.scala b/test/files/neg/t7886.scala deleted file mode 100644 index 55d80a0a43..0000000000 --- a/test/files/neg/t7886.scala +++ /dev/null @@ -1,22 +0,0 @@ -trait Covariant[+A] -trait Contra[-A] { def accept(p: A): Unit } -trait Invariant[A] extends Covariant[A] with Contra[A] - -case class Unravel[A](m: Contra[A], msg: A) - -object Test extends Covariant[Any] { - def g(m: Contra[Any]): Unit = m accept 5 - def f(x: Any): Unit = x match { - case Unravel(m, msg) => g(m) - case _ => - } - def main(args: Array[String]) { - f(Unravel[String](new Contra[String] { def accept(x: String) = x.length }, "")) - } -} -// java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String -// at Test$$anon$1.accept(a.scala:18) -// at Test$.g(a.scala:13) -// at Test$.f(a.scala:15) -// at Test$.main(a.scala:18) -// at Test.main(a.scala) diff --git a/test/files/pos/pattern-typing.scala b/test/files/pos/pattern-typing.scala deleted file mode 100644 index 7286cc38af..0000000000 --- a/test/files/pos/pattern-typing.scala +++ /dev/null @@ -1,29 +0,0 @@ -import scala.language.higherKinds - -trait Bound[B] - -package p1 { - case class Sub[B <: Bound[B]](p: B) - object Test { - def g[A](x: Bound[A]) = () - def f(x: Any) = x match { case Sub(p) => g(p) } - } -} - -package p2 { - trait Traversable[+A] { def head: A = ??? } - trait Seq[+A] extends Traversable[A] { def length: Int = ??? } - - case class SubHK[B <: Bound[B], CC[X] <: Traversable[X]](xs: CC[B]) - class MyBound extends Bound[MyBound] - class MySeq extends Seq[MyBound] - - object Test { - def g[B](x: Bound[B]) = () - - def f1(x: Any) = x match { case SubHK(xs) => xs } - def f2[B <: Bound[B], CC[X] <: Traversable[X]](sub: SubHK[B, CC]): CC[B] = sub match { case SubHK(xs) => xs } - def f3 = g(f1(SubHK(new MySeq)).head) - def f4 = g(f2(SubHK(new MySeq)).head) - } -} diff --git a/test/files/pos/t5900a.scala b/test/files/pos/t5900a.scala new file mode 100644 index 0000000000..cb02f67fb2 --- /dev/null +++ b/test/files/pos/t5900a.scala @@ -0,0 +1,9 @@ +case class Transition[S](x: S) + +object C + +object Test { + (??? : Any) match { + case Transition(C) => + } +} diff --git a/test/pending/neg/t7886.scala b/test/pending/neg/t7886.scala new file mode 100644 index 0000000000..55d80a0a43 --- /dev/null +++ b/test/pending/neg/t7886.scala @@ -0,0 +1,22 @@ +trait Covariant[+A] +trait Contra[-A] { def accept(p: A): Unit } +trait Invariant[A] extends Covariant[A] with Contra[A] + +case class Unravel[A](m: Contra[A], msg: A) + +object Test extends Covariant[Any] { + def g(m: Contra[Any]): Unit = m accept 5 + def f(x: Any): Unit = x match { + case Unravel(m, msg) => g(m) + case _ => + } + def main(args: Array[String]) { + f(Unravel[String](new Contra[String] { def accept(x: String) = x.length }, "")) + } +} +// java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String +// at Test$$anon$1.accept(a.scala:18) +// at Test$.g(a.scala:13) +// at Test$.f(a.scala:15) +// at Test$.main(a.scala:18) +// at Test.main(a.scala) diff --git a/test/pending/pos/pattern-typing.scala b/test/pending/pos/pattern-typing.scala new file mode 100644 index 0000000000..7286cc38af --- /dev/null +++ b/test/pending/pos/pattern-typing.scala @@ -0,0 +1,29 @@ +import scala.language.higherKinds + +trait Bound[B] + +package p1 { + case class Sub[B <: Bound[B]](p: B) + object Test { + def g[A](x: Bound[A]) = () + def f(x: Any) = x match { case Sub(p) => g(p) } + } +} + +package p2 { + trait Traversable[+A] { def head: A = ??? } + trait Seq[+A] extends Traversable[A] { def length: Int = ??? } + + case class SubHK[B <: Bound[B], CC[X] <: Traversable[X]](xs: CC[B]) + class MyBound extends Bound[MyBound] + class MySeq extends Seq[MyBound] + + object Test { + def g[B](x: Bound[B]) = () + + def f1(x: Any) = x match { case SubHK(xs) => xs } + def f2[B <: Bound[B], CC[X] <: Traversable[X]](sub: SubHK[B, CC]): CC[B] = sub match { case SubHK(xs) => xs } + def f3 = g(f1(SubHK(new MySeq)).head) + def f4 = g(f2(SubHK(new MySeq)).head) + } +} -- cgit v1.2.3 From 1067e23506aca5083ca915f1dbd7dba689170b36 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Thu, 13 Feb 2014 10:31:03 -0800 Subject: SI-8280 regression in implicit selection. In 2fa2db7840 I fixed a bug where applicable implicit conversions would not be found for numeric types if one introduced any aliasing or singleton types, for the usual reasons involving the absence of uniform type normalization. See pos/t7228 for examples - that test case has 20 errors in 2.10.3 but compiles in master. An unintended side effect was making implicit search less oblivious. It turns out that in so doing I had created ambiguity where there was none before. Not because it was any more ambiguous, but because the compiler now had the wits to notice the ambiguity at an earlier time. The fix for this is not intuitive. The way the internal logic is, we need to keep the wool over implicit search's eyes, which leads to those unrecognized types being passed to adapt, where they are recognized and weak subtyping suffices to be more specific. It is sufficient for SI-7228 that weak subtyping be done correctly - the other change, which is reverted here, was exposing the type arguments of Function1 when a view exists as a subtype of Function1. It is also possible this could be remedied by calling weak_<:< somewhere which is presently <:<, but I don't know where and it has a far greater chance of affecting something else than does this, which is a straight reversion of a post-2.10.3 change. --- .../scala/tools/nsc/typechecker/Implicits.scala | 5 +- test/files/run/t8280.check | 9 +++ test/files/run/t8280.scala | 82 ++++++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t8280.check create mode 100644 test/files/run/t8280.scala (limited to 'test') diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala index 776920ed42..8f5778862d 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala @@ -306,7 +306,10 @@ trait Implicits { */ object Function1 { val Sym = FunctionClass(1) - def unapply(tp: Type) = tp baseType Sym match { + // It is tempting to think that this should be inspecting "tp baseType Sym" + // rather than tp. See test case run/t8280 and the commit message which + // accompanies it for explanation why that isn't done. + def unapply(tp: Type) = tp match { case TypeRef(_, Sym, arg1 :: arg2 :: _) => Some((arg1, arg2)) case _ => None } diff --git a/test/files/run/t8280.check b/test/files/run/t8280.check new file mode 100644 index 0000000000..ed392841c7 --- /dev/null +++ b/test/files/run/t8280.check @@ -0,0 +1,9 @@ +Int +Int +Int +Int +Int +Int +Int +Int +Int diff --git a/test/files/run/t8280.scala b/test/files/run/t8280.scala new file mode 100644 index 0000000000..0734d63b6e --- /dev/null +++ b/test/files/run/t8280.scala @@ -0,0 +1,82 @@ +import scala.language.implicitConversions + +object Test { + def main(args: Array[String]): Unit = { + Moop1.ob1 + Moop1.ob2 + Moop1.ob3 + Moop2.ob1 + Moop2.ob2 + Moop2.ob3 + Moop3.ob1 + Moop3.ob2 + Moop3.ob3 + } +} + +// int object vs. +object Moop1 { + object ob1 { + implicit object f1 extends (Int => String) { def apply(x: Int): String = "Int" } + implicit object f2 extends (Long => String) { def apply(x: Long): String = "Long" } + + println(5: String) + } + object ob2 { + implicit object f1 extends (Int => String) { def apply(x: Int): String = "Int" } + implicit def f2(x: Long): String = "Long" + + println(5: String) + } + object ob3 { + implicit object f1 extends (Int => String) { def apply(x: Int): String = "Int" } + implicit val f2: Long => String = _ => "Long" + + println(5: String) + } +} + +// int def vs. +object Moop2 { + object ob1 { + implicit def f1(x: Int): String = "Int" + implicit object f2 extends (Long => String) { def apply(x: Long): String = "Long" } + + println(5: String) + } + object ob2 { + implicit def f1(x: Int): String = "Int" + implicit def f2(x: Long): String = "Long" + + println(5: String) + } + object ob3 { + implicit def f1(x: Int): String = "Int" + implicit val f2: Long => String = _ => "Long" + + println(5: String) + } +} + +// int val vs. +object Moop3 { + object ob1 { + implicit val f1: Int => String = _ => "Int" + implicit object f2 extends (Long => String) { def apply(x: Long): String = "Long" } + + println(5: String) + } + object ob2 { + implicit val f1: Int => String = _ => "Int" + implicit def f2(x: Long): String = "Long" + + println(5: String) + } + object ob3 { + implicit val f1: Int => String = _ => "Int" + implicit val f2: Long => String = _ => "Long" + + println(5: String) + } +} + -- cgit v1.2.3 From 922d090f5c2dd8a14f2bb7915a04c5156d29ea1b Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 13 Feb 2014 21:49:18 +0100 Subject: SI-8276 Test for cyclic error caused by (reverted) SI-1786 fix We reverted SI-1786 recently on the grounds that its means of avoiding cycles (not sharpening bounds of T[_] when T is uninitialized) caused unacceptable non-determinism (well: compilation order dependency) to type inference. Nary a day later, @gkossakowski hit a regression in scala-io. Bisection revealed that it stopped working in 2dbd17a2 and started working agiain after the revert. How's that for prescience! I've distilled the cyclic error in scala-io in this test case. I'm yet to pinpoint this, followon error, which didn't survive the shrink ray, and only appeared in the original code: error: java.lang.IndexOutOfBoundsException: 0 at scala.collection.LinearSeqOptimized$class.apply(LinearSeqOptimized.scala:51) at scala.collection.immutable.List.apply(List.scala:83) at scala.reflect.internal.tpe.TypeMaps$AsSeenFromMap.correspondingTypeArgument(TypeMaps.scala:5 --- test/files/pos/t1786-cycle.scala | 57 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 test/files/pos/t1786-cycle.scala (limited to 'test') diff --git a/test/files/pos/t1786-cycle.scala b/test/files/pos/t1786-cycle.scala new file mode 100644 index 0000000000..af5d892c6a --- /dev/null +++ b/test/files/pos/t1786-cycle.scala @@ -0,0 +1,57 @@ +trait GenTraversableLike[+A, +Repr] extends Any + +object O { + (null: Any) match { + case _: LongTraversableLike[_] => + } +} + +trait LongTraversable extends LongTraversableLike[LongTraversable] + +trait LongTraversableLike[+Repr <: LongTraversableLike[Repr]] extends GenTraversableLike[Any, Repr] + +/* +% scalac-hash v2.11.0-M8 test/files/pos/t1786-cycle.scala +[warn] v2.11.0-M8 failed, using closest available +test/files/pos/t1786-cycle.scala:11: error: illegal cyclic reference involving trait LongTraversableLike +trait LongTraversableLike[+Repr <: LongTraversableLike[Repr]] extends GenTraversableLike[Any, Repr] + ^ +one error found + +Okay again after SI-1786 was reverted. + + +|-- object O BYVALmode-EXPRmode (site: package ) +| |-- super EXPRmode-POLYmode-QUALmode (silent: in O) +| | |-- this EXPRmode (silent: in O) +| | | \-> O.type +| | \-> O.type +| |-- (null: Any) match { case (_: LongTraversableLike[(_ @ in O) +| | |-- (null: Any) BYVALmode-EXPRmode (site: value in O) +| | | |-- Any TYPEmode (site: value in O) +| | | | \-> Any +| | | |-- null : pt=Any EXPRmode (site: value in O) +| | | | \-> Null(null) +| | | \-> Any +| | |-- (_: LongTraversableLike[(_ @ )]) : pt=Any PATTERNmode (site: value in O) enrichment only +| | | |-- LongTraversableLike[(_ @ )] TYPEPATmode-TYPEmode (site: value in O) enrichment only +| | | | |-- <: LongTraversableLike[Repr] TYPEmode (site: type Repr in ) +| | | | | |-- LongTraversableLike[Repr] TYPEmode (site: type Repr in ) +| | | | | | |-- Repr NOmode (site: type Repr in ) +| | | | | | | \-> Repr +| | | | | | \-> LongTraversableLike[Repr] +| | | | | [adapt] <: LongTraversableLike[Repr] is now a TypeTree( <: LongTraversableLike[Repr]) +| | | | | \-> <: LongTraversableLike[Repr] +| | | | |-- (_ @ ) TYPEPATmode-TYPEmode (site: value in O) enrichment only +| | | | | \-> _ +| | | | |-- GenTraversableLike FUNmode-TYPEmode (site: trait LongTraversableLike) +| | | | | \-> GenTraversableLike +| | | | |-- GenTraversableLike[Any, Repr] TYPEmode (site: trait LongTraversableLike) +| | | | | |-- Any TYPEmode (site: trait LongTraversableLike) +| | | | | | \-> Any +| | | | | |-- Repr TYPEmode (site: trait LongTraversableLike) +| | | | | | \-> Repr +| | | | | caught scala.reflect.internal.Symbols$CyclicReference: illegal cyclic reference involving trait LongTraversableLike: while typing GenTraversableLike[Any, Repr] +test/files/pos/t1786-cycle.scala:11: error: illegal cyclic reference involving trait LongTraversableLike +trait LongTraversableLike[+Repr <: LongTraversableLike[Repr]] extends GenT +*/ \ No newline at end of file -- cgit v1.2.3 From d3a302b022adc5eaeb1fcbdffaffd5fd438726e0 Mon Sep 17 00:00:00 2001 From: Rex Kerr Date: Thu, 13 Feb 2014 17:47:27 -0800 Subject: SI-6632 ListBuffer's updated accepts negative positions Changed the behavior in SeqLike.updated (which would silently accept negatives and throw on empty.tail) to throw IndexOutOfBoundException. Updated tests to verify the behavior in ListBuffer. Everything else SeqLike will come along for the ride. --- src/library/scala/collection/SeqLike.scala | 5 ++++- test/files/run/t6632.check | 2 ++ test/files/run/t6632.scala | 29 +++++++++++------------------ 3 files changed, 17 insertions(+), 19 deletions(-) (limited to 'test') diff --git a/src/library/scala/collection/SeqLike.scala b/src/library/scala/collection/SeqLike.scala index 960c277f67..fdfb1f2efc 100644 --- a/src/library/scala/collection/SeqLike.scala +++ b/src/library/scala/collection/SeqLike.scala @@ -509,11 +509,14 @@ trait SeqLike[+A, +Repr] extends Any with IterableLike[A, Repr] with GenSeqLike[ } def updated[B >: A, That](index: Int, elem: B)(implicit bf: CanBuildFrom[Repr, B, That]): That = { + if (index < 0) throw new IndexOutOfBoundsException(index.toString) val b = bf(repr) val (prefix, rest) = this.splitAt(index) + val restColl = toCollection(rest) + if (restColl.isEmpty) throw new IndexOutOfBoundsException(index.toString) b ++= toCollection(prefix) b += elem - b ++= toCollection(rest).view.tail + b ++= restColl.view.tail b.result() } diff --git a/test/files/run/t6632.check b/test/files/run/t6632.check index 1f084b1dac..26cf061b5f 100644 --- a/test/files/run/t6632.check +++ b/test/files/run/t6632.check @@ -1,3 +1,5 @@ java.lang.IndexOutOfBoundsException: -1 java.lang.IndexOutOfBoundsException: -2 java.lang.IndexOutOfBoundsException: -3 +java.lang.IndexOutOfBoundsException: -1 +java.lang.IndexOutOfBoundsException: 5 diff --git a/test/files/run/t6632.scala b/test/files/run/t6632.scala index 0242e60104..f338b73fa6 100644 --- a/test/files/run/t6632.scala +++ b/test/files/run/t6632.scala @@ -3,27 +3,20 @@ object Test extends App { def newLB = ListBuffer('a, 'b, 'c, 'd, 'e) - val lb0 = newLB + def iiobe[A](f: => A) = + try { f } + catch { case ex: IndexOutOfBoundsException => println(ex) } - try { - lb0.insert(-1, 'x) - } catch { - case ex: IndexOutOfBoundsException => println(ex) - } + val lb0 = newLB + iiobe( lb0.insert(-1, 'x) ) val lb1 = newLB - - try { - lb1.insertAll(-2, Array('x, 'y, 'z)) - } catch { - case ex: IndexOutOfBoundsException => println(ex) - } + iiobe( lb1.insertAll(-2, Array('x, 'y, 'z)) ) val lb2 = newLB + iiobe( lb2.update(-3, 'u) ) - try { - lb2.update(-3, 'u) - } catch { - case ex: IndexOutOfBoundsException => println(ex) - } -} \ No newline at end of file + val lb3 = newLB + iiobe( lb3.updated(-1, 'u) ) + iiobe( lb3.updated(5, 'u) ) +} -- cgit v1.2.3 From 33fc68171105bb8d884219381c220076c5651316 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 13 Feb 2014 12:28:06 -0800 Subject: SI-8177 specializeSym must use memberInfo on high side When determining whether member `symLo` of `tpLo` has a stronger type than member `symHi` of `tpHi`, should we use memberType or memberInfo? Well, memberType transforms (using `asSeenFrom`) `sym.tpe`, whereas memberInfo performs the same transform on `sym.info`. For term symbols, this ends up being the same thing (`sym.tpe == sym.info`). For type symbols, however, the `.info` of an abstract type member is defined by its bounds, whereas its `.tpe` is a `TypeRef` to that type symbol, so that `sym.tpe <:< sym.info`, but not the other way around. Thus, for the strongest (correct) result, we should use `memberType` on the low side. On the high side, we should use the result appropriate for the right side of the `<:<` above (`memberInfo`). I also optimized the method a little bit by avoiding calling memberType if the symbol on the high side isn't eligble (e.g., it's a class). PS: I had to add a workaround to reifyType, because we now dealias a little less eagerly, which means a type selection on refinement class symbols makes it to reify this broke the t8104 tests. I also had to update the run/t6992 test, which should now test the right thing. Tests should be commented and/or use sensible names. What is it testing? What is the expected outcome? We should not be left guessing. --- .../scala/reflect/reify/codegen/GenSymbols.scala | 2 +- .../scala/reflect/reify/codegen/GenTypes.scala | 4 +- src/reflect/scala/reflect/internal/Types.scala | 54 +++++++++++++------ test/files/neg/t0764.check | 7 +++ test/files/neg/t0764.scala | 45 ++++++++++++++++ test/files/neg/t0764b.check | 47 ++++++++++++++++ test/files/neg/t0764b.scala | 63 ++++++++++++++++++++++ test/files/neg/t8177a.check | 6 +++ test/files/neg/t8177a.scala | 6 +++ test/files/pos/t0764.scala | 27 ---------- test/files/pos/t0764b.scala | 61 --------------------- test/files/pos/t8177h.scala | 5 ++ test/files/run/t6992/Test_2.scala | 2 +- 13 files changed, 221 insertions(+), 108 deletions(-) create mode 100644 test/files/neg/t0764.check create mode 100644 test/files/neg/t0764.scala create mode 100644 test/files/neg/t0764b.check create mode 100644 test/files/neg/t0764b.scala create mode 100644 test/files/neg/t8177a.check create mode 100644 test/files/neg/t8177a.scala delete mode 100644 test/files/pos/t0764.scala delete mode 100644 test/files/pos/t0764b.scala create mode 100644 test/files/pos/t8177h.scala (limited to 'test') diff --git a/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala b/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala index 3a97089d51..2965db17c6 100644 --- a/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala +++ b/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala @@ -93,7 +93,7 @@ trait GenSymbols { // todo. make sure that free methods and free local defs work correctly if (sym.isExistential) reifySymDef(sym) else if (sym.isTerm) reifyFreeTerm(Ident(sym)) - else reifyFreeType(Ident(sym)) + else reifyFreeType(Ident(sym)) // TODO: reify refinement classes } } diff --git a/src/compiler/scala/reflect/reify/codegen/GenTypes.scala b/src/compiler/scala/reflect/reify/codegen/GenTypes.scala index a90a3a338b..3e2acc28e5 100644 --- a/src/compiler/scala/reflect/reify/codegen/GenTypes.scala +++ b/src/compiler/scala/reflect/reify/codegen/GenTypes.scala @@ -55,7 +55,9 @@ trait GenTypes { case tpe @ ConstantType(value) => mirrorFactoryCall(nme.ConstantType, reifyProduct(value)) case tpe @ TypeRef(pre, sym, args) => - reifyProduct(tpe) + // TODO: remove special case!!! for now, as we can't reify these symbols, let's hope dealias gets us out of this bind... + if (pre.typeSymbol.isAnonOrRefinementClass && (tpe ne tpe.dealias)) reifyType(tpe.dealias) + else reifyProduct(tpe) case tpe @ TypeBounds(lo, hi) => reifyProduct(tpe) case tpe @ NullaryMethodType(restpe) => diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 6010d4eb12..27ec882549 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -4100,24 +4100,44 @@ trait Types ) } - /** Does member `sym1` of `tp1` have a stronger type - * than member `sym2` of `tp2`? + /** Does member `symLo` of `tpLo` have a stronger type + * than member `symHi` of `tpHi`? */ - protected[internal] def specializesSym(tp1: Type, sym1: Symbol, tp2: Type, sym2: Symbol, depth: Depth): Boolean = { - require((sym1 ne NoSymbol) && (sym2 ne NoSymbol), ((tp1, sym1, tp2, sym2, depth))) - val info1 = tp1.memberInfo(sym1) - val info2 = tp2.memberInfo(sym2).substThis(tp2.typeSymbol, tp1) - //System.out.println("specializes "+tp1+"."+sym1+":"+info1+sym1.locationString+" AND "+tp2+"."+sym2+":"+info2)//DEBUG - ( sym2.isTerm && isSubType(info1, info2, depth) && (!sym2.isStable || sym1.isStable) && (!sym1.hasVolatileType || sym2.hasVolatileType) - || sym2.isAbstractType && { - val memberTp1 = tp1.memberType(sym1) - // println("kinds conform? "+(memberTp1, tp1, sym2, kindsConform(List(sym2), List(memberTp1), tp2, sym2.owner))) - info2.bounds.containsType(memberTp1) && - kindsConform(List(sym2), List(memberTp1), tp1, sym1.owner) - } - || sym2.isAliasType && tp2.memberType(sym2).substThis(tp2.typeSymbol, tp1) =:= tp1.memberType(sym1) //@MAT ok - ) - } + protected[internal] def specializesSym(preLo: Type, symLo: Symbol, preHi: Type, symHi: Symbol, depth: Depth): Boolean = + (symHi.isAliasType || symHi.isTerm || symHi.isAbstractType) && { + // only now that we know symHi is a viable candidate ^^^^^^^, do the expensive checks: ----V + require((symLo ne NoSymbol) && (symHi ne NoSymbol), ((preLo, symLo, preHi, symHi, depth))) + + val tpHi = preHi.memberInfo(symHi).substThis(preHi.typeSymbol, preLo) + + // Should we use memberType or memberInfo? + // memberType transforms (using `asSeenFrom`) `sym.tpe`, + // whereas memberInfo performs the same transform on `sym.info`. + // For term symbols, this ends up being the same thing (`sym.tpe == sym.info`). + // For type symbols, however, the `.info` of an abstract type member + // is defined by its bounds, whereas its `.tpe` is a `TypeRef` to that type symbol, + // so that `sym.tpe <:< sym.info`, but not the other way around. + // + // Thus, for the strongest (correct) result, + // we should use `memberType` on the low side. + // + // On the high side, we should use the result appropriate + // for the right side of the `<:<` above (`memberInfo`). + val tpLo = preLo.memberType(symLo) + + debuglog(s"specializesSymHi: $preHi . $symHi : $tpHi") + debuglog(s"specializesSymLo: $preLo . $symLo : $tpLo") + + if (symHi.isTerm) + (isSubType(tpLo, tpHi, depth) && + (!symHi.isStable || symLo.isStable) && // sub-member must remain stable + (!symLo.hasVolatileType || symHi.hasVolatileType)) // sub-member must not introduce volatility + else if (symHi.isAbstractType) + ((tpHi.bounds containsType tpLo) && + kindsConform(symHi :: Nil, tpLo :: Nil, preLo, symLo.owner)) + else // we know `symHi.isAliasType` (see above) + tpLo =:= tpHi + } /** A function implementing `tp1` matches `tp2`. */ final def matchesType(tp1: Type, tp2: Type, alwaysMatchSimple: Boolean): Boolean = { diff --git a/test/files/neg/t0764.check b/test/files/neg/t0764.check new file mode 100644 index 0000000000..0c7cff1e1e --- /dev/null +++ b/test/files/neg/t0764.check @@ -0,0 +1,7 @@ +t0764.scala:13: error: type mismatch; + found : Node{type T = _1.type} where val _1: Node{type T = NextType} + required: Node{type T = Main.this.AType} + (which expands to) Node{type T = Node{type T = NextType}} + new Main[AType]( (value: AType).prepend ) + ^ +one error found diff --git a/test/files/neg/t0764.scala b/test/files/neg/t0764.scala new file mode 100644 index 0000000000..9f77a59414 --- /dev/null +++ b/test/files/neg/t0764.scala @@ -0,0 +1,45 @@ +class Top[A] { + type AType = A +} + +trait Node { outer => + type T <: Node + def prepend = new Node { type T = outer.type } +} + +class Main[NextType <: Node](value: Node { type T = NextType }) + extends Top[Node { type T = NextType }] { + + new Main[AType]( (value: AType).prepend ) +} + +/* we've been back-and-forth on this one -- see PRs on SI-8177 for the reasoning +I think it should compile and that the following error is due to broken =:= on existentials + found : Node{type T = _1.type} where val _1: Node{type T = NextType} + required: Node{type T = Main.this.AType} + (which expands to) Node{type T = Node{type T = NextType}} + +I claim (omitting the forSome for brevity, even though the premature skolemization is probably the issue) +_1.type =:= Main.this.AType +because +(1) _1.type <:< Main.this.AType and (2) Main.this.AType <:< _1.type +(1), because: +_1.type <:< Node{type T = NextType} (because skolemization and _1's upper bound) +(2), because: +Node{type T = NextType} <:< _1.type forSome val _1: Node{type T = NextType} +because: +Node{type T = NextType} <:< T forSome {type T <: Node{type T = NextType} with Singleton} +because +Node{type T = NextType} <:< Node{type T = NextType} with Singleton + +hmmm.. might the with Singleton be throwing a wrench in our existential house? + +Behold the equivalent program which type checks without the fix for SI-8177. +(Expand type alias, convert type member to type param; +note the covariance to encode subtyping on type members.) + +class Node[+T <: Node[_]] { def prepend = new Node[this.type] } +class Main[NextType <: Node[_]](value: Node[NextType]) { + new Main(value.prepend) +} +*/ \ No newline at end of file diff --git a/test/files/neg/t0764b.check b/test/files/neg/t0764b.check new file mode 100644 index 0000000000..4040954e7c --- /dev/null +++ b/test/files/neg/t0764b.check @@ -0,0 +1,47 @@ +t0764b.scala:27: error: type mismatch; + found : p1.t0764.Node{type T = p1.t0764..type} + required: p1.t0764.NodeAlias[p1.t0764.NodeAlias[A]] + (which expands to) p1.t0764.Node{type T = p1.t0764.Node{type T = A}} + private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail + ^ +t0764b.scala:28: error: type mismatch; + found : p1.t0764.Node{type T = p1.t0764..type} + required: p1.t0764.NodeAlias[p1.t0764.Node{type T = A}] + (which expands to) p1.t0764.Node{type T = p1.t0764.Node{type T = A}} + private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail + ^ +t0764b.scala:34: error: type mismatch; + found : p1.t0764.Node{type T = p1.t0764..type} + required: p1.t0764.Node{type T = p1.t0764.NodeAlias[A]} + (which expands to) p1.t0764.Node{type T = p1.t0764.Node{type T = A}} + private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail + ^ +t0764b.scala:35: error: type mismatch; + found : p1.t0764.Node{type T = p1.t0764..type} + required: p1.t0764.Node{type T = p1.t0764.Node{type T = A}} + private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail + ^ +t0764b.scala:51: error: type mismatch; + found : p2.t0764.Node{type T = p2.t0764..type} + required: p2.t0764.NodeAlias[p2.t0764.NodeAlias[A]] + (which expands to) p2.t0764.Node{type T = p2.t0764.Node{type T = A}} + private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail + ^ +t0764b.scala:52: error: type mismatch; + found : p2.t0764.Node{type T = p2.t0764..type} + required: p2.t0764.NodeAlias[p2.t0764.Node{type T = A}] + (which expands to) p2.t0764.Node{type T = p2.t0764.Node{type T = A}} + private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail + ^ +t0764b.scala:58: error: type mismatch; + found : p2.t0764.Node{type T = p2.t0764..type} + required: p2.t0764.Node{type T = p2.t0764.NodeAlias[A]} + (which expands to) p2.t0764.Node{type T = p2.t0764.Node{type T = A}} + private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail + ^ +t0764b.scala:59: error: type mismatch; + found : p2.t0764.Node{type T = p2.t0764..type} + required: p2.t0764.Node{type T = p2.t0764.Node{type T = A}} + private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail + ^ +8 errors found diff --git a/test/files/neg/t0764b.scala b/test/files/neg/t0764b.scala new file mode 100644 index 0000000000..14c623c67a --- /dev/null +++ b/test/files/neg/t0764b.scala @@ -0,0 +1,63 @@ +// see neg/t0764 why this should probably be a pos/ test -- alas something's wrong with existential subtyping (?) + +// In all cases when calling "prepend" the receiver 'v' +// has static type NodeAlias[A] or (equivalently) Node { type T = A }. +// Since prepend explicitly returns the singleton type of the receiver, +// the return type of prepend in all cases is "v.type", and so the call +// to "new Main" can be parameterized with any of the following, in order +// of decreasing specificity with a tie for second place: +// +// new Main[v.type](v.prepend) +// new Main[NodeAlias[A]](v.prepend) +// new Main[Node { type T = A }](v.prepend) +// new Main(v.prepend) + +// the `fail` comments below denote what didn't compile before SI-8177 fixed all of them + +package p1 { + object t0764 { + type NodeAlias[A] = Node { type T = A } + trait Node { outer => + type T <: Node + def prepend: Node { type T = outer.type } = ??? + } + + class Main1[A <: Node](v: NodeAlias[A]) { + private[this] def f1 = new Main1(v.prepend) // fail + private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main1[v.type](v.prepend) // ok + } + + class Main2[A <: Node](v: Node { type T = A }) { + private[this] def f1 = new Main2(v.prepend) // fail + private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main2[v.type](v.prepend) // ok + } + } +} + +package p2 { + object t0764 { + type NodeAlias[A] = Node { type T = A } + trait Node { outer => + type T <: Node + def prepend: NodeAlias[outer.type] = ??? + } + + class Main1[A <: Node](v: NodeAlias[A]) { + private[this] def f1 = new Main1(v.prepend) // ok! <<========== WOT + private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main1[v.type](v.prepend) // ok + } + + class Main2[A <: Node](v: Node { type T = A }) { + private[this] def f1 = new Main2(v.prepend) // fail + private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail + private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail + private[this] def f4 = new Main2[v.type](v.prepend) // ok + } + } +} diff --git a/test/files/neg/t8177a.check b/test/files/neg/t8177a.check new file mode 100644 index 0000000000..0d01206e0c --- /dev/null +++ b/test/files/neg/t8177a.check @@ -0,0 +1,6 @@ +t8177a.scala:5: error: type mismatch; + found : A{type Result = Int} + required: A{type Result = String} + : A { type Result = String} = x + ^ +one error found diff --git a/test/files/neg/t8177a.scala b/test/files/neg/t8177a.scala new file mode 100644 index 0000000000..d1e47f8c1e --- /dev/null +++ b/test/files/neg/t8177a.scala @@ -0,0 +1,6 @@ +trait A { type Result } + +class PolyTests { + def wrong(x: A { type Result = Int }) + : A { type Result = String} = x +} \ No newline at end of file diff --git a/test/files/pos/t0764.scala b/test/files/pos/t0764.scala deleted file mode 100644 index f1084f5ff8..0000000000 --- a/test/files/pos/t0764.scala +++ /dev/null @@ -1,27 +0,0 @@ -class Top[A] { - type AType = A -} - -trait Node { outer => - type T <: Node - def prepend = new Node { type T = outer.type } -} - -class Main[NextType <: Node](value: Node { type T = NextType }) - extends Top[Node { type T = NextType }] { - - new Main[AType]( (value: AType).prepend ) -} - -/* this used to be a neg test, even though it should've compiled -SI-8177 fixed this. - -Behold the equivalent program which type checks without the fix for SI-8177. -(Expand type alias, convert type member to type param; -note the covariance to encode subtyping on type members.) - -class Node[+T <: Node[_]] { def prepend = new Node[this.type] } -class Main[NextType <: Node[_]](value: Node[NextType]) { - new Main(value.prepend) -} -*/ \ No newline at end of file diff --git a/test/files/pos/t0764b.scala b/test/files/pos/t0764b.scala deleted file mode 100644 index 6ae3c105c9..0000000000 --- a/test/files/pos/t0764b.scala +++ /dev/null @@ -1,61 +0,0 @@ -// In all cases when calling "prepend" the receiver 'v' -// has static type NodeAlias[A] or (equivalently) Node { type T = A }. -// Since prepend explicitly returns the singleton type of the receiver, -// the return type of prepend in all cases is "v.type", and so the call -// to "new Main" can be parameterized with any of the following, in order -// of decreasing specificity with a tie for second place: -// -// new Main[v.type](v.prepend) -// new Main[NodeAlias[A]](v.prepend) -// new Main[Node { type T = A }](v.prepend) -// new Main(v.prepend) - -// the `fail` comments below denote what didn't compile before SI-8177 fixed all of them - -package p1 { - object t0764 { - type NodeAlias[A] = Node { type T = A } - trait Node { outer => - type T <: Node - def prepend: Node { type T = outer.type } = ??? - } - - class Main1[A <: Node](v: NodeAlias[A]) { - private[this] def f1 = new Main1(v.prepend) // fail - private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main1[v.type](v.prepend) // ok - } - - class Main2[A <: Node](v: Node { type T = A }) { - private[this] def f1 = new Main2(v.prepend) // fail - private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main2[v.type](v.prepend) // ok - } - } -} - -package p2 { - object t0764 { - type NodeAlias[A] = Node { type T = A } - trait Node { outer => - type T <: Node - def prepend: NodeAlias[outer.type] = ??? - } - - class Main1[A <: Node](v: NodeAlias[A]) { - private[this] def f1 = new Main1(v.prepend) // ok! <<========== WOT - private[this] def f2 = new Main1[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main1[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main1[v.type](v.prepend) // ok - } - - class Main2[A <: Node](v: Node { type T = A }) { - private[this] def f1 = new Main2(v.prepend) // fail - private[this] def f2 = new Main2[NodeAlias[A]](v.prepend) // fail - private[this] def f3 = new Main2[Node { type T = A }](v.prepend) // fail - private[this] def f4 = new Main2[v.type](v.prepend) // ok - } - } -} diff --git a/test/files/pos/t8177h.scala b/test/files/pos/t8177h.scala new file mode 100644 index 0000000000..90b8a26ce7 --- /dev/null +++ b/test/files/pos/t8177h.scala @@ -0,0 +1,5 @@ +class Module { self => + type settingsType <: Any + final type commonModuleType = Module {type settingsType = self.settingsType} + def foo(s: self.type): commonModuleType = s +} diff --git a/test/files/run/t6992/Test_2.scala b/test/files/run/t6992/Test_2.scala index 05282d6f5b..0f226b01ce 100644 --- a/test/files/run/t6992/Test_2.scala +++ b/test/files/run/t6992/Test_2.scala @@ -2,7 +2,7 @@ import scala.language.reflectiveCalls object Test extends App { val foo = Macros.foo("T") - println(scala.reflect.runtime.universe.weakTypeOf[foo.T].typeSymbol.typeSignature) + println(scala.reflect.runtime.universe.weakTypeOf[foo.T]) val bar = Macros.bar("test") println(bar.test) -- cgit v1.2.3 From a4a13199d0811d01cb008804ae34c594abdc415e Mon Sep 17 00:00:00 2001 From: Rex Kerr Date: Thu, 13 Feb 2014 19:37:14 -0800 Subject: SI-8188 NPE during deserialization of TrieMap The writer was using the constructor headf and ef instead of the internal vars hashingobj and equalityobj. --- .../scala/collection/concurrent/TrieMap.scala | 4 ++-- test/files/run/t8188.scala | 25 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t8188.scala (limited to 'test') diff --git a/src/library/scala/collection/concurrent/TrieMap.scala b/src/library/scala/collection/concurrent/TrieMap.scala index 6632f30e51..fccc1d81b9 100644 --- a/src/library/scala/collection/concurrent/TrieMap.scala +++ b/src/library/scala/collection/concurrent/TrieMap.scala @@ -655,8 +655,8 @@ extends scala.collection.concurrent.Map[K, V] /* internal methods */ private def writeObject(out: java.io.ObjectOutputStream) { - out.writeObject(hashf) - out.writeObject(ef) + out.writeObject(hashingobj) + out.writeObject(equalityobj) val it = iterator while (it.hasNext) { diff --git a/test/files/run/t8188.scala b/test/files/run/t8188.scala new file mode 100644 index 0000000000..ec3a968e4a --- /dev/null +++ b/test/files/run/t8188.scala @@ -0,0 +1,25 @@ +object Test { + def main(args: Array[String]) { + import java.io.ByteArrayInputStream + import java.io.ByteArrayOutputStream + import java.io.ObjectInputStream + import java.io.ObjectOutputStream + import scala.collection.concurrent.TrieMap + + def ser[T](o: T): Array[Byte] = { + val baos = new ByteArrayOutputStream() + new ObjectOutputStream(baos).writeObject(o) + baos.toByteArray() + } + + def deser[T](bs: Array[Byte]): T = + new ObjectInputStream(new ByteArrayInputStream(bs)).readObject().asInstanceOf[T] + + def cloneViaSerialization[T](t: T): T = deser(ser(t)) + + val f = cloneViaSerialization(_: TrieMap[Int, Int]) + val tm = TrieMap(1 -> 2) + assert( f(f(tm)) == tm ) + assert( ser(tm).length == ser(f(tm)).length ) + } +} -- cgit v1.2.3 From 5984461227c70dd48d9c87c6e5afe0cf8c58f8f1 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Fri, 14 Feb 2014 11:06:01 -0800 Subject: SI-8177 tidy up in type reification --- src/compiler/scala/reflect/reify/codegen/GenTypes.scala | 4 +--- src/reflect/scala/reflect/internal/Types.scala | 2 +- test/files/neg/t8104/Test_2.scala | 2 +- test/files/run/t6992.check | 1 + test/files/run/t6992/Test_2.scala | 4 +++- test/files/run/t8104.check | 3 ++- test/files/run/t8104/Test_2.scala | 5 ++++- 7 files changed, 13 insertions(+), 8 deletions(-) (limited to 'test') diff --git a/src/compiler/scala/reflect/reify/codegen/GenTypes.scala b/src/compiler/scala/reflect/reify/codegen/GenTypes.scala index 3e2acc28e5..a90a3a338b 100644 --- a/src/compiler/scala/reflect/reify/codegen/GenTypes.scala +++ b/src/compiler/scala/reflect/reify/codegen/GenTypes.scala @@ -55,9 +55,7 @@ trait GenTypes { case tpe @ ConstantType(value) => mirrorFactoryCall(nme.ConstantType, reifyProduct(value)) case tpe @ TypeRef(pre, sym, args) => - // TODO: remove special case!!! for now, as we can't reify these symbols, let's hope dealias gets us out of this bind... - if (pre.typeSymbol.isAnonOrRefinementClass && (tpe ne tpe.dealias)) reifyType(tpe.dealias) - else reifyProduct(tpe) + reifyProduct(tpe) case tpe @ TypeBounds(lo, hi) => reifyProduct(tpe) case tpe @ NullaryMethodType(restpe) => diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 27ec882549..b2e36a2732 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -1143,7 +1143,7 @@ trait Types /** A class for this-types of the form .this.type */ abstract case class ThisType(sym: Symbol) extends SingletonType with ThisTypeApi { - if (!sym.isClass) { + if (!sym.isClass && !sym.isFreeType) { // SI-6640 allow StubSymbols to reveal what's missing from the classpath before we trip the assertion. sym.failIfStub() abort(s"ThisType($sym) for sym which is not a class") diff --git a/test/files/neg/t8104/Test_2.scala b/test/files/neg/t8104/Test_2.scala index 585f76c00f..a3bd940188 100644 --- a/test/files/neg/t8104/Test_2.scala +++ b/test/files/neg/t8104/Test_2.scala @@ -9,7 +9,7 @@ object Test extends App { case class C(x: Int, y: Int) import scala.reflect.runtime.universe._ - def reprify[T, Repr](x: T)(implicit generic: Generic.Aux[T, Repr], tag: TypeTag[Repr]) = println(tag) + def reprify[T, Repr](x: T)(implicit generic: Generic.Aux[T, Repr], tag: WeakTypeTag[Repr]) = println(tag) reprify(C(40, 2)) // this is a compilation error at the moment as explained in SI-8104 diff --git a/test/files/run/t6992.check b/test/files/run/t6992.check index 1a0684c995..021f32ec95 100644 --- a/test/files/run/t6992.check +++ b/test/files/run/t6992.check @@ -1,3 +1,4 @@ +Test.foo.T Int 42 42 diff --git a/test/files/run/t6992/Test_2.scala b/test/files/run/t6992/Test_2.scala index 0f226b01ce..1ed8958d38 100644 --- a/test/files/run/t6992/Test_2.scala +++ b/test/files/run/t6992/Test_2.scala @@ -2,7 +2,9 @@ import scala.language.reflectiveCalls object Test extends App { val foo = Macros.foo("T") - println(scala.reflect.runtime.universe.weakTypeOf[foo.T]) + val ttpe = scala.reflect.runtime.universe.weakTypeOf[foo.T] + println(ttpe) + println(ttpe.typeSymbol.typeSignature) val bar = Macros.bar("test") println(bar.test) diff --git a/test/files/run/t8104.check b/test/files/run/t8104.check index c2593eb199..40523a2868 100644 --- a/test/files/run/t8104.check +++ b/test/files/run/t8104.check @@ -1 +1,2 @@ -TypeTag[(Int, Int)] +WeakTypeTag[.this.Repr] +(Int, Int) diff --git a/test/files/run/t8104/Test_2.scala b/test/files/run/t8104/Test_2.scala index 630176f175..55c080a563 100644 --- a/test/files/run/t8104/Test_2.scala +++ b/test/files/run/t8104/Test_2.scala @@ -9,7 +9,10 @@ object Test extends App { case class C(x: Int, y: Int) import scala.reflect.runtime.universe._ - def reprify[T, Repr](x: T)(implicit generic: Generic.Aux[T, Repr], tag: TypeTag[Repr]) = println(tag) + def reprify[T, Repr](x: T)(implicit generic: Generic.Aux[T, Repr], tag: WeakTypeTag[Repr]) = { + println(tag) + println(tag.tpe.typeSymbol.typeSignature) + } reprify(C(40, 2)) implicitly[Generic.Aux[C, (Int, Int)]] -- cgit v1.2.3