diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index af4fb7ea8c93..ef1f6e268fc0 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -29,14 +29,6 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { import Primitives.TestOp - /* If the selector type has a member with the right name, - * it is the host class; otherwise the symbol's owner. - */ - def findHostClass(selector: Type, sym: Symbol) = selector member sym.name match { - case NoSymbol => debuglog(s"Rejecting $selector as host class for $sym") ; sym.owner - case _ => selector.typeSymbol - } - /* ---------------- helper utils for generating methods and code ---------------- */ def emit(opc: Int): Unit = { mnode.visitInsn(opc) } @@ -65,12 +57,14 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { lineNumber(tree) tree match { - case Assign(lhs @ Select(_, _), rhs) => + case Assign(lhs @ Select(qual, _), rhs) => val isStatic = lhs.symbol.isStaticMember if (!isStatic) { genLoadQualifier(lhs) } genLoad(rhs, symInfoTK(lhs.symbol)) lineNumber(tree) - fieldStore(lhs.symbol) + // receiverClass is used in the bytecode to access the field. using sym.owner may lead to IllegalAccessError + val receiverClass = qual.tpe.typeSymbol + fieldStore(lhs.symbol, receiverClass) case Assign(lhs, rhs) => val s = lhs.symbol @@ -170,19 +164,12 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { bc.aload(elementType) } else if (isArraySet(code)) { - args match { - case a1 :: a2 :: Nil => - genLoad(a1, INT) - genLoad(a2) - // the following line should really be here, but because of bugs in erasure - // we pretend we generate whatever type is expected from us. - //generatedType = UNIT - bc.astore(elementType) - case _ => - abort(s"Too many arguments for array set operation: $tree") - } - } - else { + val List(a1, a2) = args + genLoad(a1, INT) + genLoad(a2) + generatedType = UNIT + bc.astore(elementType) + } else { generatedType = INT emit(asm.Opcodes.ARRAYLENGTH) } @@ -358,26 +345,24 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { assert(tree.symbol.isModule, s"Selection of non-module from empty package: $tree sym: ${tree.symbol} at: ${tree.pos}") genLoadModule(tree) - case Select(qualifier, selector) => + case Select(qualifier, _) => val sym = tree.symbol generatedType = symInfoTK(sym) - val hostClass = findHostClass(qualifier.tpe, sym) - debuglog(s"Host class of $sym with qual $qualifier (${qualifier.tpe}) is $hostClass") val qualSafeToElide = isQualifierSafeToElide(qualifier) def genLoadQualUnlessElidable(): Unit = { if (!qualSafeToElide) { genLoadQualifier(tree) } } + // receiverClass is used in the bytecode to access the field. using sym.owner may lead to IllegalAccessError + def receiverClass = qualifier.tpe.typeSymbol if (sym.isModule) { genLoadQualUnlessElidable() genLoadModule(tree) - } - else if (sym.isStaticMember) { + } else if (sym.isStaticMember) { genLoadQualUnlessElidable() - fieldLoad(sym, hostClass) - } - else { + fieldLoad(sym, receiverClass) + } else { genLoadQualifier(tree) - fieldLoad(sym, hostClass) + fieldLoad(sym, receiverClass) } case t @ Ident(name) => @@ -444,24 +429,18 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { /* * must-single-thread */ - def fieldLoad( field: Symbol, hostClass: Symbol = null): Unit = { - fieldOp(field, isLoad = true, hostClass) - } + def fieldLoad( field: Symbol, hostClass: Symbol = null): Unit = fieldOp(field, isLoad = true, hostClass) + /* * must-single-thread */ - def fieldStore(field: Symbol, hostClass: Symbol = null): Unit = { - fieldOp(field, isLoad = false, hostClass) - } + def fieldStore(field: Symbol, hostClass: Symbol = null): Unit = fieldOp(field, isLoad = false, hostClass) /* * must-single-thread */ private def fieldOp(field: Symbol, isLoad: Boolean, hostClass: Symbol): Unit = { - // LOAD_FIELD.hostClass , CALL_METHOD.hostClass , and #4283 - val owner = - if (hostClass == null) internalName(field.owner) - else internalName(hostClass) + val owner = internalName(if (hostClass == null) field.owner else hostClass) val fieldJName = field.javaSimpleName.toString val fieldDescr = symInfoTK(field).descriptor val isStatic = field.isStaticMember @@ -706,19 +685,33 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { if (t.symbol ne Object_synchronized) genTypeApply(t) else genSynchronized(app, expectedType) - // 'super' call: Note: since constructors are supposed to - // return an instance of what they construct, we have to take - // special care. On JVM they are 'void', and Scala forbids (syntactically) - // to call super constructors explicitly and/or use their 'returned' value. - // therefore, we can ignore this fact, and generate code that leaves nothing - // on the stack (contrary to what the type in the AST says). - case Apply(fun @ Select(Super(_, mix), _), args) => - val invokeStyle = InvokeStyle.Super - // if (fun.symbol.isConstructor) Static(true) else SuperCall(mix); + case Apply(fun @ Select(Super(_, _), _), args) => + def initModule(): Unit = { + // we initialize the MODULE$ field immediately after the super ctor + if (!isModuleInitialized && + jMethodName == INSTANCE_CONSTRUCTOR_NAME && + fun.symbol.javaSimpleName.toString == INSTANCE_CONSTRUCTOR_NAME && + claszSymbol.isStaticModuleClass) { + isModuleInitialized = true + mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) + mnode.visitFieldInsn( + asm.Opcodes.PUTSTATIC, + thisName, + MODULE_INSTANCE_FIELD, + "L" + thisName + ";" + ) + } + } + // 'super' call: Note: since constructors are supposed to + // return an instance of what they construct, we have to take + // special care. On JVM they are 'void', and Scala forbids (syntactically) + // to call super constructors explicitly and/or use their 'returned' value. + // therefore, we can ignore this fact, and generate code that leaves nothing + // on the stack (contrary to what the type in the AST says). mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) genLoadArguments(args, paramTKs(app)) - genCallMethod(fun.symbol, invokeStyle, pos = app.pos) - generatedType = asmMethodType(fun.symbol).returnType + generatedType = genCallMethod(fun.symbol, InvokeStyle.Super, app.pos) + initModule() // 'new' constructor call: Note: since constructors are // thought to return an instance of what they construct, @@ -740,7 +733,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { mnode.visitTypeInsn(asm.Opcodes.NEW, rt.internalName) bc dup generatedType genLoadArguments(args, paramTKs(app)) - genCallMethod(ctor, InvokeStyle.Special) + genCallMethod(ctor, InvokeStyle.Special, app.pos) case _ => abort(s"Cannot instantiate $tpt of kind: $generatedType") @@ -763,58 +756,57 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { case app @ Apply(fun, args) => val sym = fun.symbol - if (sym.isLabel) { // jump to a label + if (sym.isLabel) { // jump to a label assert(int.hasLabelDefs) genLoadLabelArguments(args, labelDef(sym), app.pos) bc goTo programPoint(sym) } else if (isPrimitive(fun)) { // primitive method call generatedType = genPrimitiveOp(app, expectedType) - } else { // normal method call - - def genNormalMethodCall(): Unit = { - - val invokeStyle = - if (sym.isStaticMember) InvokeStyle.Static - else if (sym.isPrivate || sym.isClassConstructor) InvokeStyle.Special - else InvokeStyle.Virtual - - if (invokeStyle.hasInstance) { - genLoadQualifier(fun) + } else { // normal method call + val invokeStyle = + if (sym.isStaticMember) InvokeStyle.Static + else if (sym.isPrivate || sym.isClassConstructor) InvokeStyle.Special + else InvokeStyle.Virtual + + if (invokeStyle.hasInstance) genLoadQualifier(fun) + genLoadArguments(args, paramTKs(app)) + + val Select(qual, _) = fun // fun is a Select, also checked in genLoadQualifier + if (isArrayClone(fun)) { + // Special-case Array.clone, introduced in 36ef60e. The goal is to generate this call + // as "[I.clone" instead of "java/lang/Object.clone". This is consistent with javac. + // Arrays have a public method `clone` (jls 10.7). + // + // The JVMS is not explicit about this, but that receiver type can be an array type + // descriptor (instead of a class internal name): + // invokevirtual #2; //Method "[I".clone:()Ljava/lang/Object + // + // Note that using `Object.clone()` would work as well, but only because the JVM + // relaxes protected access specifically if the receiver is an array: + // http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/87ee5ee27509/src/share/vm/interpreter/linkResolver.cpp#l439 + // Example: `class C { override def clone(): Object = "hi" }` + // Emitting `def f(c: C) = c.clone()` as `Object.clone()` gives a VerifyError. + val target: String = tpeTK(qual).asRefBType.classOrArrayType + val methodBType = asmMethodType(sym) + bc.invokevirtual(target, sym.javaSimpleName.toString, methodBType.descriptor) + generatedType = methodBType.returnType + } else { + val receiverClass = if (!invokeStyle.isVirtual) null else { + // receiverClass is used in the bytecode to as the method receiver. using sym.owner + // may lead to IllegalAccessErrors, see 9954eaf / aladdin bug 455. + val qualSym = qual.tpe.typeSymbol + if (qualSym == ArrayClass) { + // For invocations like `Array(1).hashCode` or `.wait()`, use Object as receiver + // in the bytecode. Using the array descriptor (like we do for clone above) seems + // to work as well, but it seems safer not to change this. Javac also uses Object. + // Note that array apply/update/length are handled by isPrimitive (above). + assert(sym.owner == ObjectClass, s"unexpected array call: $app") + ObjectClass + } else qualSym } - - genLoadArguments(args, paramTKs(app)) - - // In "a couple cases", squirrel away a extra information (hostClass, targetTypeKind). TODO Document what "in a couple cases" refers to. - var hostClass: Symbol = null - - fun match { - case Select(qual, _) if isArrayClone(fun) && invokeStyle.isVirtual => - val targetTypeKind = tpeTK(qual) - val target: String = targetTypeKind.asRefBType.classOrArrayType - bc.invokevirtual(target, "clone", "()Ljava/lang/Object;") - generatedType = targetTypeKind - - case Select(qual, _) => - val qualSym = findHostClass(qual.tpe, sym) - - hostClass = qualSym - if (qual.tpe.typeSymbol != qualSym) { - log(s"Precisified host class for $sym from ${qual.tpe.typeSymbol.fullName} to ${qualSym.fullName}") - } - genCallMethod(sym, invokeStyle, hostClass, app.pos) - generatedType = asmMethodType(sym).returnType - - case _ => - genCallMethod(sym, invokeStyle, hostClass, app.pos) - generatedType = asmMethodType(sym).returnType - - } - } // end of genNormalMethodCall() - - genNormalMethodCall() - + generatedType = genCallMethod(sym, invokeStyle, app.pos, receiverClass) + } } - } generatedType @@ -1113,7 +1105,6 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { def genStringConcat(tree: Tree): BType = { lineNumber(tree) liftStringConcat(tree) match { - // Optimization for expressions of the form "" + x. We can avoid the StringBuilder. case List(Literal(Constant("")), arg) => genLoad(arg, ObjectReference) @@ -1135,77 +1126,80 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { bc.genConcat(elemType) } bc.genEndConcat - } - StringRef } - def genCallMethod(method: Symbol, style: InvokeStyle, hostClass0: Symbol = null, pos: Position = NoPosition): Unit = { - - val siteSymbol = claszSymbol - val hostSymbol = if (hostClass0 == null) method.owner else hostClass0 + /** + * Generate a method invocation. If `specificReceiver != null`, it is used as receiver in the + * invocation instruction, otherwise `method.owner`. A specific receiver class is needed to + * prevent an IllegalAccessError, (aladdin bug 455). + */ + def genCallMethod(method: Symbol, style: InvokeStyle, pos: Position = NoPosition, specificReceiver: Symbol = null): BType = { val methodOwner = method.owner - // info calls so that types are up to date; erasure may add lateINTERFACE to traits - hostSymbol.info ; methodOwner.info - def needsInterfaceCall(sym: Symbol) = ( - sym.isInterface - || sym.isJavaDefined && sym.isNonBottomSubClass(ClassfileAnnotationClass) - ) + // the class used in the invocation's method descriptor in the classfile + val receiverClass = { + if (specificReceiver != null) + assert(style.isVirtual || specificReceiver == methodOwner, s"specificReceiver can only be specified for virtual calls. $method - $specificReceiver") + + val useSpecificReceiver = specificReceiver != null && !specificReceiver.isBottomClass + val receiver = if (useSpecificReceiver) specificReceiver else methodOwner + + // workaround for a JVM bug: https://bugs.openjdk.java.net/browse/JDK-8154587 + // when an interface method overrides a member of Object (note that all interfaces implicitly + // have superclass Object), the receiver needs to be the interface declaring the override (and + // not a sub-interface that inherits it). example: + // trait T { override def clone(): Object = "" } + // trait U extends T + // class C extends U + // class D { def f(u: U) = u.clone() } + // The invocation `u.clone()` needs `T` as a receiver: + // - using Object is illegal, as Object.clone is protected + // - using U results in a `NoSuchMethodError: U.clone. This is the JVM bug. + // Note that a mixin forwarder is generated, so the correct method is executed in the end: + // class C { override def clone(): Object = super[T].clone() } + val isTraitMethodOverridingObjectMember = { + receiver != methodOwner && // fast path - the boolean is used to pick either of these two, if they are the same it does not matter + style.isVirtual && + receiver.isEmittedInterface && + Object_Type.decl(method.name).exists && { // fast path - compute overrideChain on the next line only if necessary + val syms = method.allOverriddenSymbols + !syms.isEmpty && syms.last.owner == ObjectClass + } + } + if (isTraitMethodOverridingObjectMember) methodOwner else receiver + } + + receiverClass.info // ensure types the type is up to date; erasure may add lateINTERFACE to traits + val receiverName = internalName(receiverClass) - // whether to reference the type of the receiver or - // the type of the method owner - val useMethodOwner = ( - !style.isVirtual - || hostSymbol.isBottomClass - || methodOwner == ObjectClass - ) - val receiver = if (useMethodOwner) methodOwner else hostSymbol - val jowner = internalName(receiver) val jname = method.javaSimpleName.toString val bmType = asmMethodType(method) val mdescr = bmType.descriptor - def initModule(): Unit = { - // we initialize the MODULE$ field immediately after the super ctor - if (!isModuleInitialized && - jMethodName == INSTANCE_CONSTRUCTOR_NAME && - jname == INSTANCE_CONSTRUCTOR_NAME && - siteSymbol.isStaticModuleClass) { - isModuleInitialized = true - mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) - mnode.visitFieldInsn( - asm.Opcodes.PUTSTATIC, - thisName, - MODULE_INSTANCE_FIELD, - "L" + thisName + ";" - ) + val isInterface = receiverClass.isEmittedInterface + import InvokeStyle._ + if (style == Super) { + // DOTTY: this differ from how super-calls in traits are handled in the scalac backend, + // this is intentional but could change in the future, see https://github.com/lampepfl/dotty/issues/5928 + bc.invokespecial(receiverName, jname, mdescr, isInterface) + } else { + val opc = style match { + case Static => Opcodes.INVOKESTATIC + case Special => Opcodes.INVOKESPECIAL + case Virtual => if (isInterface) Opcodes.INVOKEINTERFACE else Opcodes.INVOKEVIRTUAL } + bc.emitInvoke(opc, receiverName, jname, mdescr, isInterface) } - val isInterface = receiver.isEmittedInterface - if (style.isStatic) { bc.invokestatic (jowner, jname, mdescr, itf = isInterface) } - else if (style.isSpecial) { bc.invokespecial (jowner, jname, mdescr, itf = isInterface) } - else if (style.isVirtual) { - if (needsInterfaceCall(receiver)) { bc.invokeinterface(jowner, jname, mdescr) } - else { bc.invokevirtual (jowner, jname, mdescr) } - } - else { - assert(style.isSuper, s"An unknown InvokeStyle: $style") - bc.invokespecial(jowner, jname, mdescr, itf = isInterface) - initModule() - } - + bmType.returnType } // end of genCallMethod() /* Generate the scala ## method. */ def genScalaHash(tree: Tree): BType = { - genLoadModule(ScalaRunTimeModule) // TODO why load ScalaRunTimeModule if ## has InvokeStyle of Static(false) ? genLoad(tree, ObjectReference) genCallMethod(hashMethodSym, InvokeStyle.Static) - - INT } /* diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala index 69e67a15d707..81e3284e1fff 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala @@ -6,6 +6,7 @@ import scala.tools.asm import scala.annotation.switch import scala.collection.mutable import Primitives.{NE, EQ, TestOp, ArithmeticOp} +import scala.tools.asm.tree.MethodInsnNode /* * A high-level facade to the ASM API for bytecode generation. @@ -104,7 +105,7 @@ trait BCodeIdiomatic { */ abstract class JCodeMethodN { - def jmethod: asm.MethodVisitor + def jmethod: asm.tree.MethodNode import asm.Opcodes; @@ -394,21 +395,27 @@ trait BCodeIdiomatic { // can-multi-thread final def invokespecial(owner: String, name: String, desc: String, itf: Boolean): Unit = { - jmethod.visitMethodInsn(Opcodes.INVOKESPECIAL, owner, name, desc, itf) + emitInvoke(Opcodes.INVOKESPECIAL, owner, name, desc, itf) } // can-multi-thread final def invokestatic(owner: String, name: String, desc: String, itf: Boolean): Unit = { - jmethod.visitMethodInsn(Opcodes.INVOKESTATIC, owner, name, desc, itf) + emitInvoke(Opcodes.INVOKESTATIC, owner, name, desc, itf) } // can-multi-thread final def invokeinterface(owner: String, name: String, desc: String): Unit = { - jmethod.visitMethodInsn(Opcodes.INVOKEINTERFACE, owner, name, desc, true) + emitInvoke(Opcodes.INVOKEINTERFACE, owner, name, desc, itf = true) } // can-multi-thread final def invokevirtual(owner: String, name: String, desc: String): Unit = { - jmethod.visitMethodInsn(Opcodes.INVOKEVIRTUAL, owner, name, desc, false) + emitInvoke(Opcodes.INVOKEVIRTUAL, owner, name, desc, itf = false) } + def emitInvoke(opcode: Int, owner: String, name: String, desc: String, itf: Boolean): Unit = { + val node = new MethodInsnNode(opcode, owner, name, desc, itf) + jmethod.instructions.add(node) + } + + // can-multi-thread final def goTo(label: asm.Label): Unit = { jmethod.visitJumpInsn(Opcodes.GOTO, label) } // can-multi-thread diff --git a/compiler/src/dotty/tools/backend/jvm/BackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/BackendInterface.scala index 5a9c2baa0f55..c34946445284 100644 --- a/compiler/src/dotty/tools/backend/jvm/BackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/BackendInterface.scala @@ -442,6 +442,8 @@ abstract class BackendInterface extends BackendInterfaceDefinitions { } abstract class SymbolHelper { + def exists: Boolean + // names def showFullName: String def javaSimpleName: String @@ -534,6 +536,7 @@ abstract class BackendInterface extends BackendInterfaceDefinitions { def enclosingClassSym: Symbol def originalLexicallyEnclosingClass: Symbol def nextOverriddenSymbol: Symbol + def allOverriddenSymbols: List[Symbol] // members @@ -603,6 +606,7 @@ abstract class BackendInterface extends BackendInterfaceDefinitions { */ def sortedMembersBasedOnFlags(required: Flags, excluded: Flags): List[Symbol] def members: List[Symbol] + def decl(name: Name): Symbol def decls: List[Symbol] def underlying: Type def parents: List[Type] diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index 789934722e6a..b30a58606998 100644 --- a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -627,6 +627,8 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma } implicit def symHelper(sym: Symbol): SymbolHelper = new SymbolHelper { + def exists: Boolean = sym.exists + // names def showFullName: String = sym.showFullName def javaSimpleName: String = toDenot(sym).name.mangledString // addModuleSuffix(simpleName.dropLocal) @@ -680,7 +682,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma (sym.is(Flags.JavaStatic) || toDenot(sym).hasAnnotation(ctx.definitions.ScalaStaticAnnot)) // guard against no sumbol cause this code is executed to select which call type(static\dynamic) to use to call array.clone - def isBottomClass: Boolean = (sym ne defn.NullClass) && (sym ne defn.NothingClass) + def isBottomClass: Boolean = (sym eq defn.NullClass) || (sym eq defn.NothingClass) def isBridge: Boolean = sym.is(Flags.Bridge) def isArtifact: Boolean = sym.is(Flags.Artifact) def hasEnumFlag: Boolean = sym.isAllOf(Flags.JavaEnumTrait) @@ -759,12 +761,14 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma toDenot(sym)(shiftedContext).lexicallyEnclosingClass(shiftedContext) } else NoSymbol def nextOverriddenSymbol: Symbol = toDenot(sym).nextOverriddenSymbol + def allOverriddenSymbols: List[Symbol] = toDenot(sym).allOverriddenSymbols.toList // members def primaryConstructor: Symbol = toDenot(sym).primaryConstructor /** For currently compiled classes: All locally defined classes including local classes. * The empty list for classes that are not currently compiled. + */ def nestedClasses: List[Symbol] = definedClasses(ctx.flattenPhase) @@ -876,6 +880,8 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma def memberInfo(s: Symbol): Type = tp.memberInfo(s) + def decl(name: Name): Symbol = tp.decl(name).symbol + def decls: List[Symbol] = tp.decls.toList def members: List[Symbol] = tp.allMembers.map(_.symbol).toList diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala index 55914caabb47..464929c6ff53 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala @@ -24,6 +24,7 @@ import java.io.{File => JFile, InputStream} trait DottyBytecodeTest { import AsmNode._ import ASMConverters._ + import DottyBytecodeTest._ protected object Opcode { val newarray = 188 @@ -81,6 +82,24 @@ trait DottyBytecodeTest { classNode.fields.asScala.find(_.name == name) getOrElse sys.error(s"Didn't find field '$name' in class '${classNode.name}'") + def getInstructions(c: ClassNode, name: String): List[Instruction] = + instructionsFromMethod(getMethod(c, name)) + + def assertSameCode(method: MethodNode, expected: List[Instruction]): Unit = + assertSameCode(instructionsFromMethod(method).dropNonOp, expected) + def assertSameCode(actual: List[Instruction], expected: List[Instruction]): Unit = { + assert(actual === expected, s"\nExpected: $expected\nActual : $actual") + } + + def assertInvoke(m: MethodNode, receiver: String, method: String): Unit = + assertInvoke(instructionsFromMethod(m), receiver, method) + def assertInvoke(l: List[Instruction], receiver: String, method: String): Unit = { + assert(l.exists { + case Invoke(_, `receiver`, `method`, _, _) => true + case _ => false + }, l.stringLines) + } + def diffInstructions(isa: List[Instruction], isb: List[Instruction]): String = { val len = Math.max(isa.length, isb.length) val sb = new StringBuilder @@ -194,3 +213,9 @@ trait DottyBytecodeTest { ) } } +object DottyBytecodeTest { + implicit class listStringLines[T](val l: List[T]) extends AnyVal { + def stringLines = l.mkString("\n") + } +} + diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 3f2a7273d5b3..39331d541ce0 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -8,6 +8,8 @@ import asm._ import asm.tree._ import scala.tools.asm.Opcodes +import scala.jdk.CollectionConverters._ + class TestBCode extends DottyBytecodeTest { import ASMConverters._ @@ -783,4 +785,134 @@ class TestBCode extends DottyBytecodeTest { assertParamName(b2, "evidence$2") } } + + @Test + def invocationReceivers(): Unit = { + import Opcodes._ + + checkBCode(List(invocationReceiversTestCode.definitions("Object"))) { dir => + val c1 = loadClassNode(dir.lookupName("C1.class", directory = false).input) + val c2 = loadClassNode(dir.lookupName("C2.class", directory = false).input) + // Scala 2 uses "invokestatic T.clone$" here, see https://github.com/lampepfl/dotty/issues/5928 + assertSameCode(getMethod(c1, "clone"), List(VarOp(ALOAD, 0), Invoke(INVOKESPECIAL, "T", "clone", "()Ljava/lang/Object;", true), Op(ARETURN))) + assertInvoke(getMethod(c1, "f1"), "T", "clone") + assertInvoke(getMethod(c1, "f2"), "T", "clone") + assertInvoke(getMethod(c1, "f3"), "C1", "clone") + assertInvoke(getMethod(c2, "f1"), "T", "clone") + assertInvoke(getMethod(c2, "f2"), "T", "clone") + assertInvoke(getMethod(c2, "f3"), "C1", "clone") + } + checkBCode(List(invocationReceiversTestCode.definitions("String"))) { dir => + val c1b = loadClassNode(dir.lookupName("C1.class", directory = false).input) + val c2b = loadClassNode(dir.lookupName("C2.class", directory = false).input) + val tb = loadClassNode(dir.lookupName("T.class", directory = false).input) + val ub = loadClassNode(dir.lookupName("U.class", directory = false).input) + + def ms(c: ClassNode, n: String) = c.methods.asScala.toList.filter(_.name == n) + assert(ms(tb, "clone").length == 1) + assert(ms(ub, "clone").isEmpty) + val List(c1Clone) = ms(c1b, "clone").filter(_.desc == "()Ljava/lang/Object;") + assert((c1Clone.access | Opcodes.ACC_BRIDGE) != 0) + assertSameCode(c1Clone, List(VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "C1", "clone", "()Ljava/lang/String;", false), Op(ARETURN))) + + def iv(m: MethodNode) = getInstructions(c1b, "f1").collect({case i: Invoke => i}) + assertSameCode(iv(getMethod(c1b, "f1")), List(Invoke(INVOKEINTERFACE, "T", "clone", "()Ljava/lang/String;", true))) + assertSameCode(iv(getMethod(c1b, "f2")), List(Invoke(INVOKEINTERFACE, "T", "clone", "()Ljava/lang/String;", true))) + // invokeinterface T.clone in C1 is OK here because it is not an override of Object.clone (different signature) + assertSameCode(iv(getMethod(c1b, "f3")), List(Invoke(INVOKEINTERFACE, "T", "clone", "()Ljava/lang/String;", true))) + } + } + + @Test + def invocationReceiversProtected(): Unit = { + // http://lrytz.github.io/scala-aladdin-bugtracker/displayItem.do%3Fid=455.html / 9954eaf + // also https://github.com/scala/bug/issues/1430 / 0bea2ab (same but with interfaces) + val aC = + """package a; + |/*package private*/ abstract class A { + | public int f() { return 1; } + | public int t; + |} + """.stripMargin + val bC = + """package a; + |public class B extends A { } + """.stripMargin + val iC = + """package a; + |/*package private*/ interface I { int f(); } + """.stripMargin + val jC = + """package a; + |public interface J extends I { } + """.stripMargin + val cC = + """package b + |class C { + | def f1(b: a.B) = b.f + | def f2(b: a.B) = { b.t = b.t + 1 } + | def f3(j: a.J) = j.f + |} + """.stripMargin + + checkBCode(scalaSources = List(cC), javaSources = List(aC, bC, iC, jC)) { dir => + val clsIn = dir.subdirectoryNamed("b").lookupName("C.class", directory = false).input + val c = loadClassNode(clsIn) + + assertInvoke(getMethod(c, "f1"), "a/B", "f") // receiver needs to be B (A is not accessible in class C, package b) + assertInvoke(getMethod(c, "f3"), "a/J", "f") // receiver needs to be J + } + } + + @Test + def specialInvocationReceivers(): Unit = { + val code = + """class C { + | def f1(a: Array[String]) = a.clone() + | def f2(a: Array[Int]) = a.hashCode() + | def f3(n: Nothing) = n.hashCode() + | def f4(n: Null) = n.toString() + | + |} + """.stripMargin + checkBCode(List(code)) { dir => + val c = loadClassNode(dir.lookupName("C.class", directory = false).input) + + assertInvoke(getMethod(c, "f1"), "[Ljava/lang/String;", "clone") // array descriptor as receiver + assertInvoke(getMethod(c, "f2"), "java/lang/Object", "hashCode") // object receiver + assertInvoke(getMethod(c, "f3"), "java/lang/Object", "hashCode") + assertInvoke(getMethod(c, "f4"), "java/lang/Object", "toString") + } + } +} + +object invocationReceiversTestCode { + // if cloneType is more specific than Object (e.g., String), a bridge method is generated. + def definitions(cloneType: String): String = + s"""trait T { override def clone(): $cloneType = "hi" } + |trait U extends T + |class C1 extends U with Cloneable { + | // The comments below are true when `cloneType` is Object. + | // C1 gets a forwarder for clone that invokes T.clone. this is needed because JVM method + | // resolution always prefers class members, so it would resolve to Object.clone, even if + | // C1 is a subtype of the interface T which has an overriding default method for clone. + | + | // invokeinterface T.clone + | def f1 = (this: T).clone() + | + | // cannot invokeinterface U.clone (NoSuchMethodError). Object.clone would work here, but + | // not in the example in C2 (illegal access to protected). T.clone works in all cases and + | // resolves correctly. + | def f2 = (this: U).clone() + | + | // invokevirtual C1.clone() + | def f3 = (this: C1).clone() + |} + | + |class C2 { + | def f1(t: T) = t.clone() // invokeinterface T.clone + | def f2(t: U) = t.clone() // invokeinterface T.clone -- Object.clone would be illegal (protected, explained in C1) + | def f3(t: C1) = t.clone() // invokevirtual C1.clone -- Object.clone would be illegal + |} + """.stripMargin } diff --git a/compiler/test/dotty/tools/backend/jvm/InlineBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/InlineBytecodeTests.scala index 69f00168e93f..3d26c871ecaa 100644 --- a/compiler/test/dotty/tools/backend/jvm/InlineBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/InlineBytecodeTests.scala @@ -341,7 +341,7 @@ class InlineBytecodeTests extends DottyBytecodeTest { VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "Test", "given_Int", "()I", false), Invoke(INVOKESTATIC, "scala/runtime/BoxesRunTime", "boxToInteger", "(I)Ljava/lang/Integer;", false), - Invoke(INVOKEINTERFACE, "scala/Function1", "apply", "(Ljava/lang/Object;)Ljava/lang/Object;", true), + Invoke(INVOKEINTERFACE, "dotty/runtime/function/JFunction1$mcZI$sp", "apply", "(Ljava/lang/Object;)Ljava/lang/Object;", true), Invoke(INVOKESTATIC, "scala/runtime/BoxesRunTime", "unboxToBoolean", "(Ljava/lang/Object;)Z", false), Op(IRETURN) ) @@ -352,7 +352,7 @@ class InlineBytecodeTests extends DottyBytecodeTest { } assert(instructions.tail == expected, - "`fg was not properly inlined in `test`\n" + diffInstructions(instructions, expected)) + "`fg was not properly inlined in `test`\n" + diffInstructions(instructions.tail, expected)) } } diff --git a/tests/pos-java-interop/i6546/A.java b/tests/pos-java-interop/i6546/A.java new file mode 100644 index 000000000000..d1297891217f --- /dev/null +++ b/tests/pos-java-interop/i6546/A.java @@ -0,0 +1,8 @@ +package pkg; +class P { + public void foo() { + } +} + +public class A extends P { +} diff --git a/tests/pos-java-interop/i6546/B.scala b/tests/pos-java-interop/i6546/B.scala new file mode 100644 index 000000000000..6a233c04fc79 --- /dev/null +++ b/tests/pos-java-interop/i6546/B.scala @@ -0,0 +1,6 @@ +object B { + def main(args: Array[String]): Unit = { + val a = new pkg.A + a.foo() + } +} diff --git a/tests/run/invocationReceivers1.scala b/tests/run/invocationReceivers1.scala new file mode 100644 index 000000000000..72a0c7e9a028 --- /dev/null +++ b/tests/run/invocationReceivers1.scala @@ -0,0 +1,45 @@ +trait T { override def clone(): Object = "hi" } +trait U extends T +class C1 extends U with Cloneable { + // C1 gets a forwarder for clone that invokes T.clone. this is needed because JVM method + // resolution always prefers class members, so it would resolve to Object.clone, even if + // C1 is a subtype of the interface T which has an overriding default method for clone. + + // invokeinterface T.clone + def f1 = (this: T).clone() + + // cannot invokeinterface U.clone (NoSuchMethodError). Object.clone would work here, but + // not in the example in C2 (illegal access to protected). T.clone works in all cases and + // resolves correctly. + def f2 = (this: U).clone() + + // invokevirtual C1.clone() + def f3 = (this: C1).clone() +} + +class C2 { + def f1(t: T) = t.clone() // invokeinterface T.clone + def f2(t: U) = t.clone() // invokeinterface T.clone -- Object.clone would be illegal (protected, explained in C1) + def f3(t: C1) = t.clone() // invokevirtual C1.clone -- Object.clone would be illegal +} + +object Test { + def main(arg: Array[String]): Unit = { + val r = new StringBuffer() + val c1 = new C1 + r.append(c1.f1) + r.append(c1.f2) + r.append(c1.f3) + val t = new T { } + val u = new U { } + val c2 = new C2 + r.append(c2.f1(t)) + r.append(c2.f1(u)) + r.append(c2.f1(c1)) + r.append(c2.f2(u)) + r.append(c2.f2(c1)) + r.append(c2.f3(c1)) + r.toString + } +} + diff --git a/tests/run/invocationReceivers2.scala b/tests/run/invocationReceivers2.scala new file mode 100644 index 000000000000..823f3c0b7902 --- /dev/null +++ b/tests/run/invocationReceivers2.scala @@ -0,0 +1,36 @@ +trait T { override def clone(): String = "hi" } +trait U extends T +class C1 extends U with Cloneable { + def f1 = (this: T).clone() + + def f2 = (this: U).clone() + + def f3 = (this: C1).clone() +} + +class C2 { + def f1(t: T) = t.clone() + def f2(t: U) = t.clone() + def f3(t: C1) = t.clone() +} + +object Test { + def main(arg: Array[String]): Unit = { + val r = new StringBuffer() + val c1 = new C1 + r.append(c1.f1) + r.append(c1.f2) + r.append(c1.f3) + val t = new T { } + val u = new U { } + val c2 = new C2 + r.append(c2.f1(t)) + r.append(c2.f1(u)) + r.append(c2.f1(c1)) + r.append(c2.f2(u)) + r.append(c2.f2(c1)) + r.append(c2.f3(c1)) + r.toString + } +} +