From 66fe64f8f72ba7d574e07d3308d72cd3766a5763 Mon Sep 17 00:00:00 2001 From: Brian McKenna Date: Mon, 7 Jan 2013 18:17:05 +1000 Subject: SI-6923 Context now buffers warnings as well as errors Code that was silently typed would not report warnings, even if it returned a successful result. This appeared in the following code which didn't show warnings even with -Ywarn-adapted-args: def foo(a: Any) = a; foo(1, 2) While the following would show the expected warning: def foo[A](a: Any) = a; foo(1, 2) --- src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 9 +++++++++ src/compiler/scala/tools/nsc/typechecker/Typers.scala | 10 +++++++++- test/files/neg/names-defaults-neg.check | 10 +++++++++- test/files/neg/t4851.check | 8 +++++++- test/files/neg/t4851/S.scala | 5 +++++ 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index 0907f1088a..af2aeefecd 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -144,6 +144,7 @@ trait Contexts { self: Analyzer => def typingIndent = " " * typingIndentLevel var buffer: Set[AbsTypeError] = _ + var warningsBuffer: Set[(Position, String)] = _ def enclClassOrMethod: Context = if ((owner eq NoSymbol) || (owner.isClass) || (owner.isMethod)) this @@ -165,6 +166,7 @@ trait Contexts { self: Analyzer => def errBuffer = buffer def hasErrors = buffer.nonEmpty + def hasWarnings = warningsBuffer.nonEmpty def state: Int = mode def restoreState(state0: Int) = mode = state0 @@ -193,6 +195,11 @@ trait Contexts { self: Analyzer => buffer.clear() current } + def flushAndReturnWarningsBuffer(): Set[(Position, String)] = { + val current = warningsBuffer.clone() + warningsBuffer.clear() + current + } def logError(err: AbsTypeError) = buffer += err @@ -282,6 +289,7 @@ trait Contexts { self: Analyzer => c.retyping = this.retyping c.openImplicits = this.openImplicits c.buffer = if (this.buffer == null) LinkedHashSet[AbsTypeError]() else this.buffer // need to initialize + c.warningsBuffer = if (this.warningsBuffer == null) LinkedHashSet[(Position, String)]() else this.warningsBuffer registerContext(c.asInstanceOf[analyzer.Context]) debuglog("[context] ++ " + c.unit + " / " + tree.summaryString) c @@ -406,6 +414,7 @@ trait Contexts { self: Analyzer => def warning(pos: Position, msg: String): Unit = warning(pos, msg, false) def warning(pos: Position, msg: String, force: Boolean) { if (reportErrors || force) unit.warning(pos, msg) + else if (bufferErrors) warningsBuffer += ((pos, msg)) } def isLocal(): Boolean = tree match { diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 9d390476db..8722aa3666 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -728,7 +728,15 @@ trait Typers extends Modes with Adaptations with Tags { if (context1.hasErrors) { stopStats() SilentTypeError(context1.errBuffer.head) - } else SilentResultValue(result) + } else { + // If we have a successful result, emit any warnings it created. + if (context1.hasWarnings) { + context1.flushAndReturnWarningsBuffer() foreach { + case (pos, msg) => unit.warning(pos, msg) + } + } + SilentResultValue(result) + } } else { assert(context.bufferErrors || isPastTyper, "silent mode is not available past typer") withSavedContext(context){ diff --git a/test/files/neg/names-defaults-neg.check b/test/files/neg/names-defaults-neg.check index 6f9dc7d127..f6bd703e1f 100644 --- a/test/files/neg/names-defaults-neg.check +++ b/test/files/neg/names-defaults-neg.check @@ -134,9 +134,17 @@ names-defaults-neg.scala:144: error: variable definition needs type because 'x' names-defaults-neg.scala:147: error: variable definition needs type because 'x' is used as a named argument in its body. object t6 { var x = t.f(x = 1) } ^ +names-defaults-neg.scala:147: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment +in the current scope. The resulting type inference error (see above) can be fixed by providing an explicit type in the local definition for x. + object t6 { var x = t.f(x = 1) } + ^ names-defaults-neg.scala:150: error: variable definition needs type because 'x' is used as a named argument in its body. class t9 { var x = t.f(x = 1) } ^ +names-defaults-neg.scala:150: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment +in the current scope. The resulting type inference error (see above) can be fixed by providing an explicit type in the local definition for x. + class t9 { var x = t.f(x = 1) } + ^ names-defaults-neg.scala:164: error: variable definition needs type because 'x' is used as a named argument in its body. def u3 { var x = u.f(x = 1) } ^ @@ -156,5 +164,5 @@ in the current scope. The resulting type inference error (see above) can be fixe names-defaults-neg.scala:180: error: reference to x is ambiguous; it is both a method parameter and a variable in scope. class u18 { var x: Int = u.f(x = 1) } ^ -two warnings found +four warnings found 41 errors found diff --git a/test/files/neg/t4851.check b/test/files/neg/t4851.check index 8011350f23..9633fdffed 100644 --- a/test/files/neg/t4851.check +++ b/test/files/neg/t4851.check @@ -40,4 +40,10 @@ S.scala:10: error: Adapting argument list by inserting (): this is unlikely to b after adaptation: new J2((): Unit) val z2 = new J2() ^ -7 errors found +S.scala:14: error: Adapting argument list by creating a 3-tuple: this may not be what you want. + signature: Test.anyId(a: Any): Any + given arguments: 1, 2, 3 + after adaptation: Test.anyId((1, 2, 3): (Int, Int, Int)) + val w1 = anyId(1, 2 ,3) + ^ +8 errors found diff --git a/test/files/neg/t4851/S.scala b/test/files/neg/t4851/S.scala index 1550892967..0a442ac7a9 100644 --- a/test/files/neg/t4851/S.scala +++ b/test/files/neg/t4851/S.scala @@ -10,6 +10,9 @@ object Test { val z2 = new J2() val z3 = new J2(()) + def anyId(a: Any) = a + val w1 = anyId(1, 2 ,3) + def main(args: Array[String]): Unit = { println(x1) println(x2) @@ -19,5 +22,7 @@ object Test { println(z1) println(z2) println(z3) + + println(w1) } } -- cgit v1.2.3 From 765386ff970af8d53aaa66a42b030e83043d471d Mon Sep 17 00:00:00 2001 From: James Iry Date: Mon, 14 Jan 2013 11:24:11 -0800 Subject: SI-5568 Fixes verify error from getClass on refinement of value type ().asInstanceOf[AnyRef with Unit].getClass and 5.asInstanceOf[AnyRef with Int].getClass would cause a verify error. Going the other way, i.e. [Unit with AnyRef] or [Int with AnyRef] worked fine. This commit fixes it that both directions work out to BoxedUnit or java.lang.Integer. --- .../scala/tools/nsc/transform/Erasure.scala | 23 ++++++++++++++++------ test/files/run/t5568.check | 9 +++++++++ test/files/run/t5568.scala | 18 +++++++++++++++++ 3 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 test/files/run/t5568.check create mode 100644 test/files/run/t5568.scala diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index 7b9b13ae1c..58494cb18e 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -340,12 +340,18 @@ abstract class Erasure extends AddInterfaces case _ => tp.deconst } } - // Methods on Any/Object which we rewrite here while we still know what - // is a primitive and what arrived boxed. - private lazy val interceptedMethods = Set[Symbol](Any_##, Object_##, Any_getClass, AnyVal_getClass) ++ ( - // Each value class has its own getClass for ultra-precise class object typing. + + // Each value class has its own getClass for ultra-precise class object typing. + private lazy val primitiveGetClassMethods = Set[Symbol](Any_getClass, AnyVal_getClass) ++ ( ScalaValueClasses map (_.tpe member nme.getClass_) ) + + // ## requires a little translation + private lazy val poundPoundMethods = Set[Symbol](Any_##, Object_##) + + // Methods on Any/Object which we rewrite here while we still know what + // is a primitive and what arrived boxed. + private lazy val interceptedMethods = poundPoundMethods ++ primitiveGetClassMethods // -------- erasure on trees ------------------------------------------ @@ -1136,7 +1142,7 @@ abstract class Erasure extends AddInterfaces args) } } else if (args.isEmpty && interceptedMethods(fn.symbol)) { - if (fn.symbol == Any_## || fn.symbol == Object_##) { + if (poundPoundMethods.contains(fn.symbol)) { // This is unattractive, but without it we crash here on ().## because after // erasure the ScalaRunTime.hash overload goes from Unit => Int to BoxedUnit => Int. // This must be because some earlier transformation is being skipped on ##, but so @@ -1152,9 +1158,14 @@ abstract class Erasure extends AddInterfaces } else if (isPrimitiveValueClass(qual.tpe.typeSymbol)) { // Rewrite 5.getClass to ScalaRunTime.anyValClass(5) global.typer.typed(gen.mkRuntimeCall(nme.anyValClass, List(qual, typer.resolveClassTag(tree.pos, qual.tpe.widen)))) - } else if (fn.symbol == AnyVal_getClass) { + } else if (primitiveGetClassMethods.contains(fn.symbol)) { + // if we got here then we're trying to send a primitive getClass method to + // a) an Any, or + // b) a non-primitive, e.g. because the qualifier's type is a refinement type where part of the refinement is a primitive. + // if we use Object_getClass then things work out because we will call getClass on the boxed form of the Any or primitive tree setSymbol Object_getClass } else { + debugwarn(s"The symbol '${fn.symbol}' was interecepted but didn't match any cases, that means the intercepted methods set doesn't match the code") tree } } else qual match { diff --git a/test/files/run/t5568.check b/test/files/run/t5568.check new file mode 100644 index 0000000000..67aaf16e07 --- /dev/null +++ b/test/files/run/t5568.check @@ -0,0 +1,9 @@ +void +int +class scala.runtime.BoxedUnit +class scala.runtime.BoxedUnit +class java.lang.Integer +class java.lang.Integer +5 +5 +5 diff --git a/test/files/run/t5568.scala b/test/files/run/t5568.scala new file mode 100644 index 0000000000..7fc51fe86f --- /dev/null +++ b/test/files/run/t5568.scala @@ -0,0 +1,18 @@ +object Test { + final val UNIT: AnyRef with Unit = ().asInstanceOf[AnyRef with Unit] + + def main(args: Array[String]): Unit = { + // these should give unboxed results + println(().getClass) + println(5.getClass) + // these should give boxed results + println(().asInstanceOf[AnyRef with Unit].getClass) + println(().asInstanceOf[Unit with AnyRef].getClass) + println(5.asInstanceOf[AnyRef with Int].getClass) + println(5.asInstanceOf[Int with AnyRef].getClass) + //make sure ## wasn't broken + println(5.##) + println((5.asInstanceOf[AnyRef]).##) + println((5:Any).##) + } +} -- cgit v1.2.3 From c6065591c981e38aedf50618faee945a8b1e5423 Mon Sep 17 00:00:00 2001 From: James Iry Date: Tue, 15 Jan 2013 11:51:36 -0800 Subject: SI-5568 Comment improvements for getClass on primitive intersection. Based on code review here are a few comment cleanups and the removal of some dead test code. --- src/compiler/scala/tools/nsc/transform/Erasure.scala | 14 +++++++++----- test/files/run/t5568.scala | 2 -- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index 58494cb18e..41aada473a 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -341,7 +341,7 @@ abstract class Erasure extends AddInterfaces } } - // Each value class has its own getClass for ultra-precise class object typing. + // Each primitive value class has its own getClass for ultra-precise class object typing. private lazy val primitiveGetClassMethods = Set[Symbol](Any_getClass, AnyVal_getClass) ++ ( ScalaValueClasses map (_.tpe member nme.getClass_) ) @@ -1159,10 +1159,14 @@ abstract class Erasure extends AddInterfaces // Rewrite 5.getClass to ScalaRunTime.anyValClass(5) global.typer.typed(gen.mkRuntimeCall(nme.anyValClass, List(qual, typer.resolveClassTag(tree.pos, qual.tpe.widen)))) } else if (primitiveGetClassMethods.contains(fn.symbol)) { - // if we got here then we're trying to send a primitive getClass method to - // a) an Any, or - // b) a non-primitive, e.g. because the qualifier's type is a refinement type where part of the refinement is a primitive. - // if we use Object_getClass then things work out because we will call getClass on the boxed form of the Any or primitive + // if we got here then we're trying to send a primitive getClass method to either + // a) an Any, in which cage Object_getClass works because Any erases to object. Or + // + // b) a non-primitive, e.g. because the qualifier's type is a refinement type where one parent + // of the refinement is a primitive and another is AnyRef. In that case + // we get a primitive form of _getClass trying to target a boxed value + // so we need replace that method name with Object_getClass to get correct behavior. + // See SI-5568. tree setSymbol Object_getClass } else { debugwarn(s"The symbol '${fn.symbol}' was interecepted but didn't match any cases, that means the intercepted methods set doesn't match the code") diff --git a/test/files/run/t5568.scala b/test/files/run/t5568.scala index 7fc51fe86f..14599d9ed2 100644 --- a/test/files/run/t5568.scala +++ b/test/files/run/t5568.scala @@ -1,6 +1,4 @@ object Test { - final val UNIT: AnyRef with Unit = ().asInstanceOf[AnyRef with Unit] - def main(args: Array[String]): Unit = { // these should give unboxed results println(().getClass) -- cgit v1.2.3 From a6b34b60feaea763fd5a056eb55f25aa1f57988a Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 15 Jan 2013 15:01:54 -0800 Subject: SI-6956 determine switchability by type, not tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constant folding will set the type of a constant tree to `ConstantType(Constant(folded))`, while the tree itself can be many different things (in casu, an Ident). We used to look at the tree directly when deciding whether to emit a switch. Now we look at the tree's type. Voilà. --- .../tools/nsc/typechecker/PatternMatching.scala | 6 +++-- test/files/run/t6956.check | 1 + test/files/run/t6956.scala | 26 ++++++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t6956.check create mode 100644 test/files/run/t6956.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index d8cfd5a765..454d9210ff 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -3494,8 +3494,10 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL val alternativesSupported = true val canJump = true - object SwitchablePattern { def unapply(pat: Tree): Option[Tree] = pat match { - case Literal(const@Constant((_: Byte ) | (_: Short) | (_: Int ) | (_: Char ))) => + // Constant folding sets the type of a constant tree to `ConstantType(Constant(folded))` + // The tree itself can be a literal, an ident, a selection, ... + object SwitchablePattern { def unapply(pat: Tree): Option[Tree] = pat.tpe match { + case ConstantType(const@Constant((_: Byte ) | (_: Short) | (_: Int ) | (_: Char ))) => Some(Literal(Constant(const.intValue))) // TODO: Java 7 allows strings in switches case _ => None }} diff --git a/test/files/run/t6956.check b/test/files/run/t6956.check new file mode 100644 index 0000000000..0cfbf08886 --- /dev/null +++ b/test/files/run/t6956.check @@ -0,0 +1 @@ +2 diff --git a/test/files/run/t6956.scala b/test/files/run/t6956.scala new file mode 100644 index 0000000000..4a6583ca45 --- /dev/null +++ b/test/files/run/t6956.scala @@ -0,0 +1,26 @@ +import scala.tools.partest.IcodeTest + +class Switches { + private[this] final val ONE = 1 + + def switchBad(i: Byte): Int = i match { + case ONE => 1 + case 2 => 2 + case 3 => 3 + case _ => 0 + } + + def switchOkay(i: Byte): Int = i match { + case 1 => 1 + case 2 => 2 + case 3 => 3 + case _ => 0 + } +} + +object Test extends IcodeTest { + // ensure we get two switches out of this -- ignore the rest of the output for robustness + // exclude the constant we emit for the "SWITCH ..." string below (we get the icode for all the code you see in this file) + override def show() = println(collectIcode("").filter(x => x.indexOf("SWITCH ...") >= 0 && x.indexOf("CONSTANT(") == -1).size) +} + -- cgit v1.2.3 From ce563164a3e64d8a7a5ca1f49dd62377d603b5d9 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 15 Jan 2013 16:13:14 -0800 Subject: use Constant::isIntRange even if it's NIH --- src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index 454d9210ff..f1c70f46d8 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -3497,7 +3497,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // Constant folding sets the type of a constant tree to `ConstantType(Constant(folded))` // The tree itself can be a literal, an ident, a selection, ... object SwitchablePattern { def unapply(pat: Tree): Option[Tree] = pat.tpe match { - case ConstantType(const@Constant((_: Byte ) | (_: Short) | (_: Int ) | (_: Char ))) => + case ConstantType(const) if const.isIntRange => Some(Literal(Constant(const.intValue))) // TODO: Java 7 allows strings in switches case _ => None }} -- cgit v1.2.3 From 8a74b7bd136f691d9d60c7dd10ddf96a45e32329 Mon Sep 17 00:00:00 2001 From: amin Date: Wed, 16 Jan 2013 09:59:12 +0100 Subject: Closes SI-6952: add correct error positions for Dynamic feature check. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 6 +++--- test/files/neg/t6040.check | 4 +++- test/files/neg/t6952.check | 13 +++++++++++++ test/files/neg/t6952.scala | 4 ++++ 4 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 test/files/neg/t6952.check create mode 100644 test/files/neg/t6952.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index b4c5365516..2efc1df1e5 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1740,8 +1740,8 @@ trait Typers extends Modes with Adaptations with Tags { */ def validateParentClasses(parents: List[Tree], selfType: Type) { val pending = ListBuffer[AbsTypeError]() - def validateDynamicParent(parent: Symbol) = - if (parent == DynamicClass) checkFeature(parent.pos, DynamicsFeature) + def validateDynamicParent(parent: Symbol, parentPos: Position) = + if (parent == DynamicClass) checkFeature(parentPos, DynamicsFeature) def validateParentClass(parent: Tree, superclazz: Symbol) = if (!parent.isErrorTyped) { @@ -1791,7 +1791,7 @@ trait Typers extends Modes with Adaptations with Tags { if (parents exists (p => p != parent && p.tpe.typeSymbol == psym && !psym.isError)) pending += ParentInheritedTwiceError(parent, psym) - validateDynamicParent(psym) + validateDynamicParent(psym, parent.pos) } if (!parents.isEmpty && parents.forall(!_.isErrorTyped)) { diff --git a/test/files/neg/t6040.check b/test/files/neg/t6040.check index f6757f97e3..f91df0c46d 100644 --- a/test/files/neg/t6040.check +++ b/test/files/neg/t6040.check @@ -1,7 +1,9 @@ -error: extension of type scala.Dynamic needs to be enabled +t6040.scala:1: error: extension of type scala.Dynamic needs to be enabled by making the implicit value language.dynamics visible. This can be achieved by adding the import clause 'import scala.language.dynamics' or by setting the compiler option -language:dynamics. See the Scala docs for value scala.language.dynamics for a discussion why the feature needs to be explicitly enabled. +class X extends Dynamic + ^ one error found diff --git a/test/files/neg/t6952.check b/test/files/neg/t6952.check new file mode 100644 index 0000000000..f1e1881404 --- /dev/null +++ b/test/files/neg/t6952.check @@ -0,0 +1,13 @@ +t6952.scala:2: error: extension of type scala.Dynamic needs to be enabled +by making the implicit value language.dynamics visible. +This can be achieved by adding the import clause 'import scala.language.dynamics' +or by setting the compiler option -language:dynamics. +See the Scala docs for value scala.language.dynamics for a discussion +why the feature needs to be explicitly enabled. +trait B extends Dynamic + ^ +t6952.scala:3: error: extension of type scala.Dynamic needs to be enabled +by making the implicit value language.dynamics visible. +trait C extends A with Dynamic + ^ +two errors found diff --git a/test/files/neg/t6952.scala b/test/files/neg/t6952.scala new file mode 100644 index 0000000000..257ea3be68 --- /dev/null +++ b/test/files/neg/t6952.scala @@ -0,0 +1,4 @@ +trait A +trait B extends Dynamic +trait C extends A with Dynamic +trait D extends B -- cgit v1.2.3 From d9d6494fa7704ebacfa74e92a964381895bbf8d4 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 16 Jan 2013 15:11:31 +0100 Subject: SI-6976 Fix value class separate compilation crasher. We can't guarantee that the owner of the value class is initialized, and if it isn't, the search for the companion module will turn up bubkis. This is a localized fix, but I'd be suprised if there weren't other places that suffered from the same problem. Wouldn't it be nicer to have something like: // doesn't force info sym.raw.info sym.raw.companionModule // forces info sym.info sym.companionModule --- .../tools/nsc/transform/ExtensionMethods.scala | 12 ++++++---- test/files/pos/t6976/Exts_1.scala | 10 ++++++++ test/files/pos/t6976/ImplicitBug_1.scala | 27 ++++++++++++++++++++++ test/files/pos/t6976/ImplicitBug_2.scala | 7 ++++++ 4 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 test/files/pos/t6976/Exts_1.scala create mode 100644 test/files/pos/t6976/ImplicitBug_1.scala create mode 100644 test/files/pos/t6976/ImplicitBug_2.scala diff --git a/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala b/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala index 6f3d7932a5..5318524870 100644 --- a/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala +++ b/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala @@ -64,14 +64,18 @@ abstract class ExtensionMethods extends Transform with TypingTransformers { } } - /** Return the extension method that corresponds to given instance method `meth`. - */ + private def companionModuleForce(sym: Symbol) = { + sym.andAlso(_.owner.initialize) // See SI-6976. `companionModule` only calls `rawInfo`. (Why?) + sym.companionModule + } + + /** Return the extension method that corresponds to given instance method `meth`. */ def extensionMethod(imeth: Symbol): Symbol = atPhase(currentRun.refchecksPhase) { - val companionInfo = imeth.owner.companionModule.info + val companionInfo = companionModuleForce(imeth.owner).info val candidates = extensionNames(imeth) map (companionInfo.decl(_)) filter (_.exists) val matching = candidates filter (alt => normalize(alt.tpe, imeth.owner) matches imeth.tpe) assert(matching.nonEmpty, - s"no extension method found for $imeth:${imeth.tpe} among ${candidates map (c => c.name+":"+c.tpe)} / ${extensionNames(imeth)}") + s"no extension method found for $imeth:${imeth.tpe} among ${candidates.map(c => c.name+":"+c.tpe).toList} / ${extensionNames(imeth).toList}") matching.head } diff --git a/test/files/pos/t6976/Exts_1.scala b/test/files/pos/t6976/Exts_1.scala new file mode 100644 index 0000000000..9b3a69edd9 --- /dev/null +++ b/test/files/pos/t6976/Exts_1.scala @@ -0,0 +1,10 @@ +object Exts { + implicit class AnyExts[T](val o: T) extends AnyVal { + def moo = "moo!" + } +} + +trait Exts { + import language.implicitConversions + implicit def AnyExts[T](o: T) = Exts.AnyExts(o) +} diff --git a/test/files/pos/t6976/ImplicitBug_1.scala b/test/files/pos/t6976/ImplicitBug_1.scala new file mode 100644 index 0000000000..c9031bab2e --- /dev/null +++ b/test/files/pos/t6976/ImplicitBug_1.scala @@ -0,0 +1,27 @@ +// This one is weird and nasty. Not sure if this is scalac or sbt +// (tried with 0.12 & 0.12.2-RC2) bug. +// +// A level of indirection is required to trigger this bug. +// Exts seems to need to be defined in separate file. +// +// Steps to reproduce: +// 1. sbt clean +// 2. sbt run (it works) +// 3. Comment A & uncomment B. +// 4. sbt run (it fails) +// 5. Switch it back & sbt run. It still fails. +// +// In this project sbt clean helps. However in a large project where this +// bug was found compiler crashed even after doing sbt clean. The only +// way to work around this was to reference Exts object explicitly (C) in +// the source file using its implicit classes. + +// Lets suppose this is a mega-trait combining all sorts of helper +// functionality. +trait Support extends Exts + +object ImplicitsBug extends App with Support { // A +// object ImplicitsBug extends App with Exts { // B + //Exts // C) this reference helped in the large project. + println(3.moo) +} diff --git a/test/files/pos/t6976/ImplicitBug_2.scala b/test/files/pos/t6976/ImplicitBug_2.scala new file mode 100644 index 0000000000..2fea5e2993 --- /dev/null +++ b/test/files/pos/t6976/ImplicitBug_2.scala @@ -0,0 +1,7 @@ +trait Support extends Exts + +// object ImplicitsBug extends App with Support { // A +object ImplicitsBug extends App with Exts { // B + //Exts // C) this reference helped in the large project. + println(3.moo) +} -- cgit v1.2.3 From b07228aebe7aa620af45a681ef60d945ffc65665 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 16 Jan 2013 23:05:12 +0100 Subject: SI-6601 Publicise derived value contstructor after pickler Otherwise the access restrictions are not enforced under separate compilation. See also SI-6608. --- src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala | 1 - src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 2 ++ test/files/neg/t6601.check | 4 ++++ test/files/neg/t6601/AccessPrivateConstructor_2.scala | 3 +++ test/files/neg/t6601/PrivateConstructor_1.scala | 1 + 5 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t6601.check create mode 100644 test/files/neg/t6601/AccessPrivateConstructor_2.scala create mode 100644 test/files/neg/t6601/PrivateConstructor_1.scala diff --git a/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala b/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala index 6f3d7932a5..79e51d5daa 100644 --- a/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala +++ b/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala @@ -140,7 +140,6 @@ abstract class ExtensionMethods extends Transform with TypingTransformers { wrap over other value classes anyway. checkNonCyclic(currentOwner.pos, Set(), currentOwner) */ extensionDefs(currentOwner.companionModule) = new mutable.ListBuffer[Tree] - currentOwner.primaryConstructor.makeNotPrivate(NoSymbol) super.transform(tree) } else if (currentOwner.isStaticOwner) { super.transform(tree) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 9bd3aa8fe5..7118375b82 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1677,6 +1677,8 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans val bridges = addVarargBridges(currentOwner) checkAllOverrides(currentOwner) checkAnyValSubclass(currentOwner) + if (currentOwner.isDerivedValueClass) + currentOwner.primaryConstructor makeNotPrivate NoSymbol // SI-6601, must be done *after* pickler! if (bridges.nonEmpty) deriveTemplate(tree)(_ ::: bridges) else tree case dc@TypeTreeWithDeferredRefCheck() => abort("adapt should have turned dc: TypeTreeWithDeferredRefCheck into tpt: TypeTree, with tpt.original == dc") diff --git a/test/files/neg/t6601.check b/test/files/neg/t6601.check new file mode 100644 index 0000000000..1410e1b11a --- /dev/null +++ b/test/files/neg/t6601.check @@ -0,0 +1,4 @@ +AccessPrivateConstructor_2.scala:2: error: constructor PrivateConstructor in class PrivateConstructor cannot be accessed in class AccessPrivateConstructor + new PrivateConstructor("") // Scalac should forbid accessing to the private constructor! + ^ +one error found diff --git a/test/files/neg/t6601/AccessPrivateConstructor_2.scala b/test/files/neg/t6601/AccessPrivateConstructor_2.scala new file mode 100644 index 0000000000..816bc10d79 --- /dev/null +++ b/test/files/neg/t6601/AccessPrivateConstructor_2.scala @@ -0,0 +1,3 @@ +class AccessPrivateConstructor { + new PrivateConstructor("") // Scalac should forbid accessing to the private constructor! +} diff --git a/test/files/neg/t6601/PrivateConstructor_1.scala b/test/files/neg/t6601/PrivateConstructor_1.scala new file mode 100644 index 0000000000..f09d7ad068 --- /dev/null +++ b/test/files/neg/t6601/PrivateConstructor_1.scala @@ -0,0 +1 @@ +class PrivateConstructor private(val s: String) extends AnyVal -- cgit v1.2.3 From f5397818aa6c9822ce6593e3eec02edfffdc4f2e Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Wed, 16 Jan 2013 16:27:05 -0800 Subject: SI-6942 more efficient unreachability analysis Avoid blowing the stack/the analysis budget by more eagerly translating the propositions that model matches to CNF. First building a large proposition that represents the match, and then converting to CNF tends to blow the stack. Luckily, it's easy to convert to CNF as we go. The optimization relies on `CNF(P1 /\ ... /\ PN) == CNF(P1) ++ CNF(...) ++ CNF(PN)`: Normalizing a conjunction of propositions yields the same formula as concatenating the normalized conjuncts. CNF conversion is expensive for large propositions, so we push it down into the conjunction and then concatenate the resulting arrays of clauses (which is cheap). (CNF converts a free-form proposition into an `Array[Set[Lit]]`, where: - the Array's elements are /\'ed together; - and the Set's elements are \/'ed; - a Lit is a possibly negated variable.) NOTE: - removeVarEq may throw an AnalysisBudget.Exception - also reworked the interface used to build formula, so that we can more easily plug in SAT4J when the time comes --- .../tools/nsc/typechecker/PatternMatching.scala | 72 ++++--- test/files/pos/t6942.flags | 1 + test/files/pos/t6942/Bar.java | 235 +++++++++++++++++++++ test/files/pos/t6942/t6942.scala | 64 ++++++ 4 files changed, 348 insertions(+), 24 deletions(-) create mode 100644 test/files/pos/t6942.flags create mode 100644 test/files/pos/t6942/Bar.java create mode 100644 test/files/pos/t6942/t6942.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index f1c70f46d8..df1267b98f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -1954,7 +1954,8 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // // TODO: for V1 representing x1 and V2 standing for x1.head, encode that // V1 = Nil implies -(V2 = Ci) for all Ci in V2's domain (i.e., it is unassignable) - def removeVarEq(props: List[Prop], modelNull: Boolean = false): (Prop, List[Prop]) = { + // may throw an AnalysisBudget.Exception + def removeVarEq(props: List[Prop], modelNull: Boolean = false): (Formula, List[Formula]) = { val start = if (Statistics.canEnable) Statistics.startTimer(patmatAnaVarEq) else null val vars = new scala.collection.mutable.HashSet[Var] @@ -1978,10 +1979,10 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL props foreach gatherEqualities.apply if (modelNull) vars foreach (_.registerNull) - val pure = props map rewriteEqualsToProp.apply + val pure = props map (p => eqFreePropToSolvable(rewriteEqualsToProp(p))) - var eqAxioms: Prop = True - def addAxiom(p: Prop) = eqAxioms = And(eqAxioms, p) + val eqAxioms = formulaBuilder + @inline def addAxiom(p: Prop) = addFormula(eqAxioms, eqFreePropToSolvable(p)) patmatDebug("removeVarEq vars: "+ vars) vars.foreach { v => @@ -2007,23 +2008,37 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL } } - patmatDebug("eqAxioms:\n"+ cnfString(eqFreePropToSolvable(eqAxioms))) - patmatDebug("pure:"+ pure.map(p => cnfString(eqFreePropToSolvable(p))).mkString("\n")) + patmatDebug("eqAxioms:\n"+ cnfString(toFormula(eqAxioms))) + patmatDebug("pure:"+ pure.map(p => cnfString(p)).mkString("\n")) if (Statistics.canEnable) Statistics.stopTimer(patmatAnaVarEq, start) - (eqAxioms, pure) + (toFormula(eqAxioms), pure) } + // an interface that should be suitable for feeding a SAT solver when the time comes type Formula + type FormulaBuilder + + // creates an empty formula builder to which more formulae can be added + def formulaBuilder: FormulaBuilder + + // val f = formulaBuilder; addFormula(f, f1); ... addFormula(f, fN) + // toFormula(f) == andFormula(f1, andFormula(..., fN)) + def addFormula(buff: FormulaBuilder, f: Formula): Unit + def toFormula(buff: FormulaBuilder): Formula + + // the conjunction of formulae `a` and `b` def andFormula(a: Formula, b: Formula): Formula + // equivalent formula to `a`, but simplified in a lightweight way (drop duplicate clauses) + def simplifyFormula(a: Formula): Formula // may throw an AnalysisBudget.Exception def propToSolvable(p: Prop): Formula = { val (eqAxioms, pure :: Nil) = removeVarEq(List(p), modelNull = false) - eqFreePropToSolvable(And(eqAxioms, pure)) + andFormula(eqAxioms, pure) } // may throw an AnalysisBudget.Exception @@ -2039,24 +2054,34 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL } trait CNF extends Logic { - // CNF: a formula is a conjunction of clauses - type Formula = Array[Clause] /** Override Array creation for efficiency (to not go through reflection). */ private implicit val clauseTag: scala.reflect.ClassTag[Clause] = new scala.reflect.ClassTag[Clause] { def runtimeClass: java.lang.Class[Clause] = classOf[Clause] final override def newArray(len: Int): Array[Clause] = new Array[Clause](len) } + + import scala.collection.mutable.ArrayBuffer + type FormulaBuilder = ArrayBuffer[Clause] + def formulaBuilder = ArrayBuffer[Clause]() + def addFormula(buff: FormulaBuilder, f: Formula): Unit = buff ++= f + def toFormula(buff: FormulaBuilder): Formula = buff.toArray + + // CNF: a formula is a conjunction of clauses + type Formula = Array[Clause] def formula(c: Clause*): Formula = c.toArray - def andFormula(a: Formula, b: Formula): Formula = a ++ b + type Clause = Set[Lit] // a clause is a disjunction of distinct literals - type Clause = Set[Lit] def clause(l: Lit*): Clause = l.toSet - private def merge(a: Clause, b: Clause) = a ++ b type Lit def Lit(sym: Sym, pos: Boolean = true): Lit + def andFormula(a: Formula, b: Formula): Formula = a ++ b + def simplifyFormula(a: Formula): Formula = a.distinct + + private def merge(a: Clause, b: Clause) = a ++ b + // throws an AnalysisBudget.Exception when the prop results in a CNF that's too big // TODO: be smarter/more efficient about this (http://lara.epfl.ch/w/sav09:tseitin_s_encoding) def eqFreePropToSolvable(p: Prop): Formula = { @@ -2621,23 +2646,22 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL val propsCasesOk = testCasesOk map (t => symbolicCase(t, modelNull = true)) val propsCasesFail = testCasesFail map (t => Not(symbolicCase(t, modelNull = true))) - val (eqAxiomsFail, symbolicCasesFail) = removeVarEq(propsCasesFail, modelNull = true) - val (eqAxiomsOk, symbolicCasesOk) = removeVarEq(propsCasesOk, modelNull = true) try { - // most of the time eqAxiomsFail == eqAxiomsOk, but the different approximations might cause different variables to disapper in general - val eqAxiomsCNF = - if (eqAxiomsFail == eqAxiomsOk) eqFreePropToSolvable(eqAxiomsFail) - else eqFreePropToSolvable(And(eqAxiomsFail, eqAxiomsOk)) + val (eqAxiomsFail, symbolicCasesFail) = removeVarEq(propsCasesFail, modelNull = true) + val (eqAxiomsOk, symbolicCasesOk) = removeVarEq(propsCasesOk, modelNull = true) + val eqAxioms = simplifyFormula(andFormula(eqAxiomsOk, eqAxiomsFail)) // I'm pretty sure eqAxiomsOk == eqAxiomsFail, but not 100% sure. + + val prefix = formulaBuilder + addFormula(prefix, eqAxioms) - var prefix = eqAxiomsCNF var prefixRest = symbolicCasesFail var current = symbolicCasesOk var reachable = true var caseIndex = 0 patmatDebug("reachability, vars:\n"+ ((propsCasesFail flatMap gatherVariables).distinct map (_.describe) mkString ("\n"))) - patmatDebug("equality axioms:\n"+ cnfString(eqAxiomsCNF)) + patmatDebug("equality axioms:\n"+ cnfString(eqAxiomsOk)) // invariant (prefixRest.length == current.length) && (prefix.reverse ++ prefixRest == symbolicCasesFail) // termination: prefixRest.length decreases by 1 @@ -2647,11 +2671,11 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL prefixRest = prefixRest.tail if (prefixRest.isEmpty) reachable = true else { - prefix = andFormula(eqFreePropToSolvable(prefHead), prefix) + addFormula(prefix, prefHead) current = current.tail - val model = findModelFor(andFormula(eqFreePropToSolvable(current.head), prefix)) + val model = findModelFor(andFormula(current.head, toFormula(prefix))) - // patmatDebug("trying to reach:\n"+ cnfString(eqFreePropToSolvable(current.head)) +"\nunder prefix:\n"+ cnfString(prefix)) + // patmatDebug("trying to reach:\n"+ cnfString(current.head) +"\nunder prefix:\n"+ cnfString(prefix)) // if (NoModel ne model) patmatDebug("reached: "+ modelString(model)) reachable = NoModel ne model diff --git a/test/files/pos/t6942.flags b/test/files/pos/t6942.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t6942.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t6942/Bar.java b/test/files/pos/t6942/Bar.java new file mode 100644 index 0000000000..592f62efb4 --- /dev/null +++ b/test/files/pos/t6942/Bar.java @@ -0,0 +1,235 @@ +package foo; + +public enum Bar { + ANGUILLA /*("US")*/, + ANTIGUA_AND_BARBUDA /*("US")*/, + ARGENTINA /*("US")*/, + ARUBA /*("US")*/, + BAHAMAS /*("US")*/, + BARBADOS /*("US")*/, + BELIZE /*("US")*/, + BERMUDA /*("US")*/, + BOLIVIA /*("US")*/, + BRAZIL /*("US")*/, + BRITISH_VIRGIN_ISLANDS /*("US")*/, + CANADA /*("US")*/, + CAYMAN_ISLANDS /*("US")*/, + CHILE /*("US")*/, + CHRISTMAS_ISLANDS /*("US")*/, + COCOS /*("US")*/, + COLOMBIA /*("US")*/, + COSTA_RICA /*("US")*/, + CUBA /*("US")*/, + DOMINICA /*("US")*/, + DOMINICAN_REPUBLIC /*("US")*/, + ECUADOR /*("US")*/, + EL_SALVADOR /*("US")*/, + FALKLAND_ISLANDS /*("US")*/, + GRENADA /*("US")*/, + GUADALOUPE /*("US")*/, + GUATEMALA /*("US")*/, + HAITI /*("US")*/, + HONDURAS /*("US")*/, + NETHERLANDS_ANTILLES /*("US")*/, + NICARAGUA /*("US")*/, + PANAMA /*("US")*/, + PARAGUAY /*("US")*/, + PERU /*("US")*/, + PUERTO_RICO /*("US")*/, + JAMAICA /*("US")*/, + MARTINIQUE /*("US")*/, + MEXICO /*("US")*/, + MONTSERRAT /*("US")*/, + ST_KITTS /*("US")*/, + ST_LUCIA /*("US")*/, + ST_VINCENT /*("US")*/, + SUPRA_NATIONAL /*("US")*/, + TRINIDAD /*("US")*/, + TURKS_AND_CAICOS /*("US")*/, + UNITED_STATES /*("US")*/, + URUGUAY /*("US")*/, + VENEZUELA /*("US")*/, + VIRGIN_ISLANDS /*("US")*/, + + AUSTRALIA /*("AP")*/, + BANGLADESH /*("AP")*/, + BHUTAN /*("AP")*/, + CAMBODIA /*("AP")*/, + CHINA /*("AP")*/, + COOK_ISLANDS /*("AP")*/, + EAST_TIMOR /*("AP")*/, + FIJI /*("AP")*/, + GUAM /*("AP")*/, + HONG_KONG /*("AP")*/, + INDIA /*("AP")*/, + INDONESIA /*("AP")*/, + JAPAN /*("AP")*/, + KIRIBATI /*("AP")*/, + LAOS /*("AP")*/, + MACAU /*("AP")*/, + MALAYSIA /*("AP")*/, + MICRONESIA /*("AP")*/, + MONGOLIA /*("AP")*/, + MYANMAR /*("AP")*/, + NEPAL /*("AP")*/, + NEW_CALEDONIA /*("AP")*/, + NEW_ZEALAND /*("AP")*/, + NORFOLK_ISLAND /*("AP")*/, + NORTH_KOREA /*("AP")*/, + PAKISTAN /*("AP")*/, + PALAU /*("AP")*/, + PAPUA_NEW_GUINEA /*("AP")*/, + PHILIPPINES /*("AP")*/, + PITCAIRN_ISLANDS /*("AP")*/, + SAMOA /*("AP")*/, + WEST_SAMOA /*("AP")*/, + SINGAPORE /*("AP")*/, + SOUTH_KOREA /*("AP")*/, + SRI_LANKA /*("AP")*/, + TAIWAN /*("AP")*/, + THAILAND /*("AP")*/, + TOKELAU /*("AP")*/, + TONGA /*("AP")*/, + TUVALU /*("AP")*/, + VANUATU /*("AP")*/, + VIETNAM /*("AP")*/, + + AFGHANISTAN /*("EU")*/, + ALBANIA /*("EU")*/, + ALGERIA /*("EU")*/, + ANDORRA /*("EU")*/, + ANGOLA /*("EU")*/, + ARMENIA /*("EU")*/, + AUSTRIA /*("EU")*/, + AZERBAIJAN /*("EU")*/, + BAHRAIN /*("EU")*/, + BELARUS /*("EU")*/, + BELGIUM /*("EU")*/, + BENIN /*("EU")*/, + BOSNIA_AND_HERZEGOVINA /*("EU")*/, + BOTSWANA /*("EU")*/, + BOUVET_ISLAND /*("EU")*/, + BRUNEI /*("EU")*/, + BULGARIA /*("EU")*/, + BURKINA_FASO /*("EU")*/, + BURUNDI /*("EU")*/, + CAMEROON /*("EU")*/, + CAPE_VERDE /*("EU")*/, + CHAD /*("EU")*/, + COMOROS /*("EU")*/, + CONGO /*("EU")*/, + CROATIA /*("EU")*/, + CYPRUS /*("EU")*/, + CZECH_REPUBLIC /*("EU")*/, + DR_CONGO /*("EU")*/, + DENMARK /*("EU")*/, + DJIBOUTI /*("EU")*/, + EGYPT /*("EU")*/, + EQUATORIAL_GUINEA /*("EU")*/, + ERITREA /*("EU")*/, + ESTONIA /*("EU")*/, + ETHIOPIA /*("EU")*/, + FAEROE_ISLANDS /*("EU")*/, + FINLAND /*("EU")*/, + FRANCE /*("EU")*/, + FRENCH_GUIANA /*("EU")*/, + GABON /*("EU")*/, + GAMBIA /*("EU")*/, + GEORGIA /*("EU")*/, + GERMANY /*("EU")*/, + GHANA /*("EU")*/, + GIBRALTAR /*("EU")*/, + GREAT_BRITAIN /*("EU")*/, + GREECE /*("EU")*/, + GREENLAND /*("EU")*/, + GUINEA /*("EU")*/, + GUINEA_BISSAU /*("EU")*/, + GUYANA /*("EU")*/, + HUNGARY /*("EU")*/, + ICELAND /*("EU")*/, + IRAN /*("EU")*/, + IRAQ /*("EU")*/, + IRELAND /*("EU")*/, + ISLE_OF_MAN /*("EU")*/, + ISRAEL /*("EU")*/, + ITALY /*("EU")*/, + IVORY_COAST /*("EU")*/, + JERSEY /*("EU")*/, + JORDAN /*("EU")*/, + KAZAKHSTAN /*("EU")*/, + KENYA /*("EU")*/, + KUWAIT /*("EU")*/, + KYRGYZSTAN /*("EU")*/, + LATVIA /*("EU")*/, + LEBANON /*("EU")*/, + LESOTHO /*("EU")*/, + LIBERIA /*("EU")*/, + LIBYA /*("EU")*/, + LIECHTENSTEIN /*("EU")*/, + LITHUANIA /*("EU")*/, + LUXEMBOURG /*("EU")*/, + MACEDONIA /*("EU")*/, + MADAGASCAR /*("EU")*/, + MALAWI /*("EU")*/, + MALDIVES /*("EU")*/, + MALI /*("EU")*/, + MALTA /*("EU")*/, + MARSHALL_ISLAND /*("EU")*/, + MAURITANIA /*("EU")*/, + MAURITIUS /*("EU")*/, + MAYOTTE /*("EU")*/, + MOLDOVA /*("EU")*/, + MONACO /*("EU")*/, + MOROCCO /*("EU")*/, + MOZAMBIQUE /*("EU")*/, + NAMIBIA /*("EU")*/, + NETHERLANDS /*("EU")*/, + NIGER_REPUBLIC /*("EU")*/, + NIGERIA /*("EU")*/, + NORWAY /*("EU")*/, + OMAN /*("EU")*/, + PALESTINE /*("EU")*/, + POLAND /*("EU")*/, + PORTUGAL /*("EU")*/, + QATAR /*("EU")*/, + REUNION /*("EU")*/, + ROMANIA /*("EU")*/, + RUSSIA /*("EU")*/, + RWANDA /*("EU")*/, + SAN_MARINO /*("EU")*/, + SAO_TOME /*("EU")*/, + SAUDI_ARABIA /*("EU")*/, + SENEGAL /*("EU")*/, + SERBIA /*("EU")*/, + SEYCHELLES /*("EU")*/, + SEIRRA_LEONE /*("EU")*/, + SLOVAKIA /*("EU")*/, + SLOVENIA /*("EU")*/, + SOMALIA /*("EU")*/, + SOUTH_AFRICA /*("EU")*/, + SPAIN /*("EU")*/, + ST_HELENA /*("EU")*/, + SUDAN /*("EU")*/, + SURINAME /*("EU")*/, + SVALBARD /*("EU")*/, + SWAZILAND /*("EU")*/, + SWEDEN /*("EU")*/, + SWITZERLAND /*("EU")*/, + SYRIA /*("EU")*/, + TAJIKSTAN /*("EU")*/, + TANZANIA /*("EU")*/, + TOGO /*("EU")*/, + TUNISIA /*("EU")*/, + TURKEY /*("EU")*/, + TURKMENISTAN /*("EU")*/, + UAE /*("EU")*/, + UGANDA /*("EU")*/, + UKRAINE /*("EU")*/, + UZBEKISTAN /*("EU")*/, + VATICAN_CITY /*("EU")*/, + WESTERN_SAHARA /*("EU")*/, + YEMEN /*("EU")*/, + ZAMBIA /*("EU")*/, + ZIMBABWE /*("EU")*/; + +} \ No newline at end of file diff --git a/test/files/pos/t6942/t6942.scala b/test/files/pos/t6942/t6942.scala new file mode 100644 index 0000000000..77963d2634 --- /dev/null +++ b/test/files/pos/t6942/t6942.scala @@ -0,0 +1,64 @@ +// not a peep out of the pattern matcher's unreachability analysis +// its budget should suffice for these simple matches (they do have a large search space) +class Test { + import foo.Bar // a large enum + def exhaustUnreachabilitysStack_ENUM_STYLE = (null: Bar) match { + case Bar.BULGARIA => + case _ => + } + + // lots of strings + def exhaustUnreachabilitysStack_StringStyle = "foo" match { + case "a" => + case "b" => + case "c" => + case "d" => + case "e" => + case "f" => + case "aa" => + case "ba" => + case "ca" => + case "da" => + case "ea" => + case "f1a" => + case "a1a" => + case "b1a" => + case "c1a" => + case "d1a" => + case "e1a" => + case "f1a2" => + case "f1a0" => + case "a1a2" => + case "b1a2" => + case "c1a2" => + case "d1a2" => + case "e1a2" => + case "f1a3" => + case "_a" => + case "_b" => + case "_c" => + case "_d" => + case "_e" => + case "_f" => + case "_aa" => + case "_ba" => + case "_ca" => + case "_da" => + case "_ea" => + case "_f1a" => + case "_a1a" => + case "_b1a" => + case "_c1a" => + case "_d1a" => + case "_e1a" => + case "_f1a0" => + case "_f1a2" => + case "_a1a2" => + case "_b1a2" => + case "_c1a2" => + case "_d1a2" => + case "_e1a2" => + case "_f1a3" => + case _ => + } +} -- cgit v1.2.3 From 964776f528a8ec4da889638ab41e5dbc8a9164a1 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 17 Jan 2013 11:45:10 -0800 Subject: use ArrayBuffer instead of Array to build Formulae --- src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index df1267b98f..cdceb2d992 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -2064,11 +2064,11 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL type FormulaBuilder = ArrayBuffer[Clause] def formulaBuilder = ArrayBuffer[Clause]() def addFormula(buff: FormulaBuilder, f: Formula): Unit = buff ++= f - def toFormula(buff: FormulaBuilder): Formula = buff.toArray + def toFormula(buff: FormulaBuilder): Formula = buff // CNF: a formula is a conjunction of clauses - type Formula = Array[Clause] - def formula(c: Clause*): Formula = c.toArray + type Formula = FormulaBuilder + def formula(c: Clause*): Formula = ArrayBuffer(c: _*) type Clause = Set[Lit] // a clause is a disjunction of distinct literals -- cgit v1.2.3 From cbd0205999d19e378f9f7ac8ca685a134862cf47 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 19 Jan 2013 11:57:52 +0100 Subject: SI-6902 Check unreachability under @unchecked Only exhaustiveness checking should be disabled if the scrutinee of a match as annotated as `: @unchecked`. This was the pre-2.10.x behaviour. This also fixes a variation of the closed ticket, SI-6011. The exhaustiveness check is needed to safely fallback from emitting a table switch if duplicate cases are detected. --- .../tools/nsc/typechecker/PatternMatching.scala | 17 ++++++++-------- test/files/neg/t6902.check | 10 ++++++++++ test/files/neg/t6902.flags | 1 + test/files/neg/t6902.scala | 23 ++++++++++++++++++++++ test/files/run/t6011c.scala | 13 ++++++++++++ 5 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 test/files/neg/t6902.check create mode 100644 test/files/neg/t6902.flags create mode 100644 test/files/neg/t6902.scala create mode 100644 test/files/run/t6011c.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index fa8aff5cdd..9746b05c03 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -3225,6 +3225,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // TODO: make more fine-grained, as we don't always need to jump def canJump: Boolean + /** Should exhaustivity analysis be skipped? */ def unchecked: Boolean @@ -3458,12 +3459,10 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL case Some(cds) => cds } - val allReachable = unchecked || { - // a switch with duplicate cases yields a verify error, - // and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch - // (even though the verify error would disappear, the behaviour would change) - unreachableCase(caseDefsWithGuards) map (cd => reportUnreachable(cd.body.pos)) isEmpty - } + // a switch with duplicate cases yields a verify error, + // and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch + // (even though the verify error would disappear, the behaviour would change) + val allReachable = unreachableCase(caseDefsWithGuards) map (cd => reportUnreachable(cd.body.pos)) isEmpty if (!allReachable) Nil else if (noGuards(caseDefsWithGuards)) { @@ -3710,10 +3709,10 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL with SymbolicMatchAnalysis with DPLLSolver { self: TreeMakers => override def optimizeCases(prevBinder: Symbol, cases: List[List[TreeMaker]], pt: Type, unchecked: Boolean): (List[List[TreeMaker]], List[Tree]) = { + unreachableCase(prevBinder, cases, pt) foreach { caseIndex => + reportUnreachable(cases(caseIndex).last.pos) + } if (!unchecked) { - unreachableCase(prevBinder, cases, pt) foreach { caseIndex => - reportUnreachable(cases(caseIndex).last.pos) - } val counterExamples = exhaustive(prevBinder, cases, pt) if (counterExamples.nonEmpty) reportMissingCases(prevBinder.pos, counterExamples) diff --git a/test/files/neg/t6902.check b/test/files/neg/t6902.check new file mode 100644 index 0000000000..8ad7fd37f9 --- /dev/null +++ b/test/files/neg/t6902.check @@ -0,0 +1,10 @@ +t6902.scala:4: error: unreachable code + case Some(b) => 3 // no warning was emitted + ^ +t6902.scala:9: error: unreachable code + case Some(b) => 3 // no warning was emitted + ^ +t6902.scala:21: error: unreachable code + case 1 => 3 // crash + ^ +three errors found diff --git a/test/files/neg/t6902.flags b/test/files/neg/t6902.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/neg/t6902.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/neg/t6902.scala b/test/files/neg/t6902.scala new file mode 100644 index 0000000000..ce5ff8b6fb --- /dev/null +++ b/test/files/neg/t6902.scala @@ -0,0 +1,23 @@ +object Test { + Some(Some(1)) collect { + case Some(a) => 2 + case Some(b) => 3 // no warning was emitted + } + + (Some(1): @ unchecked) match { + case Some(a) => 2 + case Some(b) => 3 // no warning was emitted + } + + // A variation of SI-6011, which eluded the fix + // in 2.10.0. + // + // duplicate keys in SWITCH, can't pick arbitrarily one of them to evict, see SI-6011. + // at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:50) + // at scala.tools.nsc.Global.abort(Global.scala:249) + // at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder$jcode$.emitSWITCH(GenASM.scala:1850) + ((1: Byte): @unchecked @annotation.switch) match { + case 1 => 2 + case 1 => 3 // crash + } +} diff --git a/test/files/run/t6011c.scala b/test/files/run/t6011c.scala new file mode 100644 index 0000000000..0647e3f81a --- /dev/null +++ b/test/files/run/t6011c.scala @@ -0,0 +1,13 @@ +object Test extends App { + // A variation of SI-6011, which eluded the fix + // in 2.10.0. + // + // duplicate keys in SWITCH, can't pick arbitrarily one of them to evict, see SI-6011. + // at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:50) + // at scala.tools.nsc.Global.abort(Global.scala:249) + // at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder$jcode$.emitSWITCH(GenASM.scala:1850) + ((1: Byte): @unchecked @annotation.switch) match { + case 1 => 2 + case 1 => 3 // crash + } +} -- cgit v1.2.3 From 3486d47508686e4b96560e176280fa9fc536fd41 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 19 Jan 2013 13:01:31 +0100 Subject: SI-6439 Avoid spurious REPL warnings about companionship `val m` isn't a companion of `trait m`, check the pair of eponymous symbols are a ((class|trait), object) pair before emitting the warning. In order to correctly check this one a type alias is involved, `definedSymbols` must avoid normalizing through type aliases. AFAICT this is an improvement to the other clients of that Map, one such power mode progression is demonstrated at the end of the test case. --- .../scala/tools/nsc/interpreter/IMain.scala | 3 +- test/files/run/t6439.check | 66 ++++++++++++++++++++++ test/files/run/t6439.scala | 22 ++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t6439.check create mode 100644 test/files/run/t6439.scala diff --git a/src/compiler/scala/tools/nsc/interpreter/IMain.scala b/src/compiler/scala/tools/nsc/interpreter/IMain.scala index b46d28dec3..a55f0af116 100644 --- a/src/compiler/scala/tools/nsc/interpreter/IMain.scala +++ b/src/compiler/scala/tools/nsc/interpreter/IMain.scala @@ -386,6 +386,7 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends oldReq <- definedNameMap get name.companionName newSym <- req.definedSymbols get name oldSym <- oldReq.definedSymbols get name.companionName + if Seq(oldSym, newSym).permutations exists { case Seq(s1, s2) => s1.isClass && s2.isModule } } { afterTyper(replwarn(s"warning: previously defined $oldSym is not a companion to $newSym.")) replwarn("Companions must be defined together; you may wish to use :paste mode for this.") @@ -970,7 +971,7 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends // } lazy val definedSymbols = ( termNames.map(x => x -> applyToResultMember(x, x => x)) ++ - typeNames.map(x => x -> compilerTypeOf(x).typeSymbol) + typeNames.map(x => x -> compilerTypeOf(x).typeSymbolDirect) ).toMap[Name, Symbol] withDefaultValue NoSymbol lazy val typesOfDefinedTerms = mapFrom[Name, Name, Type](termNames)(x => applyToResultMember(x, _.tpe)) diff --git a/test/files/run/t6439.check b/test/files/run/t6439.check new file mode 100644 index 0000000000..178ea739f5 --- /dev/null +++ b/test/files/run/t6439.check @@ -0,0 +1,66 @@ +Type in expressions to have them evaluated. +Type :help for more information. + +scala> + +scala> class A +defined class A + +scala> object A // warn +defined module A +warning: previously defined class A is not a companion to object A. +Companions must be defined together; you may wish to use :paste mode for this. + +scala> trait B +defined trait B + +scala> object B // warn +defined module B +warning: previously defined trait B is not a companion to object B. +Companions must be defined together; you may wish to use :paste mode for this. + +scala> object C +defined module C + +scala> object Bippy +defined module Bippy + +scala> class C // warn +defined class C +warning: previously defined object C is not a companion to class C. +Companions must be defined together; you may wish to use :paste mode for this. + +scala> class D +defined class D + +scala> def D = 0 // no warn +D: Int + +scala> val D = 0 // no warn +D: Int = 0 + +scala> object E +defined module E + +scala> var E = 0 // no warn +E: Int = 0 + +scala> object F +defined module F + +scala> type F = Int // no warn +defined type alias F + +scala> :power +** Power User mode enabled - BEEP WHIR GYVE ** +** :phase has been set to 'typer'. ** +** scala.tools.nsc._ has been imported ** +** global._, definitions._ also imported ** +** Try :help, :vals, power. ** + +scala> intp("F") // this now works as a result of changing .typeSymbol to .typeSymbolDirect in IMain#Request#definedSymbols +res0: $r.intp.global.Symbol = type F + +scala> + +scala> diff --git a/test/files/run/t6439.scala b/test/files/run/t6439.scala new file mode 100644 index 0000000000..70a2dbafaf --- /dev/null +++ b/test/files/run/t6439.scala @@ -0,0 +1,22 @@ +import scala.tools.partest.ReplTest + +object Test extends ReplTest { + def code = """ +class A +object A // warn +trait B +object B // warn +object C +object Bippy +class C // warn +class D +def D = 0 // no warn +val D = 0 // no warn +object E +var E = 0 // no warn +object F +type F = Int // no warn +:power +intp("F") // this now works as a result of changing .typeSymbol to .typeSymbolDirect in IMain#Request#definedSymbols + """ +} -- cgit v1.2.3 From 52a53280172695f651bd3913a0fcb1ac6260c33d Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 19 Jan 2013 23:05:32 +0100 Subject: Addressing warnings. - SI-6923 uncovered a few valid warnings, these have been addressed. - A pair of "catches all throwable" warnings appeared; one of the is spurious and the subject of SI-6994. --- src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala | 2 +- src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala | 2 +- src/library/scala/collection/mutable/HashSet.scala | 2 +- src/library/scala/collection/parallel/mutable/ParArray.scala | 3 --- src/library/scala/collection/parallel/mutable/ParHashSet.scala | 2 +- 5 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala index bbab545d9e..64051b56ec 100644 --- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala +++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala @@ -1360,7 +1360,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { debuglog("obtained env: " + e) e.keySet == env.keySet } catch { - case _ => + case _: Throwable => debuglog("Could not unify.") false } diff --git a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala index 4268398081..dc367b11fd 100644 --- a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala +++ b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala @@ -730,7 +730,7 @@ trait ContextErrors { } catch { // the code above tries various tricks to detect the relevant portion of the stack trace // if these tricks fail, just fall back to uninformative, but better than nothing, getMessage - case NonFatal(ex) => + case NonFatal(ex) => // currently giving a spurious warning, see SI-6994 macroLogVerbose("got an exception when processing a macro generated exception\n" + "offender = " + stackTraceString(realex) + "\n" + "error = " + stackTraceString(ex)) diff --git a/src/library/scala/collection/mutable/HashSet.scala b/src/library/scala/collection/mutable/HashSet.scala index 74f2a6c762..c60e363f8f 100644 --- a/src/library/scala/collection/mutable/HashSet.scala +++ b/src/library/scala/collection/mutable/HashSet.scala @@ -88,7 +88,7 @@ extends AbstractSet[A] } private def readObject(in: java.io.ObjectInputStream) { - init(in, x => x) + init(in, x => ()) } /** Toggles whether a size map is used to track hash map statistics. diff --git a/src/library/scala/collection/parallel/mutable/ParArray.scala b/src/library/scala/collection/parallel/mutable/ParArray.scala index e4c8e5fae2..0a4f30131f 100644 --- a/src/library/scala/collection/parallel/mutable/ParArray.scala +++ b/src/library/scala/collection/parallel/mutable/ParArray.scala @@ -469,7 +469,6 @@ self => Array.copy(arr, i, targetarr, 0, until - i) pac.buff.size = pac.buff.size + until - i pac.buff.lastPtr.size = until - i - pac } otherwise { copy2builder_quick(cb, arr, until, i) i = until @@ -531,7 +530,6 @@ self => val targetarr: Array[Any] = pac.lastbuff.internalArray.asInstanceOf[Array[Any]] reverse2combiner_quick(targetarr, arr, 0, i, until) pac.lastbuff.setInternalSize(sz) - pac } otherwise { cb.ifIs[UnrolledParArrayCombiner[T]] { pac => @@ -542,7 +540,6 @@ self => reverse2combiner_quick(targetarr, arr, 0, i, until) pac.buff.size = pac.buff.size + sz pac.buff.lastPtr.size = sz - pac } otherwise super.reverse2combiner(cb) } cb diff --git a/src/library/scala/collection/parallel/mutable/ParHashSet.scala b/src/library/scala/collection/parallel/mutable/ParHashSet.scala index 3b1278f3be..57fab57348 100644 --- a/src/library/scala/collection/parallel/mutable/ParHashSet.scala +++ b/src/library/scala/collection/parallel/mutable/ParHashSet.scala @@ -85,7 +85,7 @@ extends ParSet[T] } private def readObject(in: java.io.ObjectInputStream) { - init(in, x => x) + init(in, x => ()) } import scala.collection.DebugUtils._ -- cgit v1.2.3 From 8f498847020c5623559fa42d5d23e44fb74c8d22 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 20 Jan 2013 00:18:38 +0100 Subject: SI-6994 Avoid spurious promiscuous catch warning It was being issued upon re-typechecking of a transformed tree. Now we disable the warning post-typer. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 2 +- test/files/pos/t6994.flags | 1 + test/files/pos/t6994.scala | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 test/files/pos/t6994.flags create mode 100644 test/files/pos/t6994.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index cbcddba487..5ad6c6bd73 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5367,7 +5367,7 @@ trait Typers extends Modes with Adaptations with Tags { var block1 = typed(tree.block, pt) var catches1 = typedCases(tree.catches, ThrowableClass.tpe, pt) - for (cdef <- catches1 if cdef.guard.isEmpty) { + for (cdef <- catches1 if !isPastTyper && cdef.guard.isEmpty) { def warn(name: Name) = context.warning(cdef.pat.pos, s"This catches all Throwables. If this is really intended, use `case ${name.decoded} : Throwable` to clear this warning.") def unbound(t: Tree) = t.symbol == null || t.symbol == NoSymbol cdef.pat match { diff --git a/test/files/pos/t6994.flags b/test/files/pos/t6994.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t6994.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t6994.scala b/test/files/pos/t6994.scala new file mode 100644 index 0000000000..d707196423 --- /dev/null +++ b/test/files/pos/t6994.scala @@ -0,0 +1,8 @@ +object Test { + object NF { + def unapply(t: Throwable): Option[Throwable] = None + } + val x = (try { None } catch { case NF(ex) => None }) getOrElse 0 + // Was emitting a spurious warning post typer: + // "This catches all Throwables. If this is really intended, use `case ex6 : Throwable` to clear this warning." +} -- cgit v1.2.3 From 1a7de4314ac72bca81e31ad3ac0af7bee7eed26b Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 20 Jan 2013 12:31:22 +0100 Subject: SI-6666 Restrict hidden `this` access in self/super calls. Detect when classes (user authored or compiler generated) local to a self or super constructor argument would require premature access to the in-construction instance. The same restriction applies for classes and objects; for objects, the premature access would result in a null via MODULE$ field. A residual error has been lodged as SI-6997. I'd like to remove calls to `Symbol#outerClass` (which relies on the flaky flag INCONSTRUCTOR, see my comments in the JIRA issue for more discussion) from `LambdaLift` and `ExplicitOuter`, and instead use the stack of active self/super calls to know when to skip an enclosing class. That will obviate that flag. --- .../scala/tools/nsc/transform/LambdaLift.scala | 38 ++++-- test/files/neg/t6666.check | 40 +++++++ test/files/neg/t6666.scala | 132 +++++++++++++++++++++ 3 files changed, 203 insertions(+), 7 deletions(-) create mode 100644 test/files/neg/t6666.check create mode 100644 test/files/neg/t6666.scala diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index 4a23e65ad2..a4f75f424f 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -320,12 +320,24 @@ abstract class LambdaLift extends InfoTransform { private def memberRef(sym: Symbol) = { val clazz = sym.owner.enclClass //Console.println("memberRef from "+currentClass+" to "+sym+" in "+clazz) - val qual = if (clazz == currentClass) gen.mkAttributedThis(clazz) - else { - sym resetFlag(LOCAL | PRIVATE) - if (clazz.isStaticOwner) gen.mkAttributedQualifier(clazz.thisType) - else outerPath(outerValue, currentClass.outerClass, clazz) - } + def prematureSelfReference() { + val what = + if (clazz.isStaticOwner) clazz.fullLocationString + else s"the unconstructed `this` of ${clazz.fullLocationString}" + val msg = s"Implementation restriction: access of ${sym.fullLocationString} from ${currentClass.fullLocationString}, would require illegal premature access to $what" + currentUnit.error(curTree.pos, msg) + } + val qual = + if (clazz == currentClass) gen.mkAttributedThis(clazz) + else { + sym resetFlag (LOCAL | PRIVATE) + if (selfOrSuperCalls exists (_.owner == clazz)) { + prematureSelfReference() + EmptyTree + } + else if (clazz.isStaticOwner) gen.mkAttributedQualifier(clazz.thisType) + else outerPath(outerValue, currentClass.outerClass, clazz) + } Select(qual, sym) setType sym.tpe } @@ -495,13 +507,25 @@ abstract class LambdaLift extends InfoTransform { private def preTransform(tree: Tree) = super.transform(tree) setType lifted(tree.tpe) + /** The stack of constructor symbols in which a call to this() or to the super + * constructor is active. + */ + private val selfOrSuperCalls = mutable.Stack[Symbol]() + @inline private def inSelfOrSuperCall[A](sym: Symbol)(a: => A) = try { + selfOrSuperCalls push sym + a + } finally selfOrSuperCalls.pop() + override def transform(tree: Tree): Tree = tree match { case Select(ReferenceToBoxed(idt), elem) if elem == nme.elem => postTransform(preTransform(idt), isBoxedRef = false) case ReferenceToBoxed(idt) => postTransform(preTransform(idt), isBoxedRef = true) case _ => - postTransform(preTransform(tree)) + def transformTree = postTransform(preTransform(tree)) + if (treeInfo isSelfOrSuperConstrCall tree) + inSelfOrSuperCall(currentOwner)(transformTree) + else transformTree } /** Transform statements and add lifted definitions to them. */ diff --git a/test/files/neg/t6666.check b/test/files/neg/t6666.check new file mode 100644 index 0000000000..d0378173ea --- /dev/null +++ b/test/files/neg/t6666.check @@ -0,0 +1,40 @@ +t6666.scala:23: error: Implementation restriction: access of method x$2 in object O1 from anonymous class 2, would require illegal premature access to object O1 + F.byname(x) + ^ +t6666.scala:30: error: Implementation restriction: access of value x$3 in object O2 from anonymous class 3, would require illegal premature access to object O2 + F.byname(x) + ^ +t6666.scala:37: error: Implementation restriction: access of method x$4 in object O3 from anonymous class 4, would require illegal premature access to object O3 + F.hof(() => x) + ^ +t6666.scala:50: error: Implementation restriction: access of method x$6 in class C1 from anonymous class 7, would require illegal premature access to the unconstructed `this` of class C1 + F.byname(x) + ^ +t6666.scala:54: error: Implementation restriction: access of value x$7 in class C2 from anonymous class 8, would require illegal premature access to the unconstructed `this` of class C2 + F.byname(x) + ^ +t6666.scala:58: error: Implementation restriction: access of method x$8 in class C3 from anonymous class 9, would require illegal premature access to the unconstructed `this` of class C3 + F.hof(() => x) + ^ +t6666.scala:62: error: Implementation restriction: access of method x$9 in class C4 from object Nested$5, would require illegal premature access to the unconstructed `this` of class C4 + object Nested { def xx = x} + ^ +t6666.scala:68: error: Implementation restriction: access of method x$10 in class C5 from object Nested$6, would require illegal premature access to the unconstructed `this` of class C5 + object Nested { def xx = x} + ^ +t6666.scala:83: error: Implementation restriction: access of method x$12 in class C11 from anonymous class 12, would require illegal premature access to the unconstructed `this` of class C11 + F.byname(x) + ^ +t6666.scala:102: error: Implementation restriction: access of method x$13 in class C13 from anonymous class 13, would require illegal premature access to the unconstructed `this` of class C13 + F.hof(() => x) + ^ +t6666.scala:111: error: Implementation restriction: access of method x$14 in class C14 from object Nested$7, would require illegal premature access to the unconstructed `this` of class C14 + object Nested { def xx = x} + ^ +t6666.scala:122: error: Implementation restriction: access of method x$15 in class C15 from object Nested$8, would require illegal premature access to the unconstructed `this` of class C15 + object Nested { def xx = x} + ^ +t6666.scala:131: error: Implementation restriction: access of method foo$1 in class COuter from class CInner$1, would require illegal premature access to the unconstructed `this` of class COuter + class CInner extends C({foo}) + ^ +13 errors found diff --git a/test/files/neg/t6666.scala b/test/files/neg/t6666.scala new file mode 100644 index 0000000000..d37ffaf141 --- /dev/null +++ b/test/files/neg/t6666.scala @@ -0,0 +1,132 @@ +class C(a: Any) +object F { + def byname(a: => Any) = println(a) + def hof(a: () => Any) = println(a()) +} + +class COkay extends C(0) { + def this(a: Any) { + this() + def x = "".toString + F.byname(x) + } +} + +// +// The thunk's apply method accesses the MODULE$ +// field before it is set. +// +// 0: getstatic #23; //Field O1$.MODULE$:LO1$; +// 3: invokevirtual #26; //Method O1$.O1$$x$1:()Ljava/lang/String; +object O1 extends C({ + def x = "".toString + F.byname(x) +}) + +// java.lang.NullPointerException +// at O2$$anonfun$$init$$1.apply(:11) +object O2 extends C({ + lazy val x = "".toString + F.byname(x) +}) + +// java.lang.NullPointerException +// at O3$$anonfun$$init$$1.apply(:11) +object O3 extends C({ + def x = "".toString + F.hof(() => x) +}) + +// Okay, the nested classes don't get an outer pointer passed, +// just an extra param for `x: String`. +object O6 extends C({ + val x = "".toString + F.byname(x); F.hof(() => x); (new { val xx = x }.xx) +}) + + +class C1 extends C({ + def x = "".toString + F.byname(x) +}) +class C2 extends C({ + lazy val x = "".toString + F.byname(x) +}) +class C3 extends C({ + def x = "".toString + F.hof(() => x) +}) +class C4 extends C({ + def x = "".toString + object Nested { def xx = x} + Nested.xx +}) +class C5 extends C({ + def x = "".toString + val y = { + object Nested { def xx = x} + Nested.xx + } +}) + +// okay, for same reason as O6 +class C6 extends C({ + val x = "".toString + F.byname(x); F.hof(() => x); (new { val xx = x }.xx) +}) + +class C11(a: Any) { + def this() = { + this({ + def x = "".toString + F.byname(x) + }) + } +} + +// Crashes earlier in lazyVals. +// class C12(a: Any) { +// def this() = { +// this({ +// lazy val x = "".toString +// F.byname(x) +// }) +// } +// } + +class C13(a: Any) { + def this() = { + this({ + def x = "".toString + F.hof(() => x) + }) + } +} + +class C14(a: Any) { + def this() = { + this({ + def x = "".toString + object Nested { def xx = x} + Nested.xx + }) + } +} + +class C15(a: Any) { + def this() = { + this({ + def x = "".toString + val y = { + object Nested { def xx = x} + Nested.xx + } + }) + } +} + +class COuter extends C({ + def foo = 0 + class CInner extends C({foo}) +}) \ No newline at end of file -- cgit v1.2.3 From f6168b8a4d661985b0fb4d6d3cbba256bfc69607 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 21 Jan 2013 10:45:36 +0100 Subject: SI-6231 Report unsupported free var capture by a trait. If a class nested in a trait captures a free variable from the enclosing scope of the trait, the transformation to add that variable to the `init` method of the trait implementation class happens *after* the abstract trait interface has been extracted. This would lead to a crash when trying to find the corresponding interface method. This commit detects this situation and reports an implementation restriction. The enclosed test case shows a workaround. To lift this restriction, LambdaLifter should add the getters and make sure they end up in the trait interface. Looks like Martin tried this once: // LambdaLift.scala // // Disabled attempt to to add getters to freeParams // this does not work yet. Problem is that local symbols need local names // and references to local symbols need to be transformed into // method calls to setters. // def paramGetter(param: Symbol): Tree = { // val getter = param.newGetter setFlag TRANS_FLAG resetFlag PARAMACCESSOR // mark because we have to add them to interface // sym.info.decls.enter(getter) // val rhs = Select(gen.mkAttributedThis(sym), param) setType param.tpe // DefDef(getter, rhs) setPos tree.pos setType NoType // } // val newDefs = if (sym.isTrait) freeParams ::: (ps map paramGetter) else freeParams --- .../scala/tools/nsc/transform/LambdaLift.scala | 3 ++- src/compiler/scala/tools/nsc/transform/Mixin.scala | 19 +++++++++++++++++-- test/files/neg/t6231.check | 6 ++++++ test/files/neg/t6231.scala | 15 +++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 test/files/neg/t6231.check create mode 100644 test/files/neg/t6231.scala diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index 4a23e65ad2..cd2bffcfde 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -352,6 +352,7 @@ abstract class LambdaLift extends InfoTransform { copyDefDef(tree)(vparamss = List(vparams ++ freeParams)) case ClassDef(_, _, _, _) => + // SI-6231 // Disabled attempt to to add getters to freeParams // this does not work yet. Problem is that local symbols need local names // and references to local symbols need to be transformed into @@ -369,7 +370,7 @@ abstract class LambdaLift extends InfoTransform { tree } -/* Something like this will be necessary to eliminate the implementation +/* SI-6231: Something like this will be necessary to eliminate the implementation * restiction from paramGetter above: * We need to pass getters to the interface of an implementation class. private def fixTraitGetters(lifted: List[Tree]): List[Tree] = diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala index 57bdaea17a..3cd943aa74 100644 --- a/src/compiler/scala/tools/nsc/transform/Mixin.scala +++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala @@ -1215,9 +1215,24 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL { // refer to fields in some implementation class via an abstract // getter in the interface. val iface = toInterface(sym.owner.tpe).typeSymbol - val getter = sym getter iface orElse abort("No getter for " + sym + " in " + iface) + val ifaceGetter = sym getter iface + + def si6231Restriction() { + // See SI-6231 comments in LamdaLift for ideas on how to lift the restriction. + val msg = sm"""Implementation restriction: local ${iface.fullLocationString} is unable to automatically capture the + |free variable ${sym} on behalf of ${currentClass}. You can manually assign it to a val inside the trait, + |and refer that that val in ${currentClass}. For more details, see SI-6231.""" + reporter.error(tree.pos, msg) + } - typedPos(tree.pos)((qual DOT getter)()) + if (ifaceGetter == NoSymbol) { + if (sym.isParamAccessor) { + si6231Restriction() + EmptyTree + } + else abort("No getter for " + sym + " in " + iface) + } + else typedPos(tree.pos)((qual DOT ifaceGetter)()) case Assign(Apply(lhs @ Select(qual, _), List()), rhs) => // assign to fields in some implementation class via an abstract diff --git a/test/files/neg/t6231.check b/test/files/neg/t6231.check new file mode 100644 index 0000000000..b27961d393 --- /dev/null +++ b/test/files/neg/t6231.check @@ -0,0 +1,6 @@ +t6231.scala:4: error: Implementation restriction: local trait Bug$X$1 is unable to automatically capture the +free variable value ev$1 on behalf of anonymous class anonfun$qux$1. You can manually assign it to a val inside the trait, +and refer that that val in anonymous class anonfun$qux$1. For more details, see SI-6231. + def qux = { () => ev } + ^ +one error found diff --git a/test/files/neg/t6231.scala b/test/files/neg/t6231.scala new file mode 100644 index 0000000000..1e5b4e0e1a --- /dev/null +++ b/test/files/neg/t6231.scala @@ -0,0 +1,15 @@ +object Bug { + def bar(ev: Any) = { + trait X { + def qux = { () => ev } + } + new X {}.qux() + + // workaround + trait Y { + val ev2 = ev // manually capture `ev` so that `ev2` is added to the trait interface. + def qux = { () => ev2 } + } + } +} + -- cgit v1.2.3 From 277f0fe5a86701e48cc594cad7ff0bd14a5c864b Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Mon, 21 Jan 2013 10:35:18 -0800 Subject: Removed class files. Someone checked in a pair of .class files. --- test/files/neg/t5753/Impls$class.class | Bin 626 -> 0 bytes test/files/neg/t5753/Impls.class | Bin 866 -> 0 bytes 2 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test/files/neg/t5753/Impls$class.class delete mode 100644 test/files/neg/t5753/Impls.class diff --git a/test/files/neg/t5753/Impls$class.class b/test/files/neg/t5753/Impls$class.class deleted file mode 100644 index 476329174e..0000000000 Binary files a/test/files/neg/t5753/Impls$class.class and /dev/null differ diff --git a/test/files/neg/t5753/Impls.class b/test/files/neg/t5753/Impls.class deleted file mode 100644 index dfcf89ed44..0000000000 Binary files a/test/files/neg/t5753/Impls.class and /dev/null differ -- cgit v1.2.3 From d972336af84b0d26c37faaa8b506adc8bba7e02c Mon Sep 17 00:00:00 2001 From: Szabolcs Berecz Date: Mon, 21 Jan 2013 23:50:27 +0100 Subject: Use the same default scalac options in all three partest frontends Make ConsoleRunner, AntRunner and SBTRunner take scalac options from "partest.scalac_opts" property. Also remove leftover "-deprecation" option from test/partest. The change to SBTRunner was not tested as sbt test is currently broken. --- src/partest/scala/tools/partest/PartestTask.scala | 2 +- src/partest/scala/tools/partest/nest/SBTRunner.scala | 2 +- test/partest | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/partest/scala/tools/partest/PartestTask.scala b/src/partest/scala/tools/partest/PartestTask.scala index d9f2bfe765..0199400ada 100644 --- a/src/partest/scala/tools/partest/PartestTask.scala +++ b/src/partest/scala/tools/partest/PartestTask.scala @@ -355,7 +355,7 @@ class PartestTask extends Task with CompilationPathProperty { javacmd foreach (x => antFileManager.JAVACMD = x.getAbsolutePath) javaccmd foreach (x => antFileManager.JAVAC_CMD = x.getAbsolutePath) - scalacArgsFlat foreach (antFileManager.SCALAC_OPTS = _) + scalacArgsFlat foreach (antFileManager.SCALAC_OPTS ++= _) timeout foreach (antFileManager.timeout = _) type TFSet = (Array[File], String, String) diff --git a/src/partest/scala/tools/partest/nest/SBTRunner.scala b/src/partest/scala/tools/partest/nest/SBTRunner.scala index b0ce6579ac..20f9c701d5 100644 --- a/src/partest/scala/tools/partest/nest/SBTRunner.scala +++ b/src/partest/scala/tools/partest/nest/SBTRunner.scala @@ -46,7 +46,7 @@ object SBTRunner extends DirectRunner { case x => sys.error("Unknown command line options: " + x) } val config = parseArgs(args, CommandLineOptions()) - fileManager.SCALAC_OPTS = config.scalacOptions + fileManager.SCALAC_OPTS ++= config.scalacOptions fileManager.CLASSPATH = config.classpath getOrElse sys.error("No classpath set") def findClasspath(jar: String, name: String): Option[String] = { diff --git a/test/partest b/test/partest index 8352f8a946..186c0d1580 100755 --- a/test/partest +++ b/test/partest @@ -77,7 +77,6 @@ fi # last arg wins, so if JAVA_OPTS already contains -Xmx or -Xms the # supplied argument will be used. JAVA_OPTS="-Xmx1024M -Xms64M $JAVA_OPTS" -[ -n "$SCALAC_OPTS" ] || SCALAC_OPTS="-deprecation" partestDebugStr="" if [ ! -z "${PARTEST_DEBUG}" ] ; then -- cgit v1.2.3 From e12a5b88acd80a41574d51c88a7776f99c3d2580 Mon Sep 17 00:00:00 2001 From: James Iry Date: Wed, 16 Jan 2013 16:44:44 -0800 Subject: SI-6987 Fixes fsc compile server verbose output Internally the fsc server code was setting a "verbose" flag, but it was always false. Fixing that gives server's verbose output, but because the output was buffered and not flushed the server's output wasn't seen until the compile run was complete. This commit fixes the verbose flag and flushes the server side output. --- src/compiler/scala/tools/nsc/CompileServer.scala | 3 ++- src/compiler/scala/tools/util/SocketServer.scala | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/compiler/scala/tools/nsc/CompileServer.scala b/src/compiler/scala/tools/nsc/CompileServer.scala index c23c1e6154..74118d1e20 100644 --- a/src/compiler/scala/tools/nsc/CompileServer.scala +++ b/src/compiler/scala/tools/nsc/CompileServer.scala @@ -92,10 +92,11 @@ class StandardCompileServer extends SocketServer { val args = input.split("\0", -1).toList val newSettings = new FscSettings(fscError) - this.verbose = newSettings.verbose.value val command = newOfflineCompilerCommand(args, newSettings) + this.verbose = newSettings.verbose.value info("Settings after normalizing paths: " + newSettings) + if (!command.files.isEmpty) info("Input files after normalizing paths: " + (command.files mkString ",")) printMemoryStats() // Update the idle timeout if given diff --git a/src/compiler/scala/tools/util/SocketServer.scala b/src/compiler/scala/tools/util/SocketServer.scala index d29a370c28..21775a01d1 100644 --- a/src/compiler/scala/tools/util/SocketServer.scala +++ b/src/compiler/scala/tools/util/SocketServer.scala @@ -16,7 +16,7 @@ trait CompileOutputCommon { def verbose: Boolean def info(msg: String) = if (verbose) echo(msg) - def echo(msg: String) = Console println msg + def echo(msg: String) = {Console println msg; Console.flush} def warn(msg: String) = System.err println msg def fatal(msg: String) = { warn(msg) ; sys.exit(1) } } -- cgit v1.2.3 From 1dab5bf91361b8d6073db05ba1dff1bc01a83220 Mon Sep 17 00:00:00 2001 From: James Iry Date: Thu, 17 Jan 2013 11:46:10 -0800 Subject: SI-6987 Tests fsc verbose output This commit includes a test of fsc's verbose output. In order for it to work, CompileServer's main method had to be modified to remove a sys exit 0 at the end. It was redundant and made testing a bit harder. In order to prevent a race condition between server and client start up, this commit also adds a server callback that decrements a CountDownLatch that the main testing thread waits for. Finally, the server had to be modified to use Console.withErr and Console.withOut instead of mutating the global System.err and System.out variables. Otherwise the test would be unreliable. --- src/compiler/scala/tools/nsc/CompileServer.scala | 41 ++++++++++++++-------- src/compiler/scala/tools/util/SocketServer.scala | 2 +- test/files/run/t6987.check | 1 + test/files/run/t6987.scala | 43 ++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 15 deletions(-) create mode 100644 test/files/run/t6987.check create mode 100644 test/files/run/t6987.scala diff --git a/src/compiler/scala/tools/nsc/CompileServer.scala b/src/compiler/scala/tools/nsc/CompileServer.scala index 74118d1e20..7a0a072bb8 100644 --- a/src/compiler/scala/tools/nsc/CompileServer.scala +++ b/src/compiler/scala/tools/nsc/CompileServer.scala @@ -174,11 +174,22 @@ object CompileServer extends StandardCompileServer { /** A directory holding redirected output */ private lazy val redirectDir = (compileSocket.tmpDir / "output-redirects").createDirectory() - private def redirect(setter: PrintStream => Unit, filename: String) { - setter(new PrintStream((redirectDir / filename).createFile().bufferedOutput())) - } - - def main(args: Array[String]) { + private def createRedirect(filename: String) = + new PrintStream((redirectDir / filename).createFile().bufferedOutput()) + + def main(args: Array[String]) = + execute(() => (), args) + + /** + * Used for internal testing. The callback is called upon + * server start, notifying the caller that the server is + * ready to run. WARNING: the callback runs in the + * server's thread, blocking the server from doing any work + * until the callback is finished. Callbacks should be kept + * simple and clients should not try to interact with the + * server while the callback is processing. + */ + def execute(startupCallback : () => Unit, args: Array[String]) { val debug = args contains "-v" if (debug) { @@ -186,14 +197,16 @@ object CompileServer extends StandardCompileServer { echo("Redirect dir is " + redirectDir) } - redirect(System.setOut, "scala-compile-server-out.log") - redirect(System.setErr, "scala-compile-server-err.log") - System.err.println("...starting server on socket "+port+"...") - System.err.flush() - compileSocket setPort port - run() - - compileSocket deletePort port - sys exit 0 + Console.withErr(createRedirect("scala-compile-server-err.log")) { + Console.withOut(createRedirect("scala-compile-server-out.log")) { + Console.err.println("...starting server on socket "+port+"...") + Console.err.flush() + compileSocket setPort port + startupCallback() + run() + + compileSocket deletePort port + } + } } } diff --git a/src/compiler/scala/tools/util/SocketServer.scala b/src/compiler/scala/tools/util/SocketServer.scala index 21775a01d1..1b06ce2ff2 100644 --- a/src/compiler/scala/tools/util/SocketServer.scala +++ b/src/compiler/scala/tools/util/SocketServer.scala @@ -17,7 +17,7 @@ trait CompileOutputCommon { def info(msg: String) = if (verbose) echo(msg) def echo(msg: String) = {Console println msg; Console.flush} - def warn(msg: String) = System.err println msg + def warn(msg: String) = {Console.err println msg; Console.flush} def fatal(msg: String) = { warn(msg) ; sys.exit(1) } } diff --git a/test/files/run/t6987.check b/test/files/run/t6987.check new file mode 100644 index 0000000000..86fc96c679 --- /dev/null +++ b/test/files/run/t6987.check @@ -0,0 +1 @@ +got successful verbose results! diff --git a/test/files/run/t6987.scala b/test/files/run/t6987.scala new file mode 100644 index 0000000000..37e91d61ae --- /dev/null +++ b/test/files/run/t6987.scala @@ -0,0 +1,43 @@ +import java.io._ +import tools.nsc.{CompileClient, CompileServer} +import java.util.concurrent.{CountDownLatch, TimeUnit} + +object Test extends App { + val startupLatch = new CountDownLatch(1) + // we have to explicitly launch our server because when the client launches a server it uses + // the "scala" shell command meaning whatever version of scala (and whatever version of libraries) + // happens to be in the path gets used + val t = new Thread(new Runnable { + def run() = { + CompileServer.execute(() => startupLatch.countDown(), Array[String]()) + } + }) + t setDaemon true + t.start() + if (!startupLatch.await(2, TimeUnit.MINUTES)) + sys error "Timeout waiting for server to start" + + val baos = new ByteArrayOutputStream() + val ps = new PrintStream(baos) + + val success = (scala.Console withOut ps) { + // shut down the server via the client using the verbose flag + CompileClient.process(Array("-shutdown", "-verbose")) + } + + // now make sure we got success and a verbose result + val msg = baos.toString() + + if (success) { + if (msg contains "Settings after normalizing paths") { + println("got successful verbose results!") + } else { + println("did not get the string expected, full results were:") + println(msg) + } + } else { + println("got a failure. Full results were:") + println(msg) + } + scala.Console.flush +} -- cgit v1.2.3 From 4dceb2268780462823a98168d7350687d5cf27a8 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 23 Jan 2013 00:28:21 +0100 Subject: [backport] Fix SI-6637 (misoptimization in erasure) commit f9ef5300ab561628e53c654df9000c75f488d74a Author: Jan Niehusmann Date: Fri Nov 9 15:05:58 2012 +0100 Fix SI-6637 (misoptimization in erasure) Move the optimization one level deeper so the expression being tested with isInstanceOf is always evaluated. (cherry picked from commit b540aaee4ba30e2dd980456a44e8c6d732222df1) --- .../scala/tools/nsc/transform/Erasure.scala | 22 +++++++++++----------- test/files/run/t6637.check | 1 + test/files/run/t6637.scala | 8 ++++++++ 3 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 test/files/run/t6637.check create mode 100644 test/files/run/t6637.scala diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index 41aada473a..889d309ba9 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -1057,17 +1057,17 @@ abstract class Erasure extends AddInterfaces Apply(Select(qual, cmpOp), List(gen.mkAttributedQualifier(targ.tpe))) } case RefinedType(parents, decls) if (parents.length >= 2) => - // Optimization: don't generate isInstanceOf tests if the static type - // conforms, because it always succeeds. (Or at least it had better.) - // At this writing the pattern matcher generates some instance tests - // involving intersections where at least one parent is statically known true. - // That needs fixing, but filtering the parents here adds an additional - // level of robustness (in addition to the short term fix.) - val parentTests = parents filterNot (qual.tpe <:< _) - - if (parentTests.isEmpty) Literal(Constant(true)) - else gen.evalOnce(qual, currentOwner, unit) { q => - atPos(tree.pos) { + gen.evalOnce(qual, currentOwner, unit) { q => + // Optimization: don't generate isInstanceOf tests if the static type + // conforms, because it always succeeds. (Or at least it had better.) + // At this writing the pattern matcher generates some instance tests + // involving intersections where at least one parent is statically known true. + // That needs fixing, but filtering the parents here adds an additional + // level of robustness (in addition to the short term fix.) + val parentTests = parents filterNot (qual.tpe <:< _) + + if (parentTests.isEmpty) Literal(Constant(true)) + else atPos(tree.pos) { parentTests map mkIsInstanceOf(q) reduceRight gen.mkAnd } } diff --git a/test/files/run/t6637.check b/test/files/run/t6637.check new file mode 100644 index 0000000000..9766475a41 --- /dev/null +++ b/test/files/run/t6637.check @@ -0,0 +1 @@ +ok diff --git a/test/files/run/t6637.scala b/test/files/run/t6637.scala new file mode 100644 index 0000000000..d3c380370b --- /dev/null +++ b/test/files/run/t6637.scala @@ -0,0 +1,8 @@ + +object Test extends App { + try { + class A ; class B ; List().head.isInstanceOf[A with B] + } catch { + case _ :java.util.NoSuchElementException => println("ok") + } +} -- cgit v1.2.3 From ba411c4c2cb4b400481ed3dffed30b6975c000c1 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 23 Jan 2013 00:40:59 +0100 Subject: [backport] Fix unsafe array opt. / opt. primitive Array(...) SI-6611, SI-6247 (partial fix) The original commits on master were a bit circuitous, this is squashed to a neat little package. I had to add type arguments to the Array.apply calls in the test case, they are inferred on master. commit 41ff05dfdbcf032157b3509ace633f2e7a12295c Author: Jason Zaugg Date: Sun Nov 4 14:44:59 2012 +0100 Refactor guards checking for a particular overload of Array.apply. (cherry picked from commit 092345a24c22a821204fb358d33272ae8f7353be) commit 1e5c942deccaf64f8d57bd8891b912381d7f220a Author: Jason Zaugg Date: Sun Nov 4 14:17:25 2012 +0100 Expand optimization of Array(e1, ..., en) to primitive arrays. (cherry picked from commit 8265175ecc42293997d59049f430396c77a2b891) commit ab1bf77e39f2dfeacf3fc107ccb2907a1867f04c Author: Jason Zaugg Date: Sat Nov 3 13:34:20 2012 +0100 SI-6611 Tighten up an unsafe array optimization The net was cast too wide and was unsafely optimizing away array copies. (cherry picked from commit dad886659faca4fba2d4937c9bc6780591b02c27) And also: Optimize primitive Array(e1, ..., en) Expands an existing optimization for reference arrays to apply to primitives, as well. Fixes one aspect of SI-6247. (cherry picked from commit cac5a08611f9511ba4d94b99db630404efae190a) Conflicts: src/compiler/scala/tools/nsc/transform/CleanUp.scala More principled tree copying. Canonical > home-spun. Conflicts: src/compiler/scala/tools/nsc/transform/CleanUp.scala --- .../scala/tools/nsc/transform/CleanUp.scala | 19 ++++--- src/library/scala/Array.scala | 10 ++++ .../scala/reflect/internal/Definitions.scala | 15 +++--- src/reflect/scala/reflect/internal/TreeInfo.scala | 14 +++++ test/files/instrumented/t6611.check | 1 + test/files/instrumented/t6611.scala | 35 +++++++++++++ test/files/run/t6611.scala | 61 ++++++++++++++++++++++ 7 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 test/files/instrumented/t6611.check create mode 100644 test/files/instrumented/t6611.scala create mode 100644 test/files/run/t6611.scala diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala index 0c9cb31d58..a6ea45d8b4 100644 --- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala +++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala @@ -15,6 +15,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL { import global._ import definitions._ import CODE._ + import treeInfo.StripCast /** the following two members override abstract members in Transform */ val phaseName: String = "cleanup" @@ -606,14 +607,16 @@ abstract class CleanUp extends Transform with ast.TreeDSL { } transformApply - // This transform replaces Array(Predef.wrapArray(Array(...)), ) - // with just Array(...) - case Apply(appMeth, List(Apply(wrapRefArrayMeth, List(array)), _)) - if (wrapRefArrayMeth.symbol == Predef_wrapRefArray && - appMeth.symbol == ArrayModule_overloadedApply.suchThat { - _.tpe.resultType.dealias.typeSymbol == ObjectClass - }) => - super.transform(array) + // Replaces `Array(Predef.wrapArray(ArrayValue(...).$asInstanceOf[...]), )` + // with just `ArrayValue(...).$asInstanceOf[...]` + // + // See SI-6611; we must *only* do this for literal vararg arrays. + case Apply(appMeth, List(Apply(wrapRefArrayMeth, List(arg @ StripCast(ArrayValue(_, _)))), _)) + if wrapRefArrayMeth.symbol == Predef_wrapRefArray && appMeth.symbol == ArrayModule_genericApply => + super.transform(arg) + case Apply(appMeth, List(elem0, Apply(wrapArrayMeth, List(rest @ ArrayValue(elemtpt, _))))) + if wrapArrayMeth.symbol == Predef_wrapArray(elemtpt.tpe) && appMeth.symbol == ArrayModule_apply(elemtpt.tpe) => + super.transform(treeCopy.ArrayValue(rest, rest.elemtpt, elem0 :: rest.elems)) case _ => super.transform(tree) diff --git a/src/library/scala/Array.scala b/src/library/scala/Array.scala index 90684b5fdd..b9f51803ec 100644 --- a/src/library/scala/Array.scala +++ b/src/library/scala/Array.scala @@ -115,6 +115,8 @@ object Array extends FallbackArrayBuilding { * @param xs the elements to put in the array * @return an array containing all elements from xs. */ + // Subject to a compiler optimization in Cleanup. + // Array(e0, ..., en) is translated to { val a = new Array(3); a(i) = ei; a } def apply[T: ClassTag](xs: T*): Array[T] = { val array = new Array[T](xs.length) var i = 0 @@ -123,6 +125,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Boolean` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Boolean, xs: Boolean*): Array[Boolean] = { val array = new Array[Boolean](xs.length + 1) array(0) = x @@ -132,6 +135,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Byte` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Byte, xs: Byte*): Array[Byte] = { val array = new Array[Byte](xs.length + 1) array(0) = x @@ -141,6 +145,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Short` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Short, xs: Short*): Array[Short] = { val array = new Array[Short](xs.length + 1) array(0) = x @@ -150,6 +155,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Char` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Char, xs: Char*): Array[Char] = { val array = new Array[Char](xs.length + 1) array(0) = x @@ -159,6 +165,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Int` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Int, xs: Int*): Array[Int] = { val array = new Array[Int](xs.length + 1) array(0) = x @@ -168,6 +175,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Long` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Long, xs: Long*): Array[Long] = { val array = new Array[Long](xs.length + 1) array(0) = x @@ -177,6 +185,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Float` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Float, xs: Float*): Array[Float] = { val array = new Array[Float](xs.length + 1) array(0) = x @@ -186,6 +195,7 @@ object Array extends FallbackArrayBuilding { } /** Creates an array of `Double` objects */ + // Subject to a compiler optimization in Cleanup, see above. def apply(x: Double, xs: Double*): Array[Double] = { val array = new Array[Double](xs.length + 1) array(0) = x diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 2a7b55cb5a..f28b3567a2 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -337,12 +337,13 @@ trait Definitions extends api.StandardDefinitions { lazy val PredefModule = requiredModule[scala.Predef.type] lazy val PredefModuleClass = PredefModule.moduleClass - def Predef_classOf = getMemberMethod(PredefModule, nme.classOf) - def Predef_identity = getMemberMethod(PredefModule, nme.identity) - def Predef_conforms = getMemberMethod(PredefModule, nme.conforms) - def Predef_wrapRefArray = getMemberMethod(PredefModule, nme.wrapRefArray) - def Predef_??? = getMemberMethod(PredefModule, nme.???) - def Predef_implicitly = getMemberMethod(PredefModule, nme.implicitly) + def Predef_classOf = getMemberMethod(PredefModule, nme.classOf) + def Predef_identity = getMemberMethod(PredefModule, nme.identity) + def Predef_conforms = getMemberMethod(PredefModule, nme.conforms) + def Predef_wrapRefArray = getMemberMethod(PredefModule, nme.wrapRefArray) + def Predef_wrapArray(tp: Type) = getMemberMethod(PredefModule, wrapArrayMethodName(tp)) + def Predef_??? = getMemberMethod(PredefModule, nme.???) + def Predef_implicitly = getMemberMethod(PredefModule, nme.implicitly) /** Is `sym` a member of Predef with the given name? * Note: DON't replace this by sym == Predef_conforms/etc, as Predef_conforms is a `def` @@ -466,6 +467,8 @@ trait Definitions extends api.StandardDefinitions { // arrays and their members lazy val ArrayModule = requiredModule[scala.Array.type] lazy val ArrayModule_overloadedApply = getMemberMethod(ArrayModule, nme.apply) + def ArrayModule_genericApply = ArrayModule_overloadedApply.suchThat(_.paramss.flatten.last.tpe.typeSymbol == ClassTagClass) // [T: ClassTag](xs: T*): Array[T] + def ArrayModule_apply(tp: Type) = ArrayModule_overloadedApply.suchThat(_.tpe.resultType =:= arrayType(tp)) // (p1: AnyVal1, ps: AnyVal1*): Array[AnyVal1] lazy val ArrayClass = getRequiredClass("scala.Array") // requiredClass[scala.Array[_]] lazy val Array_apply = getMemberMethod(ArrayClass, nme.apply) lazy val Array_update = getMemberMethod(ArrayClass, nme.update) diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 8908036442..e1a18570b2 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -234,6 +234,20 @@ abstract class TreeInfo { tree } + /** Strips layers of `.asInstanceOf[T]` / `_.$asInstanceOf[T]()` from an expression */ + def stripCast(tree: Tree): Tree = tree match { + case TypeApply(sel @ Select(inner, _), _) if isCastSymbol(sel.symbol) => + stripCast(inner) + case Apply(TypeApply(sel @ Select(inner, _), _), Nil) if isCastSymbol(sel.symbol) => + stripCast(inner) + case t => + t + } + + object StripCast { + def unapply(tree: Tree): Some[Tree] = Some(stripCast(tree)) + } + /** Is tree a self or super constructor call? */ def isSelfOrSuperConstrCall(tree: Tree) = { // stripNamedApply for SI-3584: adaptToImplicitMethod in Typers creates a special context diff --git a/test/files/instrumented/t6611.check b/test/files/instrumented/t6611.check new file mode 100644 index 0000000000..5cd691e93a --- /dev/null +++ b/test/files/instrumented/t6611.check @@ -0,0 +1 @@ +Method call statistics: diff --git a/test/files/instrumented/t6611.scala b/test/files/instrumented/t6611.scala new file mode 100644 index 0000000000..4c52f8a5ef --- /dev/null +++ b/test/files/instrumented/t6611.scala @@ -0,0 +1,35 @@ +import scala.tools.partest.instrumented.Instrumentation._ + +object Test { + def main(args: Array[String]) { + startProfiling() + + // tests optimization in Cleanup for varargs reference arrays + Array("") + + + Array(true) + Array(true, false) + Array(1: Byte) + Array(1: Byte, 2: Byte) + Array(1: Short) + Array(1: Short, 2: Short) + Array(1) + Array(1, 2) + Array(1L) + Array(1L, 2L) + Array(1d) + Array(1d, 2d) + Array(1f) + Array(1f, 2f) + + /* Not currently optimized: + Array[Int](1, 2) etc + Array(()) + Array((), ()) + */ + + stopProfiling() + printStatistics() + } +} diff --git a/test/files/run/t6611.scala b/test/files/run/t6611.scala new file mode 100644 index 0000000000..0947a48f90 --- /dev/null +++ b/test/files/run/t6611.scala @@ -0,0 +1,61 @@ +object Test extends App { + locally { + val a = Array("1") + val a2 = Array(a: _*) + assert(a ne a2) + } + + locally { + val a = Array("1": Object) + val a2 = Array[Object](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(true) + val a2 = Array[Boolean](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(1: Short) + val a2 = Array[Short](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(1: Byte) + val a2 = Array[Byte](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(1) + val a2 = Array[Int](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(1L) + val a2 = Array[Long](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(1f) + val a2 = Array[Float](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(1d) + val a2 = Array[Double](a: _*) + assert(a ne a2) + } + + locally { + val a = Array(()) + val a2 = Array[Unit](a: _*) + assert(a ne a2) + } +} -- cgit v1.2.3 From 96ed055769483d661b09f346cd1641f956f3172a Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 23 Jan 2013 01:10:05 +0100 Subject: [backport] SI-6567 Warning for Option(implicitView(foo)) commit 284bd754fa5dfc8bc626b0c5ebe85d872dd044cb Author: Jason Zaugg Date: Sat Nov 3 16:19:46 2012 +0100 SI-6567 Warning for Option(implicitView(foo)) I've seen the reported problem before in the wild. It seems worthy of a special warning, so long as we advocate Option.apply as an alternative to `if (x == null) Some(x) else None`. It is behind -Xlint at the moment, an option that could do with some promotion. (cherry picked from commit 0bcb9e9169146e3f589c6c9f65cc4a5523b78120) --- src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 11 ++++++++++- src/reflect/scala/reflect/internal/Definitions.scala | 10 ++++++---- test/files/neg/t6567.check | 7 +++++++ test/files/neg/t6567.flags | 1 + test/files/neg/t6567.scala | 11 +++++++++++ 5 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 test/files/neg/t6567.check create mode 100644 test/files/neg/t6567.flags create mode 100644 test/files/neg/t6567.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 7118375b82..969bb8aceb 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1062,6 +1062,12 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans def apply(tp: Type) = mapOver(tp).normalize } + def checkImplicitViewOptionApply(pos: Position, fn: Tree, args: List[Tree]): Unit = if (settings.lint.value) (fn, args) match { + case (tap@TypeApply(fun, targs), List(view: ApplyImplicitView)) if fun.symbol == Option_apply => + unit.warning(pos, s"Suspicious application of an implicit view (${view.fun}) in the argument to Option.apply.") // SI-6567 + case _ => + } + def checkSensible(pos: Position, fn: Tree, args: List[Tree]) = fn match { case Select(qual, name @ (nme.EQ | nme.NE | nme.eq | nme.ne)) if args.length == 1 => def isReferenceOp = name == nme.eq || name == nme.ne @@ -1563,7 +1569,10 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans case Apply(fn, args) => // sensicality should be subsumed by the unreachability/exhaustivity/irrefutability analyses in the pattern matcher - if (!inPattern) checkSensible(tree.pos, fn, args) + if (!inPattern) { + checkImplicitViewOptionApply(tree.pos, fn, args) + checkSensible(tree.pos, fn, args) + } currentApplication = tree tree } diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 2a7b55cb5a..4bd0aca9c4 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -536,10 +536,12 @@ trait Definitions extends api.StandardDefinitions { lazy val ScalaLongSignatureAnnotation = requiredClass[scala.reflect.ScalaLongSignature] // Option classes - lazy val OptionClass: ClassSymbol = requiredClass[Option[_]] - lazy val SomeClass: ClassSymbol = requiredClass[Some[_]] - lazy val NoneModule: ModuleSymbol = requiredModule[scala.None.type] - lazy val SomeModule: ModuleSymbol = requiredModule[scala.Some.type] + lazy val OptionClass: ClassSymbol = requiredClass[Option[_]] + lazy val OptionModule: ModuleSymbol = requiredModule[scala.Option.type] + lazy val Option_apply = getMemberMethod(OptionModule, nme.apply) + lazy val SomeClass: ClassSymbol = requiredClass[Some[_]] + lazy val NoneModule: ModuleSymbol = requiredModule[scala.None.type] + lazy val SomeModule: ModuleSymbol = requiredModule[scala.Some.type] def compilerTypeFromTag(tt: ApiUniverse # WeakTypeTag[_]): Type = tt.in(rootMirror).tpe def compilerSymbolFromTag(tt: ApiUniverse # WeakTypeTag[_]): Symbol = tt.in(rootMirror).tpe.typeSymbol diff --git a/test/files/neg/t6567.check b/test/files/neg/t6567.check new file mode 100644 index 0000000000..4c513e64cd --- /dev/null +++ b/test/files/neg/t6567.check @@ -0,0 +1,7 @@ +t6567.scala:8: error: Suspicious application of an implicit view (Test.this.a2b) in the argument to Option.apply. + Option[B](a) + ^ +t6567.scala:10: error: Suspicious application of an implicit view (Test.this.a2b) in the argument to Option.apply. + val b: Option[B] = Option(a) + ^ +two errors found diff --git a/test/files/neg/t6567.flags b/test/files/neg/t6567.flags new file mode 100644 index 0000000000..e93641e931 --- /dev/null +++ b/test/files/neg/t6567.flags @@ -0,0 +1 @@ +-Xlint -Xfatal-warnings \ No newline at end of file diff --git a/test/files/neg/t6567.scala b/test/files/neg/t6567.scala new file mode 100644 index 0000000000..650e5e39ae --- /dev/null +++ b/test/files/neg/t6567.scala @@ -0,0 +1,11 @@ +class A +class B + +object Test { + val a: A = null + implicit def a2b(a: A) = new B + + Option[B](a) + + val b: Option[B] = Option(a) +} -- cgit v1.2.3 From d592216a12d8bd145f6a670554e9217944c2b169 Mon Sep 17 00:00:00 2001 From: James Iry Date: Wed, 23 Jan 2013 01:30:10 -0800 Subject: SI-7011 Fix finding constructor type in captured var definitions If a captured var was initialized with an empty tree then finding the type of the empty tree was being handled improperly. The fix is to look for primary constructors on the tree's type symbol rather than the tree's symbol. A test is included. In order to make the problem more testable the debug logging of the issue is changed to a debug warn. --- src/compiler/scala/tools/nsc/transform/LambdaLift.scala | 4 ++-- test/files/pos/t7011.flags | 1 + test/files/pos/t7011.scala | 7 +++++++ 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/t7011.flags create mode 100644 test/files/pos/t7011.scala diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index 4a23e65ad2..c59d0b5b27 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -430,10 +430,10 @@ abstract class LambdaLift extends InfoTransform { /* Creating a constructor argument if one isn't present. */ val constructorArg = rhs match { case EmptyTree => - sym.primaryConstructor.info.paramTypes match { + sym.tpe.typeSymbol.primaryConstructor.info.paramTypes match { case List(tp) => gen.mkZero(tp) case _ => - log("Couldn't determine how to properly construct " + sym) + debugwarn("Couldn't determine how to properly construct " + sym) rhs } case arg => arg diff --git a/test/files/pos/t7011.flags b/test/files/pos/t7011.flags new file mode 100644 index 0000000000..a4c161553e --- /dev/null +++ b/test/files/pos/t7011.flags @@ -0,0 +1 @@ +-Ydebug -Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7011.scala b/test/files/pos/t7011.scala new file mode 100644 index 0000000000..539f662bc0 --- /dev/null +++ b/test/files/pos/t7011.scala @@ -0,0 +1,7 @@ +object bar { + def foo { + lazy val x = 42 + + {()=>x} + } +} \ No newline at end of file -- cgit v1.2.3 From 8297843765c7195bb7c3ad30e91de6779b9bff99 Mon Sep 17 00:00:00 2001 From: James Iry Date: Wed, 23 Jan 2013 11:57:06 -0800 Subject: SI-6434 Pretty print function types with by name arg as (=> A) => B We were pretty printing a function type with one by name arg as => A => B, but because => is right associative that's formally equivalent to => (A => B) and that's entirely a different thing. This commit changes the pretty printer in Typers.scala to check for a byname argument on a function type and wrap it in parens. A REPL test is included. --- src/reflect/scala/reflect/internal/Types.scala | 6 ++++-- test/files/run/t6434.check | 10 ++++++++++ test/files/run/t6434.scala | 8 ++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 test/files/run/t6434.check create mode 100644 test/files/run/t6434.scala diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index c2637e6967..dbc00edb1a 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -2481,8 +2481,10 @@ trait Types extends api.Types { self: SymbolTable => // from (T1, T2) => R. targs match { case in :: out :: Nil if !isTupleType(in) => - // A => B => C should be (A => B) => C or A => (B => C) - val in_s = if (isFunctionType(in)) "(" + in + ")" else "" + in + // A => B => C should be (A => B) => C or A => (B => C). + // Also if A is byname, then we want (=> A) => B because => is right associative and => A => B + // would mean => (A => B) which is a different type + val in_s = if (isFunctionType(in) || isByNameParamType(in)) "(" + in + ")" else "" + in val out_s = if (isFunctionType(out)) "(" + out + ")" else "" + out in_s + " => " + out_s case xs => diff --git a/test/files/run/t6434.check b/test/files/run/t6434.check new file mode 100644 index 0000000000..f898b6b781 --- /dev/null +++ b/test/files/run/t6434.check @@ -0,0 +1,10 @@ +Type in expressions to have them evaluated. +Type :help for more information. + +scala> def f(x: => Int): Int = x +f: (x: => Int)Int + +scala> f _ +res0: (=> Int) => Int = + +scala> diff --git a/test/files/run/t6434.scala b/test/files/run/t6434.scala new file mode 100644 index 0000000000..e4a4579613 --- /dev/null +++ b/test/files/run/t6434.scala @@ -0,0 +1,8 @@ +import scala.tools.partest.ReplTest + +object Test extends ReplTest { + def code = +"""def f(x: => Int): Int = x +f _ +""" +} -- cgit v1.2.3 From f2e45fccfe53ff79ae34f9ca6cae1afa0d153e53 Mon Sep 17 00:00:00 2001 From: Nada Amin Date: Fri, 28 Sep 2012 01:32:44 +0200 Subject: Fix class loader issues in instrumentation tests. The ASM ClassWriter uses a wimpy class loader when computing common superclasses. This could cause a ClassNotFoundException in the transform method (at reader.accept). This exception gets swallowed, resulting in a class that should be instrumented to silently not be. The fix is to override getCommonSuperClass to use the correct class loader. Trivia: This bug was discovered while 'stress-testing' this instrumentation scheme on the Coursera students, to check that they implement one method in terms of another in the assignment. --- .../tools/partest/javaagent/ASMTransformer.java | 30 ++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/partest/scala/tools/partest/javaagent/ASMTransformer.java b/src/partest/scala/tools/partest/javaagent/ASMTransformer.java index 494a5a99be..b6bec2f598 100644 --- a/src/partest/scala/tools/partest/javaagent/ASMTransformer.java +++ b/src/partest/scala/tools/partest/javaagent/ASMTransformer.java @@ -26,9 +26,35 @@ public class ASMTransformer implements ClassFileTransformer { className.startsWith("instrumented/")); } - public byte[] transform(ClassLoader loader, String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) { + public byte[] transform(final ClassLoader classLoader, String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) { if (shouldTransform(className)) { - ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS) { + // this is copied verbatim from the superclass, + // except that we use the outer class loader + @Override protected String getCommonSuperClass(final String type1, final String type2) { + Class c, d; + try { + c = Class.forName(type1.replace('/', '.'), false, classLoader); + d = Class.forName(type2.replace('/', '.'), false, classLoader); + } catch (Exception e) { + throw new RuntimeException(e.toString()); + } + if (c.isAssignableFrom(d)) { + return type1; + } + if (d.isAssignableFrom(c)) { + return type2; + } + if (c.isInterface() || d.isInterface()) { + return "java/lang/Object"; + } else { + do { + c = c.getSuperclass(); + } while (!c.isAssignableFrom(d)); + return c.getName().replace('.', '/'); + } + } + }; ProfilerVisitor visitor = new ProfilerVisitor(writer); ClassReader reader = new ClassReader(classfileBuffer); reader.accept(visitor, 0); -- cgit v1.2.3 From 0a967e1cc18d631a58d467d9bec0e67860ca3520 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Fri, 9 Nov 2012 22:22:42 -0800 Subject: Correct whitespace in `ASMTransformer.java`. Let's stick to 2 spaces for indentation (and no tabs). --- .../tools/partest/javaagent/ASMTransformer.java | 54 +++++++++++----------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/partest/scala/tools/partest/javaagent/ASMTransformer.java b/src/partest/scala/tools/partest/javaagent/ASMTransformer.java index b6bec2f598..7338e2b01b 100644 --- a/src/partest/scala/tools/partest/javaagent/ASMTransformer.java +++ b/src/partest/scala/tools/partest/javaagent/ASMTransformer.java @@ -28,33 +28,33 @@ public class ASMTransformer implements ClassFileTransformer { public byte[] transform(final ClassLoader classLoader, String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) { if (shouldTransform(className)) { - ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS) { - // this is copied verbatim from the superclass, - // except that we use the outer class loader - @Override protected String getCommonSuperClass(final String type1, final String type2) { - Class c, d; - try { - c = Class.forName(type1.replace('/', '.'), false, classLoader); - d = Class.forName(type2.replace('/', '.'), false, classLoader); - } catch (Exception e) { - throw new RuntimeException(e.toString()); - } - if (c.isAssignableFrom(d)) { - return type1; - } - if (d.isAssignableFrom(c)) { - return type2; - } - if (c.isInterface() || d.isInterface()) { - return "java/lang/Object"; - } else { - do { - c = c.getSuperclass(); - } while (!c.isAssignableFrom(d)); - return c.getName().replace('.', '/'); - } - } - }; + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS) { + // this is copied verbatim from the superclass, + // except that we use the outer class loader + @Override protected String getCommonSuperClass(final String type1, final String type2) { + Class c, d; + try { + c = Class.forName(type1.replace('/', '.'), false, classLoader); + d = Class.forName(type2.replace('/', '.'), false, classLoader); + } catch (Exception e) { + throw new RuntimeException(e.toString()); + } + if (c.isAssignableFrom(d)) { + return type1; + } + if (d.isAssignableFrom(c)) { + return type2; + } + if (c.isInterface() || d.isInterface()) { + return "java/lang/Object"; + } else { + do { + c = c.getSuperclass(); + } while (!c.isAssignableFrom(d)); + return c.getName().replace('.', '/'); + } + } + }; ProfilerVisitor visitor = new ProfilerVisitor(writer); ClassReader reader = new ClassReader(classfileBuffer); reader.accept(visitor, 0); -- cgit v1.2.3 From b2776b40b28d312a8cc0cad0f35b2c3cb81abefb Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Fri, 9 Nov 2012 22:29:09 -0800 Subject: Set `canRetransform` flag to `false` in instrumentation. We do not need to retransform classes once they are loaded. All instrumentation byte-code is pushed at loading time. This fixes a problem with Java 7 that was failing to add a transformer because we did not declare retransformation capability in `MANIFEST.MF` file in Java agent jar. Java 6 allowed to add transformer due to a bug. --- src/partest/scala/tools/partest/javaagent/ProfilingAgent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/partest/scala/tools/partest/javaagent/ProfilingAgent.java b/src/partest/scala/tools/partest/javaagent/ProfilingAgent.java index c2e4dc69f4..3b18987040 100644 --- a/src/partest/scala/tools/partest/javaagent/ProfilingAgent.java +++ b/src/partest/scala/tools/partest/javaagent/ProfilingAgent.java @@ -20,6 +20,6 @@ public class ProfilingAgent { // and the test-case itself won't be loaded yet. We rely here on the fact that ASMTransformer does // not depend on Scala library. In case our assumptions are wrong we can always insert call to // inst.retransformClasses. - inst.addTransformer(new ASMTransformer(), true); + inst.addTransformer(new ASMTransformer(), false); } } -- cgit v1.2.3 From a9bbfec8d58f68bd9105789754373f205d9981b1 Mon Sep 17 00:00:00 2001 From: Grzegorz Kossakowski Date: Fri, 9 Nov 2012 22:58:47 -0800 Subject: Do not recompute stack frames when instrumenting bytecode. It turns out that we do not need to do that. See comment in `ProfilerVisitor.java`. Also, since recomputing stack frame map was the only reason we needed to implement `getCommonSuperClass` we can now remove its implementation that was causing problems on Java 7 due to a cyclic dependency involving class loader because we would try to load a class we are currently transforming and transformer is triggered just before classloading. //cc @namin who worked on this code with me. --- .../tools/partest/javaagent/ASMTransformer.java | 33 ++++++---------------- .../tools/partest/javaagent/ProfilerVisitor.java | 13 +++++++++ 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/partest/scala/tools/partest/javaagent/ASMTransformer.java b/src/partest/scala/tools/partest/javaagent/ASMTransformer.java index 7338e2b01b..878c8613d5 100644 --- a/src/partest/scala/tools/partest/javaagent/ASMTransformer.java +++ b/src/partest/scala/tools/partest/javaagent/ASMTransformer.java @@ -26,33 +26,16 @@ public class ASMTransformer implements ClassFileTransformer { className.startsWith("instrumented/")); } - public byte[] transform(final ClassLoader classLoader, String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) { + public byte[] transform(final ClassLoader classLoader, final String className, Class classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) { if (shouldTransform(className)) { - ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS) { - // this is copied verbatim from the superclass, - // except that we use the outer class loader + ClassWriter writer = new ClassWriter(ClassWriter.COMPUTE_MAXS) { @Override protected String getCommonSuperClass(final String type1, final String type2) { - Class c, d; - try { - c = Class.forName(type1.replace('/', '.'), false, classLoader); - d = Class.forName(type2.replace('/', '.'), false, classLoader); - } catch (Exception e) { - throw new RuntimeException(e.toString()); - } - if (c.isAssignableFrom(d)) { - return type1; - } - if (d.isAssignableFrom(c)) { - return type2; - } - if (c.isInterface() || d.isInterface()) { - return "java/lang/Object"; - } else { - do { - c = c.getSuperclass(); - } while (!c.isAssignableFrom(d)); - return c.getName().replace('.', '/'); - } + // Since we are not recomputing stack frame map, this should never be called we override this method because + // default implementation uses reflection for implementation and might try to load the class that we are + // currently processing. That leads to weird results like swallowed exceptions and classes being not + // transformed. + throw new RuntimeException("Unexpected call to getCommonSuperClass(" + type1 + ", " + type2 + + ") while transforming " + className); } }; ProfilerVisitor visitor = new ProfilerVisitor(writer); diff --git a/src/partest/scala/tools/partest/javaagent/ProfilerVisitor.java b/src/partest/scala/tools/partest/javaagent/ProfilerVisitor.java index ac83f66506..8306327b14 100644 --- a/src/partest/scala/tools/partest/javaagent/ProfilerVisitor.java +++ b/src/partest/scala/tools/partest/javaagent/ProfilerVisitor.java @@ -33,6 +33,19 @@ public class ProfilerVisitor extends ClassVisitor implements Opcodes { // only instrument non-abstract methods if((access & ACC_ABSTRACT) == 0) { assert(className != null); + /* The following instructions do not modify compressed stack frame map so + * we don't need to worry about recalculating stack frame map. Specifically, + * let's quote "ASM 4.0, A Java bytecode engineering library" guide (p. 40): + * + * In order to save space, a compiled method does not contain one frame per + * instruction: in fact it contains only the frames for the instructions + * that correspond to jump targets or exception handlers, or that follow + * unconditional jump instructions. Indeed the other frames can be easily + * and quickly inferred from these ones. + * + * Instructions below are just loading constants and calling a method so according + * to definition above they do not contribute to compressed stack frame map. + */ mv.visitLdcInsn(className); mv.visitLdcInsn(name); mv.visitLdcInsn(desc); -- cgit v1.2.3 From 873aecceea7a6a95e95a015c6c05686d31183621 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Thu, 24 Jan 2013 12:19:31 -0800 Subject: Fix broken build. It's all system admin, all the time, here at scala ranch. --- test/files/run/reify_magicsymbols.check | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/files/run/reify_magicsymbols.check b/test/files/run/reify_magicsymbols.check index e2aa46a364..c9d892d793 100644 --- a/test/files/run/reify_magicsymbols.check +++ b/test/files/run/reify_magicsymbols.check @@ -10,4 +10,4 @@ List[Null] List[Nothing] AnyRef{def foo(x: Int): Int} Int* => Unit -=> Int => Unit +(=> Int) => Unit -- cgit v1.2.3 From 0d01cc1c300b718afd1fe69d5030d36d8000e0cd Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Tue, 15 Jan 2013 00:28:38 -0800 Subject: SI-6969, mishandling of SoftReferences in method cache. More interesting to test than it was to fix. The soft reference is now dereferenced once, the locally stored underlying value ascertained to be non-null, and the remainder of the references to the value use the local var. The enclosed test reliably NPEs without this patch. --- .../scala/tools/nsc/transform/CleanUp.scala | 26 ++++++++++++++------ test/files/run/t6969.check | 1 + test/files/run/t6969.scala | 28 ++++++++++++++++++++++ 3 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 test/files/run/t6969.check create mode 100644 test/files/run/t6969.scala diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala index a6ea45d8b4..44510ab0c2 100644 --- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala +++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala @@ -207,12 +207,17 @@ abstract class CleanUp extends Transform with ast.TreeDSL { var reflPoly$Cache: SoftReference[scala.runtime.MethodCache] = new SoftReference(new EmptyMethodCache()) def reflMethod$Method(forReceiver: JClass[_]): JMethod = { - var method: JMethod = reflPoly$Cache.find(forReceiver) - if (method != null) + var methodCache: MethodCache = reflPoly$Cache.find(forReceiver) + if (methodCache eq null) { + methodCache = new EmptyMethodCache + reflPoly$Cache = new SoftReference(methodCache) + } + var method: JMethod = methodCache.find(forReceiver) + if (method ne null) return method else { method = ScalaRunTime.ensureAccessible(forReceiver.getMethod("xyz", reflParams$Cache)) - reflPoly$Cache = new SoftReference(reflPoly$Cache.get.add(forReceiver, method)) + reflPoly$Cache = new SoftReference(methodCache.add(forReceiver, method)) return method } } @@ -229,16 +234,22 @@ abstract class CleanUp extends Transform with ast.TreeDSL { def getPolyCache = gen.mkCast(fn(REF(reflPolyCacheSym), nme.get), MethodCacheClass.tpe) addStaticMethodToClass((reflMethodSym, forReceiverSym) => { + val methodCache = reflMethodSym.newVariable(mkTerm("methodCache"), ad.pos) setInfo MethodCacheClass.tpe val methodSym = reflMethodSym.newVariable(mkTerm("method"), ad.pos) setInfo MethodClass.tpe BLOCK( - IF (getPolyCache OBJ_EQ NULL) THEN (REF(reflPolyCacheSym) === mkNewPolyCache) ENDIF, - VAL(methodSym) === ((getPolyCache DOT methodCache_find)(REF(forReceiverSym))) , - IF (REF(methodSym) OBJ_!= NULL) . + VAR(methodCache) === getPolyCache, + IF (REF(methodCache) OBJ_EQ NULL) THEN BLOCK( + REF(methodCache) === NEW(TypeTree(EmptyMethodCacheClass.tpe)), + REF(reflPolyCacheSym) === gen.mkSoftRef(REF(methodCache)) + ) ENDIF, + + VAR(methodSym) === (REF(methodCache) DOT methodCache_find)(REF(forReceiverSym)), + IF (REF(methodSym) OBJ_NE NULL) . THEN (Return(REF(methodSym))) ELSE { def methodSymRHS = ((REF(forReceiverSym) DOT Class_getMethod)(LIT(method), REF(reflParamsCacheSym))) - def cacheRHS = ((getPolyCache DOT methodCache_add)(REF(forReceiverSym), REF(methodSym))) + def cacheRHS = ((REF(methodCache) DOT methodCache_add)(REF(forReceiverSym), REF(methodSym))) BLOCK( REF(methodSym) === (REF(ensureAccessibleMethod) APPLY (methodSymRHS)), REF(reflPolyCacheSym) === gen.mkSoftRef(cacheRHS), @@ -247,6 +258,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL { } ) }) + } /* ### HANDLING METHODS NORMALLY COMPILED TO OPERATORS ### */ diff --git a/test/files/run/t6969.check b/test/files/run/t6969.check new file mode 100644 index 0000000000..78297812c9 --- /dev/null +++ b/test/files/run/t6969.check @@ -0,0 +1 @@ +All threads completed. diff --git a/test/files/run/t6969.scala b/test/files/run/t6969.scala new file mode 100644 index 0000000000..8cfc28c1e5 --- /dev/null +++ b/test/files/run/t6969.scala @@ -0,0 +1,28 @@ +object Test { + private type Clearable = { def clear(): Unit } + private def choke() = { + try new Array[Object]((Runtime.getRuntime().maxMemory min Int.MaxValue).toInt) + catch { + case _: OutOfMemoryError => // what do you mean, out of memory? + case t: Throwable => println(t) + } + } + private def f(x: Clearable) = x.clear() + class Choker(id: Int) extends Thread { + private def g(iteration: Int) = { + val map = scala.collection.mutable.Map[Int, Int](1 -> 2) + try f(map) catch { case t: NullPointerException => println(s"Failed at $id/$iteration") ; throw t } + choke() + } + override def run() { + 1 to 50 foreach g + } + } + + def main(args: Array[String]): Unit = { + val threads = 1 to 3 map (id => new Choker(id)) + threads foreach (_.start()) + threads foreach (_.join()) + println("All threads completed.") + } +} -- cgit v1.2.3