From e720babdaf999878642c18cebec4c18e93988d54 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 15 Sep 2014 23:32:09 -0700 Subject: SI-6502 Repl reset/replay take settings args The reset and replay commands take arbitrary command line args. When settings args are supplied, the compiler is recreated. For uniformity, the settings command performs only the usual arg parsing: use -flag:true instead of +flag, and clearing a setting is promoted to the command line, so that -Xlint: is not an error but clears the flags. ``` scala> maqicode.Test main null :8: error: not found: value maqicode maqicode.Test main null ^ scala> :reset -classpath/a target/scala-2.11/sample_2.11-1.0.jar Resetting interpreter state. Forgetting all expression results and named terms: $intp scala> maqicode.Test main null Hello, world. scala> val i = 42 i: Int = 42 scala> s"$i is the loneliest numbah." res1: String = 42 is the loneliest numbah. scala> :replay -classpath "" Replaying: maqicode.Test main null Hello, world. Replaying: val i = 42 i: Int = 42 Replaying: s"$i is the loneliest numbah." res1: String = 42 is the loneliest numbah. scala> :replay -classpath/a "" Replaying: maqicode.Test main null :8: error: not found: value maqicode maqicode.Test main null ^ Replaying: val i = 42 i: Int = 42 Replaying: s"$i is the loneliest numbah." res1: String = 42 is the loneliest numbah. ``` Clearing a clearable setting: ``` scala> :reset -Xlint:missing-interpolator Resetting interpreter state. scala> { val i = 42 ; "$i is the loneliest numbah." } :8: warning: possible missing interpolator: detected interpolated identifier `$i` { val i = 42 ; "$i is the loneliest numbah." } ^ res0: String = $i is the loneliest numbah. scala> :reset -Xlint: Resetting interpreter state. Forgetting this session history: { val i = 42 ; "$i is the loneliest numbah." } scala> { val i = 42 ; "$i is the loneliest numbah." } res0: String = $i is the loneliest numbah. ``` --- test/files/run/t4594-repl-settings.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test/files') diff --git a/test/files/run/t4594-repl-settings.scala b/test/files/run/t4594-repl-settings.scala index 4202991607..db5dc19866 100644 --- a/test/files/run/t4594-repl-settings.scala +++ b/test/files/run/t4594-repl-settings.scala @@ -14,7 +14,7 @@ object Test extends SessionTest { |warning: there was one deprecation warning; re-run with -deprecation for details |a: String | - |scala> :settings +deprecation + |scala> :settings -deprecation | |scala> def b = depp |:8: warning: method depp is deprecated: Please don't do that. -- cgit v1.2.3 From ac4662d9c189449918bbd904692054f76985bf60 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 29 Sep 2014 21:15:04 +0200 Subject: SI-8445, SI-6622 test cases, already fixed They were most likely fixed in #3931 / e3107465c3. The test case for SI-6622 is taken from Jason's PR #2654. I adjusted the EnclosingMethod to be `null` in two places in the check file, for the classes that are owned by fields (not methods). --- test/files/run/t6622.check | 10 ++++++++++ test/files/run/t6622.scala | 50 ++++++++++++++++++++++++++++++++++++++++++++++ test/files/run/t8445.check | 1 + test/files/run/t8445.scala | 11 ++++++++++ 4 files changed, 72 insertions(+) create mode 100644 test/files/run/t6622.check create mode 100644 test/files/run/t6622.scala create mode 100644 test/files/run/t8445.check create mode 100644 test/files/run/t8445.scala (limited to 'test/files') diff --git a/test/files/run/t6622.check b/test/files/run/t6622.check new file mode 100644 index 0000000000..5d006d88e6 --- /dev/null +++ b/test/files/run/t6622.check @@ -0,0 +1,10 @@ + O1.resultVal isMemberClass = false, null +class A$1 + O1.resultDef isMemberClass = false, public void O1$.resultDef() +class A$2 + C2.resultVal isMemberClass = false, null +class $B$1 + O3.resultDef isMemberClass = false, public void O3$.resultDef() +class C$1 + O4.resultDefDefault isMemberClass = false, public java.lang.Object O4$.resultDefDefault$default$1() +class C$2 diff --git a/test/files/run/t6622.scala b/test/files/run/t6622.scala new file mode 100644 index 0000000000..de8ffa01bf --- /dev/null +++ b/test/files/run/t6622.scala @@ -0,0 +1,50 @@ +import Test.check + +object O1 { + lazy val resultVal = { + class A + check("O1.resultVal", classOf[A]) + } + + def resultDef = { + class A + check("O1.resultDef", classOf[A]) + } +} + +class C2 { + val resultVal = { + val tmp = { + class B + check("C2.resultVal", classOf[B]) + } + } +} + +object O3 { + def resultDef = { + class C + check("O3.resultDef", classOf[C]) + } +} + +object O4 { + def resultDefDefault(a: Any = { + class C + check("O4.resultDefDefault", classOf[C]) + }) = (); +} + + +object Test extends App { + def check(desc: String, clazz: Class[_]) { + println(s" $desc isMemberClass = ${clazz.isMemberClass}, ${clazz.getEnclosingMethod}") + println(reflect.runtime.currentMirror.classSymbol(clazz)) + } + + O1.resultVal + O1.resultDef + new C2().resultVal + O3.resultDef + O4.resultDefDefault() +} diff --git a/test/files/run/t8445.check b/test/files/run/t8445.check new file mode 100644 index 0000000000..41fd6d3ed1 --- /dev/null +++ b/test/files/run/t8445.check @@ -0,0 +1 @@ +warning: there was one feature warning; re-run with -feature for details diff --git a/test/files/run/t8445.scala b/test/files/run/t8445.scala new file mode 100644 index 0000000000..ed196b62a2 --- /dev/null +++ b/test/files/run/t8445.scala @@ -0,0 +1,11 @@ +object X { + class Y + def y = new Y { + class Z + def z = classOf[Z] + } +} + +object Test extends App { + assert(X.y.z.getEnclosingClass.getName == "X$$anon$1") +} -- cgit v1.2.3 From bcd1e4f3a3c7a683bb7cf435ef8163ecaf90f126 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 30 Sep 2014 14:50:30 +1000 Subject: SI-8868 Fix unpickling of local dummy symbols These pop up as the owner of symbols in annotation arguments, such as the ones introduced by the names/defaults desugaring. The first two test cases here motivate the two patches to Unpicker. The third requires both fixes, but exploits the problem directly, without using `@deprecated` and named arguments. See also 14fa7be / SI-8708 for a recently remedied kindred bug. --- src/reflect/scala/reflect/internal/pickling/UnPickler.scala | 12 ++++++++++-- test/files/pos/t8868a/Sub_2.scala | 1 + test/files/pos/t8868a/T_1.scala | 6 ++++++ test/files/pos/t8868b/Sub_2.scala | 2 ++ test/files/pos/t8868b/T_1.scala | 4 ++++ test/files/pos/t8868c/Sub_2.scala | 2 ++ test/files/pos/t8868c/T_1.scala | 9 +++++++++ 7 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/t8868a/Sub_2.scala create mode 100644 test/files/pos/t8868a/T_1.scala create mode 100644 test/files/pos/t8868b/Sub_2.scala create mode 100644 test/files/pos/t8868b/T_1.scala create mode 100644 test/files/pos/t8868c/Sub_2.scala create mode 100644 test/files/pos/t8868c/T_1.scala (limited to 'test/files') diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala index 8d4c3f752f..5433bfad60 100644 --- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala +++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala @@ -243,8 +243,14 @@ abstract class UnPickler { } getOrElse "") } + def localDummy = { + if (nme.isLocalDummyName(name)) + owner.newLocalDummy(NoPosition) + else NoSymbol + } + // (1) Try name. - fromName(name) orElse { + localDummy orElse fromName(name) orElse { // (2) Try with expanded name. Can happen if references to private // symbols are read from outside: for instance when checking the children // of a class. See #1722. @@ -298,6 +304,7 @@ abstract class UnPickler { * (.) ... * (1) `local child` represents local child classes, see comment in Pickler.putSymbol. * Since it is not a member, it should not be entered in the owner's scope. + * (2) Similarly, we ignore local dummy symbols, as seen in SI-8868 */ def shouldEnterInOwnerScope = { sym.owner.isClass && @@ -307,7 +314,8 @@ abstract class UnPickler { !sym.isRefinementClass && !sym.isTypeParameter && !sym.isExistentiallyBound && - sym.rawname != tpnme.LOCAL_CHILD // (1) + sym.rawname != tpnme.LOCAL_CHILD && // (1) + !nme.isLocalDummyName(sym.rawname) // (2) } markFlagsCompleted(sym)(mask = AllFlags) diff --git a/test/files/pos/t8868a/Sub_2.scala b/test/files/pos/t8868a/Sub_2.scala new file mode 100644 index 0000000000..a19b529c88 --- /dev/null +++ b/test/files/pos/t8868a/Sub_2.scala @@ -0,0 +1 @@ +class Sub extends T diff --git a/test/files/pos/t8868a/T_1.scala b/test/files/pos/t8868a/T_1.scala new file mode 100644 index 0000000000..9fb97b1413 --- /dev/null +++ b/test/files/pos/t8868a/T_1.scala @@ -0,0 +1,6 @@ +class C + +trait T { + @deprecated(since = "", message = "") + class X +} diff --git a/test/files/pos/t8868b/Sub_2.scala b/test/files/pos/t8868b/Sub_2.scala new file mode 100644 index 0000000000..58b44db2b3 --- /dev/null +++ b/test/files/pos/t8868b/Sub_2.scala @@ -0,0 +1,2 @@ +class Sub extends T + diff --git a/test/files/pos/t8868b/T_1.scala b/test/files/pos/t8868b/T_1.scala new file mode 100644 index 0000000000..0b71cfdaa3 --- /dev/null +++ b/test/files/pos/t8868b/T_1.scala @@ -0,0 +1,4 @@ +@deprecated(since = "2.4.0", message = "") +trait T { + class X +} diff --git a/test/files/pos/t8868c/Sub_2.scala b/test/files/pos/t8868c/Sub_2.scala new file mode 100644 index 0000000000..58b44db2b3 --- /dev/null +++ b/test/files/pos/t8868c/Sub_2.scala @@ -0,0 +1,2 @@ +class Sub extends T + diff --git a/test/files/pos/t8868c/T_1.scala b/test/files/pos/t8868c/T_1.scala new file mode 100644 index 0000000000..dc541950d8 --- /dev/null +++ b/test/files/pos/t8868c/T_1.scala @@ -0,0 +1,9 @@ +class C(a: Any) extends annotation.StaticAnnotation + +@C({val x = 0; x}) +trait T { + class X + + @C({val x = 0; x}) + def foo = 42 +} -- cgit v1.2.3 From 6346c6b46d2991def37a9ae81ed5f4f8b90d5efd Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 29 Sep 2014 15:14:24 +0200 Subject: SI-8087 keep annotations on mixed-in private[this] fields Related to SI-2511 / eea7956, which fixed the same issue for non `private[this]` fields. If you have trait T { private[this] val f = 0 } class C extends T Mixin geneartes an accessor method `T.f` with owner `T`. When generating the field in `C`, the Mixin.mixinTraitMembers calls `fAccessor.accessed`. The implementation of `accessed` does a lookup for a member named `"f "` (note the space). The bug is that `private[this]` fields are not renamed to have space (`LOCAL_SUFFIX_STRING`) in their name, so the accessed was not found, and no annotations were copied from it. --- src/reflect/scala/reflect/internal/Symbols.scala | 17 ++++++++++++----- test/files/run/t8087.scala | 12 ++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 test/files/run/t8087.scala (limited to 'test/files') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 44fce2c9ab..b0c23ef45d 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -2023,12 +2023,19 @@ trait Symbols extends api.Symbols { self: SymbolTable => info.decls.filter(sym => !sym.isMethod && sym.isParamAccessor).toList /** The symbol accessed by this accessor (getter or setter) function. */ - final def accessed: Symbol = accessed(owner.info) - - /** The symbol accessed by this accessor function, but with given owner type. */ - final def accessed(ownerTp: Type): Symbol = { + final def accessed: Symbol = { assert(hasAccessorFlag, this) - ownerTp decl localName + val localField = owner.info decl localName + + if (localField == NoSymbol && this.hasFlag(MIXEDIN)) { + // SI-8087: private[this] fields don't have a `localName`. When searching the accessed field + // for a mixin accessor of such a field, we need to look for `name` instead. + // The phase travel ensures that the field is found (`owner` is the trait class symbol, the + // field gets removed from there in later phases). + enteringPhase(picklerPhase)(owner.info).decl(name).suchThat(!_.isAccessor) + } else { + localField + } } /** The module corresponding to this module class (note that this diff --git a/test/files/run/t8087.scala b/test/files/run/t8087.scala new file mode 100644 index 0000000000..6047211756 --- /dev/null +++ b/test/files/run/t8087.scala @@ -0,0 +1,12 @@ +trait Foo { + @volatile private[this] var x: String = "" + @volatile private var y: String = "" +} + +class Bar extends Foo + +object Test extends App { + classOf[Bar].getDeclaredFields.foreach(f => { + assert(java.lang.reflect.Modifier.isVolatile(f.getModifiers), f.getName) + }) +} -- cgit v1.2.3 From 84d46719a9ab3caa7ff46af4630d75f8c407c3d2 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 1 Oct 2014 08:51:36 +1000 Subject: SI-8869 Prevent ill-kindedness in type lambdas When type checking an type application, the arguments are allowed to be of kinds other than *. This leniency is controlled by the `ContextMode` bit `TypeConstructorAllowed`. (More fine grained checking of matching arity a bounds of type constructors is deferred until the refchecks phase to avoid cycles during typechecking.) However, this bit is propagated to child contexts, which means that we fail to report this error in the lexical context marked here: T[({type x = Option}#x)] `-------------' This commit resets this bit to false in any child context relates to a different tree from its parent. --- src/compiler/scala/tools/nsc/typechecker/Contexts.scala | 2 ++ test/files/neg/t8869.check | 7 +++++++ test/files/neg/t8869.scala | 10 ++++++++++ 3 files changed, 19 insertions(+) create mode 100644 test/files/neg/t8869.check create mode 100644 test/files/neg/t8869.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala index a79f162140..eb29ccf4e1 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala @@ -480,6 +480,8 @@ trait Contexts { self: Analyzer => // SI-8245 `isLazy` need to skip lazy getters to ensure `return` binds to the right place c.enclMethod = if (isDefDef && !owner.isLazy) c else enclMethod + if (tree != outer.tree) c(TypeConstructorAllowed) = false + registerContext(c.asInstanceOf[analyzer.Context]) debuglog("[context] ++ " + c.unit + " / " + tree.summaryString) c diff --git a/test/files/neg/t8869.check b/test/files/neg/t8869.check new file mode 100644 index 0000000000..40b8570f9f --- /dev/null +++ b/test/files/neg/t8869.check @@ -0,0 +1,7 @@ +t8869.scala:5: error: class Option takes type parameters + def value: TC[({type l1[x] = Option})#l1] = ??? // error not reported! + ^ +t8869.scala:7: error: class Option takes type parameters + type l2[x] = Option // error correctly reported + ^ +two errors found diff --git a/test/files/neg/t8869.scala b/test/files/neg/t8869.scala new file mode 100644 index 0000000000..0c7f0c9451 --- /dev/null +++ b/test/files/neg/t8869.scala @@ -0,0 +1,10 @@ +class TC[T[_]] { + def identity[A](a: T[A]): T[A] = a +} +object Test { + def value: TC[({type l1[x] = Option})#l1] = ??? // error not reported! + + type l2[x] = Option // error correctly reported + def value1: TC[l2] = ??? +} + -- cgit v1.2.3 From 922a9fc919169ad27f27b25cb48450b368d9329f Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 15 Sep 2014 16:42:07 +1000 Subject: SI-8845 Control flow pot-pourri crashes GenASM, but not -BCode Given that we'll switch to GenBCode in 2.12, the test case showing the bug is fixed under that option is all I plan to offer for this bug. The flags file contains `-Ynooptimize` to stay locked into `GenBCode`. --- test/files/run/t8845.flags | 1 + test/files/run/t8845.scala | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 test/files/run/t8845.flags create mode 100644 test/files/run/t8845.scala (limited to 'test/files') diff --git a/test/files/run/t8845.flags b/test/files/run/t8845.flags new file mode 100644 index 0000000000..aada25f80d --- /dev/null +++ b/test/files/run/t8845.flags @@ -0,0 +1 @@ +-Ybackend:GenBCode -Ynooptimize diff --git a/test/files/run/t8845.scala b/test/files/run/t8845.scala new file mode 100644 index 0000000000..8ccdbdadc7 --- /dev/null +++ b/test/files/run/t8845.scala @@ -0,0 +1,17 @@ +// crashes compiler under GenASM, works under GenBCode. +object Interpreter { + def mkDataProp(i: Int) = i + def break(n: Int): Unit = + try { + n match { + case _ => + val newDesc = mkDataProp(n) + n match { case _ => return } + } + } catch { case e: Throwable => } + finally { } +} + +object Test extends App { + Interpreter.break(0) +} -- cgit v1.2.3 From 0d8ca1f5fe335565f861ce2c58f7f684f15a4064 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Fri, 31 Jan 2014 19:43:08 +0100 Subject: SI-8217 allow abstract type members in objects Previously, abstract type members were allowed in objects only when inherited, but not when declared directly. This inconsistency was not intended. In dotty, abstract type members are allowed in values and represent existentials; so upon discussion, it was decided to fix things to conform to dotty and allow such type members. Adriaan also asked to keep rejecting abstract type members in methods even though they would conceivably make sense. Discussions happened on #3407, scala/scala-dist#127. This code is improved from #3442, keeps closer to the current logic, and passes tests. Existing tests that have been converted to `pos` tests show that this works, and a new test has been added to show that local aliases (ie term-owned) without a RHS are still rejected. --- src/compiler/scala/tools/nsc/typechecker/Namers.scala | 1 + test/files/neg/t3240.check | 4 ---- test/files/neg/t3240.scala | 8 -------- test/files/neg/t8217-local-alias-requires-rhs.check | 10 ++++++++++ test/files/neg/t8217-local-alias-requires-rhs.scala | 15 +++++++++++++++ test/files/neg/t845.check | 4 ---- test/files/neg/t845.scala | 16 ---------------- test/files/pos/t3240.scala | 8 ++++++++ test/files/pos/t845.scala | 16 ++++++++++++++++ 9 files changed, 50 insertions(+), 32 deletions(-) delete mode 100644 test/files/neg/t3240.check delete mode 100644 test/files/neg/t3240.scala create mode 100644 test/files/neg/t8217-local-alias-requires-rhs.check create mode 100644 test/files/neg/t8217-local-alias-requires-rhs.scala delete mode 100644 test/files/neg/t845.check delete mode 100644 test/files/neg/t845.scala create mode 100644 test/files/pos/t3240.scala create mode 100644 test/files/pos/t845.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index fdff2f3076..e876d4a6af 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -1643,6 +1643,7 @@ trait Namers extends MethodSynthesis { def symbolAllowsDeferred = ( sym.isValueParameter || sym.isTypeParameterOrSkolem + || (sym.isAbstractType && sym.owner.isClass) || context.tree.isInstanceOf[ExistentialTypeTree] ) // Does the symbol owner require no undefined members? diff --git a/test/files/neg/t3240.check b/test/files/neg/t3240.check deleted file mode 100644 index efae682c66..0000000000 --- a/test/files/neg/t3240.check +++ /dev/null @@ -1,4 +0,0 @@ -t3240.scala:3: error: only classes can have declared but undefined members - type t - ^ -one error found diff --git a/test/files/neg/t3240.scala b/test/files/neg/t3240.scala deleted file mode 100644 index cf197a406d..0000000000 --- a/test/files/neg/t3240.scala +++ /dev/null @@ -1,8 +0,0 @@ -class A { - val foo = new { - type t - def apply(a: Option[t], defVal: Any) = { - a.getOrElse(defVal).asInstanceOf[t] - } - } -} \ No newline at end of file diff --git a/test/files/neg/t8217-local-alias-requires-rhs.check b/test/files/neg/t8217-local-alias-requires-rhs.check new file mode 100644 index 0000000000..0d4f0864ba --- /dev/null +++ b/test/files/neg/t8217-local-alias-requires-rhs.check @@ -0,0 +1,10 @@ +t8217-local-alias-requires-rhs.scala:6: error: only classes can have declared but undefined members + type B + ^ +t8217-local-alias-requires-rhs.scala:3: error: only classes can have declared but undefined members + type A + ^ +t8217-local-alias-requires-rhs.scala:14: error: only classes can have declared but undefined members + def this(a: Any) = { this(); type C } + ^ +three errors found diff --git a/test/files/neg/t8217-local-alias-requires-rhs.scala b/test/files/neg/t8217-local-alias-requires-rhs.scala new file mode 100644 index 0000000000..12b7976835 --- /dev/null +++ b/test/files/neg/t8217-local-alias-requires-rhs.scala @@ -0,0 +1,15 @@ +trait Alias { + def foo = { + type A + } + val bar = { + type B + object O { + type OK + } + } +} + +class C { + def this(a: Any) = { this(); type C } +} diff --git a/test/files/neg/t845.check b/test/files/neg/t845.check deleted file mode 100644 index 07ed7e417b..0000000000 --- a/test/files/neg/t845.check +++ /dev/null @@ -1,4 +0,0 @@ -t845.scala:4: error: only classes can have declared but undefined members - type Bar; - ^ -one error found diff --git a/test/files/neg/t845.scala b/test/files/neg/t845.scala deleted file mode 100644 index ddf6a16f32..0000000000 --- a/test/files/neg/t845.scala +++ /dev/null @@ -1,16 +0,0 @@ -package test; - -object Test extends App { - type Bar; - trait FooImpl; - - trait Bob { - def bar : Bar with FooImpl; - } - def ifn[A,B](a : A)(f : A => B) = - if (a != null) f(a) else null; - - val bob : Bob = null; - val bar = ifn(bob)(_.bar); - assert(bar == null); -} diff --git a/test/files/pos/t3240.scala b/test/files/pos/t3240.scala new file mode 100644 index 0000000000..cf197a406d --- /dev/null +++ b/test/files/pos/t3240.scala @@ -0,0 +1,8 @@ +class A { + val foo = new { + type t + def apply(a: Option[t], defVal: Any) = { + a.getOrElse(defVal).asInstanceOf[t] + } + } +} \ No newline at end of file diff --git a/test/files/pos/t845.scala b/test/files/pos/t845.scala new file mode 100644 index 0000000000..ddf6a16f32 --- /dev/null +++ b/test/files/pos/t845.scala @@ -0,0 +1,16 @@ +package test; + +object Test extends App { + type Bar; + trait FooImpl; + + trait Bob { + def bar : Bar with FooImpl; + } + def ifn[A,B](a : A)(f : A => B) = + if (a != null) f(a) else null; + + val bob : Bob = null; + val bar = ifn(bob)(_.bar); + assert(bar == null); +} -- cgit v1.2.3 From ee2b7d615be8e7d893b78d770cf68a95c8fa0621 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 19 Sep 2014 00:11:39 +1000 Subject: SI-8267 Avoid existentials after polymorphic overload resolution ... which can be introduced by `memberType` for methods with parameter types dependent on class type parameters. Here's an example of such a type: ``` scala> class Bippy { trait Foo[A] } defined class Bippy scala> final class RichBippy[C <: Bippy with Singleton](val c1: C) { | def g[A](x: A)(ev: c1.Foo[A]): Int = 2 | } defined class RichBippy 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> val g = typeOf[RichBippy[_]].member(TermName("g")) g: $r.intp.global.Symbol = method g scala> val c = new Bippy c: Bippy = Bippy@92e2c93 scala> val memberType = typeOf[RichBippy[c.type]].memberType(g) memberType: $r.intp.global.Type = ([A](x: A)(ev: _7.c1.Foo[A])Int) forSome { val _7: RichBippy[c.type] } ``` In this example, if we were to typecheck the selection `new RichBippy[c.type].g` that existential type would be short lived. Consider this approximation of `Typer#typedInternal`: ```scala val tree1: Tree = typed1(tree, mode, ptWild) val result = adapt(tree1, mode, ptPlugins, tree) ``` Given that `tree1.tpe` is not an overloaded, adapt will find its way to: ``` case tp if mode.typingExprNotLhs && isExistentialType(tp) => adapt(tree setType tp.dealias.skolemizeExistential(context.owner, tree), mode, pt, original) ``` Which would open the existential as per: ``` scala> memberType.skolemizeExistential res2: $r.intp.global.Type = [A](x: A)(ev: _7.c1.Foo[A])Int ``` However, if do have overloaded alternatives, as in the test case, we have to remember to call `adapt` again *after* we have picked the winning alternative. We actually don't have a centralised place where overload resolution occurs, as the process differs depending on the context of the selection. (Are there explicit type arguments? Inferred type arguments? Do we need to use the expected type to pick a winner?) This commit finds the existing places that call adapt after overloade resolution and routes those calls through a marker method. It then adds one more call to this in `inferPolyAlternatives`, which fixes the bug. --- .../scala/tools/nsc/typechecker/Typers.scala | 27 ++++++++++++++---- test/files/pos/t8267.scala | 33 ++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 test/files/pos/t8267.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 422b940cd3..b498d9e667 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1100,7 +1100,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper adaptConstant(value) case OverloadedType(pre, alts) if !mode.inFunMode => // (1) inferExprAlternative(tree, pt) - adapt(tree, mode, pt, original) + adaptAfterOverloadResolution(tree, mode, pt, original) case NullaryMethodType(restpe) => // (2) adapt(tree setType restpe, mode, pt, original) case TypeRef(_, ByNameParamClass, arg :: Nil) if mode.inExprMode => // (2) @@ -1133,6 +1133,12 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper } } + // This just exists to help keep track of the spots where we have to adapt a tree after + // overload resolution. These proved hard to find during the fix for SI-8267. + def adaptAfterOverloadResolution(tree: Tree, mode: Mode, pt: Type = WildcardType, original: Tree = EmptyTree): Tree = { + adapt(tree, mode, pt, original) + } + def instantiate(tree: Tree, mode: Mode, pt: Type): Tree = { inferExprInstance(tree, context.extractUndetparams(), pt) adapt(tree, mode, pt) @@ -3171,7 +3177,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper if (sym1 != NoSymbol) sym = sym1 } if (sym == NoSymbol) fun - else adapt(fun setSymbol sym setType pre.memberType(sym), mode.forFunMode, WildcardType) + else adaptAfterOverloadResolution(fun setSymbol sym setType pre.memberType(sym), mode.forFunMode) } else fun } @@ -3216,7 +3222,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper setError(tree) else { inferMethodAlternative(fun, undetparams, argTpes, pt) - doTypedApply(tree, adapt(fun, mode.forFunMode, WildcardType), args1, mode, pt) + doTypedApply(tree, adaptAfterOverloadResolution(fun, mode.forFunMode, WildcardType), args1, mode, pt) } } handleOverloaded @@ -3799,7 +3805,18 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper protected def typedTypeApply(tree: Tree, mode: Mode, fun: Tree, args: List[Tree]): Tree = fun.tpe match { case OverloadedType(pre, alts) => inferPolyAlternatives(fun, mapList(args)(treeTpe)) - val tparams = fun.symbol.typeParams //@M TODO: fun.symbol.info.typeParams ? (as in typedAppliedTypeTree) + + // SI-8267 `memberType` can introduce existentials *around* a PolyType/MethodType, see AsSeenFromMap#captureThis. + // If we had selected a non-overloaded symbol, `memberType` would have been called in `makeAccessible` + // and the resulting existential type would have been skolemized in `adapt` *before* we typechecked + // the enclosing type-/ value- application. + // + // However, if the selection is overloaded, we defer calling `memberType` until we can select a single + // alternative here. It is therefore necessary to skolemize the existential here. + // + val fun1 = adaptAfterOverloadResolution(fun, mode.forFunMode | TAPPmode) + + val tparams = fun1.symbol.typeParams //@M TODO: fun.symbol.info.typeParams ? (as in typedAppliedTypeTree) val args1 = if (sameLength(args, tparams)) { //@M: in case TypeApply we can't check the kind-arities of the type arguments, // as we don't know which alternative to choose... here we do @@ -3813,7 +3830,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper // ...actually this was looping anyway, see bug #278. return TypedApplyWrongNumberOfTpeParametersError(fun, fun) - typedTypeApply(tree, mode, fun, args1) + typedTypeApply(tree, mode, fun1, args1) case SingleType(_, _) => typedTypeApply(tree, mode, fun setType fun.tpe.widen, args) case PolyType(tparams, restpe) if tparams.nonEmpty => diff --git a/test/files/pos/t8267.scala b/test/files/pos/t8267.scala new file mode 100644 index 0000000000..37b498fe3e --- /dev/null +++ b/test/files/pos/t8267.scala @@ -0,0 +1,33 @@ +class Bippy { trait Foo[A] } + +final class RichBippy[C <: Bippy with Singleton](val c1: C) { + def f: Int = 1 + def f[A](x: A)(ev: c1.Foo[A]): Int = 2 + + def g[A <: Nothing](x: A): Int = 1 + def g[A](x: A)(ev: c1.Foo[A]): Int = 2 + + def h[A](x: A)(ev: c1.Foo[A]): Int = 1 + + def i(x: Nothing): Int = 1 + def i(x: AnyRef)(ev: c1.Foo[x.type]): Int = 2 +} + +object p { + + val c = new Bippy + val d0 = new RichBippy[c.type](c) + def d1 = new RichBippy[c.type](c) + + d0.f[Int](5)(null: c.Foo[Int]) // ok + d1.f[Int](5)(null: c.Foo[Int]) // fails + + d0.g[Int](5)(null: c.Foo[Int]) // ok + d1.g[Int](5)(null: c.Foo[Int]) // fails + + d0.h[Int](5)(null: c.Foo[Int]) // ok + d1.h[Int](5)(null: c.Foo[Int]) // ok + + d0.i("")(null) // ok + d1.i("")(null) // ok +} -- cgit v1.2.3 From 1122e9e55aad915c1ecb90e6d1e11d2faba58dab Mon Sep 17 00:00:00 2001 From: Iulian Dragos Date: Fri, 3 Oct 2014 18:52:09 +0200 Subject: Revert "Disable flaky presentation compiler test." This reverts commit 8986ee4fd56c53d563165d992185c6c532f35790. Scaladoc seems to work reliably for 2.11.x. We are using it in the IDE builds and haven't noticed any flakiness, so we'd like to get reinstate this test. --- test/disabled/presentation/doc.check | 1 - test/disabled/presentation/doc/doc.scala | 147 --------------------- test/disabled/presentation/doc/src/Class.scala | 1 - test/disabled/presentation/doc/src/p/Base.scala | 11 -- test/disabled/presentation/doc/src/p/Derived.scala | 9 -- test/files/presentation/doc.check | 1 + test/files/presentation/doc/doc.scala | 147 +++++++++++++++++++++ test/files/presentation/doc/src/Class.scala | 1 + test/files/presentation/doc/src/p/Base.scala | 11 ++ test/files/presentation/doc/src/p/Derived.scala | 9 ++ 10 files changed, 169 insertions(+), 169 deletions(-) delete mode 100644 test/disabled/presentation/doc.check delete mode 100755 test/disabled/presentation/doc/doc.scala delete mode 100755 test/disabled/presentation/doc/src/Class.scala delete mode 100755 test/disabled/presentation/doc/src/p/Base.scala delete mode 100755 test/disabled/presentation/doc/src/p/Derived.scala create mode 100644 test/files/presentation/doc.check create mode 100755 test/files/presentation/doc/doc.scala create mode 100755 test/files/presentation/doc/src/Class.scala create mode 100755 test/files/presentation/doc/src/p/Base.scala create mode 100755 test/files/presentation/doc/src/p/Derived.scala (limited to 'test/files') diff --git a/test/disabled/presentation/doc.check b/test/disabled/presentation/doc.check deleted file mode 100644 index 5a3ff13151..0000000000 --- a/test/disabled/presentation/doc.check +++ /dev/null @@ -1 +0,0 @@ -reload: Base.scala, Class.scala, Derived.scala diff --git a/test/disabled/presentation/doc/doc.scala b/test/disabled/presentation/doc/doc.scala deleted file mode 100755 index f2233f1828..0000000000 --- a/test/disabled/presentation/doc/doc.scala +++ /dev/null @@ -1,147 +0,0 @@ -import scala.reflect.internal.util.{ BatchSourceFile, SourceFile } -import scala.tools.nsc.doc -import scala.tools.nsc.doc.base._ -import scala.tools.nsc.doc.base.comment._ -import scala.tools.nsc.interactive._ -import scala.tools.nsc.interactive.tests._ - -object Test extends InteractiveTest { - val tags = Seq( - "@example `\"abb\".permutations = Iterator(abb, bab, bba)`", - "@version 1.0, 09/07/2012", - "@since 2.10", - "@todo this is unsafe!", - "@note Don't inherit!", - "@see something else" - ) - - val names = Seq("Class", "Def", "Val", "Var", "AbstracType", "TypeAlias", "Trait", "InnerClass") - val bareText = - """abstract class %s { - | def %s = "" - | val %s = "" - | var %s: String = _ - | type %s - | type %s = String - | class %s - |} - |trait %s""".stripMargin.format(names: _*) - - def docComment(nTags: Int) = "/**\n%s*/".format(tags.take(nTags).mkString("\n")) - - def text(name: String, nTags: Int) = { - val nameIndex = bareText.indexOf(name) - val (pre, post) = bareText.splitAt(nameIndex) - val crIndex = pre.lastIndexOf("\n") - val (prepre, prepost) = pre.splitAt(crIndex) - prepre + docComment(nTags) + prepost + post - } - - override lazy val compiler = { - prepareSettings(settings) - new Global(settings, compilerReporter) with MemberLookupBase with CommentFactoryBase with doc.ScaladocGlobalTrait { - outer => - - val global: this.type = this - - override lazy val analyzer = new { - val global: outer.type = outer - } with doc.ScaladocAnalyzer with InteractiveAnalyzer { - override def newTyper(context: Context): InteractiveTyper with ScaladocTyper = - new Typer(context) with InteractiveTyper with ScaladocTyper - } - - def chooseLink(links: List[LinkTo]): LinkTo = links.head - def internalLink(sym: Symbol, site: Symbol) = None - def toString(link: LinkTo) = link.toString - def warnNoLink = false - def findExternalLink(sym: Symbol, name: String) = None - - override def forScaladoc = true - - def getComment(sym: Symbol, source: SourceFile, fragments: List[(Symbol,SourceFile)]): Option[Comment] = { - val docResponse = new Response[(String, String, Position)] - askDocComment(sym, source, sym.owner, fragments, docResponse) - docResponse.get.left.toOption flatMap { - case (expanded, raw, pos) => - if (expanded.isEmpty) - None - else - Some(ask { () => parseAtSymbol(expanded, raw, pos, sym.owner) }) - } - } - } - } - - override def runDefaultTests() { - import compiler._ - def findSource(name: String) = sourceFiles.find(_.file.name == name).get - - val className = names.head - for (name <- names; - i <- 1 to tags.length) { - val newText = text(name, i) - val source = findSource("Class.scala") - val batch = new BatchSourceFile(source.file, newText.toCharArray) - val reloadResponse = new Response[Unit] - compiler.askReload(List(batch), reloadResponse) - reloadResponse.get.left.toOption match { - case None => - println("Couldn't reload") - case Some(_) => - val parseResponse = new Response[Tree] - askParsedEntered(batch, true, parseResponse) - parseResponse.get.left.toOption match { - case None => - println("Couldn't parse") - case Some(_) => - val sym = compiler.ask { () => - val toplevel = compiler.rootMirror.EmptyPackage.info.decl(TypeName(name)) - if (toplevel eq NoSymbol) { - val clazz = compiler.rootMirror.EmptyPackage.info.decl(TypeName(className)) - val term = clazz.info.decl(TermName(name)) - if (term eq NoSymbol) clazz.info.decl(TypeName(name)) else - if (term.isAccessor) term.accessed else term - } else toplevel - } - - getComment(sym, batch, (sym,batch)::Nil) match { - case None => println(s"Got no doc comment for $name") - case Some(comment) => - import comment._ - def cnt(bodies: Iterable[Body]) = bodies.size - val actual = cnt(example) + cnt(version) + cnt(since) + cnt(todo) + cnt(note) + cnt(see) - if (actual != i) - println(s"Got docComment with $actual tags instead of $i, file text:\n$newText") - } - } - } - } - - // Check inter-classes documentation one-time retrieved ok. - val baseSource = findSource("Base.scala") - val derivedSource = findSource("Derived.scala") - def existsText(where: Any, text: String): Boolean = where match { - case s: String => s contains text - case s: Seq[_] => s exists (existsText(_, text)) - case p: Product => p.productIterator exists (existsText(_, text)) - case c: Comment => existsText(c.body, text) - } - val (derived, base) = compiler.ask { () => - val derived = compiler.rootMirror.RootPackage.info.decl(newTermName("p")).info.decl(newTypeName("Derived")) - (derived, derived.ancestors(0)) - } - val cmt1 = getComment(derived, derivedSource, (base, baseSource)::(derived, derivedSource)::Nil) - if (!existsText(cmt1, "This is Derived comment")) - println("Unexpected Derived class comment:"+cmt1) - - val (fooDerived, fooBase) = compiler.ask { () => - val decl = derived.tpe.decl(newTermName("foo")) - (decl, decl.allOverriddenSymbols(0)) - } - - val cmt2 = getComment(fooDerived, derivedSource, (fooBase, baseSource)::(fooDerived, derivedSource)::Nil) - if (!existsText(cmt2, "Base method has documentation")) - println("Unexpected foo method comment:"+cmt2) - } -} diff --git a/test/disabled/presentation/doc/src/Class.scala b/test/disabled/presentation/doc/src/Class.scala deleted file mode 100755 index a974bd6f5c..0000000000 --- a/test/disabled/presentation/doc/src/Class.scala +++ /dev/null @@ -1 +0,0 @@ -object Class \ No newline at end of file diff --git a/test/disabled/presentation/doc/src/p/Base.scala b/test/disabled/presentation/doc/src/p/Base.scala deleted file mode 100755 index d91632b6f6..0000000000 --- a/test/disabled/presentation/doc/src/p/Base.scala +++ /dev/null @@ -1,11 +0,0 @@ -package p - -/** - * @define BaseComment This is $BaseVar comment. - */ -trait Base { - /** - * Base method has documentation. - */ - def foo: String -} diff --git a/test/disabled/presentation/doc/src/p/Derived.scala b/test/disabled/presentation/doc/src/p/Derived.scala deleted file mode 100755 index 1a9c9a26d1..0000000000 --- a/test/disabled/presentation/doc/src/p/Derived.scala +++ /dev/null @@ -1,9 +0,0 @@ -package p - -/** - * $BaseComment - * @define BaseVar Derived - */ -class Derived extends Base { - def foo = "" -} diff --git a/test/files/presentation/doc.check b/test/files/presentation/doc.check new file mode 100644 index 0000000000..5a3ff13151 --- /dev/null +++ b/test/files/presentation/doc.check @@ -0,0 +1 @@ +reload: Base.scala, Class.scala, Derived.scala diff --git a/test/files/presentation/doc/doc.scala b/test/files/presentation/doc/doc.scala new file mode 100755 index 0000000000..f2233f1828 --- /dev/null +++ b/test/files/presentation/doc/doc.scala @@ -0,0 +1,147 @@ +import scala.reflect.internal.util.{ BatchSourceFile, SourceFile } +import scala.tools.nsc.doc +import scala.tools.nsc.doc.base._ +import scala.tools.nsc.doc.base.comment._ +import scala.tools.nsc.interactive._ +import scala.tools.nsc.interactive.tests._ + +object Test extends InteractiveTest { + val tags = Seq( + "@example `\"abb\".permutations = Iterator(abb, bab, bba)`", + "@version 1.0, 09/07/2012", + "@since 2.10", + "@todo this is unsafe!", + "@note Don't inherit!", + "@see something else" + ) + + val names = Seq("Class", "Def", "Val", "Var", "AbstracType", "TypeAlias", "Trait", "InnerClass") + val bareText = + """abstract class %s { + | def %s = "" + | val %s = "" + | var %s: String = _ + | type %s + | type %s = String + | class %s + |} + |trait %s""".stripMargin.format(names: _*) + + def docComment(nTags: Int) = "/**\n%s*/".format(tags.take(nTags).mkString("\n")) + + def text(name: String, nTags: Int) = { + val nameIndex = bareText.indexOf(name) + val (pre, post) = bareText.splitAt(nameIndex) + val crIndex = pre.lastIndexOf("\n") + val (prepre, prepost) = pre.splitAt(crIndex) + prepre + docComment(nTags) + prepost + post + } + + override lazy val compiler = { + prepareSettings(settings) + new Global(settings, compilerReporter) with MemberLookupBase with CommentFactoryBase with doc.ScaladocGlobalTrait { + outer => + + val global: this.type = this + + override lazy val analyzer = new { + val global: outer.type = outer + } with doc.ScaladocAnalyzer with InteractiveAnalyzer { + override def newTyper(context: Context): InteractiveTyper with ScaladocTyper = + new Typer(context) with InteractiveTyper with ScaladocTyper + } + + def chooseLink(links: List[LinkTo]): LinkTo = links.head + def internalLink(sym: Symbol, site: Symbol) = None + def toString(link: LinkTo) = link.toString + def warnNoLink = false + def findExternalLink(sym: Symbol, name: String) = None + + override def forScaladoc = true + + def getComment(sym: Symbol, source: SourceFile, fragments: List[(Symbol,SourceFile)]): Option[Comment] = { + val docResponse = new Response[(String, String, Position)] + askDocComment(sym, source, sym.owner, fragments, docResponse) + docResponse.get.left.toOption flatMap { + case (expanded, raw, pos) => + if (expanded.isEmpty) + None + else + Some(ask { () => parseAtSymbol(expanded, raw, pos, sym.owner) }) + } + } + } + } + + override def runDefaultTests() { + import compiler._ + def findSource(name: String) = sourceFiles.find(_.file.name == name).get + + val className = names.head + for (name <- names; + i <- 1 to tags.length) { + val newText = text(name, i) + val source = findSource("Class.scala") + val batch = new BatchSourceFile(source.file, newText.toCharArray) + val reloadResponse = new Response[Unit] + compiler.askReload(List(batch), reloadResponse) + reloadResponse.get.left.toOption match { + case None => + println("Couldn't reload") + case Some(_) => + val parseResponse = new Response[Tree] + askParsedEntered(batch, true, parseResponse) + parseResponse.get.left.toOption match { + case None => + println("Couldn't parse") + case Some(_) => + val sym = compiler.ask { () => + val toplevel = compiler.rootMirror.EmptyPackage.info.decl(TypeName(name)) + if (toplevel eq NoSymbol) { + val clazz = compiler.rootMirror.EmptyPackage.info.decl(TypeName(className)) + val term = clazz.info.decl(TermName(name)) + if (term eq NoSymbol) clazz.info.decl(TypeName(name)) else + if (term.isAccessor) term.accessed else term + } else toplevel + } + + getComment(sym, batch, (sym,batch)::Nil) match { + case None => println(s"Got no doc comment for $name") + case Some(comment) => + import comment._ + def cnt(bodies: Iterable[Body]) = bodies.size + val actual = cnt(example) + cnt(version) + cnt(since) + cnt(todo) + cnt(note) + cnt(see) + if (actual != i) + println(s"Got docComment with $actual tags instead of $i, file text:\n$newText") + } + } + } + } + + // Check inter-classes documentation one-time retrieved ok. + val baseSource = findSource("Base.scala") + val derivedSource = findSource("Derived.scala") + def existsText(where: Any, text: String): Boolean = where match { + case s: String => s contains text + case s: Seq[_] => s exists (existsText(_, text)) + case p: Product => p.productIterator exists (existsText(_, text)) + case c: Comment => existsText(c.body, text) + } + val (derived, base) = compiler.ask { () => + val derived = compiler.rootMirror.RootPackage.info.decl(newTermName("p")).info.decl(newTypeName("Derived")) + (derived, derived.ancestors(0)) + } + val cmt1 = getComment(derived, derivedSource, (base, baseSource)::(derived, derivedSource)::Nil) + if (!existsText(cmt1, "This is Derived comment")) + println("Unexpected Derived class comment:"+cmt1) + + val (fooDerived, fooBase) = compiler.ask { () => + val decl = derived.tpe.decl(newTermName("foo")) + (decl, decl.allOverriddenSymbols(0)) + } + + val cmt2 = getComment(fooDerived, derivedSource, (fooBase, baseSource)::(fooDerived, derivedSource)::Nil) + if (!existsText(cmt2, "Base method has documentation")) + println("Unexpected foo method comment:"+cmt2) + } +} diff --git a/test/files/presentation/doc/src/Class.scala b/test/files/presentation/doc/src/Class.scala new file mode 100755 index 0000000000..a974bd6f5c --- /dev/null +++ b/test/files/presentation/doc/src/Class.scala @@ -0,0 +1 @@ +object Class \ No newline at end of file diff --git a/test/files/presentation/doc/src/p/Base.scala b/test/files/presentation/doc/src/p/Base.scala new file mode 100755 index 0000000000..d91632b6f6 --- /dev/null +++ b/test/files/presentation/doc/src/p/Base.scala @@ -0,0 +1,11 @@ +package p + +/** + * @define BaseComment This is $BaseVar comment. + */ +trait Base { + /** + * Base method has documentation. + */ + def foo: String +} diff --git a/test/files/presentation/doc/src/p/Derived.scala b/test/files/presentation/doc/src/p/Derived.scala new file mode 100755 index 0000000000..1a9c9a26d1 --- /dev/null +++ b/test/files/presentation/doc/src/p/Derived.scala @@ -0,0 +1,9 @@ +package p + +/** + * $BaseComment + * @define BaseVar Derived + */ +class Derived extends Base { + def foo = "" +} -- cgit v1.2.3 From a87db212098a5e69176652c93284f425bb7e1b09 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sun, 5 Oct 2014 10:26:02 +1000 Subject: SI-8291 Fix implicitNotFound message with type aliases This pattern of code is typically a bug: if (f(tp.typeSymbol)) { g(tp.typeArgs) } Intead, one needs to take the base type of `tp` wrt `tp.typeSymbol`. This commit does exactly that when formatting the `@implicitNotFound` custom error message. Patch found on the back of an envelope in the handwriting of @adriaanm --- src/compiler/scala/tools/nsc/typechecker/Implicits.scala | 4 +++- test/files/neg/t8291.check | 7 +++++++ test/files/neg/t8291.scala | 7 +++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t8291.check create mode 100644 test/files/neg/t8291.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala index b85c8e6d42..74c28122a1 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala @@ -1478,8 +1478,10 @@ trait Implicits { }) private lazy val typeParamNames: List[String] = sym.typeParams.map(_.decodedName) + private def typeArgsAtSym(paramTp: Type) = paramTp.baseType(sym).typeArgs + + def format(paramName: Name, paramTp: Type): String = format(typeArgsAtSym(paramTp) map (_.toString)) - def format(paramName: Name, paramTp: Type): String = format(paramTp.typeArgs map (_.toString)) def format(typeArgs: List[String]): String = interpolate(msg, Map((typeParamNames zip typeArgs): _*)) // TODO: give access to the name and type of the implicit argument, etc? diff --git a/test/files/neg/t8291.check b/test/files/neg/t8291.check new file mode 100644 index 0000000000..c9972e5575 --- /dev/null +++ b/test/files/neg/t8291.check @@ -0,0 +1,7 @@ +t8291.scala:5: error: Could not find implicit for Int or String + implicitly[X[Int, String]] + ^ +t8291.scala:6: error: Could not find implicit for Int or String + implicitly[Z[String]] + ^ +two errors found diff --git a/test/files/neg/t8291.scala b/test/files/neg/t8291.scala new file mode 100644 index 0000000000..b344586a56 --- /dev/null +++ b/test/files/neg/t8291.scala @@ -0,0 +1,7 @@ +@scala.annotation.implicitNotFound("Could not find implicit for ${T} or ${U}") trait X[T, U] + +object Test { + type Z[U] = X[Int, U] + implicitly[X[Int, String]] + implicitly[Z[String]] +} -- cgit v1.2.3 From 179419c7ef29c5f987d7a73bf403435d6937764e Mon Sep 17 00:00:00 2001 From: Gerard Basler Date: Mon, 15 Sep 2014 01:12:42 +0200 Subject: SI-7746 patmat: fix non-determinism, infeasible counter examples Fixes non-determinism within the DPLL algorithm and disallows infeasible counter examples directly in the formula. The function to compute all solutions was flawed and thus only returned a subset of the solutions. The algorithm would stop too soon and thus depending on the ordering of the symbols return more or less solutions. I also added printing a warning when the search was stopped because the max recursion depth was reached. This is very useful as an explanation of spuriously failing regression tests, since less counter examples might be reported. In such a case the recursion depth should be set to infinite by adding `-Ypatmat-exhaust-depth off`. The mapping of the solutions of the DPLL algorithm to counter examples has been adapted to take the additional solutions from the solver into account: Consider for example `t8430.scala`: ```Scala sealed trait CL3Literal case object IntLit extends CL3Literal case object CharLit extends CL3Literal case object BooleanLit extends CL3Literal case object UnitLit extends CL3Literal sealed trait Tree case class LetL(value: CL3Literal) extends Tree case object LetP extends Tree case object LetC extends Tree case object LetF extends Tree object Test { (tree: Tree) => tree match {case LetL(CharLit) => ??? } } ``` This test contains 2 domains, `IntLit, CharLit, ...` and `LetL, LetP, ...`, the corresponding formula to check exhaustivity looks like: ``` V1=LetC.type#13 \/ V1=LetF.type#14 \/ V1=LetL#11 \/ V1=LetP.type#15 /\ V2=BooleanLit.type#16 \/ V2=CharLit#12 \/ V2=IntLit.type#17 \/ V2=UnitLit.type#18 /\ -V1=LetL#11 \/ -V2=CharLit#12 \/ \/ ``` The first two lines assign a value of the domain to the scrutinee (and the correponding member in case of `LetL`) and prohibit the counter example `LetL(CharLit)` since it's covered by the pattern match. The used Boolean encoding allows that scrutinee `V1` can be equal to `LetC` and `LetF` at the same time and thus, during enumeration of all possible solutions of the formula, such a solution will be found, since only one literal needs to be set to true, to satisfy that clause. That means, if at least one of the literals of such a clause was in the `unassigned` list of the DPLL procedure, we will get solutions where the scrutinee is equal to more than one element of the domain. A remedy would be to add constraints that forbid both literals to be true at the same time. His is infeasible for big domains (see `pos/t8531.scala`), since we would have to add a quadratic number of clauses (one clause for each pair in the domain). A much simpler solution is to just filter the invalid results. Since both values for `unassigned` literals are explored, we will eventually find a valid counter example. --- .../scala/tools/nsc/settings/ScalaSettings.scala | 2 + .../scala/tools/nsc/transform/patmat/Logic.scala | 3 +- .../tools/nsc/transform/patmat/MatchAnalysis.scala | 55 +++++++++++----- .../scala/tools/nsc/transform/patmat/Solving.scala | 74 ++++++++++++++++++---- test/files/neg/patmatexhaust.check | 2 +- test/files/neg/patmatexhaust.flags | 2 +- test/files/neg/patmatexhaust.scala | 2 +- test/files/neg/t8430.check | 12 ++-- test/files/neg/t8430.flags | 2 +- test/files/run/virtpatmat_nested_lists.flags | 1 + test/files/run/virtpatmat_opt_sharing.flags | 1 + 11 files changed, 115 insertions(+), 41 deletions(-) create mode 100644 test/files/run/virtpatmat_nested_lists.flags create mode 100644 test/files/run/virtpatmat_opt_sharing.flags (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index 466e397dd7..850534f2cc 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -262,6 +262,8 @@ trait ScalaSettings extends AbsScalaSettings val Yreifydebug = BooleanSetting("-Yreify-debug", "Trace reification.") val Ytyperdebug = BooleanSetting("-Ytyper-debug", "Trace all type assignments.") val Ypatmatdebug = BooleanSetting("-Ypatmat-debug", "Trace pattern matching translation.") + val YpatmatExhaustdepth = IntSetting("-Ypatmat-exhaust-depth", "off", 20, Some((10, Int.MaxValue)), + str => Some(if(str.equalsIgnoreCase("off")) Int.MaxValue else str.toInt)) val Yquasiquotedebug = BooleanSetting("-Yquasiquote-debug", "Trace quasiquote-related activities.") // TODO 2.12 Remove diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala index 0899507bab..3331805c1b 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala @@ -10,7 +10,6 @@ package tools.nsc.transform.patmat import scala.language.postfixOps import scala.collection.mutable import scala.reflect.internal.util.Statistics -import scala.reflect.internal.util.Position import scala.reflect.internal.util.HashSet trait Logic extends Debugging { @@ -72,6 +71,8 @@ trait Logic extends Debugging { def unapply(v: Var): Some[Tree] } + def reportWarning(message: String): Unit + // resets hash consing -- only supposed to be called by TreeMakersToProps def prepareNewAnalysis(): Unit diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index b2dc6e4e52..a8852a3ff3 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -9,7 +9,6 @@ package scala.tools.nsc.transform.patmat import scala.language.postfixOps import scala.collection.mutable import scala.reflect.internal.util.Statistics -import scala.reflect.internal.util.Position trait TreeAndTypeAnalysis extends Debugging { import global._ @@ -400,6 +399,7 @@ trait MatchAnalysis extends MatchApproximation { trait MatchAnalyzer extends MatchApproximator { def uncheckedWarning(pos: Position, msg: String) = currentRun.reporting.uncheckedWarning(pos, msg) def warn(pos: Position, ex: AnalysisBudget.Exception, kind: String) = uncheckedWarning(pos, s"Cannot check match for $kind.\n${ex.advice}") + def reportWarning(message: String) = global.reporter.warning(typer.context.tree.pos, message) // TODO: model dependencies between variables: if V1 corresponds to (x: List[_]) and V2 is (x.hd), V2 cannot be assigned when V1 = null or V1 = Nil // right now hackily implement this by pruning counter-examples @@ -524,9 +524,11 @@ trait MatchAnalysis extends MatchApproximation { val matchFailModels = findAllModelsFor(propToSolvable(matchFails)) val scrutVar = Var(prevBinderTree) - val counterExamples = matchFailModels.map(modelToCounterExample(scrutVar)) - - val pruned = CounterExample.prune(counterExamples).map(_.toString).sorted + val counterExamples = matchFailModels.flatMap(modelToCounterExample(scrutVar)) + // sorting before pruning is important here in order to + // keep neg/t7020.scala stable + // since e.g. List(_, _) would cover List(1, _) + val pruned = CounterExample.prune(counterExamples.sortBy(_.toString)).map(_.toString) if (Statistics.canEnable) Statistics.stopTimer(patmatAnaExhaust, start) pruned @@ -616,7 +618,7 @@ trait MatchAnalysis extends MatchApproximation { // (the variables don't take into account type information derived from other variables, // so, naively, you might try to construct a counter example like _ :: Nil(_ :: _, _ :: _), // since we didn't realize the tail of the outer cons was a Nil) - def modelToCounterExample(scrutVar: Var)(model: Model): CounterExample = { + def modelToCounterExample(scrutVar: Var)(model: Model): Option[CounterExample] = { // x1 = ... // x1.hd = ... // x1.tl = ... @@ -674,6 +676,7 @@ trait MatchAnalysis extends MatchApproximation { private val fields: mutable.Map[Symbol, VariableAssignment] = mutable.HashMap.empty // need to prune since the model now incorporates all super types of a constant (needed for reachability) private lazy val uniqueEqualTo = equalTo filterNot (subsumed => equalTo.exists(better => (better ne subsumed) && instanceOfTpImplies(better.tp, subsumed.tp))) + private lazy val inSameDomain = uniqueEqualTo forall (const => variable.domainSyms.exists(_.exists(_.const.tp =:= const.tp))) private lazy val prunedEqualTo = uniqueEqualTo filterNot (subsumed => variable.staticTpCheckable <:< subsumed.tp) private lazy val ctor = (prunedEqualTo match { case List(TypeConst(tp)) => tp case _ => variable.staticTpCheckable }).typeSymbol.primaryConstructor private lazy val ctorParams = if (ctor.paramss.isEmpty) Nil else ctor.paramss.head @@ -694,13 +697,13 @@ trait MatchAnalysis extends MatchApproximation { // NoExample if the constructor call is ill-typed // (thus statically impossible -- can we incorporate this into the formula?) // beBrief is used to suppress negative information nested in tuples -- it tends to get too noisy - def toCounterExample(beBrief: Boolean = false): CounterExample = - if (!allFieldAssignmentsLegal) NoExample + def toCounterExample(beBrief: Boolean = false): Option[CounterExample] = + if (!allFieldAssignmentsLegal) Some(NoExample) else { debug.patmat("describing "+ ((variable, equalTo, notEqualTo, fields, cls, allFieldAssignmentsLegal))) val res = prunedEqualTo match { // a definite assignment to a value - case List(eq: ValueConst) if fields.isEmpty => ValueExample(eq) + case List(eq: ValueConst) if fields.isEmpty => Some(ValueExample(eq)) // constructor call // or we did not gather any information about equality but we have information about the fields @@ -713,30 +716,50 @@ trait MatchAnalysis extends MatchApproximation { // figure out the constructor arguments from the field assignment val argLen = (caseFieldAccs.length min ctorParams.length) - (0 until argLen).map(i => fields.get(caseFieldAccs(i)).map(_.toCounterExample(brevity)) getOrElse WildcardExample).toList + val examples = (0 until argLen).map(i => fields.get(caseFieldAccs(i)).map(_.toCounterExample(brevity)) getOrElse Some(WildcardExample)).toList + sequence(examples) } cls match { - case ConsClass => ListExample(args()) - case _ if isTupleSymbol(cls) => TupleExample(args(brevity = true)) - case _ => ConstructorExample(cls, args()) + case ConsClass => + args().map { + case List(NoExample, l: ListExample) => + // special case for neg/t7020.scala: + // if we find a counter example `??::*` we report `*::*` instead + // since the `??` originates from uniqueEqualTo containing several instanced of the same type + List(WildcardExample, l) + case args => args + }.map(ListExample) + case _ if isTupleSymbol(cls) => args(brevity = true).map(TupleExample) + case _ if cls.isSealed && cls.isAbstractClass => + // don't report sealed abstract classes, since + // 1) they can't be instantiated + // 2) we are already reporting any missing subclass (since we know the full domain) + // (see patmatexhaust.scala) + None + case _ => args().map(ConstructorExample(cls, _)) } // a definite assignment to a type - case List(eq) if fields.isEmpty => TypeExample(eq) + case List(eq) if fields.isEmpty => Some(TypeExample(eq)) // negative information case Nil if nonTrivialNonEqualTo.nonEmpty => // negation tends to get pretty verbose - if (beBrief) WildcardExample + if (beBrief) Some(WildcardExample) else { val eqTo = equalTo.headOption getOrElse TypeConst(variable.staticTpCheckable) - NegativeExample(eqTo, nonTrivialNonEqualTo) + Some(NegativeExample(eqTo, nonTrivialNonEqualTo)) } + // if uniqueEqualTo contains more than one symbol of the same domain + // then we can safely ignore these counter examples since we will eventually encounter + // both counter examples separately + case _ if inSameDomain => None + // not a valid counter-example, possibly since we have a definite type but there was a field mismatch // TODO: improve reasoning -- in the mean time, a false negative is better than an annoying false positive - case _ => NoExample + case _ => Some(NoExample) } debug.patmatResult("described as")(res) } diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala b/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala index 31b1ffa912..0d867be4b6 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/Solving.scala @@ -115,7 +115,6 @@ trait Solving extends Logic { if (Statistics.canEnable) Statistics.stopTimer(patmatCNF, start) - // if (Statistics.canEnable) patmatCNFSizes(res.size).value += 1 // debug.patmat("cnf for\n"+ p +"\nis:\n"+cnfString(res)) @@ -146,31 +145,78 @@ trait Solving extends Logic { // returns all solutions, if any (TODO: better infinite recursion backstop -- detect fixpoint??) def findAllModelsFor(f: Formula): List[Model] = { + + debug.patmat("find all models for\n"+ cnfString(f)) + val vars: Set[Sym] = f.flatMap(_ collect {case l: Lit => l.sym}).toSet // debug.patmat("vars "+ vars) // the negation of a model -(S1=True/False /\ ... /\ SN=True/False) = clause(S1=False/True, ...., SN=False/True) def negateModel(m: Model) = clause(m.toSeq.map{ case (sym, pos) => Lit(sym, !pos) } : _*) - def findAllModels(f: Formula, models: List[Model], recursionDepthAllowed: Int = 10): List[Model]= - if (recursionDepthAllowed == 0) models - else { - debug.patmat("find all models for\n"+ cnfString(f)) + /** + * The DPLL procedure only returns a minimal mapping from literal to value + * such that the CNF formula is satisfied. + * E.g. for: + * `(a \/ b)` + * The DPLL procedure will find either {a = true} or {b = true} + * as solution. + * + * The expansion step will amend both solutions with the unassigned variable + * i.e., {a = true} will be expanded to {a = true, b = true} and {a = true, b = false}. + */ + def expandUnassigned(unassigned: List[Sym], model: Model): List[Model] = { + // the number of solutions is doubled for every unassigned variable + val expandedModels = 1 << unassigned.size + var current = mutable.ArrayBuffer[Model]() + var next = mutable.ArrayBuffer[Model]() + current.sizeHint(expandedModels) + next.sizeHint(expandedModels) + + current += model + + // we use double buffering: + // read from `current` and create a two models for each model in `next` + for { + s <- unassigned + } { + for { + model <- current + } { + def force(l: Lit) = model + (l.sym -> l.pos) + + next += force(Lit(s, pos = true)) + next += force(Lit(s, pos = false)) + } + + val tmp = current + current = next + next = tmp + + next.clear() + } + + current.toList + } + + def findAllModels(f: Formula, + models: List[Model], + recursionDepthAllowed: Int = global.settings.YpatmatExhaustdepth.value): List[Model]= + if (recursionDepthAllowed == 0) { + val maxDPLLdepth = global.settings.YpatmatExhaustdepth.value + reportWarning("(Exhaustivity analysis reached max recursion depth, not all missing cases are reported. " + + s"Please try with scalac -Ypatmat-exhaust-depth ${maxDPLLdepth * 2} or -Ypatmat-exhaust-depth off.)") + models + } else { val model = findModelFor(f) // if we found a solution, conjunct the formula with the model's negation and recurse if (model ne NoModel) { val unassigned = (vars -- model.keySet).toList debug.patmat("unassigned "+ unassigned +" in "+ model) - def force(lit: Lit) = { - val model = withLit(findModelFor(dropUnit(f, lit)), lit) - if (model ne NoModel) List(model) - else Nil - } - val forced = unassigned flatMap { s => - force(Lit(s, pos = true)) ++ force(Lit(s, pos = false)) - } + + val forced = expandUnassigned(unassigned, model) debug.patmat("forced "+ forced) val negated = negateModel(model) - findAllModels(f :+ negated, model :: (forced ++ models), recursionDepthAllowed - 1) + findAllModels(f :+ negated, forced ++ models, recursionDepthAllowed - 1) } else models } diff --git a/test/files/neg/patmatexhaust.check b/test/files/neg/patmatexhaust.check index 2dad608451..bbf5e9b528 100644 --- a/test/files/neg/patmatexhaust.check +++ b/test/files/neg/patmatexhaust.check @@ -12,7 +12,7 @@ It would fail on the following inputs: (Kult(_), Kult(_)), (Qult(), Qult()) ^ patmatexhaust.scala:49: warning: match may not be exhaustive. It would fail on the following inputs: Gp(), Gu - def ma4(x:Deep) = x match { // missing cases: Gu, Gp + def ma4(x:Deep) = x match { // missing cases: Gu, Gp which is not abstract so must be included ^ patmatexhaust.scala:55: warning: unreachable code case _ if 1 == 0 => diff --git a/test/files/neg/patmatexhaust.flags b/test/files/neg/patmatexhaust.flags index 85d8eb2ba2..3b01ca062c 100644 --- a/test/files/neg/patmatexhaust.flags +++ b/test/files/neg/patmatexhaust.flags @@ -1 +1 @@ --Xfatal-warnings +-Xfatal-warnings -Ypatmat-exhaust-depth off \ No newline at end of file diff --git a/test/files/neg/patmatexhaust.scala b/test/files/neg/patmatexhaust.scala index f937197829..26f0c12a91 100644 --- a/test/files/neg/patmatexhaust.scala +++ b/test/files/neg/patmatexhaust.scala @@ -46,7 +46,7 @@ class TestSealedExhaustive { // compile only case _ => } - def ma4(x:Deep) = x match { // missing cases: Gu, Gp + def ma4(x:Deep) = x match { // missing cases: Gu, Gp which is not abstract so must be included case Ga => } diff --git a/test/files/neg/t8430.check b/test/files/neg/t8430.check index 7c6a73ce53..dbc0c70bba 100644 --- a/test/files/neg/t8430.check +++ b/test/files/neg/t8430.check @@ -1,25 +1,25 @@ t8430.scala:15: warning: match may not be exhaustive. -It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP +It would fail on the following inputs: LetC, LetF, LetL(BooleanLit), LetL(IntLit), LetL(UnitLit), LetP (tree: Tree) => tree match {case LetL(CharLit) => ??? } ^ t8430.scala:16: warning: match may not be exhaustive. -It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP +It would fail on the following inputs: LetC, LetF, LetL(BooleanLit), LetL(IntLit), LetL(UnitLit), LetP (tree: Tree) => tree match {case LetL(CharLit) => ??? } ^ t8430.scala:17: warning: match may not be exhaustive. -It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP +It would fail on the following inputs: LetC, LetF, LetL(BooleanLit), LetL(IntLit), LetL(UnitLit), LetP (tree: Tree) => tree match {case LetL(CharLit) => ??? } ^ t8430.scala:18: warning: match may not be exhaustive. -It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP +It would fail on the following inputs: LetC, LetF, LetL(BooleanLit), LetL(IntLit), LetL(UnitLit), LetP (tree: Tree) => tree match {case LetL(CharLit) => ??? } ^ t8430.scala:19: warning: match may not be exhaustive. -It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP +It would fail on the following inputs: LetC, LetF, LetL(BooleanLit), LetL(IntLit), LetL(UnitLit), LetP (tree: Tree) => tree match {case LetL(CharLit) => ??? } ^ t8430.scala:20: warning: match may not be exhaustive. -It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP +It would fail on the following inputs: LetC, LetF, LetL(BooleanLit), LetL(IntLit), LetL(UnitLit), LetP (tree: Tree) => tree match {case LetL(CharLit) => ??? } ^ error: No warnings can be incurred under -Xfatal-warnings. diff --git a/test/files/neg/t8430.flags b/test/files/neg/t8430.flags index 85d8eb2ba2..6f60189a8d 100644 --- a/test/files/neg/t8430.flags +++ b/test/files/neg/t8430.flags @@ -1 +1 @@ --Xfatal-warnings +-Xfatal-warnings -Ypatmat-exhaust-depth off diff --git a/test/files/run/virtpatmat_nested_lists.flags b/test/files/run/virtpatmat_nested_lists.flags new file mode 100644 index 0000000000..ca9a4c0697 --- /dev/null +++ b/test/files/run/virtpatmat_nested_lists.flags @@ -0,0 +1 @@ +-Ypatmat-exhaust-depth off \ No newline at end of file diff --git a/test/files/run/virtpatmat_opt_sharing.flags b/test/files/run/virtpatmat_opt_sharing.flags new file mode 100644 index 0000000000..ca9a4c0697 --- /dev/null +++ b/test/files/run/virtpatmat_opt_sharing.flags @@ -0,0 +1 @@ +-Ypatmat-exhaust-depth off \ No newline at end of file -- cgit v1.2.3 From 964a197cd90e561d05c9d725cc13895f18b6a6d0 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 4 Oct 2014 12:33:11 -0700 Subject: SI-8843 AbsFileCL acts like a CL Let the AbstractFileClassLoader override just the usual suspects. Normal delegation behavior should ensue. That's instead of overriding `getResourceAsStream`, which was intended that "The repl classloader now works more like you'd expect a classloader to." (Workaround for "Don't know how to construct an URL for something which exists only in memory.") Also override `findResources` so that `getResources` does the obvious thing, namely, return one iff `getResource` does. The translating class loader for REPL only special-cases `foo.class`: as a fallback, take `foo` as `$line42.$read$something$foo` and try that class file. That's the use case for "works like you'd expect it to." There was a previous fix to ensure `getResource` doesn't take a class name. The convenience behavior, that `classBytes` takes either a class name or a resource path ending in ".class", has been promoted to `ScalaClassLoader`. --- .../internal/util/AbstractFileClassLoader.scala | 66 ++++------ .../reflect/internal/util/ScalaClassLoader.scala | 6 +- src/repl/scala/tools/nsc/interpreter/IMain.scala | 34 +++-- test/files/run/t8843-repl-xlat.scala | 33 +++++ .../scala/reflect/internal/PrintersTest.scala | 6 +- .../util/AbstractFileClassLoaderTest.scala | 138 +++++++++++++++++++++ 6 files changed, 230 insertions(+), 53 deletions(-) create mode 100644 test/files/run/t8843-repl-xlat.scala create mode 100644 test/junit/scala/reflect/internal/util/AbstractFileClassLoaderTest.scala (limited to 'test/files') diff --git a/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala b/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala index 10a8b4c812..30dcbc21ca 100644 --- a/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala +++ b/src/reflect/scala/reflect/internal/util/AbstractFileClassLoader.scala @@ -5,16 +5,16 @@ package scala package reflect.internal.util -import scala.reflect.io.AbstractFile +import scala.collection.{ mutable, immutable } +import scala.reflect.io.{ AbstractFile, Streamable } +import java.net.{ URL, URLConnection, URLStreamHandler } import java.security.cert.Certificate import java.security.{ ProtectionDomain, CodeSource } -import java.net.{ URL, URLConnection, URLStreamHandler } -import scala.collection.{ mutable, immutable } +import java.util.{ Collections => JCollections, Enumeration => JEnumeration } -/** - * A class loader that loads files from a {@link scala.tools.nsc.io.AbstractFile}. +/** A class loader that loads files from a {@link scala.tools.nsc.io.AbstractFile}. * - * @author Lex Spoon + * @author Lex Spoon */ class AbstractFileClassLoader(val root: AbstractFile, parent: ClassLoader) extends ClassLoader(parent) @@ -22,7 +22,7 @@ class AbstractFileClassLoader(val root: AbstractFile, parent: ClassLoader) { protected def classNameToPath(name: String): String = if (name endsWith ".class") name - else name.replace('.', '/') + ".class" + else s"${name.replace('.', '/')}.class" protected def findAbstractFile(name: String): AbstractFile = { var file: AbstractFile = root @@ -56,35 +56,25 @@ class AbstractFileClassLoader(val root: AbstractFile, parent: ClassLoader) file } - // parent delegation in JCL uses getResource; so either add parent.getResAsStream - // or implement findResource, which we do here as a study in scarlet (my complexion - // after looking at CLs and URLs) - override def findResource(name: String): URL = findAbstractFile(name) match { + override protected def findClass(name: String): Class[_] = { + val bytes = classBytes(name) + if (bytes.length == 0) + throw new ClassNotFoundException(name) + else + defineClass(name, bytes, 0, bytes.length, protectionDomain) + } + override protected def findResource(name: String): URL = findAbstractFile(name) match { case null => null - case file => new URL(null, "repldir:" + file.path, new URLStreamHandler { + case file => new URL(null, s"memory:${file.path}", new URLStreamHandler { override def openConnection(url: URL): URLConnection = new URLConnection(url) { - override def connect() { } + override def connect() = () override def getInputStream = file.input } }) } - - // this inverts delegation order: super.getResAsStr calls parent.getRes if we fail - override def getResourceAsStream(name: String) = findAbstractFile(name) match { - case null => super.getResourceAsStream(name) - case file => file.input - } - // ScalaClassLoader.classBytes uses getResAsStream, so we'll try again before delegating - override def classBytes(name: String): Array[Byte] = findAbstractFile(classNameToPath(name)) match { - case null => super.classBytes(name) - case file => file.toByteArray - } - override def findClass(name: String): Class[_] = { - val bytes = classBytes(name) - if (bytes.length == 0) - throw new ClassNotFoundException(name) - else - defineClass(name, bytes, 0, bytes.length, protectionDomain) + override protected def findResources(name: String): JEnumeration[URL] = findResource(name) match { + case null => JCollections.enumeration(JCollections.emptyList[URL]) //JCollections.emptyEnumeration[URL] + case url => JCollections.enumeration(JCollections.singleton(url)) } lazy val protectionDomain = { @@ -106,15 +96,13 @@ class AbstractFileClassLoader(val root: AbstractFile, parent: ClassLoader) throw new UnsupportedOperationException() } - override def getPackage(name: String): Package = { - findAbstractDir(name) match { - case null => super.getPackage(name) - case file => packages.getOrElseUpdate(name, { - val ctor = classOf[Package].getDeclaredConstructor(classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[URL], classOf[ClassLoader]) - ctor.setAccessible(true) - ctor.newInstance(name, null, null, null, null, null, null, null, this) - }) - } + override def getPackage(name: String): Package = findAbstractDir(name) match { + case null => super.getPackage(name) + case file => packages.getOrElseUpdate(name, { + val ctor = classOf[Package].getDeclaredConstructor(classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[URL], classOf[ClassLoader]) + ctor.setAccessible(true) + ctor.newInstance(name, null, null, null, null, null, null, null, this) + }) } override def getPackages(): Array[Package] = diff --git a/src/reflect/scala/reflect/internal/util/ScalaClassLoader.scala b/src/reflect/scala/reflect/internal/util/ScalaClassLoader.scala index 63ea6e2c49..41011f6c6b 100644 --- a/src/reflect/scala/reflect/internal/util/ScalaClassLoader.scala +++ b/src/reflect/scala/reflect/internal/util/ScalaClassLoader.scala @@ -53,8 +53,10 @@ trait ScalaClassLoader extends JClassLoader { } /** An InputStream representing the given class name, or null if not found. */ - def classAsStream(className: String) = - getResourceAsStream(className.replaceAll("""\.""", "/") + ".class") + def classAsStream(className: String) = getResourceAsStream { + if (className endsWith ".class") className + else s"${className.replace('.', '/')}.class" // classNameToPath + } /** Run the main method of a class to be loaded by this classloader */ def run(objectName: String, arguments: Seq[String]) { diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala index 6e30b73e0e..20b5a79aaa 100644 --- a/src/repl/scala/tools/nsc/interpreter/IMain.scala +++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala @@ -295,22 +295,38 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set def originalPath(name: Name): String = typerOp path name def originalPath(sym: Symbol): String = typerOp path sym def flatPath(sym: Symbol): String = flatOp shift sym.javaClassName + def translatePath(path: String) = { val sym = if (path endsWith "$") symbolOfTerm(path.init) else symbolOfIdent(path) sym.toOption map flatPath } + + /** If path represents a class resource in the default package, + * see if the corresponding symbol has a class file that is a REPL artifact + * residing at a different resource path. Translate X.class to $line3/$read$$iw$$iw$X.class. + */ + def translateSimpleResource(path: String): Option[String] = { + if (!(path contains '/') && (path endsWith ".class")) { + val name = path stripSuffix ".class" + val sym = if (name endsWith "$") symbolOfTerm(name.init) else symbolOfIdent(name) + def pathOf(s: String) = s"${s.replace('.', '/')}.class" + sym.toOption map (s => pathOf(flatPath(s))) + } else { + None + } + } def translateEnclosingClass(n: String) = symbolOfTerm(n).enclClass.toOption map flatPath + /** If unable to find a resource foo.class, try taking foo as a symbol in scope + * and use its java class name as a resource to load. + * + * $intp.classLoader classBytes "Bippy" or $intp.classLoader getResource "Bippy.class" just work. + */ private class TranslatingClassLoader(parent: ClassLoader) extends util.AbstractFileClassLoader(replOutput.dir, parent) { - /** Overridden here to try translating a simple name to the generated - * class name if the original attempt fails. This method is used by - * getResourceAsStream as well as findClass. - */ - override protected def findAbstractFile(name: String): AbstractFile = - super.findAbstractFile(name) match { - case null if _initializeComplete => translatePath(name) map (super.findAbstractFile(_)) orNull - case file => file - } + override protected def findAbstractFile(name: String): AbstractFile = super.findAbstractFile(name) match { + case null if _initializeComplete => translateSimpleResource(name) map super.findAbstractFile orNull + case file => file + } } private def makeClassLoader(): util.AbstractFileClassLoader = new TranslatingClassLoader(parentClassLoader match { diff --git a/test/files/run/t8843-repl-xlat.scala b/test/files/run/t8843-repl-xlat.scala new file mode 100644 index 0000000000..6426dbe7d4 --- /dev/null +++ b/test/files/run/t8843-repl-xlat.scala @@ -0,0 +1,33 @@ + +import scala.tools.partest.SessionTest + +// Handy hamburger helper for repl resources +object Test extends SessionTest { + def session = +"""Type in expressions to have them evaluated. +Type :help for more information. + +scala> $intp.isettings.unwrapStrings = false +$intp.isettings.unwrapStrings: Boolean = false + +scala> class Bippy +defined class Bippy + +scala> $intp.classLoader getResource "Bippy.class" +res0: java.net.URL = memory:(memory)/$line4/$read$$iw$$iw$Bippy.class + +scala> ($intp.classLoader getResources "Bippy.class").nextElement +res1: java.net.URL = memory:(memory)/$line4/$read$$iw$$iw$Bippy.class + +scala> ($intp.classLoader classBytes "Bippy").nonEmpty +res2: Boolean = true + +scala> ($intp.classLoader classAsStream "Bippy") != null +res3: Boolean = true + +scala> $intp.classLoader getResource "Bippy" +res4: java.net.URL = null + +scala> :quit""" +} + diff --git a/test/junit/scala/reflect/internal/PrintersTest.scala b/test/junit/scala/reflect/internal/PrintersTest.scala index 1458b942dc..ca9b4671b2 100644 --- a/test/junit/scala/reflect/internal/PrintersTest.scala +++ b/test/junit/scala/reflect/internal/PrintersTest.scala @@ -24,10 +24,10 @@ object PrinterHelper { resultCode.lines mkString s"$LF" def assertResultCode(code: String)(parsedCode: String = "", typedCode: String = "", wrap: Boolean = false, printRoot: Boolean = false) = { - def toolboxTree(tree: => Tree) = try{ + def toolboxTree(tree: => Tree) = try { tree } catch { - case e:scala.tools.reflect.ToolBoxError => throw new Exception(e.getMessage + ": " + code) + case e:scala.tools.reflect.ToolBoxError => throw new Exception(e.getMessage + ": " + code, e) } def wrapCode(source: String) = { @@ -1186,4 +1186,4 @@ trait QuasiTreesPrintTests { | }; | () |}""") -} \ No newline at end of file +} diff --git a/test/junit/scala/reflect/internal/util/AbstractFileClassLoaderTest.scala b/test/junit/scala/reflect/internal/util/AbstractFileClassLoaderTest.scala new file mode 100644 index 0000000000..a2537ddab7 --- /dev/null +++ b/test/junit/scala/reflect/internal/util/AbstractFileClassLoaderTest.scala @@ -0,0 +1,138 @@ +package scala.reflect.internal.util + +import org.junit.Assert._ +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(classOf[JUnit4]) +class AbstractFileClassLoaderTest { + + import scala.reflect.io._ + import scala.io.Source + import scala.io.Codec.UTF8 + import scala.reflect.io.Streamable + import java.net.{ URLClassLoader, URL } + + implicit def `we love utf8` = UTF8 + implicit class `abs file ops`(f: AbstractFile) { + def writeContent(s: String): Unit = Streamable.closing(f.bufferedOutput)(os => os write s.getBytes(UTF8.charSet)) + } + implicit class `url slurp`(url: URL) { + def slurp(): String = Streamable.slurp(url) + } + + val NoClassLoader: ClassLoader = null + + def fuzzBuzzBooz: (AbstractFile, AbstractFile) = { + val fuzz = new VirtualDirectory("fuzz", None) + val buzz = fuzz subdirectoryNamed "buzz" + val booz = buzz fileNamed "booz.class" + (fuzz, booz) + } + + @Test + def afclGetsParent(): Unit = { + val p = new URLClassLoader(Array.empty[URL]) + val d = new VirtualDirectory("vd", None) + val x = new AbstractFileClassLoader(d, p) + assertSame(p, x.getParent) + } + + @Test + def afclGetsResource(): Unit = { + val (fuzz, booz) = fuzzBuzzBooz + booz writeContent "hello, world" + val x = new AbstractFileClassLoader(fuzz, NoClassLoader) + val r = x.getResource("buzz/booz.class") + assertNotNull(r) + assertEquals("hello, world", r.slurp()) + } + + @Test + def afclGetsResourceFromParent(): Unit = { + val (fuzz, booz) = fuzzBuzzBooz + val (fuzz_, booz_) = fuzzBuzzBooz + booz writeContent "hello, world" + booz_ writeContent "hello, world_" + val p = new AbstractFileClassLoader(fuzz, NoClassLoader) + val x = new AbstractFileClassLoader(fuzz_, p) + val r = x.getResource("buzz/booz.class") + assertNotNull(r) + assertEquals("hello, world", r.slurp()) + } + + @Test + def afclGetsResourceInDefaultPackage(): Unit = { + val fuzz = new VirtualDirectory("fuzz", None) + val booz = fuzz fileNamed "booz.class" + val bass = fuzz fileNamed "bass" + booz writeContent "hello, world" + bass writeContent "lo tone" + val x = new AbstractFileClassLoader(fuzz, NoClassLoader) + val r = x.getResource("booz.class") + assertNotNull(r) + assertEquals("hello, world", r.slurp()) + assertEquals("lo tone", (x getResource "bass").slurp()) + } + + // SI-8843 + @Test + def afclGetsResources(): Unit = { + val (fuzz, booz) = fuzzBuzzBooz + booz writeContent "hello, world" + val x = new AbstractFileClassLoader(fuzz, NoClassLoader) + val e = x.getResources("buzz/booz.class") + assertTrue(e.hasMoreElements) + assertEquals("hello, world", e.nextElement.slurp()) + assertFalse(e.hasMoreElements) + } + + @Test + def afclGetsResourcesFromParent(): Unit = { + val (fuzz, booz) = fuzzBuzzBooz + val (fuzz_, booz_) = fuzzBuzzBooz + booz writeContent "hello, world" + booz_ writeContent "hello, world_" + val p = new AbstractFileClassLoader(fuzz, NoClassLoader) + val x = new AbstractFileClassLoader(fuzz_, p) + val e = x.getResources("buzz/booz.class") + assertTrue(e.hasMoreElements) + assertEquals("hello, world", e.nextElement.slurp()) + assertTrue(e.hasMoreElements) + assertEquals("hello, world_", e.nextElement.slurp()) + assertFalse(e.hasMoreElements) + } + + @Test + def afclGetsResourceAsStream(): Unit = { + val (fuzz, booz) = fuzzBuzzBooz + booz writeContent "hello, world" + val x = new AbstractFileClassLoader(fuzz, NoClassLoader) + val r = x.getResourceAsStream("buzz/booz.class") + assertNotNull(r) + assertEquals("hello, world", Streamable.closing(r)(is => Source.fromInputStream(is).mkString)) + } + + @Test + def afclGetsClassBytes(): Unit = { + val (fuzz, booz) = fuzzBuzzBooz + booz writeContent "hello, world" + val x = new AbstractFileClassLoader(fuzz, NoClassLoader) + val b = x.classBytes("buzz/booz.class") + assertEquals("hello, world", new String(b, UTF8.charSet)) + } + + @Test + def afclGetsClassBytesFromParent(): Unit = { + val (fuzz, booz) = fuzzBuzzBooz + val (fuzz_, booz_) = fuzzBuzzBooz + booz writeContent "hello, world" + booz_ writeContent "hello, world_" + + val p = new AbstractFileClassLoader(fuzz, NoClassLoader) + val x = new AbstractFileClassLoader(fuzz_, p) + val b = x.classBytes("buzz/booz.class") + assertEquals("hello, world", new String(b, UTF8.charSet)) + } +} -- cgit v1.2.3 From 0c25979244877b4431066700a6e945f145771c3c Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 29 Sep 2014 17:52:40 +0200 Subject: SI-8731 warning if @switch is ignored For matches with two or fewer cases, @switch is ignored. This should not happen silently. --- .../nsc/transform/patmat/MatchTreeMaking.scala | 18 +++++++++- test/files/neg/t6902.scala | 2 +- test/files/neg/t8731.check | 9 +++++ test/files/neg/t8731.flags | 1 + test/files/neg/t8731.scala | 15 +++++++++ test/files/pos/switch-small.flags | 1 - test/files/pos/switch-small.scala | 8 ----- test/files/run/t5830.check | 1 + test/files/run/t5830.scala | 13 ++++---- test/files/run/t6011c.scala | 2 +- test/junit/scala/issues/BytecodeTests.scala | 39 ++++++++++++++++++++++ .../scala/tools/nsc/backend/jvm/CodeGenTools.scala | 3 ++ 12 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 test/files/neg/t8731.check create mode 100644 test/files/neg/t8731.flags create mode 100644 test/files/neg/t8731.scala delete mode 100644 test/files/pos/switch-small.flags delete mode 100644 test/files/pos/switch-small.scala create mode 100644 test/junit/scala/issues/BytecodeTests.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala index 1974befb45..3abec521df 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala @@ -550,8 +550,24 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging { case _ => false } val suppression = Suppression(suppressExhaustive, supressUnreachable) + val hasSwitchAnnotation = treeInfo.isSwitchAnnotation(tpt.tpe) // matches with two or fewer cases need not apply for switchiness (if-then-else will do) - val requireSwitch = treeInfo.isSwitchAnnotation(tpt.tpe) && casesNoSubstOnly.lengthCompare(2) > 0 + // `case 1 | 2` is considered as two cases. + def exceedsTwoCasesOrAlts = { + // avoids traversing the entire list if there are more than 3 elements + def lengthMax3[T](l: List[T]): Int = l match { + case a :: b :: c :: _ => 3 + case cases => + cases.map({ + case AlternativesTreeMaker(_, alts, _) :: _ => lengthMax3(alts) + case c => 1 + }).sum + } + lengthMax3(casesNoSubstOnly) > 2 + } + val requireSwitch = hasSwitchAnnotation && exceedsTwoCasesOrAlts + if (hasSwitchAnnotation && !requireSwitch) + reporter.warning(scrut.pos, "matches with two cases or fewer are emitted using if-then-else instead of switch") (suppression, requireSwitch) case _ => (Suppression.NoSuppression, false) diff --git a/test/files/neg/t6902.scala b/test/files/neg/t6902.scala index ce5ff8b6fb..627c324279 100644 --- a/test/files/neg/t6902.scala +++ b/test/files/neg/t6902.scala @@ -16,7 +16,7 @@ object Test { // 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 { + ((1: Byte): @unchecked) match { case 1 => 2 case 1 => 3 // crash } diff --git a/test/files/neg/t8731.check b/test/files/neg/t8731.check new file mode 100644 index 0000000000..2a9af475fc --- /dev/null +++ b/test/files/neg/t8731.check @@ -0,0 +1,9 @@ +t8731.scala:5: warning: matches with two cases or fewer are emitted using if-then-else instead of switch + def f(x: Int) = (x: @annotation.switch) match { + ^ +t8731.scala:10: warning: could not emit switch for @switch annotated match + def g(x: Int) = (x: @annotation.switch) match { + ^ +error: No warnings can be incurred under -Xfatal-warnings. +two warnings found +one error found diff --git a/test/files/neg/t8731.flags b/test/files/neg/t8731.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/neg/t8731.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/neg/t8731.scala b/test/files/neg/t8731.scala new file mode 100644 index 0000000000..d93fe706ad --- /dev/null +++ b/test/files/neg/t8731.scala @@ -0,0 +1,15 @@ +class C { + // not a compile-time constant due to return type + final val K: Int = 20 + + def f(x: Int) = (x: @annotation.switch) match { + case K => 0 + case 2 => 1 + } + + def g(x: Int) = (x: @annotation.switch) match { + case K => 0 + case 2 => 1 + case 3 => 2 + } +} diff --git a/test/files/pos/switch-small.flags b/test/files/pos/switch-small.flags deleted file mode 100644 index 85d8eb2ba2..0000000000 --- a/test/files/pos/switch-small.flags +++ /dev/null @@ -1 +0,0 @@ --Xfatal-warnings diff --git a/test/files/pos/switch-small.scala b/test/files/pos/switch-small.scala deleted file mode 100644 index 9de9ca028e..0000000000 --- a/test/files/pos/switch-small.scala +++ /dev/null @@ -1,8 +0,0 @@ -import annotation._ - -object Test { - def f(x: Int) = (x: @switch) match { - case 1 => 1 - case _ => 2 - } -} diff --git a/test/files/run/t5830.check b/test/files/run/t5830.check index 675387eb8e..9260854676 100644 --- a/test/files/run/t5830.check +++ b/test/files/run/t5830.check @@ -1,5 +1,6 @@ a with oef a with oef +a with oef a def with oef def diff --git a/test/files/run/t5830.scala b/test/files/run/t5830.scala index 5d808bfa28..03b9c540e0 100644 --- a/test/files/run/t5830.scala +++ b/test/files/run/t5830.scala @@ -1,12 +1,11 @@ import scala.annotation.switch object Test extends App { - // TODO: should not emit a switch - // def noSwitch(ch: Char, eof: Boolean) = (ch: @switch) match { - // case 'a' if eof => println("a with oef") // then branch - // } + def noSwitch(ch: Char, eof: Boolean) = ch match { + case 'a' if eof => println("a with oef") // then branch + } - def onlyThen(ch: Char, eof: Boolean) = (ch: @switch) match { + def onlyThen(ch: Char, eof: Boolean) = ch match { case 'a' if eof => println("a with oef") // then branch case 'c' => } @@ -18,7 +17,7 @@ object Test extends App { case 'c' => } - def defaultUnguarded(ch: Char, eof: Boolean) = (ch: @switch) match { + def defaultUnguarded(ch: Char, eof: Boolean) = ch match { case ' ' if eof => println("spacey oef") case _ => println("default") } @@ -44,7 +43,7 @@ object Test extends App { // case 'c' => // } - // noSwitch('a', true) + noSwitch('a', true) onlyThen('a', true) // 'a with oef' ifThenElse('a', true) // 'a with oef' ifThenElse('a', false) // 'a' diff --git a/test/files/run/t6011c.scala b/test/files/run/t6011c.scala index 0647e3f81a..96a685b9cf 100644 --- a/test/files/run/t6011c.scala +++ b/test/files/run/t6011c.scala @@ -6,7 +6,7 @@ object Test extends App { // 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 { + ((1: Byte): @unchecked) match { case 1 => 2 case 1 => 3 // crash } diff --git a/test/junit/scala/issues/BytecodeTests.scala b/test/junit/scala/issues/BytecodeTests.scala new file mode 100644 index 0000000000..7a05472277 --- /dev/null +++ b/test/junit/scala/issues/BytecodeTests.scala @@ -0,0 +1,39 @@ +package scala.issues + +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.junit.Test +import scala.tools.asm.Opcodes +import scala.tools.nsc.backend.jvm.CodeGenTools._ +import org.junit.Assert._ +import scala.collection.JavaConverters._ +import scala.tools.partest.ASMConverters._ + +@RunWith(classOf[JUnit4]) +class BytecodeTests { + val compiler = newCompiler() + + @Test + def t8731(): Unit = { + val code = + """class C { + | def f(x: Int) = (x: @annotation.switch) match { + | case 1 => 0 + | case 2 => 1 + | case 3 => 2 + | } + | final val K = 10 + | def g(x: Int) = (x: @annotation.switch) match { + | case K => 0 + | case 1 => 10 + | case 2 => 20 + | } + |} + """.stripMargin + + val List(c) = compileClasses(compiler)(code) + + assertTrue(getSingleMethod(c, "f").instructions.count(_.isInstanceOf[TableSwitch]) == 1) + assertTrue(getSingleMethod(c, "g").instructions.count(_.isInstanceOf[LookupSwitch]) == 1) + } +} diff --git a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala index 15bc1f427d..b892eb36cf 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala @@ -76,4 +76,7 @@ object CodeGenTools { def assertSameCode(actual: List[Instruction], expected: List[Instruction]): Unit = { assertTrue(s"\nExpected: $expected\nActual : $actual", actual === expected) } + + def getSingleMethod(classNode: ClassNode, name: String): Method = + convertMethod(classNode.methods.asScala.toList.find(_.name == name).get) } -- cgit v1.2.3 From 1ee6352a79d0b9001f4f1249708e7c6350b241ae Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Tue, 7 Oct 2014 15:09:28 +1000 Subject: SI-8888 Avoid ClassFormatError under -Ydelambdafy:method The pattern matcher phase (conceivably, among others) can generate code that binds local `Ident`s symbolically, rather than according to the lexical scope. This means that a lambda can capture more than one local of the same name. In the enclosed test case, this ends up creating the following tree after delambdafy [[syntax trees at end of delambdafy]] // delambday-patmat-path-dep.scala matchEnd4({ case val x1: Object = (x2: Object); case5(){ if (x1.$isInstanceOf[C]()) { val x2#19598: C = (x1.$asInstanceOf[C](): C); matchEnd4({ { (new resume$1(x2#19598, x2#19639): runtime.AbstractFunction0) }; scala.runtime.BoxedUnit.UNIT }) } else case6() }; ... }) ... class resume$1 extends AbstractFunction0 { var x2: C = null; var x2: C = null; ... } After this commit, the var members of `resume$1` are given fresh names, rather than directly using the name of the captured var: var x2$3: C = null; var x2$4: C = null; --- src/compiler/scala/tools/nsc/transform/Delambdafy.scala | 2 +- test/files/run/t8888.flags | 1 + test/files/run/t8888.scala | 12 ++++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t8888.flags create mode 100644 test/files/run/t8888.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala index 12e7b23f48..835d338ab3 100644 --- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala +++ b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala @@ -260,7 +260,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre val captureProxies2 = new LinkedHashMap[Symbol, TermSymbol] captures foreach {capture => - val sym = lambdaClass.newVariable(capture.name.toTermName, capture.pos, SYNTHETIC) + val sym = lambdaClass.newVariable(unit.freshTermName(capture.name.toString + "$"), capture.pos, SYNTHETIC) sym setInfo capture.info captureProxies2 += ((capture, sym)) } diff --git a/test/files/run/t8888.flags b/test/files/run/t8888.flags new file mode 100644 index 0000000000..48b438ddf8 --- /dev/null +++ b/test/files/run/t8888.flags @@ -0,0 +1 @@ +-Ydelambdafy:method diff --git a/test/files/run/t8888.scala b/test/files/run/t8888.scala new file mode 100644 index 0000000000..36cc1ddf3e --- /dev/null +++ b/test/files/run/t8888.scala @@ -0,0 +1,12 @@ +class C { + final def resume: Unit = (this: Any) match { + case x : C => (x: Any) match { + case y : C => + () => (x, y) // used to trigger a ClassFormatError under -Ydelambdafy:method + } + } +} + +object Test extends App { + new C().resume +} -- cgit v1.2.3 From c14e0532fcd6d68c43a3c974efec9d15b6e4b217 Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Mon, 3 Feb 2014 21:44:59 +0100 Subject: SI-4788/SI-5948 Respect RetentionPolicy of Java annotations Note that I removed the check to ignore @deprecated: - @deprecated extends StaticAnnotation, so they aren't supposed to show up in the RuntimeInvisibleAnnotation attribute anyway, and the earlier check for "extends ClassfileAnnotationClass" makes this check superflous anyway. - Otherwise, if @deprecated was extending ClassfileAnnotationClass it would seem inconsistent that we don't emit @deprecated, but would do so for @deprecatedOverriding, @deprecatedInheritance, etc. Anyway, due to ClassfileAnnotation not working in Scala, and the additional check which only allows Java-defined annotations, this is pretty pointless from every perspective. --- .../tools/nsc/backend/jvm/BCodeAsmCommon.scala | 32 +++++++++ .../scala/tools/nsc/backend/jvm/BCodeHelpers.scala | 29 ++------ .../scala/tools/nsc/backend/jvm/GenASM.scala | 76 ++++++++++++++++---- .../scala/tools/nsc/backend/jvm/GenJVMASM.scala | 83 ---------------------- .../scala/reflect/internal/Definitions.scala | 4 ++ .../scala/reflect/runtime/JavaUniverseForce.scala | 2 + test/files/run/t4788-separate-compilation.check | 5 ++ .../t4788-separate-compilation/CAnnotation_1.java | 5 ++ .../files/run/t4788-separate-compilation/C_1.scala | 2 + .../files/run/t4788-separate-compilation/D_1.scala | 5 ++ .../t4788-separate-compilation/RAnnotation_1.java | 5 ++ .../files/run/t4788-separate-compilation/R_1.scala | 2 + .../t4788-separate-compilation/SAnnotation_1.java | 5 ++ .../files/run/t4788-separate-compilation/S_1.scala | 2 + .../run/t4788-separate-compilation/Test_2.scala | 35 +++++++++ test/files/run/t4788.check | 5 ++ test/files/run/t4788/C.scala | 2 + test/files/run/t4788/CAnnotation.java | 5 ++ test/files/run/t4788/D.scala | 5 ++ test/files/run/t4788/R.scala | 2 + test/files/run/t4788/RAnnotation.java | 5 ++ test/files/run/t4788/S.scala | 2 + test/files/run/t4788/SAnnotation.java | 5 ++ test/files/run/t4788/Test.scala | 35 +++++++++ 24 files changed, 238 insertions(+), 120 deletions(-) delete mode 100644 src/compiler/scala/tools/nsc/backend/jvm/GenJVMASM.scala create mode 100644 test/files/run/t4788-separate-compilation.check create mode 100644 test/files/run/t4788-separate-compilation/CAnnotation_1.java create mode 100644 test/files/run/t4788-separate-compilation/C_1.scala create mode 100644 test/files/run/t4788-separate-compilation/D_1.scala create mode 100644 test/files/run/t4788-separate-compilation/RAnnotation_1.java create mode 100644 test/files/run/t4788-separate-compilation/R_1.scala create mode 100644 test/files/run/t4788-separate-compilation/SAnnotation_1.java create mode 100644 test/files/run/t4788-separate-compilation/S_1.scala create mode 100644 test/files/run/t4788-separate-compilation/Test_2.scala create mode 100644 test/files/run/t4788.check create mode 100644 test/files/run/t4788/C.scala create mode 100644 test/files/run/t4788/CAnnotation.java create mode 100644 test/files/run/t4788/D.scala create mode 100644 test/files/run/t4788/R.scala create mode 100644 test/files/run/t4788/RAnnotation.java create mode 100644 test/files/run/t4788/S.scala create mode 100644 test/files/run/t4788/SAnnotation.java create mode 100644 test/files/run/t4788/Test.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala index 0c0d726630..4285858bf8 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala @@ -14,6 +14,13 @@ import PartialFunction._ */ final class BCodeAsmCommon[G <: Global](val global: G) { import global._ + import definitions._ + + val ExcludedForwarderFlags = { + import scala.tools.nsc.symtab.Flags._ + // Should include DEFERRED but this breaks findMember. + ( SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | BridgeAndPrivateFlags | MACRO ) + } /** * True if `classSym` is an anonymous class or a local class. I.e., false if `classSym` is a @@ -124,4 +131,29 @@ final class BCodeAsmCommon[G <: Global](val global: G) { assert(r != NoSymbol, sym.fullLocationString) r })(collection.breakOut) + + lazy val AnnotationRetentionPolicyModule = AnnotationRetentionPolicyAttr.companionModule + lazy val AnnotationRetentionPolicySourceValue = AnnotationRetentionPolicyModule.tpe.member(TermName("SOURCE")) + lazy val AnnotationRetentionPolicyClassValue = AnnotationRetentionPolicyModule.tpe.member(TermName("CLASS")) + lazy val AnnotationRetentionPolicyRuntimeValue = AnnotationRetentionPolicyModule.tpe.member(TermName("RUNTIME")) + + /** Whether an annotation should be emitted as a Java annotation + * .initialize: if 'annot' is read from pickle, atp might be un-initialized + */ + def shouldEmitAnnotation(annot: AnnotationInfo) = { + annot.symbol.initialize.isJavaDefined && + annot.matches(ClassfileAnnotationClass) && + retentionPolicyOf(annot) != AnnotationRetentionPolicySourceValue && + annot.args.isEmpty + } + + def isRuntimeVisible(annot: AnnotationInfo): Boolean = + annot.atp.typeSymbol.getAnnotation(AnnotationRetentionAttr) + .exists(_.assocs.contains((nme.value -> LiteralAnnotArg(Constant(AnnotationRetentionPolicyRuntimeValue))))) + + private def retentionPolicyOf(annot: AnnotationInfo): Symbol = + annot.atp.typeSymbol.getAnnotation(AnnotationRetentionAttr).map(_.assocs).map(assoc => + assoc.collectFirst { + case (`nme`.value, LiteralAnnotArg(Constant(value: Symbol))) => value + }).flatten.getOrElse(AnnotationRetentionPolicyClassValue) } diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala index 14bffd67ab..806d4b277c 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala @@ -469,6 +469,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { trait BCAnnotGen extends BCInnerClassGen { import genASM.{ubytesToCharArray, arrEncode} + import bCodeAsmCommon.{shouldEmitAnnotation, isRuntimeVisible} /* * can-multi-thread @@ -533,17 +534,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { } } - /* Whether an annotation should be emitted as a Java annotation - * .initialize: if 'annot' is read from pickle, atp might be un-initialized - * - * must-single-thread - */ - private def shouldEmitAnnotation(annot: AnnotationInfo) = - annot.symbol.initialize.isJavaDefined && - annot.matches(definitions.ClassfileAnnotationClass) && - annot.args.isEmpty && - !annot.matches(definitions.DeprecatedAttr) - /* * In general, * must-single-thread @@ -563,7 +553,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { for(annot <- annotations; if shouldEmitAnnotation(annot)) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val av = cw.visitAnnotation(descriptor(typ), true) + val av = cw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot)) emitAssocs(av, assocs) } } @@ -575,7 +565,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { for(annot <- annotations; if shouldEmitAnnotation(annot)) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val av = mw.visitAnnotation(descriptor(typ), true) + val av = mw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot)) emitAssocs(av, assocs) } } @@ -587,7 +577,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { for(annot <- annotations; if shouldEmitAnnotation(annot)) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val av = fw.visitAnnotation(descriptor(typ), true) + val av = fw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot)) emitAssocs(av, assocs) } } @@ -602,7 +592,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { annot <- annots) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), true) + val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), isRuntimeVisible(annot)) emitAssocs(pannVisitor, assocs) } } @@ -625,13 +615,6 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { trait BCForwardersGen extends BCAnnotGen with BCJGenSigGen { - // ----------------------------------------------------------------------------------------- - // Static forwarders (related to mirror classes but also present in - // a plain class lacking companion module, for details see `isCandidateForForwarders`). - // ----------------------------------------------------------------------------------------- - - val ExcludedForwarderFlags = genASM.ExcludedForwarderFlags - /* Adds a @remote annotation, actual use unknown. * * Invoked from genMethod() and addForwarder(). @@ -727,7 +710,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { } debuglog(s"Potentially conflicting names for forwarders: $conflictingNames") - for (m <- moduleClass.info.membersBasedOnFlags(ExcludedForwarderFlags, symtab.Flags.METHOD)) { + for (m <- moduleClass.info.membersBasedOnFlags(bCodeAsmCommon.ExcludedForwarderFlags, symtab.Flags.METHOD)) { if (m.isType || m.isDeferred || (m.owner eq definitions.ObjectClass) || m.isConstructor) debuglog(s"No forwarder for '$m' from $jclassName to '$moduleClass'") else if (conflictingNames(m.name)) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 7c4c02c2d3..d0a12c32e5 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -20,7 +20,7 @@ import scala.annotation.tailrec * * Documentation at http://lamp.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/2012Q2/GenASM.pdf */ -abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { self => +abstract class GenASM extends SubComponent with BytecodeWriters { self => import global._ import icodes._ import icodes.opcodes._ @@ -99,6 +99,63 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { } } + private def isJavaEntryPoint(icls: IClass) = { + val sym = icls.symbol + def fail(msg: String, pos: Position = sym.pos) = { + reporter.warning(sym.pos, + sym.name + " has a main method with parameter type Array[String], but " + sym.fullName('.') + " will not be a runnable program.\n" + + " Reason: " + msg + // TODO: make this next claim true, if possible + // by generating valid main methods as static in module classes + // not sure what the jvm allows here + // + " You can still run the program by calling it as " + sym.javaSimpleName + " instead." + ) + false + } + def failNoForwarder(msg: String) = { + fail(msg + ", which means no static forwarder can be generated.\n") + } + val possibles = if (sym.hasModuleFlag) (sym.tpe nonPrivateMember nme.main).alternatives else Nil + val hasApproximate = possibles exists { m => + m.info match { + case MethodType(p :: Nil, _) => p.tpe.typeSymbol == ArrayClass + case _ => false + } + } + // At this point it's a module with a main-looking method, so either succeed or warn that it isn't. + hasApproximate && { + // Before erasure so we can identify generic mains. + enteringErasure { + val companion = sym.linkedClassOfClass + + if (hasJavaMainMethod(companion)) + failNoForwarder("companion contains its own main method") + else if (companion.tpe.member(nme.main) != NoSymbol) + // this is only because forwarders aren't smart enough yet + failNoForwarder("companion contains its own main method (implementation restriction: no main is allowed, regardless of signature)") + else if (companion.isTrait) + failNoForwarder("companion is a trait") + // Now either succeeed, or issue some additional warnings for things which look like + // attempts to be java main methods. + else (possibles exists isJavaMainMethod) || { + possibles exists { m => + m.info match { + case PolyType(_, _) => + fail("main methods cannot be generic.") + case MethodType(params, res) => + if (res.typeSymbol :: params exists (_.isAbstractType)) + fail("main methods cannot refer to type parameters or abstract types.", m.pos) + else + isJavaMainMethod(m) || fail("main method must have exact signature (Array[String])Unit", m.pos) + case tp => + fail("don't know what this is: " + tp, m.pos) + } + } + } + } + } + } + override def run() { if (settings.debug) @@ -807,15 +864,6 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { for (ThrownException(exc) <- excs.distinct) yield javaName(exc) - /** Whether an annotation should be emitted as a Java annotation - * .initialize: if 'annot' is read from pickle, atp might be un-initialized - */ - private def shouldEmitAnnotation(annot: AnnotationInfo) = - annot.symbol.initialize.isJavaDefined && - annot.matches(ClassfileAnnotationClass) && - annot.args.isEmpty && - !annot.matches(DeprecatedAttr) - def getCurrentCUnit(): CompilationUnit def getGenericSignature(sym: Symbol, owner: Symbol) = self.getGenericSignature(sym, owner, getCurrentCUnit()) @@ -877,7 +925,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { for(annot <- annotations; if shouldEmitAnnotation(annot)) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val av = cw.visitAnnotation(descriptor(typ), true) + val av = cw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot)) emitAssocs(av, assocs) } } @@ -886,7 +934,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { for(annot <- annotations; if shouldEmitAnnotation(annot)) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val av = mw.visitAnnotation(descriptor(typ), true) + val av = mw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot)) emitAssocs(av, assocs) } } @@ -895,7 +943,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { for(annot <- annotations; if shouldEmitAnnotation(annot)) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val av = fw.visitAnnotation(descriptor(typ), true) + val av = fw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot)) emitAssocs(av, assocs) } } @@ -907,7 +955,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { annot <- annots) { val AnnotationInfo(typ, args, assocs) = annot assert(args.isEmpty, args) - val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), true) + val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), isRuntimeVisible(annot)) emitAssocs(pannVisitor, assocs) } } diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVMASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVMASM.scala deleted file mode 100644 index 2bcde7f7b9..0000000000 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVMASM.scala +++ /dev/null @@ -1,83 +0,0 @@ -/* NSC -- new Scala compiler - * Copyright 2005-2013 LAMP/EPFL - * @author Jason Zaugg - */ - -package scala.tools.nsc -package backend.jvm -import scala.tools.nsc.symtab._ - -/** Code shared between the erstwhile legacy backend (aka GenJVM) - * and the new backend [[scala.tools.nsc.backend.jvm.GenASM]]. There should be - * more here, but for now I'm starting with the refactorings that are either - * straightforward to review or necessary for maintenance. - */ -trait GenJVMASM { - val global: Global - import global._ - import icodes._ - import definitions._ - - val ExcludedForwarderFlags = { - import Flags._ - // Should include DEFERRED but this breaks findMember. - ( SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | BridgeAndPrivateFlags | MACRO ) - } - - protected def isJavaEntryPoint(icls: IClass) = { - val sym = icls.symbol - def fail(msg: String, pos: Position = sym.pos) = { - reporter.warning(sym.pos, - sym.name + " has a main method with parameter type Array[String], but " + sym.fullName('.') + " will not be a runnable program.\n" + - " Reason: " + msg - // TODO: make this next claim true, if possible - // by generating valid main methods as static in module classes - // not sure what the jvm allows here - // + " You can still run the program by calling it as " + sym.javaSimpleName + " instead." - ) - false - } - def failNoForwarder(msg: String) = { - fail(msg + ", which means no static forwarder can be generated.\n") - } - val possibles = if (sym.hasModuleFlag) (sym.tpe nonPrivateMember nme.main).alternatives else Nil - val hasApproximate = possibles exists { m => - m.info match { - case MethodType(p :: Nil, _) => p.tpe.typeSymbol == ArrayClass - case _ => false - } - } - // At this point it's a module with a main-looking method, so either succeed or warn that it isn't. - hasApproximate && { - // Before erasure so we can identify generic mains. - enteringErasure { - val companion = sym.linkedClassOfClass - - if (hasJavaMainMethod(companion)) - failNoForwarder("companion contains its own main method") - else if (companion.tpe.member(nme.main) != NoSymbol) - // this is only because forwarders aren't smart enough yet - failNoForwarder("companion contains its own main method (implementation restriction: no main is allowed, regardless of signature)") - else if (companion.isTrait) - failNoForwarder("companion is a trait") - // Now either succeeed, or issue some additional warnings for things which look like - // attempts to be java main methods. - else (possibles exists isJavaMainMethod) || { - possibles exists { m => - m.info match { - case PolyType(_, _) => - fail("main methods cannot be generic.") - case MethodType(params, res) => - if (res.typeSymbol :: params exists (_.isAbstractType)) - fail("main methods cannot refer to type parameters or abstract types.", m.pos) - else - isJavaMainMethod(m) || fail("main method must have exact signature (Array[String])Unit", m.pos) - case tp => - fail("don't know what this is: " + tp, m.pos) - } - } - } - } - } - } -} diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 02578e2038..57a17693ad 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -1087,6 +1087,10 @@ trait Definitions extends api.StandardDefinitions { lazy val ClassfileAnnotationClass = requiredClass[scala.annotation.ClassfileAnnotation] lazy val StaticAnnotationClass = requiredClass[scala.annotation.StaticAnnotation] + // Java retention annotations + lazy val AnnotationRetentionAttr = requiredClass[java.lang.annotation.Retention] + lazy val AnnotationRetentionPolicyAttr = requiredClass[java.lang.annotation.RetentionPolicy] + // Annotations lazy val BridgeClass = requiredClass[scala.annotation.bridge] lazy val ElidableMethodClass = requiredClass[scala.annotation.elidable] diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index dcd262c288..18a3c5d63f 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -360,6 +360,8 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => definitions.AnnotationClass definitions.ClassfileAnnotationClass definitions.StaticAnnotationClass + definitions.AnnotationRetentionAttr + definitions.AnnotationRetentionPolicyAttr definitions.BridgeClass definitions.ElidableMethodClass definitions.ImplicitNotFoundClass diff --git a/test/files/run/t4788-separate-compilation.check b/test/files/run/t4788-separate-compilation.check new file mode 100644 index 0000000000..172ad90102 --- /dev/null +++ b/test/files/run/t4788-separate-compilation.check @@ -0,0 +1,5 @@ +Some(@Ljava/lang/Deprecated;()) +None +None +Some(@LCAnnotation;() // invisible) +Some(@LRAnnotation;()) diff --git a/test/files/run/t4788-separate-compilation/CAnnotation_1.java b/test/files/run/t4788-separate-compilation/CAnnotation_1.java new file mode 100644 index 0000000000..7120218d62 --- /dev/null +++ b/test/files/run/t4788-separate-compilation/CAnnotation_1.java @@ -0,0 +1,5 @@ +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.CLASS; + +@Retention(value=CLASS) +@interface CAnnotation {} diff --git a/test/files/run/t4788-separate-compilation/C_1.scala b/test/files/run/t4788-separate-compilation/C_1.scala new file mode 100644 index 0000000000..aba9b595e4 --- /dev/null +++ b/test/files/run/t4788-separate-compilation/C_1.scala @@ -0,0 +1,2 @@ +@CAnnotation +class C diff --git a/test/files/run/t4788-separate-compilation/D_1.scala b/test/files/run/t4788-separate-compilation/D_1.scala new file mode 100644 index 0000000000..c2479fba86 --- /dev/null +++ b/test/files/run/t4788-separate-compilation/D_1.scala @@ -0,0 +1,5 @@ +@Deprecated +class DJava + +@deprecated("", "") +class DScala diff --git a/test/files/run/t4788-separate-compilation/RAnnotation_1.java b/test/files/run/t4788-separate-compilation/RAnnotation_1.java new file mode 100644 index 0000000000..f24cf66f7b --- /dev/null +++ b/test/files/run/t4788-separate-compilation/RAnnotation_1.java @@ -0,0 +1,5 @@ +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@Retention(value=RUNTIME) +@interface RAnnotation {} diff --git a/test/files/run/t4788-separate-compilation/R_1.scala b/test/files/run/t4788-separate-compilation/R_1.scala new file mode 100644 index 0000000000..ab0cd065d9 --- /dev/null +++ b/test/files/run/t4788-separate-compilation/R_1.scala @@ -0,0 +1,2 @@ +@RAnnotation +class R diff --git a/test/files/run/t4788-separate-compilation/SAnnotation_1.java b/test/files/run/t4788-separate-compilation/SAnnotation_1.java new file mode 100644 index 0000000000..471f27d82a --- /dev/null +++ b/test/files/run/t4788-separate-compilation/SAnnotation_1.java @@ -0,0 +1,5 @@ +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.SOURCE; + +@Retention(value=SOURCE) +@interface SAnnotation {} diff --git a/test/files/run/t4788-separate-compilation/S_1.scala b/test/files/run/t4788-separate-compilation/S_1.scala new file mode 100644 index 0000000000..f8756d9bc8 --- /dev/null +++ b/test/files/run/t4788-separate-compilation/S_1.scala @@ -0,0 +1,2 @@ +@SAnnotation +class S diff --git a/test/files/run/t4788-separate-compilation/Test_2.scala b/test/files/run/t4788-separate-compilation/Test_2.scala new file mode 100644 index 0000000000..cbbb5ff386 --- /dev/null +++ b/test/files/run/t4788-separate-compilation/Test_2.scala @@ -0,0 +1,35 @@ +import java.io.PrintWriter; + +import scala.tools.partest.BytecodeTest +import scala.tools.asm.util._ +import scala.tools.nsc.util.stringFromWriter + +object Test extends BytecodeTest { + def annotationsForClass(className: String): Option[String] = { + val classNode = loadClassNode(className, skipDebugInfo = false) + val textifier = new Textifier + classNode.accept(new TraceClassVisitor(null, textifier, null)) + + val classString = stringFromWriter(w => textifier.print(w)) + classString + .split('\n') + .filterNot(_.contains("@Lscala/reflect/ScalaSignature")) + .find(_.contains("@L")) + .map(_.trim) + } + + def show { + // It seems like @java.lang.Deprecated shows up in both the + // Deprecated attribute and RuntimeVisibleAnnotation attribute, + // while @scala.deprecated only shows up in the Deprecated attribute. + // The check file just documents status quo, not sure if Scala + // should brought in line with Java or not... + // See the commit message and SI-8883 for more info. + println(annotationsForClass("DJava")) + println(annotationsForClass("DScala")) + + println(annotationsForClass("S")) + println(annotationsForClass("C")) + println(annotationsForClass("R")) + } +} diff --git a/test/files/run/t4788.check b/test/files/run/t4788.check new file mode 100644 index 0000000000..172ad90102 --- /dev/null +++ b/test/files/run/t4788.check @@ -0,0 +1,5 @@ +Some(@Ljava/lang/Deprecated;()) +None +None +Some(@LCAnnotation;() // invisible) +Some(@LRAnnotation;()) diff --git a/test/files/run/t4788/C.scala b/test/files/run/t4788/C.scala new file mode 100644 index 0000000000..aba9b595e4 --- /dev/null +++ b/test/files/run/t4788/C.scala @@ -0,0 +1,2 @@ +@CAnnotation +class C diff --git a/test/files/run/t4788/CAnnotation.java b/test/files/run/t4788/CAnnotation.java new file mode 100644 index 0000000000..7120218d62 --- /dev/null +++ b/test/files/run/t4788/CAnnotation.java @@ -0,0 +1,5 @@ +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.CLASS; + +@Retention(value=CLASS) +@interface CAnnotation {} diff --git a/test/files/run/t4788/D.scala b/test/files/run/t4788/D.scala new file mode 100644 index 0000000000..c2479fba86 --- /dev/null +++ b/test/files/run/t4788/D.scala @@ -0,0 +1,5 @@ +@Deprecated +class DJava + +@deprecated("", "") +class DScala diff --git a/test/files/run/t4788/R.scala b/test/files/run/t4788/R.scala new file mode 100644 index 0000000000..ab0cd065d9 --- /dev/null +++ b/test/files/run/t4788/R.scala @@ -0,0 +1,2 @@ +@RAnnotation +class R diff --git a/test/files/run/t4788/RAnnotation.java b/test/files/run/t4788/RAnnotation.java new file mode 100644 index 0000000000..f24cf66f7b --- /dev/null +++ b/test/files/run/t4788/RAnnotation.java @@ -0,0 +1,5 @@ +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@Retention(value=RUNTIME) +@interface RAnnotation {} diff --git a/test/files/run/t4788/S.scala b/test/files/run/t4788/S.scala new file mode 100644 index 0000000000..f8756d9bc8 --- /dev/null +++ b/test/files/run/t4788/S.scala @@ -0,0 +1,2 @@ +@SAnnotation +class S diff --git a/test/files/run/t4788/SAnnotation.java b/test/files/run/t4788/SAnnotation.java new file mode 100644 index 0000000000..471f27d82a --- /dev/null +++ b/test/files/run/t4788/SAnnotation.java @@ -0,0 +1,5 @@ +import java.lang.annotation.Retention; +import static java.lang.annotation.RetentionPolicy.SOURCE; + +@Retention(value=SOURCE) +@interface SAnnotation {} diff --git a/test/files/run/t4788/Test.scala b/test/files/run/t4788/Test.scala new file mode 100644 index 0000000000..cbbb5ff386 --- /dev/null +++ b/test/files/run/t4788/Test.scala @@ -0,0 +1,35 @@ +import java.io.PrintWriter; + +import scala.tools.partest.BytecodeTest +import scala.tools.asm.util._ +import scala.tools.nsc.util.stringFromWriter + +object Test extends BytecodeTest { + def annotationsForClass(className: String): Option[String] = { + val classNode = loadClassNode(className, skipDebugInfo = false) + val textifier = new Textifier + classNode.accept(new TraceClassVisitor(null, textifier, null)) + + val classString = stringFromWriter(w => textifier.print(w)) + classString + .split('\n') + .filterNot(_.contains("@Lscala/reflect/ScalaSignature")) + .find(_.contains("@L")) + .map(_.trim) + } + + def show { + // It seems like @java.lang.Deprecated shows up in both the + // Deprecated attribute and RuntimeVisibleAnnotation attribute, + // while @scala.deprecated only shows up in the Deprecated attribute. + // The check file just documents status quo, not sure if Scala + // should brought in line with Java or not... + // See the commit message and SI-8883 for more info. + println(annotationsForClass("DJava")) + println(annotationsForClass("DScala")) + + println(annotationsForClass("S")) + println(annotationsForClass("C")) + println(annotationsForClass("R")) + } +} -- cgit v1.2.3 From ff051e29eae9139a688664f3531028cd89df4c75 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Wed, 8 Oct 2014 21:52:15 +0200 Subject: SI-8894 dealias when looking at tuple components Classic bait-and-switch: `isTupleType` dealiases, but `typeArgs` does not. When deciding with `isTupleType`, process using `tupleComponents`. Similar for other combos. We should really enforce this using extractors, and only decouple when performance is actually impacted. --- .../scala/tools/nsc/transform/patmat/MatchAnalysis.scala | 1 - src/reflect/scala/reflect/internal/Definitions.scala | 3 ++- test/files/pos/t8894.scala | 12 ++++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/t8894.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index a8852a3ff3..21e90b1d78 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -172,7 +172,6 @@ trait TreeAndTypeAnalysis extends Debugging { // a type is "uncheckable" (for exhaustivity) if we don't statically know its subtypes (i.e., it's unsealed) // we consider tuple types with at least one component of a checkable type as a checkable type def uncheckableType(tp: Type): Boolean = { - def tupleComponents(tp: Type) = tp.normalize.typeArgs val checkable = ( (isTupleType(tp) && tupleComponents(tp).exists(tp => !uncheckableType(tp))) || enumerateSubtypes(tp).nonEmpty) diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 57a17693ad..70375d974c 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -653,6 +653,7 @@ trait Definitions extends api.StandardDefinitions { // tends to change the course of events by forcing types. def isFunctionType(tp: Type) = isFunctionTypeDirect(tp.dealiasWiden) def isTupleType(tp: Type) = isTupleTypeDirect(tp.dealiasWiden) + def tupleComponents(tp: Type) = tp.dealiasWiden.typeArgs lazy val ProductRootClass: ClassSymbol = requiredClass[scala.Product] def Product_productArity = getMemberMethod(ProductRootClass, nme.productArity) @@ -837,7 +838,7 @@ trait Definitions extends api.StandardDefinitions { def typeOfMemberNamedApply(tp: Type) = typeArgOfBaseTypeOr(tp, SeqClass)(resultOfMatchingMethod(tp, nme.apply)(IntTpe)) def typeOfMemberNamedDrop(tp: Type) = typeArgOfBaseTypeOr(tp, SeqClass)(resultOfMatchingMethod(tp, nme.drop)(IntTpe)) def typesOfSelectors(tp: Type) = - if (isTupleType(tp)) tp.typeArgs + if (isTupleType(tp)) tupleComponents(tp) else getterMemberTypes(tp, productSelectors(tp)) // SI-8128 Still using the type argument of the base type at Seq/Option if this is an old-style (2.10 compatible) diff --git a/test/files/pos/t8894.scala b/test/files/pos/t8894.scala new file mode 100644 index 0000000000..3b26f1ae7e --- /dev/null +++ b/test/files/pos/t8894.scala @@ -0,0 +1,12 @@ +class CC(val i: Int, val s: String) +object CC extends { + type P = (Int, String) + + //def unapply(c: CC): Option[(Int, String)] = Some((c.i, c.s)) // OK + def unapply(c: CC): Option[P] = Some((c.i, c.s)) // fails (because of the type alias) +} + +class Test { + val cc = new CC(23, "foo") + val CC(i, s) = cc +} \ No newline at end of file -- cgit v1.2.3 From 4902c84cb049764dddc263976affa80bd6d44997 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 7 Oct 2014 17:35:47 +0200 Subject: SI-8890 handle reference to overload with error When buffering, we must report the ambiguity error to avoid a stack overflow. When the error refers to erroneous types/symbols, we don't report it directly to the user, because there will be an underlying error that's the root cause. --- .../tools/nsc/typechecker/ContextErrors.scala | 41 +++++++++++++--------- test/files/neg/t8890.check | 4 +++ test/files/neg/t8890.scala | 11 ++++++ 3 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 test/files/neg/t8890.check create mode 100644 test/files/neg/t8890.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala index 20e462bbce..866ca37303 100644 --- a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala +++ b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala @@ -885,22 +885,31 @@ trait ContextErrors { val WrongNumber, NoParams, ArgsDoNotConform = Value } - private def issueAmbiguousTypeErrorUnlessErroneous(pos: Position, pre: Type, sym1: Symbol, sym2: Symbol, rest: String): Unit = - if (!(pre.isErroneous || sym1.isErroneous || sym2.isErroneous)) { - if (sym1.hasDefault && sym2.hasDefault && sym1.enclClass == sym2.enclClass) { - val methodName = nme.defaultGetterToMethod(sym1.name) - context.issueAmbiguousError(AmbiguousTypeError(sym1.enclClass.pos, - "in "+ sym1.enclClass +", multiple overloaded alternatives of " + methodName + - " define default arguments")) - } else { - context.issueAmbiguousError(AmbiguousTypeError(pos, - ("ambiguous reference to overloaded definition,\n" + - "both " + sym1 + sym1.locationString + " of type " + pre.memberType(sym1) + - "\nand " + sym2 + sym2.locationString + " of type " + pre.memberType(sym2) + - "\nmatch " + rest) - )) - } - } + private def issueAmbiguousTypeErrorUnlessErroneous(pos: Position, pre: Type, sym1: Symbol, sym2: Symbol, rest: String): Unit = { + // To avoid stack overflows (SI-8890), we MUST (at least) report when either `validTargets` OR `ambiguousSuppressed` + // More details: + // If `!context.ambiguousErrors`, `reporter.issueAmbiguousError` (which `context.issueAmbiguousError` forwards to) + // buffers ambiguous errors. In this case, to avoid looping, we must issue even if `!validTargets`. (TODO: why?) + // When not buffering (and thus reporting to the user), we shouldn't issue unless `validTargets`, + // otherwise we report two different errors that trace back to the same root cause, + // and unless `validTargets`, we don't know for sure the ambiguity is real anyway. + val validTargets = !(pre.isErroneous || sym1.isErroneous || sym2.isErroneous) + val ambiguousBuffered = !context.ambiguousErrors + if (validTargets || ambiguousBuffered) + context.issueAmbiguousError( + if (sym1.hasDefault && sym2.hasDefault && sym1.enclClass == sym2.enclClass) { + val methodName = nme.defaultGetterToMethod(sym1.name) + AmbiguousTypeError(sym1.enclClass.pos, + s"in ${sym1.enclClass}, multiple overloaded alternatives of $methodName define default arguments") + + } else { + AmbiguousTypeError(pos, + "ambiguous reference to overloaded definition,\n" + + s"both ${sym1.fullLocationString} of type ${pre.memberType(sym1)}\n" + + s"and ${sym2.fullLocationString} of type ${pre.memberType(sym2)}\n" + + s"match $rest") + }) + } def AccessError(tree: Tree, sym: Symbol, ctx: Context, explanation: String): AbsTypeError = AccessError(tree, sym, ctx.enclClass.owner.thisType, ctx.enclClass.owner, explanation) diff --git a/test/files/neg/t8890.check b/test/files/neg/t8890.check new file mode 100644 index 0000000000..1b69d6cf30 --- /dev/null +++ b/test/files/neg/t8890.check @@ -0,0 +1,4 @@ +t8890.scala:6: error: not found: type Str + def bar(x: Str): Unit = ??? + ^ +one error found diff --git a/test/files/neg/t8890.scala b/test/files/neg/t8890.scala new file mode 100644 index 0000000000..cbdeb11d43 --- /dev/null +++ b/test/files/neg/t8890.scala @@ -0,0 +1,11 @@ +package foo + +class A { + /** The other */ + def bar(x: Int): Unit = ??? + def bar(x: Str): Unit = ??? +} + +class B { + (new A).bar(0) +} \ No newline at end of file -- cgit v1.2.3 From e96d1a4d346b1ebac46796307a0887d7b7c4fccf Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 14 Oct 2014 16:35:35 +0200 Subject: SI-8907 Don't force symbol info in isModuleNotMethod Test case by Jason. RefChecks adds the lateMETHOD flag lazily in its info transformer. This means that forcing the `sym.info` may change the value of `sym.isMethod`. 0ccdb151f introduced a check to force the info in isModuleNotMethod, but it turns out this leads to errors on stub symbols (SI-8907). The responsibility to force info is transferred to callers, which is the case for other operations on symbols, too. --- .../scala/tools/nsc/transform/Flatten.scala | 7 +++- src/reflect/scala/reflect/internal/Symbols.scala | 44 +++++++++++----------- test/files/run/t8907.scala | 39 +++++++++++++++++++ 3 files changed, 66 insertions(+), 24 deletions(-) create mode 100644 test/files/run/t8907.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala index fa53ef48b5..4662ef6224 100644 --- a/src/compiler/scala/tools/nsc/transform/Flatten.scala +++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala @@ -77,8 +77,11 @@ abstract class Flatten extends InfoTransform { if (sym.isTerm && !sym.isStaticModule) { decls1 enter sym if (sym.isModule) { - // Nested, non-static moduls are transformed to methods. - assert(sym.isMethod, s"Non-static $sym should have the lateMETHOD flag from RefChecks") + // In theory, we could assert(sym.isMethod), because nested, non-static moduls are + // transformed to methods (lateMETHOD flag added in RefChecks). But this requires + // forcing sym.info (see comment on isModuleNotMethod), which forces stub symbols + // too eagerly (SI-8907). + // Note that module classes are not entered into the 'decls' of the ClassInfoType // of the outer class, only the module symbols are. So the current loop does // not visit module classes. Therefore we set the LIFTED flag here for module diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index b0c23ef45d..2c039ab5a7 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -735,31 +735,31 @@ trait Symbols extends api.Symbols { self: SymbolTable => final def hasGetter = isTerm && nme.isLocalName(name) - /** A little explanation for this confusing situation. - * Nested modules which have no static owner when ModuleDefs - * are eliminated (refchecks) are given the lateMETHOD flag, - * which makes them appear as methods after refchecks. - * Here's an example where one can see all four of FF FT TF TT - * for (isStatic, isMethod) at various phases. + /** + * Nested modules which have no static owner when ModuleDefs are eliminated (refchecks) are + * given the lateMETHOD flag, which makes them appear as methods after refchecks. + * + * Note: the lateMETHOD flag is added lazily in the info transformer of the RefChecks phase. + * This means that forcing the `sym.info` may change the value of `sym.isMethod`. Forcing the + * info is in the responsability of the caller. Doing it eagerly here was tried (0ccdb151f) but + * has proven to lead to bugs (SI-8907). * - * trait A1 { case class Quux() } - * object A2 extends A1 { object Flax } - * // -- namer object Quux in trait A1 - * // -M flatten object Quux in trait A1 - * // S- flatten object Flax in object A2 - * // -M posterasure object Quux in trait A1 - * // -M jvm object Quux in trait A1 - * // SM jvm object Quux in object A2 + * Here's an example where one can see all four of FF FT TF TT for (isStatic, isMethod) at + * various phases. * - * So "isModuleNotMethod" exists not for its achievement in - * brevity, but to encapsulate the relevant condition. + * trait A1 { case class Quux() } + * object A2 extends A1 { object Flax } + * // -- namer object Quux in trait A1 + * // -M flatten object Quux in trait A1 + * // S- flatten object Flax in object A2 + * // -M posterasure object Quux in trait A1 + * // -M jvm object Quux in trait A1 + * // SM jvm object Quux in object A2 + * + * So "isModuleNotMethod" exists not for its achievement in brevity, but to encapsulate the + * relevant condition. */ - def isModuleNotMethod = { - if (isModule) { - if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there. - !isMethod - } else false - } + def isModuleNotMethod = isModule && !isMethod // After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should // be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module diff --git a/test/files/run/t8907.scala b/test/files/run/t8907.scala new file mode 100644 index 0000000000..7952ac82d9 --- /dev/null +++ b/test/files/run/t8907.scala @@ -0,0 +1,39 @@ +import scala.tools.partest._ +import java.io.File + +object Test extends StoreReporterDirectTest { + def code = ??? + + def compileCode(code: String) = { + val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code) + } + + def show(): Unit = { + compileCode(""" + class C { class Inner } + + class D { + object O { + def foo(c: C)(i: c.Inner): c.Inner = ??? + } + } + """) + assert(filteredInfos.isEmpty, filteredInfos) + deleteClass("C") + compileCode(""" + class E { + def foo = { + (null: D).toString + } + } + """) + assert(storeReporter.infos.isEmpty, storeReporter.infos.mkString("\n")) // Included a MissingRequirementError before. + } + + def deleteClass(name: String) { + val classFile = new File(testOutput.path, name + ".class") + assert(classFile.exists) + assert(classFile.delete()) + } +} -- cgit v1.2.3