From 2b5fde7ef9b202e8d4ba94d93d1b91b90e14d42f Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 13 Mar 2013 11:01:37 +0100 Subject: SI-7242 Fix crash when inner object mixes in its companion Given: class C { trait T { C.this } // C$T$$$outer$ : C object T extends T { C.this } // C$T$$$outer$ : C.this.type } object T ended up with a definitions for both of the accessors. These cannot co-exist, as they have the same erased type. A crash ensued almost immediately in explitouter. Fortunately, the solution is straightforward: we can just omit the mixin outer accessor altogether, the objects own outer accessor is compatible with it. scala> :javap C.T Compiled from "" public interface C$T{ public abstract C C$T$$$outer(); } scala> :javap C.T$ Compiled from "" public class C$T$ extends java.lang.Object implements C$T{ public C C$T$$$outer(); public C$T$(C); } I also added an assertion to give a better error message in case we find ourselves here again. Also includes tests from SI-3994, which I'll mark as a duplicate. --- .../scala/tools/nsc/transform/ExplicitOuter.scala | 38 +++++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala index 2f28a16416..f6ee7be511 100644 --- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala +++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala @@ -112,6 +112,29 @@ abstract class ExplicitOuter extends InfoTransform sym setInfo clazz.outerClass.thisType } + /** + * Will the outer accessor of the `clazz` subsume the outer accessor of + * `mixin`? + * + * This arises when an inner object mixes in its companion trait. + * + * {{{ + * class C { + * trait T { C.this } // C$T$$$outer$ : C + * object T extends T { C.this } // C$T$$$outer$ : C.this.type + * } + * }}} + * + * See SI-7242. + }} + */ + private def skipMixinOuterAccessor(clazz: Symbol, mixin: Symbol) = { + // Reliant on the current scheme for name expansion, the expanded name + // of the outer accessors in a trait and its companion object are the same. + // If the assumption is one day falsified, run/t7424.scala will let us know. + clazz.fullName == mixin.fullName + } + /**

* The type transformation method: *

@@ -177,10 +200,14 @@ abstract class ExplicitOuter extends InfoTransform for (mc <- clazz.mixinClasses) { val mixinOuterAcc: Symbol = afterExplicitOuter(outerAccessor(mc)) if (mixinOuterAcc != NoSymbol) { - if (decls1 eq decls) decls1 = decls.cloneScope - val newAcc = mixinOuterAcc.cloneSymbol(clazz, mixinOuterAcc.flags & ~DEFERRED) - newAcc setInfo (clazz.thisType memberType mixinOuterAcc) - decls1 enter newAcc + if (skipMixinOuterAccessor(clazz, mc)) + debuglog(s"Reusing outer accessor symbol of $clazz for the mixin outer accessor of $mc") + else { + if (decls1 eq decls) decls1 = decls.cloneScope + val newAcc = mixinOuterAcc.cloneSymbol(clazz, mixinOuterAcc.flags & ~DEFERRED) + newAcc setInfo (clazz.thisType memberType mixinOuterAcc) + decls1 enter newAcc + } } } } @@ -390,6 +417,7 @@ abstract class ExplicitOuter extends InfoTransform val outerAcc = outerAccessor(mixinClass) overridingSymbol currentClass def mixinPrefix = (currentClass.thisType baseType mixinClass).prefix assert(outerAcc != NoSymbol, "No outer accessor for inner mixin " + mixinClass + " in " + currentClass) + assert(outerAcc.alternatives.size == 1, s"Multiple outer accessors match inner mixin $mixinClass in $currentClass : ${outerAcc.alternatives.map(_.defString)}") // I added the mixinPrefix.typeArgs.nonEmpty condition to address the // crash in SI-4970. I feel quite sure this can be improved. val path = ( @@ -492,7 +520,7 @@ abstract class ExplicitOuter extends InfoTransform } if (!currentClass.isTrait) for (mc <- currentClass.mixinClasses) - if (outerAccessor(mc) != NoSymbol) + if (outerAccessor(mc) != NoSymbol && !skipMixinOuterAccessor(currentClass, mc)) newDefs += mixinOuterAccessorDef(mc) } } -- cgit v1.2.3 From 174334b9095be2be79c164bbdea1749dab9e0cbe Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 13 Mar 2013 19:03:53 +0100 Subject: SI-6921 SI-7239 Tread lightly during exploratory typing When deciding whether an Assign is a named argument or and assignment expression, or when looking at arguments that the current selection is applied to in order to evaluate candidate implicit views, we risk polluting the tree by setting error types. This happens even if we are in 'silent' mode; that mode does silence the error report, but not the side effect on the tree. This commit adds strategic `duplicate` calls to address the problem symptomatically. Duplicating trees and retyping in general reach into the domain of bugs umbrella-ed under SI-5464, but in these places we should be safe because the tree is in the argument position, not somewhere where, for example, a case class-es synthetic companion object might be twice entered into the same scope. Longer term, we'd like to make type checking side effect free, so we wouldn't need to play whack-a-mole like this. That idea is tracked under SI-7176. --- .../tools/nsc/typechecker/NamesDefaults.scala | 2 +- .../scala/tools/nsc/typechecker/Typers.scala | 5 ++- test/files/pos/t6921.scala | 11 +++++++ test/files/pos/t7239.scala | 38 ++++++++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 test/files/pos/t6921.scala create mode 100644 test/files/pos/t7239.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala index 1c60f0a79d..f3736f1519 100644 --- a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala +++ b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala @@ -502,7 +502,7 @@ trait NamesDefaults { self: Analyzer => // disable conforms as a view... val errsBefore = reporter.ERROR.count try typer.silent { tpr => - val res = tpr.typed(arg, subst(paramtpe)) + val res = tpr.typed(arg.duplicate, subst(paramtpe)) // better warning for SI-5044: if `silent` was not actually silent give a hint to the user // [H]: the reason why `silent` is not silent is because the cyclic reference exception is // thrown in a context completely different from `context` here. The exception happens while diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index d8493d2312..3addbc2e3a 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -1375,9 +1375,8 @@ trait Typers extends Modes with Adaptations with Tags { def onError(reportError: => Tree): Tree = { context.tree match { case Apply(tree1, args) if (tree1 eq tree) && args.nonEmpty => - silent(_.typedArgs(args, mode)) match { - case SilentResultValue(xs) => - val args = xs.asInstanceOf[List[Tree]] + silent(_.typedArgs(args.map(_.duplicate), mode)) match { + case SilentResultValue(args) => if (args exists (_.isErrorTyped)) reportError else diff --git a/test/files/pos/t6921.scala b/test/files/pos/t6921.scala new file mode 100644 index 0000000000..36e70e5d2c --- /dev/null +++ b/test/files/pos/t6921.scala @@ -0,0 +1,11 @@ +class Message(messageType: String, reason: Option[String]) + +class ReproForSI6921 { + + private[this] var reason = "" + + def decideElection = { + val explanation = None + new Message("", reason = explanation) + } +} diff --git a/test/files/pos/t7239.scala b/test/files/pos/t7239.scala new file mode 100644 index 0000000000..16e9d00f17 --- /dev/null +++ b/test/files/pos/t7239.scala @@ -0,0 +1,38 @@ +object Test { + def BrokenMethod(): HasFilter[(Int, String)] = ??? + + trait HasFilter[B] { + def filter(p: B => Boolean) = ??? + } + + trait HasWithFilter { + def withFilter = ??? + } + + object addWithFilter { + trait NoImplicit + implicit def enrich(v: Any) + (implicit F0: NoImplicit): HasWithFilter = ??? + } + + BrokenMethod().withFilter(_ => true) // okay + BrokenMethod().filter(_ => true) // okay + + locally { + import addWithFilter._ + BrokenMethod().withFilter((_: (Int, String)) => true) // okay + } + + locally { + import addWithFilter._ + // adaptToMemberWithArgs sets the type of the tree `x` + // to ErrorType (while in silent mode, so the error is not + // reported. Later, when the fallback from `withFilter` + // to `filter` is attempted, the closure is taken to have + // have the type ` => Boolean`, which conforms to + // `(B => Boolean)`. Only later during pickling does the + // defensive check for erroneous types in the tree pick up + // the problem. + BrokenMethod().withFilter(x => true) // erroneous or inaccessible type. + } +} -- cgit v1.2.3 From 6e79370294777421d6f01b8a635e71dc0e2454d6 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 11 Mar 2013 22:12:11 +0100 Subject: SI-7232 Fix Java import vs defn. binding precendence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Java Spec: > A single-type-import declaration d in a compilation unit c > of package p that imports a type named n shadows, throughout > c, the declarations of: > - any top level type named n declared in another compilation > unit of p > - any type named n imported by a type-import-on-demand > declaration in c > - any type named n imported by a static-import-on-demand > declaration in c Scala Spec: > Bindings of different kinds have a precedence defined on them: > 1. Definitions and declarations that are local, inherited, or made > available by a package clause in the same compilation unit where > the definition occurs have highest precedence. > 2. Explicit imports have next highest precedence. --- .../scala/tools/nsc/typechecker/Namers.scala | 8 +++++++- .../scala/tools/nsc/typechecker/Typers.scala | 20 +++++++++++++++++++- test/files/pos/t7232.flags | 1 + test/files/pos/t7232/Foo.java | 9 +++++++++ test/files/pos/t7232/List.java | 4 ++++ test/files/pos/t7232/Test.scala | 5 +++++ test/files/pos/t7232b.flags | 1 + test/files/pos/t7232b/Foo.java | 8 ++++++++ test/files/pos/t7232b/List.java | 5 +++++ test/files/pos/t7232b/Test.scala | 5 +++++ test/files/pos/t7232c.flags | 1 + test/files/pos/t7232c/Foo.java | 10 ++++++++++ test/files/pos/t7232c/Test.scala | 4 ++++ test/files/pos/t7232d.flags | 1 + test/files/pos/t7232d/Entry.java | 4 ++++ test/files/pos/t7232d/Foo.java | 8 ++++++++ test/files/pos/t7232d/Test.scala | 4 ++++ 17 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 test/files/pos/t7232.flags create mode 100644 test/files/pos/t7232/Foo.java create mode 100644 test/files/pos/t7232/List.java create mode 100644 test/files/pos/t7232/Test.scala create mode 100644 test/files/pos/t7232b.flags create mode 100644 test/files/pos/t7232b/Foo.java create mode 100644 test/files/pos/t7232b/List.java create mode 100644 test/files/pos/t7232b/Test.scala create mode 100644 test/files/pos/t7232c.flags create mode 100644 test/files/pos/t7232c/Foo.java create mode 100644 test/files/pos/t7232c/Test.scala create mode 100644 test/files/pos/t7232d.flags create mode 100644 test/files/pos/t7232d/Entry.java create mode 100644 test/files/pos/t7232d/Foo.java create mode 100644 test/files/pos/t7232d/Test.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 7a3ab00578..379f56521b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -516,7 +516,13 @@ trait Namers extends MethodSynthesis { // Setting the position at the import means that if there is // more than one hidden name, the second will not be warned. // So it is the position of the actual hidden name. - checkNotRedundant(tree.pos withPoint fromPos, from, to) + // + // Note: java imports have precence over definitions in the same package + // so don't warn for them. There is a corresponding special treatment + // in the shadowing rules in typedIdent to (SI-7232). In any case, + // we shouldn't be emitting warnings for .java source files. + if (!context.unit.isJava) + checkNotRedundant(tree.pos withPoint fromPos, from, to) } } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index d8493d2312..93d24996eb 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -5046,7 +5046,25 @@ trait Typers extends Modes with Adaptations with Tags { else cx.depth - (cx.scope.nestingLevel - defEntry.owner.nestingLevel) var impSym: Symbol = NoSymbol // the imported symbol var imports = context.imports // impSym != NoSymbol => it is imported from imports.head - while (!reallyExists(impSym) && !imports.isEmpty && imports.head.depth > symDepth) { + + // Java: A single-type-import declaration d in a compilation unit c of package p + // that imports a type named n shadows, throughout c, the declarations of: + // + // 1) any top level type named n declared in another compilation unit of p + // + // A type-import-on-demand declaration never causes any other declaration to be shadowed. + // + // Scala: Bindings of different kinds have a precedence defined on them: + // + // 1) Definitions and declarations that are local, inherited, or made available by a + // package clause in the same compilation unit where the definition occurs have + // highest precedence. + // 2) Explicit imports have next highest precedence. + def depthOk(imp: ImportInfo) = ( + imp.depth > symDepth + || (unit.isJava && imp.isExplicitImport(name) && imp.depth == symDepth) + ) + while (!reallyExists(impSym) && !imports.isEmpty && depthOk(imports.head)) { impSym = imports.head.importedSymbol(name) if (!impSym.exists) imports = imports.tail } diff --git a/test/files/pos/t7232.flags b/test/files/pos/t7232.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t7232.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7232/Foo.java b/test/files/pos/t7232/Foo.java new file mode 100644 index 0000000000..3478301b30 --- /dev/null +++ b/test/files/pos/t7232/Foo.java @@ -0,0 +1,9 @@ +package pack; + +import java.util.List; + +public class Foo { + public static java.util.List okay() { throw new Error(); } + + public static List wrong() { throw new Error(); } +} diff --git a/test/files/pos/t7232/List.java b/test/files/pos/t7232/List.java new file mode 100644 index 0000000000..e42c63aa67 --- /dev/null +++ b/test/files/pos/t7232/List.java @@ -0,0 +1,4 @@ +package pack; + +public class List { +} diff --git a/test/files/pos/t7232/Test.scala b/test/files/pos/t7232/Test.scala new file mode 100644 index 0000000000..49c3c12aed --- /dev/null +++ b/test/files/pos/t7232/Test.scala @@ -0,0 +1,5 @@ +object Test { + import pack._ + Foo.okay().size() + Foo.wrong().size() +} diff --git a/test/files/pos/t7232b.flags b/test/files/pos/t7232b.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t7232b.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7232b/Foo.java b/test/files/pos/t7232b/Foo.java new file mode 100644 index 0000000000..94f08d545e --- /dev/null +++ b/test/files/pos/t7232b/Foo.java @@ -0,0 +1,8 @@ +package pack; + +import java.util.*; + +public class Foo { + // should be pack.List. + public static List list() { throw new Error(); } +} diff --git a/test/files/pos/t7232b/List.java b/test/files/pos/t7232b/List.java new file mode 100644 index 0000000000..ce977152b9 --- /dev/null +++ b/test/files/pos/t7232b/List.java @@ -0,0 +1,5 @@ +package pack; + +public class List { + public void packList() {} +} diff --git a/test/files/pos/t7232b/Test.scala b/test/files/pos/t7232b/Test.scala new file mode 100644 index 0000000000..6377e26bec --- /dev/null +++ b/test/files/pos/t7232b/Test.scala @@ -0,0 +1,5 @@ +object Test { + import pack._ + + Foo.list().packList() +} diff --git a/test/files/pos/t7232c.flags b/test/files/pos/t7232c.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t7232c.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7232c/Foo.java b/test/files/pos/t7232c/Foo.java new file mode 100644 index 0000000000..bbda09a2da --- /dev/null +++ b/test/files/pos/t7232c/Foo.java @@ -0,0 +1,10 @@ +package pack; + +import java.util.List; + +public class Foo { + public static class List { + public void isInnerList() {} + } + public static List innerList() { throw new Error(); } +} diff --git a/test/files/pos/t7232c/Test.scala b/test/files/pos/t7232c/Test.scala new file mode 100644 index 0000000000..aa7c710948 --- /dev/null +++ b/test/files/pos/t7232c/Test.scala @@ -0,0 +1,4 @@ +object Test { + import pack._ + Foo.innerList().isInnerList() +} diff --git a/test/files/pos/t7232d.flags b/test/files/pos/t7232d.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t7232d.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7232d/Entry.java b/test/files/pos/t7232d/Entry.java new file mode 100644 index 0000000000..0cfb6fb25b --- /dev/null +++ b/test/files/pos/t7232d/Entry.java @@ -0,0 +1,4 @@ +package pack; + +public class Entry { +} diff --git a/test/files/pos/t7232d/Foo.java b/test/files/pos/t7232d/Foo.java new file mode 100644 index 0000000000..df7114a0f0 --- /dev/null +++ b/test/files/pos/t7232d/Foo.java @@ -0,0 +1,8 @@ +package pack; + +import java.util.Map.Entry; + +public class Foo { + public static Entry mapEntry() { throw new Error(); } + public static void javaTest() { mapEntry().getKey(); } +} diff --git a/test/files/pos/t7232d/Test.scala b/test/files/pos/t7232d/Test.scala new file mode 100644 index 0000000000..89a8063b3c --- /dev/null +++ b/test/files/pos/t7232d/Test.scala @@ -0,0 +1,4 @@ +object Test { + import pack._ + Foo.mapEntry().getKey() +} -- cgit v1.2.3 From 552b6234dbbb4ff1e971e2c0f87ecd4090dd36a5 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 13 Mar 2013 23:30:05 +0100 Subject: SI-7249 Reign in overzealous Function0 optimization. The fix for SI-1247 went too far, and could result in premature evaluation of the expression that yields the Function0. This commit checks that said expression is safe to inline. If not, a wrapper `() => ....` is still required. The optimization is still enabled in sitations like the original test case, run/t1247.scala. --- src/compiler/scala/tools/nsc/transform/UnCurry.scala | 6 +++++- test/files/run/t7249.check | 1 + test/files/run/t7249.scala | 7 +++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t7249.check create mode 100644 test/files/run/t7249.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index e9f403aea0..66328e23bb 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -492,10 +492,14 @@ abstract class UnCurry extends InfoTransform } else { log(s"Argument '$arg' at line ${arg.pos.safeLine} is $formal from ${fun.fullName}") + def canUseDirectly(recv: Tree) = ( + recv.tpe.typeSymbol.isSubClass(FunctionClass(0)) + && treeInfo.isExprSafeToInline(recv) + ) arg match { // don't add a thunk for by-name argument if argument already is an application of // a Function0. We can then remove the application and use the existing Function0. - case Apply(Select(recv, nme.apply), Nil) if recv.tpe.typeSymbol isSubClass FunctionClass(0) => + case Apply(Select(recv, nme.apply), Nil) if canUseDirectly(recv) => recv case _ => newFunction0(arg) diff --git a/test/files/run/t7249.check b/test/files/run/t7249.check new file mode 100644 index 0000000000..7777e0a5a2 --- /dev/null +++ b/test/files/run/t7249.check @@ -0,0 +1 @@ +Yup! diff --git a/test/files/run/t7249.scala b/test/files/run/t7249.scala new file mode 100644 index 0000000000..375df5c3ad --- /dev/null +++ b/test/files/run/t7249.scala @@ -0,0 +1,7 @@ +object Test extends App { + def bnToLambda(s: => String): () => String = () => s + var x: () => String = () => sys.error("Nope") + val y = bnToLambda { x() } + x = () => "Yup!" + println(y()) +} -- cgit v1.2.3 From e78896f0ab7240e9bdcd98c51cbf6bc0ea277950 Mon Sep 17 00:00:00 2001 From: James Iry Date: Fri, 8 Mar 2013 06:33:12 -0800 Subject: Read version 51 (JDK 7) class files. This commit makes the ClassFileReader/ICodeReader parse class files from JDK 7 (class file version 51). It does that by skipping over the method handle related entries in the constant pool and by doing some dummy processing on invoke dynamic instructions. The inliner is updated to not try to inline a method with an invoke dynamic instruction. A place holder INVOKE_DYNAMIC instruction is added to ICode but it is designed to create an error if there's ever any attempt to analyze it. Because the inliner is the only phase that ever tries to analyze ICode instructions not generated from Scala source and because Scala source will never emit an INVOKE_DYNAMIC, the place holder INVOKE_DYNAMIC should never cause any errors. A test is included that generates a class file with INVOKE_DYNAMIC and then compiles Scala code that depends on it. --- bincompat-backward.whitelist.conf | 4 + bincompat-forward.whitelist.conf | 16 +++ .../scala/tools/nsc/backend/icode/Members.scala | 1 + .../scala/tools/nsc/backend/icode/Opcodes.scala | 19 ++++ .../scala/tools/nsc/backend/opt/Inliners.scala | 1 + .../nsc/symtab/classfile/ClassfileParser.scala | 7 +- .../tools/nsc/symtab/classfile/ICodeReader.scala | 8 ++ .../reflect/internal/ClassfileConstants.scala | 5 +- test/files/run/classfile-format-51.scala | 126 +++++++++++++++++++++ 9 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 test/files/run/classfile-format-51.scala (limited to 'src') diff --git a/bincompat-backward.whitelist.conf b/bincompat-backward.whitelist.conf index 4794666721..4f627780e6 100644 --- a/bincompat-backward.whitelist.conf +++ b/bincompat-backward.whitelist.conf @@ -158,6 +158,10 @@ filter { { matchName="scala.reflect.internal.Names#NameOps.name" problemName=MissingFieldProblem + }, + { + matchName="scala.reflect.internal.ClassfileConstants.xxxunusedxxxx" + problemName=MissingMethodProblem } ] } diff --git a/bincompat-forward.whitelist.conf b/bincompat-forward.whitelist.conf index 529fab1e14..76e189653b 100644 --- a/bincompat-forward.whitelist.conf +++ b/bincompat-forward.whitelist.conf @@ -354,6 +354,22 @@ filter { { matchName="scala.reflect.internal.StdNames#TermNames.SelectFromTypeTree" problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ClassfileConstants.CONSTANT_INVOKEDYNAMIC" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ClassfileConstants.invokedynamic" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ClassfileConstants.CONSTANT_METHODHANDLE" + problemName=MissingMethodProblem + }, + { + matchName="scala.reflect.internal.ClassfileConstants.CONSTANT_METHODTYPE" + problemName=MissingMethodProblem } ] } diff --git a/src/compiler/scala/tools/nsc/backend/icode/Members.scala b/src/compiler/scala/tools/nsc/backend/icode/Members.scala index 7ba212f42e..b74770f051 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/Members.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/Members.scala @@ -171,6 +171,7 @@ trait Members { var returnType: TypeKind = _ var recursive: Boolean = false var bytecodeHasEHs = false // set by ICodeReader only, used by Inliner to prevent inlining (SI-6188) + var bytecodeHasInvokeDynamic = false // set by ICodeReader only, used by Inliner to prevent inlining until we have proper invoke dynamic support /** local variables and method parameters */ var locals: List[Local] = Nil diff --git a/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala b/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala index 8c9a72638d..a3a0edb35d 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala @@ -409,6 +409,25 @@ trait Opcodes { self: ICodes => override def category = mthdsCat } + + /** + * A place holder entry that allows us to parse class files with invoke dynamic + * instructions. Because the compiler doesn't yet really understand the + * behavior of invokeDynamic, this op acts as a poison pill. Any attempt to analyze + * this instruction will cause a failure. The only optimization that + * should ever look at non-Scala generated icode is the inliner, and it + * has been modified to not examine any method with invokeDynamic + * instructions. So if this poison pill ever causes problems then + * there's been a serious misunderstanding + */ + // TODO do the real thing + case class INVOKE_DYNAMIC(poolEntry: Char) extends Instruction { + private def error = sys.error("INVOKE_DYNAMIC is not fully implemented and should not be analyzed") + override def consumed = error + override def produced = error + override def producedTypes = error + override def category = error + } case class BOX(boxType: TypeKind) extends Instruction { assert(boxType.isValueType && (boxType ne UNIT)) // documentation diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala index 521b6cc132..498db78636 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala @@ -961,6 +961,7 @@ abstract class Inliners extends SubComponent { if(isInlineForbidden) { rs ::= "is annotated @noinline" } if(inc.isSynchronized) { rs ::= "is synchronized method" } if(inc.m.bytecodeHasEHs) { rs ::= "bytecode contains exception handlers / finally clause" } // SI-6188 + if(inc.m.bytecodeHasInvokeDynamic) { rs ::= "bytecode contains invoke dynamic" } if(rs.isEmpty) null else rs.mkString("", ", and ", "") } diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala index 1f42fa8aab..29cac86c16 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala @@ -137,10 +137,13 @@ abstract class ClassfileParser { (in.nextByte.toInt: @switch) match { case CONSTANT_UTF8 | CONSTANT_UNICODE => in.skip(in.nextChar) - case CONSTANT_CLASS | CONSTANT_STRING => + case CONSTANT_CLASS | CONSTANT_STRING | CONSTANT_METHODTYPE=> in.skip(2) + case CONSTANT_METHODHANDLE => + in.skip(3) case CONSTANT_FIELDREF | CONSTANT_METHODREF | CONSTANT_INTFMETHODREF - | CONSTANT_NAMEANDTYPE | CONSTANT_INTEGER | CONSTANT_FLOAT => + | CONSTANT_NAMEANDTYPE | CONSTANT_INTEGER | CONSTANT_FLOAT + | CONSTANT_INVOKEDYNAMIC => in.skip(4) case CONSTANT_LONG | CONSTANT_DOUBLE => in.skip(8) diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala index 13c0d8993a..d38907d182 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala @@ -506,6 +506,12 @@ abstract class ICodeReader extends ClassfileParser { code.emit(UNBOX(toTypeKind(m.info.resultType))) else code.emit(CALL_METHOD(m, Static(false))) + case JVM.invokedynamic => + // TODO, this is just a place holder. A real implementation must parse the class constant entry + containsInvokeDynamic = true + val poolEntry = in.nextChar + in.skip(2) + code.emit(INVOKE_DYNAMIC(poolEntry)) case JVM.new_ => code.emit(NEW(REFERENCE(pool.getClassSymbol(in.nextChar)))) @@ -644,6 +650,7 @@ abstract class ICodeReader extends ClassfileParser { var containsDUPX = false var containsNEW = false var containsEHs = false + var containsInvokeDynamic = false def emit(i: Instruction) { instrs += ((pc, i)) @@ -662,6 +669,7 @@ abstract class ICodeReader extends ClassfileParser { val code = new Code(method) method.setCode(code) method.bytecodeHasEHs = containsEHs + method.bytecodeHasInvokeDynamic = containsInvokeDynamic var bb = code.startBlock def makeBasicBlocks: mutable.Map[Int, BasicBlock] = diff --git a/src/reflect/scala/reflect/internal/ClassfileConstants.scala b/src/reflect/scala/reflect/internal/ClassfileConstants.scala index 7ccb661426..c198271fb1 100644 --- a/src/reflect/scala/reflect/internal/ClassfileConstants.scala +++ b/src/reflect/scala/reflect/internal/ClassfileConstants.scala @@ -72,6 +72,9 @@ object ClassfileConstants { final val CONSTANT_METHODREF = 10 final val CONSTANT_INTFMETHODREF = 11 final val CONSTANT_NAMEANDTYPE = 12 + final val CONSTANT_METHODHANDLE = 15 + final val CONSTANT_METHODTYPE = 16 + final val CONSTANT_INVOKEDYNAMIC = 18 // tags describing the type of a literal in attribute values final val BYTE_TAG = 'B' @@ -306,7 +309,7 @@ object ClassfileConstants { final val invokespecial = 0xb7 final val invokestatic = 0xb8 final val invokeinterface = 0xb9 - final val xxxunusedxxxx = 0xba + final val invokedynamic = 0xba final val new_ = 0xbb final val newarray = 0xbc diff --git a/test/files/run/classfile-format-51.scala b/test/files/run/classfile-format-51.scala new file mode 100644 index 0000000000..9b1e612f4f --- /dev/null +++ b/test/files/run/classfile-format-51.scala @@ -0,0 +1,126 @@ +import java.io.{File, FileOutputStream} + +import scala.tools.nsc.settings.ScalaVersion +import scala.tools.partest._ +import scala.tools.asm +import asm.{AnnotationVisitor, ClassWriter, FieldVisitor, Handle, MethodVisitor, Opcodes} +import Opcodes._ + +// This test ensures that we can read JDK 7 (classfile format 51) files, including those +// with invokeDynamic instructions and associated constant pool entries +// to do that it first uses ASM to generate a class called DynamicInvoker. Then +// it runs a normal compile on the source in the 'code' field that refers to +// DynamicInvoker. Any failure will be dumped to std out. +// +// By it's nature the test can only work on JDK 7+ because under JDK 6 some of the +// classes referred to by DynamicInvoker won't be available and DynamicInvoker won't +// verify. So the test includes a version check that short-circuites the whole test +// on JDK 6 +object Test extends DirectTest { + override def extraSettings: String = "-optimise -usejavacp -d " + testOutput.path + " -cp " + testOutput.path + + def generateClass() { + val invokerClassName = "DynamicInvoker" + val bootstrapMethodName = "bootstrap" + val bootStrapMethodType = "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;" + val targetMethodName = "target" + val targetMethodType = "()Ljava/lang/String;" + + val cw = new ClassWriter(0) + cw.visit(V1_7, ACC_PUBLIC + ACC_SUPER, invokerClassName, null, "java/lang/Object", null) + + val constructor = cw.visitMethod(ACC_PUBLIC, "", "()V", null, null) + constructor.visitCode() + constructor.visitVarInsn(ALOAD, 0) + constructor.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "", "()V") + constructor.visitInsn(RETURN) + constructor.visitMaxs(1, 1) + constructor.visitEnd() + + val target = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, targetMethodName, targetMethodType, null, null) + target.visitCode() + target.visitLdcInsn("hello") + target.visitInsn(ARETURN) + target.visitMaxs(1, 1) + target.visitEnd() + + val bootstrap = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, bootstrapMethodName, bootStrapMethodType, null, null) + bootstrap.visitCode() +// val lookup = MethodHandles.lookup(); + bootstrap.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MethodHandles", "lookup", "()Ljava/lang/invoke/MethodHandles$Lookup;") + bootstrap.visitVarInsn(ASTORE, 3) // lookup + +// val clazz = lookup.lookupClass(); + bootstrap.visitVarInsn(ALOAD, 3) // lookup + bootstrap.visitMethodInsn(INVOKEVIRTUAL, "java/lang/invoke/MethodHandles$Lookup", "lookupClass", "()Ljava/lang/Class;") + bootstrap.visitVarInsn(ASTORE, 4) // clazz + +// val methodType = MethodType.fromMethodDescriptorString("()Ljava/lang/String, clazz.getClassLoader()") + bootstrap.visitLdcInsn("()Ljava/lang/String;") + bootstrap.visitVarInsn(ALOAD, 4) // CLAZZ + bootstrap.visitMethodInsn(INVOKEVIRTUAL, "java/lang/Class", "getClassLoader", "()Ljava/lang/ClassLoader;") + bootstrap.visitMethodInsn(INVOKESTATIC, "java/lang/invoke/MethodType", "fromMethodDescriptorString", "(Ljava/lang/String;Ljava/lang/ClassLoader;)Ljava/lang/invoke/MethodType;") + bootstrap.visitVarInsn(ASTORE, 5) // methodType + +// val methodHandle = lookup.findStatic(thisClass, "target", methodType) + bootstrap.visitVarInsn(ALOAD, 3) // lookup + bootstrap.visitVarInsn(ALOAD, 4) // clazz + bootstrap.visitLdcInsn("target") + bootstrap.visitVarInsn(ALOAD, 5) // methodType + bootstrap.visitMethodInsn(INVOKEVIRTUAL, "java/lang/invoke/MethodHandles$Lookup", "findStatic", "(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/MethodHandle;") + bootstrap.visitVarInsn(ASTORE, 6) // methodHandle + +// new ConstantCallSite(methodHandle) + bootstrap.visitTypeInsn(NEW, "java/lang/invoke/ConstantCallSite") + bootstrap.visitInsn(DUP) + bootstrap.visitVarInsn(ALOAD, 6) // methodHandle + bootstrap.visitMethodInsn(INVOKESPECIAL, "java/lang/invoke/ConstantCallSite", "", "(Ljava/lang/invoke/MethodHandle;)V") + bootstrap.visitInsn(ARETURN) + bootstrap.visitMaxs(4,7) + bootstrap.visitEnd() + + val test = cw.visitMethod(ACC_PUBLIC + ACC_FINAL, "test", s"()Ljava/lang/String;", null, null) + test.visitCode() + val bootstrapHandle = new Handle(H_INVOKESTATIC, invokerClassName, bootstrapMethodName, bootStrapMethodType) + test.visitInvokeDynamicInsn("invoke", targetMethodType, bootstrapHandle) + test.visitInsn(ARETURN) + test.visitMaxs(1, 1) + test.visitEnd() + + cw.visitEnd() + val bytes = cw.toByteArray() + + val fos = new FileOutputStream(new File(s"${testOutput.path}/$invokerClassName.class")) + try + fos write bytes + finally + fos.close() + + } + + def code = +""" +object Driver { + val invoker = new DynamicInvoker() + println(invoker.test()) +} +""" + + override def show(): Unit = { + // redirect err to out, for logging + val prevErr = System.err + System.setErr(System.out) + try { + // this test is only valid under JDK 1.7+ + // cheat a little by using 'ScalaVersion' because it can parse java versions just as well + val requiredJavaVersion = ScalaVersion("1.7") + val executingJavaVersion = ScalaVersion(System.getProperty("java.specification.version")) + if (executingJavaVersion >= requiredJavaVersion) { + generateClass() + compile() + } + } + finally + System.setErr(prevErr) + } +} -- cgit v1.2.3 From 67b8de7c3ca3896a8009c881c9395227d38e0250 Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Wed, 13 Mar 2013 16:32:03 +0100 Subject: [backport] SI-7237 Always choose ForkJoinTaskSupport ... on Java 6 and above. ForkJoinTaskSupport works on Hotspot, Avian and J9, while ThreadPoolTaskSupport causes the test test/files/scalacheck/parallel-collections to reliably hang on all three. We keep ThreadPoolTaskSupport around to keep the hope alive that we still have a glimpse of 1.5 support. --- src/library/scala/collection/parallel/package.scala | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/library/scala/collection/parallel/package.scala b/src/library/scala/collection/parallel/package.scala index 83aa99ad11..988886b4ea 100644 --- a/src/library/scala/collection/parallel/package.scala +++ b/src/library/scala/collection/parallel/package.scala @@ -42,11 +42,8 @@ package object parallel { private[parallel] def outofbounds(idx: Int) = throw new IndexOutOfBoundsException(idx.toString) private[parallel] def getTaskSupport: TaskSupport = - if (scala.util.Properties.isJavaAtLeast("1.6")) { - val vendor = scala.util.Properties.javaVmVendor - if ((vendor contains "Oracle") || (vendor contains "Sun") || (vendor contains "Apple")) new ForkJoinTaskSupport - else new ThreadPoolTaskSupport - } else new ThreadPoolTaskSupport + if (scala.util.Properties.isJavaAtLeast("1.6")) new ForkJoinTaskSupport + else new ThreadPoolTaskSupport val defaultTaskSupport: TaskSupport = getTaskSupport -- cgit v1.2.3 From 386a5bd68de3a95563d61b2b305e012c157f3b89 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Fri, 15 Mar 2013 21:42:15 +0100 Subject: SI-7253: respect binary compatibility constraints From the JLS one can prove that moving a method to a superclass is a binary compatible change, both forward and backward. That's because when compiling a method call `c.foo()`, where c: C, the output descriptor *must* refer to `C` and not to the class where `foo()` is actually defined. This patch just ensures that, and adds a test comparing generated descriptors against the Javac output. The sample code is from Paul Philipps, the fix and the bytecode comparison code from me. From 2006 (9954eafffd5e60676238369ab0ed5797c92b4a7b, a fix for bug 455 in the old bug tracker http://www.scala-lang.org/sites/default/files/aladdin/displayItem.do%3Fid=455.html) until 2.9, Scalac has followed this rule "often" (that is, when C is *not* an interface). This behavior was wrong, but the bug was hard to trigger. AFAICS, this can create problems only when moving a method to a super interface in a library and expecting forward binary compatibility - that is, compiling some Scala client code against the new version of the library, and trying to run this code against the old version of the library. This change grows an interface, so it is valid only if clients are supposed to *not* implement the library. Apparently, this is so rare that nobody noticed. Since 2.10 (0bea2ab5f6b211a83bbf14ea46fe57b8163c6334), Scalac follows this rule *only* when C is an interface (I assume by oversight, since the main change was an accessibility check), so the bug was finally triggered. The new code will have to emit INVOKEINTERFACE instead of INVOKEVIRTUAL a bit more often, compared to 2.9 (but not to 2.10). I don't know whether INVOKEINTERFACE is noticeably slower (it shouldn't be); but this is the safest fix since this behavior is mandated by the JLS. If somebody disagrees and believes the 2.9 is significantly faster, IMHO he should send a separate pull request (although ProGuard is probably a better place for the change). --- .../scala/tools/nsc/backend/jvm/GenASM.scala | 3 ++- .../scala/tools/nsc/backend/jvm/GenJVM.scala | 3 ++- test/files/jvm/t7253.check | 1 + test/files/jvm/t7253/Base_1.scala | 5 ++++ test/files/jvm/t7253/JavaClient_1.java | 9 +++++++ test/files/jvm/t7253/ScalaClient_1.scala | 9 +++++++ test/files/jvm/t7253/test.scala | 28 ++++++++++++++++++++++ 7 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 test/files/jvm/t7253.check create mode 100644 test/files/jvm/t7253/Base_1.scala create mode 100644 test/files/jvm/t7253/JavaClient_1.java create mode 100644 test/files/jvm/t7253/ScalaClient_1.scala create mode 100644 test/files/jvm/t7253/test.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 9b16327ffc..55b20154de 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -2260,6 +2260,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { hostSymbol.info ; methodOwner.info def isInterfaceCall(sym: Symbol) = ( + //XXX remove the test for ObjectClass. sym.isInterface && methodOwner != ObjectClass || sym.isJavaDefined && sym.isNonBottomSubClass(ClassfileAnnotationClass) ) @@ -2267,8 +2268,8 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { // the type of the method owner (if not an interface!) val useMethodOwner = ( style != Dynamic - || !isInterfaceCall(hostSymbol) && isAccessibleFrom(methodOwner, siteSymbol) || hostSymbol.isBottomClass + || methodOwner == ObjectClass ) val receiver = if (useMethodOwner) methodOwner else hostSymbol val jowner = javaName(receiver) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala index 598965b982..ed0d8144ca 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala @@ -1197,6 +1197,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with hostSymbol.info ; methodOwner.info def isInterfaceCall(sym: Symbol) = ( + //XXX remove the test for ObjectClass. sym.isInterface && methodOwner != ObjectClass || sym.isJavaDefined && sym.isNonBottomSubClass(ClassfileAnnotationClass) ) @@ -1204,8 +1205,8 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with // the type of the method owner (if not an interface!) val useMethodOwner = ( style != Dynamic - || !isInterfaceCall(hostSymbol) && isAccessibleFrom(methodOwner, siteSymbol) || hostSymbol.isBottomClass + || methodOwner == ObjectClass ) val receiver = if (useMethodOwner) methodOwner else hostSymbol val jowner = javaName(receiver) diff --git a/test/files/jvm/t7253.check b/test/files/jvm/t7253.check new file mode 100644 index 0000000000..43f53aba12 --- /dev/null +++ b/test/files/jvm/t7253.check @@ -0,0 +1 @@ +bytecode identical diff --git a/test/files/jvm/t7253/Base_1.scala b/test/files/jvm/t7253/Base_1.scala new file mode 100644 index 0000000000..a531ebb69d --- /dev/null +++ b/test/files/jvm/t7253/Base_1.scala @@ -0,0 +1,5 @@ +trait A { def f(): Int } +trait B1 extends A +abstract class B2 extends A +class B3 extends A { def f(): Int = 1 } +class B4 extends B3 diff --git a/test/files/jvm/t7253/JavaClient_1.java b/test/files/jvm/t7253/JavaClient_1.java new file mode 100644 index 0000000000..43475de2f5 --- /dev/null +++ b/test/files/jvm/t7253/JavaClient_1.java @@ -0,0 +1,9 @@ +public class JavaClient_1 { + int foo() { + ((A) null).f(); + ((B1) null).f(); + ((B2) null).f(); + ((B3) null).f(); + return ((B4) null).f(); + } +} diff --git a/test/files/jvm/t7253/ScalaClient_1.scala b/test/files/jvm/t7253/ScalaClient_1.scala new file mode 100644 index 0000000000..d244b326a8 --- /dev/null +++ b/test/files/jvm/t7253/ScalaClient_1.scala @@ -0,0 +1,9 @@ +class ScalaClient_1 { + def foo() = { + (null: A).f() + (null: B1).f() + (null: B2).f() + (null: B3).f() + (null: B4).f() + } +} diff --git a/test/files/jvm/t7253/test.scala b/test/files/jvm/t7253/test.scala new file mode 100644 index 0000000000..7fe08e8813 --- /dev/null +++ b/test/files/jvm/t7253/test.scala @@ -0,0 +1,28 @@ +import scala.tools.partest.BytecodeTest + +import scala.tools.nsc.util.JavaClassPath +import java.io.InputStream +import scala.tools.asm +import asm.ClassReader +import asm.tree.{ClassNode, InsnList} +import scala.collection.JavaConverters._ + +object Test extends BytecodeTest { + import instructions._ + + def show: Unit = { + val instrBaseSeqs = Seq("ScalaClient_1", "JavaClient_1") map (name => instructions.fromMethod(getMethod(loadClassNode(name), "foo"))) + val instrSeqs = instrBaseSeqs map (_ filter isInvoke) + cmpInstructions(instrSeqs(0), instrSeqs(1)) + } + + def cmpInstructions(isa: List[Instruction], isb: List[Instruction]) = { + if (isa == isb) println("bytecode identical") + else diffInstructions(isa, isb) + } + + def isInvoke(node: Instruction): Boolean = { + val opcode = node.opcode + (opcode == "INVOKEVIRTUAL") || (opcode == "INVOKEINTERFACE") + } +} -- cgit v1.2.3 From b0560c5ece29ad9d3f79ad6389b085a2a83fbe18 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Sat, 16 Mar 2013 02:07:42 +0100 Subject: Remove fragile code, made redundant by previous commit The JLS requires, as an exception to generate calls referring to Object for all methods defined in Object itself; hence, whenever the method owner is Object, the call should refer to the method owner. The previous commit implemented this directly in useMethodOwner, so the special case for Object in isInterfaceCall is now redudant. Hence, remove it. --- src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala | 3 +-- src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 55b20154de..0995111688 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -2260,8 +2260,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { hostSymbol.info ; methodOwner.info def isInterfaceCall(sym: Symbol) = ( - //XXX remove the test for ObjectClass. - sym.isInterface && methodOwner != ObjectClass + sym.isInterface || sym.isJavaDefined && sym.isNonBottomSubClass(ClassfileAnnotationClass) ) // whether to reference the type of the receiver or diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala index ed0d8144ca..11d46d9ae7 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala @@ -1197,8 +1197,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with hostSymbol.info ; methodOwner.info def isInterfaceCall(sym: Symbol) = ( - //XXX remove the test for ObjectClass. - sym.isInterface && methodOwner != ObjectClass + sym.isInterface || sym.isJavaDefined && sym.isNonBottomSubClass(ClassfileAnnotationClass) ) // whether to reference the type of the receiver or -- cgit v1.2.3 From 6f4a594534b09d1f129690a3571800d9dff242d3 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Sat, 16 Mar 2013 04:27:57 +0100 Subject: SI-7253: update comments and naming Rename isInterfaceCall -> needsInterfaceCall, which is more accurate since a receiver can't literally be an interface call. --- src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala | 14 +++++++------- src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 0995111688..7c46d648fe 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -2259,12 +2259,12 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { // info calls so that types are up to date; erasure may add lateINTERFACE to traits hostSymbol.info ; methodOwner.info - def isInterfaceCall(sym: Symbol) = ( + def needsInterfaceCall(sym: Symbol) = ( sym.isInterface || sym.isJavaDefined && sym.isNonBottomSubClass(ClassfileAnnotationClass) ) // whether to reference the type of the receiver or - // the type of the method owner (if not an interface!) + // the type of the method owner val useMethodOwner = ( style != Dynamic || hostSymbol.isBottomClass @@ -2291,11 +2291,11 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { } style match { - case Static(true) => dbg("invokespecial"); jcode.invokespecial (jowner, jname, jtype) - case Static(false) => dbg("invokestatic"); jcode.invokestatic (jowner, jname, jtype) - case Dynamic if isInterfaceCall(receiver) => dbg("invokinterface"); jcode.invokeinterface(jowner, jname, jtype) - case Dynamic => dbg("invokevirtual"); jcode.invokevirtual (jowner, jname, jtype) - case SuperCall(_) => + case Static(true) => dbg("invokespecial"); jcode.invokespecial (jowner, jname, jtype) + case Static(false) => dbg("invokestatic"); jcode.invokestatic (jowner, jname, jtype) + case Dynamic if needsInterfaceCall(receiver) => dbg("invokinterface"); jcode.invokeinterface(jowner, jname, jtype) + case Dynamic => dbg("invokevirtual"); jcode.invokevirtual (jowner, jname, jtype) + case SuperCall(_) => dbg("invokespecial") jcode.invokespecial(jowner, jname, jtype) initModule() diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala index 11d46d9ae7..36b294b289 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala @@ -1196,12 +1196,12 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with // info calls so that types are up to date; erasure may add lateINTERFACE to traits hostSymbol.info ; methodOwner.info - def isInterfaceCall(sym: Symbol) = ( + def needsInterfaceCall(sym: Symbol) = ( sym.isInterface || sym.isJavaDefined && sym.isNonBottomSubClass(ClassfileAnnotationClass) ) // whether to reference the type of the receiver or - // the type of the method owner (if not an interface!) + // the type of the method owner val useMethodOwner = ( style != Dynamic || hostSymbol.isBottomClass @@ -1230,11 +1230,11 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with } style match { - case Static(true) => dbg("invokespecial"); jcode.emitINVOKESPECIAL(jowner, jname, jtype) - case Static(false) => dbg("invokestatic"); jcode.emitINVOKESTATIC(jowner, jname, jtype) - case Dynamic if isInterfaceCall(receiver) => dbg("invokinterface"); jcode.emitINVOKEINTERFACE(jowner, jname, jtype) - case Dynamic => dbg("invokevirtual"); jcode.emitINVOKEVIRTUAL(jowner, jname, jtype) - case SuperCall(_) => + case Static(true) => dbg("invokespecial"); jcode.emitINVOKESPECIAL(jowner, jname, jtype) + case Static(false) => dbg("invokestatic"); jcode.emitINVOKESTATIC(jowner, jname, jtype) + case Dynamic if needsInterfaceCall(receiver) => dbg("invokinterface"); jcode.emitINVOKEINTERFACE(jowner, jname, jtype) + case Dynamic => dbg("invokevirtual"); jcode.emitINVOKEVIRTUAL(jowner, jname, jtype) + case SuperCall(_) => dbg("invokespecial") jcode.emitINVOKESPECIAL(jowner, jname, jtype) initModule() -- cgit v1.2.3 From f0468537e8b95d15b81f3480ae24555ddb029d36 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 16 Mar 2013 16:26:32 +0100 Subject: SI-7259 Fix detection of Java defined Selects The fix for SI-3120, 3ff7743, introduced a fallback within `typedSelect` that accounted for the ambiguity of a Java selection syntax. Does `A.B` refer to a member of the type `A` or of the companion object `A`? (The companion object here is a fiction used by scalac to group the static members of a Java class.) The fallback in `typedSelect` was predicated on `context.owner.enclosingTopLevelClass.isJavaDefined`. However, this was incorrectly including Select-s in top-level annotations in Scala files, which are owned by the enclosing package class, which is considered to be Java defined. This led to nonsensical error messages ("type scala not found.") Instead, this commit checks the compilation unit of the context, which is more direct and correct. (As I learned recently, `currentUnit.isJavaDefined` would *not* be correct, as a lazy type might complete a Java signature while compiling some other compilation unit!) A bonus post factum test case is included for SI-3120. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 4 +++- test/files/neg/t7259.check | 7 +++++++ test/files/neg/t7259.scala | 9 +++++++++ test/files/pos/t3120/J1.java | 4 ++++ test/files/pos/t3120/J2.java | 4 ++++ test/files/pos/t3120/Q.java | 3 +++ test/files/pos/t3120/Test.scala | 3 +++ 7 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 test/files/neg/t7259.check create mode 100644 test/files/neg/t7259.scala create mode 100644 test/files/pos/t3120/J1.java create mode 100644 test/files/pos/t3120/J2.java create mode 100644 test/files/pos/t3120/Q.java create mode 100644 test/files/pos/t3120/Test.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index d8493d2312..803768a843 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -4813,7 +4813,9 @@ trait Typers extends Modes with Adaptations with Tags { if (!reallyExists(sym)) { def handleMissing: Tree = { - if (context.owner.enclosingTopLevelClass.isJavaDefined && name.isTypeName) { + if (context.unit.isJava && name.isTypeName) { + // SI-3120 Java uses the same syntax, A.B, to express selection from the + // value A and from the type A. We have to try both. val tree1 = atPos(tree.pos) { gen.convertToSelectFromType(qual, name) } if (tree1 != EmptyTree) return typed1(tree1, mode, pt) } diff --git a/test/files/neg/t7259.check b/test/files/neg/t7259.check new file mode 100644 index 0000000000..0ad627fc3b --- /dev/null +++ b/test/files/neg/t7259.check @@ -0,0 +1,7 @@ +t7259.scala:1: error: not found: type xxxxx +@xxxxx // error: not found: type xxxx + ^ +t7259.scala:8: error: type xxxxx is not a member of package annotation +@annotation.xxxxx // error: not found: type scala + ^ +two errors found diff --git a/test/files/neg/t7259.scala b/test/files/neg/t7259.scala new file mode 100644 index 0000000000..0fdfe18822 --- /dev/null +++ b/test/files/neg/t7259.scala @@ -0,0 +1,9 @@ +@xxxxx // error: not found: type xxxx +class Ok + +// +// This had the wrong error message in 2.9 and 2.10. +// + +@annotation.xxxxx // error: not found: type scala +class WrongErrorMessage diff --git a/test/files/pos/t3120/J1.java b/test/files/pos/t3120/J1.java new file mode 100644 index 0000000000..12b23c1c98 --- /dev/null +++ b/test/files/pos/t3120/J1.java @@ -0,0 +1,4 @@ +class J1 { + public class Inner1 { } + public static class Inner2 { } +} diff --git a/test/files/pos/t3120/J2.java b/test/files/pos/t3120/J2.java new file mode 100644 index 0000000000..db6e859020 --- /dev/null +++ b/test/files/pos/t3120/J2.java @@ -0,0 +1,4 @@ +public class J2 { + public void f1(J1.Inner1 p) { } + public void f2(J1.Inner2 p) { } +} diff --git a/test/files/pos/t3120/Q.java b/test/files/pos/t3120/Q.java new file mode 100644 index 0000000000..fe2269308a --- /dev/null +++ b/test/files/pos/t3120/Q.java @@ -0,0 +1,3 @@ +public class Q { + public static void passInner(J1.Inner1 myInner) {} +} diff --git a/test/files/pos/t3120/Test.scala b/test/files/pos/t3120/Test.scala new file mode 100644 index 0000000000..c02146fba1 --- /dev/null +++ b/test/files/pos/t3120/Test.scala @@ -0,0 +1,3 @@ +object Test { + Q.passInner(null) +} -- cgit v1.2.3 From 99bdebb36d90cb0c9632fb637e3262c8c1b174f3 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sat, 16 Mar 2013 15:04:05 +0100 Subject: removes duplication in FreeDef extractors --- .../scala/reflect/reify/utils/Extractors.scala | 65 ++++++++-------------- 1 file changed, 22 insertions(+), 43 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/reflect/reify/utils/Extractors.scala b/src/compiler/scala/reflect/reify/utils/Extractors.scala index 134ae13890..59cd4e5047 100644 --- a/src/compiler/scala/reflect/reify/utils/Extractors.scala +++ b/src/compiler/scala/reflect/reify/utils/Extractors.scala @@ -164,51 +164,30 @@ trait Extractors { } } - object FreeDef { - def unapply(tree: Tree): Option[(Tree, TermName, Tree, Long, String)] = tree match { - case FreeTermDef(uref, name, binding, flags, origin) => - Some((uref, name, binding, flags, origin)) - case FreeTypeDef(uref, name, binding, flags, origin) => - Some((uref, name, binding, flags, origin)) - case _ => - None - } - } - - object FreeTermDef { - def unapply(tree: Tree): Option[(Tree, TermName, Tree, Long, String)] = tree match { - case - ValDef(_, name, _, Apply( - Select(Select(uref1 @ Ident(_), build1), newFreeTerm), - List( - _, - _, - Apply(Select(Select(uref2 @ Ident(_), build2), flagsFromBits), List(Literal(Constant(flags: Long)))), - Literal(Constant(origin: String))))) - if uref1.name == nme.UNIVERSE_SHORT && build1 == nme.build && newFreeTerm == nme.newFreeTerm && - uref2.name == nme.UNIVERSE_SHORT && build2 == nme.build && flagsFromBits == nme.flagsFromBits => - Some(uref1, name, reifyBinding(tree), flags, origin) - case _ => - None - } - } - - object FreeTypeDef { - def unapply(tree: Tree): Option[(Tree, TermName, Tree, Long, String)] = tree match { - case - ValDef(_, name, _, Apply( - Select(Select(uref1 @ Ident(_), build1), newFreeType), - List( - _, - Apply(Select(Select(uref2 @ Ident(_), build2), flagsFromBits), List(Literal(Constant(flags: Long)))), - Literal(Constant(origin: String))))) - if uref1.name == nme.UNIVERSE_SHORT && build1 == nme.build && newFreeType == nme.newFreeType && - uref2.name == nme.UNIVERSE_SHORT && build2 == nme.build && flagsFromBits == nme.flagsFromBits => - Some(uref1, name, reifyBinding(tree), flags, origin) - case _ => - None + sealed abstract class FreeDefExtractor(acceptTerms: Boolean, acceptTypes: Boolean) { + def unapply(tree: Tree): Option[(Tree, TermName, Tree, Long, String)] = { + def acceptFreeTermFactory(name: Name) = { + (acceptTerms && name == nme.newFreeTerm) || + (acceptTypes && name == nme.newFreeType) + } + tree match { + case + ValDef(_, name, _, Apply( + Select(Select(uref1 @ Ident(_), build1), freeTermFactory), + _ :+ + Apply(Select(Select(uref2 @ Ident(_), build2), flagsFromBits), List(Literal(Constant(flags: Long)))) :+ + Literal(Constant(origin: String)))) + if uref1.name == nme.UNIVERSE_SHORT && build1 == nme.build && acceptFreeTermFactory(freeTermFactory) && + uref2.name == nme.UNIVERSE_SHORT && build2 == nme.build && flagsFromBits == nme.flagsFromBits => + Some(uref1, name, reifyBinding(tree), flags, origin) + case _ => + None + } } } + object FreeDef extends FreeDefExtractor(acceptTerms = true, acceptTypes = true) + object FreeTermDef extends FreeDefExtractor(acceptTerms = true, acceptTypes = false) + object FreeTypeDef extends FreeDefExtractor(acceptTerms = false, acceptTypes = true) object FreeRef { def unapply(tree: Tree): Option[(Tree, TermName)] = tree match { -- cgit v1.2.3 From 5db04eb0ddf53f183a5b61616d037f6b558eba5c Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Mon, 18 Mar 2013 18:35:45 +0100 Subject: an amazing discovery made by Iulian Traces were stalling macro expansions by evaluating messages even when -Ymacro-debug-* flags were disabled. --- src/compiler/scala/tools/nsc/util/SimpleTracer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/util/SimpleTracer.scala b/src/compiler/scala/tools/nsc/util/SimpleTracer.scala index b103ae9cb0..2601798b96 100644 --- a/src/compiler/scala/tools/nsc/util/SimpleTracer.scala +++ b/src/compiler/scala/tools/nsc/util/SimpleTracer.scala @@ -10,7 +10,7 @@ import java.io.PrintStream * @param enabled: A condition that must be true for trace info to be produced. */ class SimpleTracer(out: PrintStream, enabled: Boolean = true) { - def apply[T](msg: String)(value: T): T = { + def apply[T](msg: => String)(value: T): T = { if (enabled) out.println(msg+value) value } -- cgit v1.2.3 From 50ee635e3c5914f75be57209f7a145fbb3e23d80 Mon Sep 17 00:00:00 2001 From: Eugene Vigdorchik Date: Mon, 18 Mar 2013 16:34:15 +0400 Subject: SI-5699 correct java parser for annotation defs. Correct java source parser not to insert a constructor with the type of its value method. --- .../scala/tools/nsc/javac/JavaParsers.scala | 8 +------- test/files/run/t5699.check | 11 ++++++++++ test/files/run/t5699.scala | 24 ++++++++++++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) create mode 100755 test/files/run/t5699.check create mode 100755 test/files/run/t5699.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/javac/JavaParsers.scala b/src/compiler/scala/tools/nsc/javac/JavaParsers.scala index 43a8402fc7..8aa9b81a72 100644 --- a/src/compiler/scala/tools/nsc/javac/JavaParsers.scala +++ b/src/compiler/scala/tools/nsc/javac/JavaParsers.scala @@ -800,13 +800,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners { val pos = in.currentPos val name = identForType() val (statics, body) = typeBody(AT, name) - def getValueMethodType(tree: Tree) = tree match { - case DefDef(_, nme.value, _, _, tpt, _) => Some(tpt.duplicate) - case _ => None - } - var templ = makeTemplate(annotationParents, body) - for (stat <- templ.body; tpt <- getValueMethodType(stat)) - templ = makeTemplate(annotationParents, makeConstructor(List(tpt)) :: templ.body) + val templ = makeTemplate(annotationParents, body) addCompanionObject(statics, atPos(pos) { ClassDef(mods, name, List(), templ) }) diff --git a/test/files/run/t5699.check b/test/files/run/t5699.check new file mode 100755 index 0000000000..df19644ae6 --- /dev/null +++ b/test/files/run/t5699.check @@ -0,0 +1,11 @@ +[[syntax trees at end of parser]] // annodef.java +package { + object MyAnnotation extends { + def () = _ + }; + class MyAnnotation extends scala.annotation.Annotation with _root_.java.lang.annotation.Annotation with scala.annotation.ClassfileAnnotation { + def () = _; + def value(): String + } +} + diff --git a/test/files/run/t5699.scala b/test/files/run/t5699.scala new file mode 100755 index 0000000000..5cef67e3b1 --- /dev/null +++ b/test/files/run/t5699.scala @@ -0,0 +1,24 @@ +import scala.tools.partest.DirectTest +import scala.tools.nsc.util.BatchSourceFile + +object Test extends DirectTest { + // Java code + override def code = """ + |public @interface MyAnnotation { String value(); } + """.stripMargin + + override def extraSettings: String = "-usejavacp -Ystop-after:typer -Xprint:parser" + + override def show(): Unit = { + // redirect err to out, for logging + val prevErr = System.err + System.setErr(System.out) + compile() + System.setErr(prevErr) + } + + override def newSources(sourceCodes: String*) = { + assert(sourceCodes.size == 1) + List(new BatchSourceFile("annodef.java", sourceCodes(0))) + } +} -- cgit v1.2.3 From df292901c3dd9688d1dabf4ebaa9d73a23ebfe80 Mon Sep 17 00:00:00 2001 From: Kato Kazuyoshi Date: Wed, 20 Mar 2013 02:44:05 +0900 Subject: SI-7013 Scaladoc: Fix StackOverflowError No one see the result value of parse and if so, it's tail-recursive. --- src/compiler/scala/tools/nsc/doc/html/SyntaxHigh.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/doc/html/SyntaxHigh.scala b/src/compiler/scala/tools/nsc/doc/html/SyntaxHigh.scala index ee78f4ea7a..6fdaaed75f 100644 --- a/src/compiler/scala/tools/nsc/doc/html/SyntaxHigh.scala +++ b/src/compiler/scala/tools/nsc/doc/html/SyntaxHigh.scala @@ -6,6 +6,7 @@ package scala.tools.nsc.doc.html import scala.xml.NodeSeq +import scala.annotation.tailrec /** Highlight the syntax of Scala code appearing in a `{{{` wiki block * (see method `HtmlPage.blockToHtml`). @@ -209,9 +210,9 @@ private[html] object SyntaxHigh { out.toString } - def parse(pre: String, i: Int): Int = { + @tailrec def parse(pre: String, i: Int): Unit = { out append pre - if (i == buf.length) return i + if (i == buf.length) return buf(i) match { case '\n' => parse("\n", i+1) @@ -277,7 +278,6 @@ private[html] object SyntaxHigh { } else parse(buf(i).toChar.toString, i+1) } - i } parse("", 0) -- cgit v1.2.3 From fc46281f9638ac8c44890b0cfca3aed4096b6197 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Wed, 20 Mar 2013 16:23:35 +0100 Subject: fixes the craziness in JavaUniverse.log This long-standing, but trivial to fix nuisance in the implementation of runtime reflection actively avoided being fixed in both 2.10.0 and 2.10.1. It's finally the time to put it to a rest. --- src/reflect/scala/reflect/runtime/JavaUniverse.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/reflect/scala/reflect/runtime/JavaUniverse.scala b/src/reflect/scala/reflect/runtime/JavaUniverse.scala index e18435d5b0..1b69ca4e89 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverse.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverse.scala @@ -17,7 +17,7 @@ class JavaUniverse extends internal.SymbolTable with ReflectSetup with runtime.S def forInteractive = false def forScaladoc = false - def log(msg: => AnyRef): Unit = println(" [] "+msg) + def log(msg: => AnyRef): Unit = if (settings.debug.value) println(" [] "+msg) type TreeCopier = InternalTreeCopierOps def newStrictTreeCopier: TreeCopier = new StrictTreeCopier -- cgit v1.2.3 From 2e0be8323bdef5582a0f5af84b3de83fcc338be4 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 23 Mar 2013 09:48:33 +0100 Subject: SI-7290 Discard duplicates in switchable alternative patterns. The pattern matcher must not allow duplicates to hit the backend when generating switches. It already eliminates then if they appear on different cases (with an unreachability warning.) This commit does the same for duplicated literal patterns in an alternative pattern: discard and warn. --- .../tools/nsc/transform/patmat/MatchOptimization.scala | 16 +++++++++++++--- test/files/neg/t7290.check | 10 ++++++++++ test/files/neg/t7290.flags | 1 + test/files/neg/t7290.scala | 10 ++++++++++ test/files/run/t7290.scala | 9 +++++++++ 5 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 test/files/neg/t7290.check create mode 100644 test/files/neg/t7290.flags create mode 100644 test/files/neg/t7290.scala create mode 100644 test/files/run/t7290.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala index dcf2413b15..23889058d3 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala @@ -22,7 +22,7 @@ import scala.reflect.internal.util.NoPosition trait MatchOptimization extends MatchTreeMaking with MatchAnalysis { import PatternMatchingStats._ import global.{Tree, Type, Symbol, NoSymbol, CaseDef, atPos, - ConstantType, Literal, Constant, gen, EmptyTree, + ConstantType, Literal, Constant, gen, EmptyTree, distinctBy, Typed, treeInfo, nme, Ident, Apply, If, Bind, lub, Alternative, deriveCaseDef, Match, MethodType, LabelDef, TypeTree, Throw} @@ -442,7 +442,7 @@ trait MatchOptimization extends MatchTreeMaking with MatchAnalysis { case SwitchableTreeMaker(pattern) :: GuardAndBodyTreeMakers(guard, body) => Some(CaseDef(pattern, guard, body)) // alternatives - case AlternativesTreeMaker(_, altss, _) :: GuardAndBodyTreeMakers(guard, body) if alternativesSupported => + case AlternativesTreeMaker(_, altss, pos) :: GuardAndBodyTreeMakers(guard, body) if alternativesSupported => val switchableAlts = altss map { case SwitchableTreeMaker(pattern) :: Nil => Some(pattern) @@ -452,7 +452,17 @@ trait MatchOptimization extends MatchTreeMaking with MatchAnalysis { // succeed if they were all switchable sequence(switchableAlts) map { switchableAlts => - CaseDef(Alternative(switchableAlts), guard, body) + def extractConst(t: Tree) = t match { + case Literal(const) => const + case _ => t + } + // SI-7290 Discard duplicate alternatives that would crash the backend + def distinctAlts = distinctBy(switchableAlts)(extractConst) + if (distinctAlts.size < switchableAlts.size) { + val duplicated = switchableAlts.groupBy(extractConst).flatMap(_._2.drop(1)) + global.currentUnit.warning(pos, s"Pattern contains duplicate alternatives: ${duplicated.mkString(", ")}") + } + CaseDef(Alternative(distinctAlts), guard, body) } case _ => // debug.patmat("can't emit switch for "+ makers) diff --git a/test/files/neg/t7290.check b/test/files/neg/t7290.check new file mode 100644 index 0000000000..15f30e8bce --- /dev/null +++ b/test/files/neg/t7290.check @@ -0,0 +1,10 @@ +t7290.scala:4: error: Pattern contains duplicate alternatives: 0 + case 0 | 0 => 0 + ^ +t7290.scala:5: error: Pattern contains duplicate alternatives: 2, 2, 2, 3 + case 2 | 2 | 2 | 3 | 2 | 3 => 0 + ^ +t7290.scala:6: error: Pattern contains duplicate alternatives: 4 + case 4 | (_ @ 4) => 0 + ^ +three errors found diff --git a/test/files/neg/t7290.flags b/test/files/neg/t7290.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/neg/t7290.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/neg/t7290.scala b/test/files/neg/t7290.scala new file mode 100644 index 0000000000..b9db7f7e8a --- /dev/null +++ b/test/files/neg/t7290.scala @@ -0,0 +1,10 @@ +object Test extends App { + val y = (0: Int) match { + case 1 => 1 + case 0 | 0 => 0 + case 2 | 2 | 2 | 3 | 2 | 3 => 0 + case 4 | (_ @ 4) => 0 + case _ => -1 + } + assert(y == 0, y) +} diff --git a/test/files/run/t7290.scala b/test/files/run/t7290.scala new file mode 100644 index 0000000000..01f7e8f68e --- /dev/null +++ b/test/files/run/t7290.scala @@ -0,0 +1,9 @@ +object Test extends App { + val y = (0: Int) match { + case 1 => 1 + case 0 | 0 => 0 + case 2 | 2 | 2 | 3 | 2 | 3 => 0 + case _ => -1 + } + assert(y == 0, y) +} -- cgit v1.2.3 From dd89b006218d76a74d0185392d5e427c0867a33c Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Sat, 23 Mar 2013 08:59:51 +0100 Subject: SI-7285 Fix match analysis with nested objects. The fix for SI-6146 introduced `nestedMemberType` to enumerate sealed subtypes based on the (prefixed) type of the scrutinee and the symbols of its sealed subclasses. That method needed to widen `ThisType(modSym)`s to `ModuleTypeRef(modSym)` before calling `asSeenFrom`. However, this could lead to confused in the match analysis, which sees `ModuleTypeRef` as distinct from singleton types on the same modules (after all, they aren't =:=). Spurious warnings ensued. This commit makes two changes: - conditionally re-narrow the result of `asSeenFrom` in `nestedMemberType`. - present `a.b.SomeModule.type` as `SomeModule` in warnings emitted by the pattern matcher. --- .../scala/tools/nsc/transform/patmat/Logic.scala | 3 +- src/reflect/scala/reflect/internal/Types.scala | 7 +- test/files/neg/t7285.check | 13 ++++ test/files/neg/t7285.flags | 1 + test/files/neg/t7285.scala | 55 ++++++++++++++ test/files/pos/t7285a.flags | 1 + test/files/pos/t7285a.scala | 83 ++++++++++++++++++++++ test/files/run/t6146b.check | 5 +- 8 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 test/files/neg/t7285.check create mode 100644 test/files/neg/t7285.flags create mode 100644 test/files/neg/t7285.scala create mode 100644 test/files/pos/t7285a.flags create mode 100644 test/files/pos/t7285a.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala index 22eabb6d6f..0fab48028e 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala @@ -574,6 +574,7 @@ trait ScalaLogic extends Interface with Logic with TreeAndTypeAnalysis { assert(tp.isInstanceOf[SingletonType]) val toString = tp match { case ConstantType(c) => c.escapedStringValue + case _ if tp.typeSymbol.isModuleClass => tp.typeSymbol.name.toString case _ => tp.toString } Const.unique(tp, new ValueConst(tp, tp.widen, toString)) @@ -623,4 +624,4 @@ trait ScalaLogic extends Interface with Logic with TreeAndTypeAnalysis { override def toString = "null" } } -} \ No newline at end of file +} diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index a27b37dae5..a2c9f1fadf 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -5036,10 +5036,11 @@ trait Types extends api.Types { self: SymbolTable => if (tp.isTrivial) tp else if (tp.prefix.typeSymbol isNonBottomSubClass owner) { val widened = tp match { - case _: ConstantType => tp // Java enum constants: don't widen to the enum type! - case _ => tp.widen // C.X.type widens to C.this.X.type, otherwise `tp asSeenFrom (pre, C)` has no effect. + case _: ConstantType => tp // Java enum constants: don't widen to the enum type! + case _ => tp.widen // C.X.type widens to C.this.X.type, otherwise `tp asSeenFrom (pre, C)` has no effect. } - widened asSeenFrom (pre, tp.typeSymbol.owner) + val memType = widened asSeenFrom (pre, tp.typeSymbol.owner) + if (tp eq widened) memType else memType.narrow } else loop(tp.prefix) memberType tp.typeSymbol diff --git a/test/files/neg/t7285.check b/test/files/neg/t7285.check new file mode 100644 index 0000000000..108f4292a8 --- /dev/null +++ b/test/files/neg/t7285.check @@ -0,0 +1,13 @@ +t7285.scala:15: error: match may not be exhaustive. +It would fail on the following input: (Up, Down) + (d1, d2) match { + ^ +t7285.scala:33: error: match may not be exhaustive. +It would fail on the following input: Down + (d1) match { + ^ +t7285.scala:51: error: match may not be exhaustive. +It would fail on the following input: (Up, Down) + (d1, d2) match { + ^ +three errors found diff --git a/test/files/neg/t7285.flags b/test/files/neg/t7285.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/neg/t7285.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/neg/t7285.scala b/test/files/neg/t7285.scala new file mode 100644 index 0000000000..14121d92b1 --- /dev/null +++ b/test/files/neg/t7285.scala @@ -0,0 +1,55 @@ +sealed abstract class Base + + +object Test1 { + sealed abstract class Base + + object Base { + case object Down extends Base { + } + + case object Up extends Base { + } + + (d1: Base, d2: Base) => + (d1, d2) match { + case (Up, Up) | (Down, Down) => false + case (Down, Up) => true + } + } +} + +object Test2 { + sealed abstract class Base + + object Base { + case object Down extends Base { + } + + case object Up extends Base { + } + + (d1: Base, d2: Base) => + (d1) match { + case Test2.Base.Up => false + } + } +} + + +object Test4 { + sealed abstract class Base + + object Base { + case object Down extends Base + + case object Up extends Base + } + + import Test4.Base._ + (d1: Base, d2: Base) => + (d1, d2) match { + case (Up, Up) | (Down, Down) => false + case (Down, Test4.Base.Up) => true + } +} diff --git a/test/files/pos/t7285a.flags b/test/files/pos/t7285a.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t7285a.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7285a.scala b/test/files/pos/t7285a.scala new file mode 100644 index 0000000000..34e79c741b --- /dev/null +++ b/test/files/pos/t7285a.scala @@ -0,0 +1,83 @@ +sealed abstract class Base + +object Test { + case object Up extends Base + + def foo(d1: Base) = + d1 match { + case Up => + } + + // Sealed subtype: ModuleTypeRef .this.Test.Up.type + // Pattern: UniqueThisType Test.this.type +} + + +object Test1 { + sealed abstract class Base + + object Base { + case object Down extends Base { + } + + case object Up extends Base { + } + + (d1: Base, d2: Base) => + (d1, d2) match { + case (Up, Up) | (Down, Down) => false + case (Down, Up) => true + case (Up, Down) => false + } + } +} + +object Test2 { + sealed abstract class Base + + object Base { + case object Down extends Base { + } + + case object Up extends Base { + } + + (d1: Base, d2: Base) => + (d1) match { + case Up | Down => false + } + } +} + +object Test3 { + sealed abstract class Base + + object Base { + case object Down extends Base + + (d1: Base, d2: Base) => + (d1, d2) match { + case (Down, Down) => false + } + } +} + +object Test4 { + sealed abstract class Base + + object Base { + case object Down extends Base { + } + + case object Up extends Base { + } + + } + import Test4.Base._ + (d1: Base, d2: Base) => + (d1, d2) match { + case (Up, Up) | (Down, Down) => false + case (Down, Test4.Base.Up) => true + case (Up, Down) => false + } +} diff --git a/test/files/run/t6146b.check b/test/files/run/t6146b.check index e08e40d9fa..49ff70697e 100644 --- a/test/files/run/t6146b.check +++ b/test/files/run/t6146b.check @@ -43,10 +43,7 @@ mt1: u.Type = O.X.S1.type scala> global.typeDeconstruct.show(mt1) res0: String = TypeRef( - pre = TypeRef( - pre = ThisType(object O) - TypeSymbol(class X extends AnyRef) - ) + pre = SingleType(pre = ThisType(object O), object X) TypeSymbol(class S1 extends C.this.F[T]) ) -- cgit v1.2.3 From cd9e03af8d61e7def5df2a0958de31ca0c163780 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 14 Mar 2013 12:50:52 +0400 Subject: SI-7246 Make $outer pointer elision Java aware In e0853b3, a space-saving optimization elided the outer pointer of inner classes if the the (protected) outer pointer of the immediate parent class was guaranteed to point to the same instance. But, this check failed to account for Java parent classes, which don't follow the Scala scheme. This commit disables the optimization in that case. The original test case in e0853b3 was anemic, I've fleshed it out to: - test the presense or absense of $outer pointers with Java reflection - test the optimization works in the presense of aliased and annotated aliased types. (The former worked already, the latter required a change to the implementation.) - Test the negative case when the prefixes don't line up and the subclass in fact needs its own $outer. This patch is based on work by Euguene Vigdorchik with some additions by Jason Zaugg. --- .../scala/tools/nsc/transform/ExplicitOuter.scala | 26 ++++++------ test/files/run/outertest.scala | 47 ++++++++++++++++++---- test/files/run/t7246.check | 1 + test/files/run/t7246/Outer.java | 4 ++ test/files/run/t7246/Test.scala | 16 ++++++++ test/files/run/t7246b.check | 2 + test/files/run/t7246b/Base.scala | 7 ++++ test/files/run/t7246b/Outer.java | 4 ++ test/files/run/t7246b/Test.scala | 14 +++++++ 9 files changed, 99 insertions(+), 22 deletions(-) create mode 100755 test/files/run/t7246.check create mode 100755 test/files/run/t7246/Outer.java create mode 100755 test/files/run/t7246/Test.scala create mode 100755 test/files/run/t7246b.check create mode 100755 test/files/run/t7246b/Base.scala create mode 100755 test/files/run/t7246b/Outer.java create mode 100755 test/files/run/t7246b/Test.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala index 2f28a16416..be3150fc76 100644 --- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala +++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala @@ -45,26 +45,24 @@ abstract class ExplicitOuter extends InfoTransform private def isInner(clazz: Symbol) = !clazz.isPackageClass && !clazz.outerClass.isStaticOwner - private def haveSameOuter(parent: Type, clazz: Symbol) = parent match { - case TypeRef(pre, sym, _) => - val owner = clazz.owner + private def haveSameOuter(parent: Type, clazz: Symbol) = { + val owner = clazz.owner + val parentSym = parent.typeSymbol - //println(s"have same outer $parent $clazz $sym ${sym.owner} $owner $pre") - - sym.isClass && owner.isClass && - (owner isSubClass sym.owner) && - owner.thisType =:= pre - - case _ => false + parentSym.isClass && owner.isClass && + (owner isSubClass parentSym.owner) && + owner.thisType =:= parent.prefix } /** Does given clazz define an outer field? */ def hasOuterField(clazz: Symbol) = { - val parents = clazz.info.parents + val parent = clazz.info.firstParent - isInner(clazz) && !clazz.isTrait && { - parents.isEmpty || !haveSameOuter(parents.head, clazz) - } + // space optimization: inherit the $outer pointer from the parent class if + // we know that it will point to the correct instance. + def canReuseParentOuterField = !parent.typeSymbol.isJavaDefined && haveSameOuter(parent, clazz) + + isInner(clazz) && !clazz.isTrait && !canReuseParentOuterField } private def outerField(clazz: Symbol): Symbol = { diff --git a/test/files/run/outertest.scala b/test/files/run/outertest.scala index 3cc96afa5b..fa0443f669 100644 --- a/test/files/run/outertest.scala +++ b/test/files/run/outertest.scala @@ -1,26 +1,57 @@ // A test for the case where the outer field of class B#J should be eliminated. -// You can verify this by running a javap on B.J + +import reflect.ClassTag + abstract class A { + abstract class I - abstract class I { + val foo = this +} +class B extends A { + class J extends I { + val bar = foo } - val foo = "foo" + type II = I + class K extends II { + val bar = foo + } + class L extends (I @annotation.tailrec) { + val bar = foo + } } -class B extends A { - class J extends I { +class C extends A { + val c: C = this + + class M extends c.I { val bar = foo } - } -object Test extends App { +object Test extends App { val b = new B - assert((new b.J).bar == b.foo) + val c0 = new C + val c = new { override val c = c0 } with C + + assert((new b.J).bar eq b) + assert((new b.K).bar eq b) + assert((new b.L).bar eq b) + assert((new c.M).bar eq c) + + def checkOuterFields[C: ClassTag](expected: Int) { + val cls = implicitly[ClassTag[C]].runtimeClass + val outerFields = cls.getDeclaredFields().filter(_.getName.contains("$outer")) + assert(outerFields.size == expected, outerFields.map(_.getName)) + } + checkOuterFields[A#I](1) // the base class must have the $outer pointer + checkOuterFields[B#J](0) // reuse parent class' $outer pointer + checkOuterFields[B#K](0) // ... through an alias + checkOuterFields[B#L](0) // ... through the annotated type + checkOuterFields[C#M](1) // different prefix, can't share. } diff --git a/test/files/run/t7246.check b/test/files/run/t7246.check new file mode 100755 index 0000000000..ce01362503 --- /dev/null +++ b/test/files/run/t7246.check @@ -0,0 +1 @@ +hello diff --git a/test/files/run/t7246/Outer.java b/test/files/run/t7246/Outer.java new file mode 100755 index 0000000000..163276fb3b --- /dev/null +++ b/test/files/run/t7246/Outer.java @@ -0,0 +1,4 @@ +public class Outer { + public class Inner { + } +} \ No newline at end of file diff --git a/test/files/run/t7246/Test.scala b/test/files/run/t7246/Test.scala new file mode 100755 index 0000000000..9f23ca8f3a --- /dev/null +++ b/test/files/run/t7246/Test.scala @@ -0,0 +1,16 @@ +object Test extends App { + + val so = new SubOuter + val si = new so.SubInner + println(si.bar) +} + +class SubOuter extends Outer { + + val foo = "hello" + + class SubInner extends Inner { + def bar = foo + } + +} \ No newline at end of file diff --git a/test/files/run/t7246b.check b/test/files/run/t7246b.check new file mode 100755 index 0000000000..5073bd8617 --- /dev/null +++ b/test/files/run/t7246b.check @@ -0,0 +1,2 @@ +base +sub diff --git a/test/files/run/t7246b/Base.scala b/test/files/run/t7246b/Base.scala new file mode 100755 index 0000000000..4e71d3313d --- /dev/null +++ b/test/files/run/t7246b/Base.scala @@ -0,0 +1,7 @@ +class Base { + val baseOuter = "base" + + class BaseInner { + val baseInner = baseOuter + } +} diff --git a/test/files/run/t7246b/Outer.java b/test/files/run/t7246b/Outer.java new file mode 100755 index 0000000000..53a79316ef --- /dev/null +++ b/test/files/run/t7246b/Outer.java @@ -0,0 +1,4 @@ +public class Outer extends Base { + public class Inner extends BaseInner { + } +} \ No newline at end of file diff --git a/test/files/run/t7246b/Test.scala b/test/files/run/t7246b/Test.scala new file mode 100755 index 0000000000..f0982ea8d0 --- /dev/null +++ b/test/files/run/t7246b/Test.scala @@ -0,0 +1,14 @@ +object Test extends App { + + val so = new SubOuter + val si = new so.SubInner + println(si.baseInner) + println(si.subInner) +} + +class SubOuter extends Outer { + val subOuter = "sub" + class SubInner extends Inner { + def subInner = subOuter + } +} -- cgit v1.2.3 From b95ca32062ae980fb45b3e93d8c90219bc1d2530 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 25 Mar 2013 12:40:48 +0100 Subject: SI-7299 Improve error message for eta-expanding 23+ param method Before, we got `error: missing arguments for method f`. --- src/compiler/scala/tools/nsc/typechecker/Typers.scala | 10 ++++++++-- test/files/neg/t7299.check | 7 +++++++ test/files/neg/t7299.scala | 6 ++++++ 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 test/files/neg/t7299.check create mode 100644 test/files/neg/t7299.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 2458fc54e1..a98f20a971 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -4461,6 +4461,12 @@ trait Typers extends Modes with Adaptations with Tags { treeCopy.New(tree, tpt1).setType(tp) } + def functionTypeWildcard(tree: Tree, arity: Int): Type = { + val tp = functionType(List.fill(arity)(WildcardType), WildcardType) + if (tp == NoType) MaxFunctionArityError(tree) + tp + } + def typedEta(expr1: Tree): Tree = expr1.tpe match { case TypeRef(_, ByNameParamClass, _) => val expr2 = Function(List(), expr1) setPos expr1.pos @@ -4472,7 +4478,7 @@ trait Typers extends Modes with Adaptations with Tags { typed1(expr2, mode, pt) case PolyType(_, MethodType(formals, _)) => if (isFunctionType(pt)) expr1 - else adapt(expr1, mode, functionType(formals map (t => WildcardType), WildcardType)) + else adapt(expr1, mode, functionTypeWildcard(expr1, formals.length)) case MethodType(formals, _) => if (isFunctionType(pt)) expr1 else expr1 match { @@ -4491,7 +4497,7 @@ trait Typers extends Modes with Adaptations with Tags { val rhs = Apply(f, args) typed(rhs) case _ => - adapt(expr1, mode, functionType(formals map (t => WildcardType), WildcardType)) + adapt(expr1, mode, functionTypeWildcard(expr1, formals.length)) } case ErrorType => expr1 diff --git a/test/files/neg/t7299.check b/test/files/neg/t7299.check new file mode 100644 index 0000000000..74340c4841 --- /dev/null +++ b/test/files/neg/t7299.check @@ -0,0 +1,7 @@ +t7299.scala:4: error: implementation restricts functions to 22 parameters + val eta1 = f _ + ^ +t7299.scala:5: error: implementation restricts functions to 22 parameters + val eta2 = g[Any] _ + ^ +two errors found diff --git a/test/files/neg/t7299.scala b/test/files/neg/t7299.scala new file mode 100644 index 0000000000..f3aae5ce5d --- /dev/null +++ b/test/files/neg/t7299.scala @@ -0,0 +1,6 @@ +object Test { + def f(a1: Int, a2: Int, a3: Int, a4: Int, a5: Int, a6: Int, a7: Int, a8: Int, a9: Int, a10: Int, a11: Int, a12: Int, a13: Int, a14: Int, a15: Int, a16: Int, a17: Int, a18: Int, a19: Int, a20: Int, a21: Int, a22: Int, a23: Int) = 0 + def g[A](a1: Int, a2: Int, a3: Int, a4: Int, a5: Int, a6: Int, a7: Int, a8: Int, a9: Int, a10: Int, a11: Int, a12: Int, a13: Int, a14: Int, a15: Int, a16: Int, a17: Int, a18: Int, a19: Int, a20: Int, a21: Int, a22: Int, a23: Int) = 0 + val eta1 = f _ + val eta2 = g[Any] _ +} -- cgit v1.2.3 From b4344e121c7e195f5244cd704d5296274d503dd1 Mon Sep 17 00:00:00 2001 From: Kato Kazuyoshi Date: Wed, 20 Mar 2013 00:55:06 +0900 Subject: SI-6580 Scaladoc: Should not close void elements Because it will generate a useless element like "". To made matters worse, Scaladoc used to generate the element with attributes (like ). That's why we had SI-6580. --- .../scala/tools/nsc/doc/base/comment/Body.scala | 18 +++++++----- test/scaladoc/run/SI-6580.check | 11 ++++++++ test/scaladoc/run/SI-6580.scala | 32 ++++++++++++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 test/scaladoc/run/SI-6580.check create mode 100644 test/scaladoc/run/SI-6580.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/doc/base/comment/Body.scala b/src/compiler/scala/tools/nsc/doc/base/comment/Body.scala index 02e662da85..eb0d751f3e 100755 --- a/src/compiler/scala/tools/nsc/doc/base/comment/Body.scala +++ b/src/compiler/scala/tools/nsc/doc/base/comment/Body.scala @@ -75,16 +75,20 @@ object EntityLink { def unapply(el: EntityLink): Option[(Inline, LinkTo)] = Some((el.title, el.link)) } final case class HtmlTag(data: String) extends Inline { - def canClose(open: HtmlTag) = { - open.data.stripPrefix("<") == data.stripPrefix("].*\z""".r + private val (isEnd, tagName) = data match { + case Pattern(s1, s2) => + (! s1.isEmpty, Some(s2.toLowerCase)) + case _ => + (false, None) } - def close = { - if (data.indexOf(" HtmlTag(s"") } } /** The summary of a comment, usually its first sentence. There must be exactly one summary per body. */ diff --git a/test/scaladoc/run/SI-6580.check b/test/scaladoc/run/SI-6580.check new file mode 100644 index 0000000000..2fb6ec3258 --- /dev/null +++ b/test/scaladoc/run/SI-6580.check @@ -0,0 +1,11 @@ +Chain(List(Chain(List(Text(Here z(1) is defined as follows:), Text( +), HtmlTag(
), Text( +), Text( ), HtmlTag(), Text( +), HtmlTag(
), Text( +), Text(plus z(1) times), Text( +), HtmlTag(
), Text( +), Text( ), HtmlTag(), Text( +), HtmlTag(
), Text( +), Text(equals QL of something +))))) +Done. diff --git a/test/scaladoc/run/SI-6580.scala b/test/scaladoc/run/SI-6580.scala new file mode 100644 index 0000000000..c544138f44 --- /dev/null +++ b/test/scaladoc/run/SI-6580.scala @@ -0,0 +1,32 @@ +import scala.tools.nsc.doc +import scala.tools.nsc.doc.model._ +import scala.tools.nsc.doc.html.page.{Index, ReferenceIndex} +import scala.tools.partest.ScaladocModelTest + +object Test extends ScaladocModelTest { + override def scaladocSettings = "" + override def code = """ + + object Test { + /** Here z(1) is defined as follows: + *
+ * + *
+ * plus z(1) times + *
+ * + *
+ * equals QL of something + */ + def f = 1 + } + + """ + + def testModel(rootPackage: Package) { + import access._ + + val f = rootPackage._object("Test")._method("f") + println(f.comment.get.short) + } +} -- cgit v1.2.3 From b7cbda78e8e280ac1354a017890cdfd8a9b77290 Mon Sep 17 00:00:00 2001 From: James Iry Date: Mon, 25 Mar 2013 07:16:22 -0700 Subject: Log when invokedynamic instruction is encountered Based on the review of https://github.com/scala/scala/pull/2257, this commit adds a debuglog when an invokedynamic instruction is found during class file parsing as a reminder that the implementation is just a place holder. --- src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala index d38907d182..c304c18c4f 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala @@ -508,6 +508,7 @@ abstract class ICodeReader extends ClassfileParser { code.emit(CALL_METHOD(m, Static(false))) case JVM.invokedynamic => // TODO, this is just a place holder. A real implementation must parse the class constant entry + debuglog("Found JVM invokedynamic instructionm, inserting place holder ICode INVOKE_DYNAMIC.") containsInvokeDynamic = true val poolEntry = in.nextChar in.skip(2) -- cgit v1.2.3 From 4e10b2c833fa846c68b81e94a08d867e7de656aa Mon Sep 17 00:00:00 2001 From: Eugene Vigdorchik Date: Sun, 17 Mar 2013 16:31:45 +0400 Subject: SI-6387 Clones accessor before name expansion When a symbol's name is expanded due to a conflict during composition (e.g. multiple traits with same-named members, but which are not both visible at the language level in the concrete class) the compiler renames some symbols with expanded names which embed the full name of the declaring class to avoid clashes. In the rare cases when the accessor overrides the member in base class, such expansion either results in AbstractMethodError when the base method is abstract, or, even worse, can change the semantics of the program. To avoid such issues, we clone the accessor symbol, clear its ACCESSOR flag and enter the symbol with an unchanged name. --- src/reflect/scala/reflect/internal/Symbols.scala | 35 ++++++++++++++++-------- test/files/run/t6387.check | 1 + test/files/run/t6387.scala | 16 +++++++++++ 3 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 test/files/run/t6387.check create mode 100644 test/files/run/t6387.scala (limited to 'src') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 45c16b7302..7274eeafe0 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -2538,20 +2538,32 @@ trait Symbols extends api.Symbols { self: SymbolTable => } /** change name by appending $$ - * Do the same for any accessed symbols or setters/getters + * Do the same for any accessed symbols or setters/getters. + * If the accessor to be renamed is overriding a base symbol, enter + * a cloned symbol with the original name but without ACCESSOR flag. */ override def expandName(base: Symbol) { - if (!hasFlag(EXPANDEDNAME)) { - setFlag(EXPANDEDNAME) - if (hasAccessorFlag && !isDeferred) { - accessed.expandName(base) - } - else if (hasGetter) { - getter(owner).expandName(base) - setter(owner).expandName(base) - } - name = nme.expandedName(name.toTermName, base) + def expand(sym: Symbol) { + if ((sym eq NoSymbol) || (sym hasFlag EXPANDEDNAME)) () // skip + else sym setFlag EXPANDEDNAME setName nme.expandedName(sym.name.toTermName, base) + } + def cloneAndExpand(accessor: Symbol) { + val clone = accessor.cloneSymbol(accessor.owner, (accessor.flags | ARTIFACT) & ~ACCESSOR) + expand(accessor) + log(s"Expanded overriding accessor to $accessor, but cloned $clone to preserve override") + accessor.owner.info.decls enter clone + } + def expandAccessor(accessor: Symbol) { + if (accessor.isOverridingSymbol) cloneAndExpand(accessor) else expand(accessor) + } + if (hasAccessorFlag && !isDeferred) { + expand(accessed) + } + else if (hasGetter) { + expandAccessor(getter(owner)) + expandAccessor(setter(owner)) } + expand(this) } protected def doCookJavaRawInfo() { @@ -3223,6 +3235,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => override def companionModule = NoSymbol override def companionSymbol = NoSymbol override def isSubClass(that: Symbol) = false + override def isOverridingSymbol = false override def filter(cond: Symbol => Boolean) = this override def defString: String = toString override def locationString: String = "" diff --git a/test/files/run/t6387.check b/test/files/run/t6387.check new file mode 100644 index 0000000000..83b33d238d --- /dev/null +++ b/test/files/run/t6387.check @@ -0,0 +1 @@ +1000 diff --git a/test/files/run/t6387.scala b/test/files/run/t6387.scala new file mode 100644 index 0000000000..bbebb5f511 --- /dev/null +++ b/test/files/run/t6387.scala @@ -0,0 +1,16 @@ +trait A { + def foo: Long +} + +object Test { + def a(): A = new A { + var foo: Long = 1000L + + val test = () => { + foo = 28 + } + } + def main(args: Array[String]) { + println(a().foo) + } +} -- cgit v1.2.3 From 0cc9496310d81354dc79e9145606a97e60bf3add Mon Sep 17 00:00:00 2001 From: Kato Kazuyoshi Date: Wed, 27 Mar 2013 01:30:17 +0900 Subject: Scaladoc: Load scripts at the bottom, and with a defer attribute To improve latency on modern browsers (which supports defer) and old browsers: * https://www.webkit.org/blog/1395/running-scripts-in-webkit/ * http://developer.yahoo.com/blogs/ydn/posts/2007/07/high_performanc_5/ --- .../scala/tools/nsc/doc/html/page/Index.scala | 19 ++++++++++----- .../scala/tools/nsc/doc/html/page/Template.scala | 27 ++++++++++++++-------- test/scaladoc/scalacheck/IndexTest.scala | 7 ------ 3 files changed, 30 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/doc/html/page/Index.scala b/src/compiler/scala/tools/nsc/doc/html/page/Index.scala index c76bdc58d9..8802d7c35c 100644 --- a/src/compiler/scala/tools/nsc/doc/html/page/Index.scala +++ b/src/compiler/scala/tools/nsc/doc/html/page/Index.scala @@ -27,13 +27,19 @@ class Index(universe: doc.Universe, val index: doc.Index) extends HtmlPage { val headers = - - - - - + private val scripts = { + val sources = + (List("jquery.js", "jquery-ui.js", "jquery.layout.js", "scheduler.js", "index.js").map { + x => relativeLinkTo(List(x, "lib")) + }) :+ "index.js" + + sources map { + src => + } + } + val body =
@@ -46,6 +52,7 @@ class Index(universe: doc.Universe, val index: doc.Index) extends HtmlPage {