diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompiler.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompiler.scala index 3c7f605b806ba..b69cc6fee0ca1 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompiler.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompiler.scala @@ -18,6 +18,7 @@ package org.apache.spark.sql.catalyst.expressions.codegen import java.io.{ByteArrayInputStream, ByteArrayOutputStream, InputStream, IOException, StringWriter} +import java.lang.reflect.{Method, Modifier} import java.net.{JarURLConnection, URI, URL} import java.util.Locale import java.util.concurrent.{Callable, ExecutionException, ExecutorService} @@ -143,10 +144,11 @@ object CodeCompiler extends Logging { * * The configured backend ([[SQLConf.CODEGEN_COMPILER]]) governs ordinary codegen. The * exception is codegen the JDK compiler is fundamentally *incapable* of compiling - not - * merely slower at - which is always routed to Janino regardless of the configured - * backend. This is deterministic routing decided up front from the execution context and - * the generated source; it is never a fallback after a failed compile. Two such cases, - * both classes the JDK compiler cannot name that Janino's lenient loader/lexer accepts: + * merely slower at compiling - which is always routed to Janino regardless of the + * configured backend. This is deterministic routing decided up front from the execution + * context and the generated source; it is never a fallback after a failed compile. Three + * such cases, all involving classes the JDK compiler cannot name that Janino's lenient + * loader/lexer accepts: * * - REPL / interactive sessions (spark-shell `$line*` wrappers, Spark Connect / * Ammonite session artifacts): reachable only through a runtime class loader and @@ -159,17 +161,31 @@ object CodeCompiler extends Logging { * an identifier in any form - Java has no backtick/escape, unlike Scala - so javac * can neither parse `a.b.package.Inner` nor resolve the flat `a.b.package$Inner`. * See [[requiresJaninoSource]]. + * - A reference to a class the Java language forbids naming - an anonymous or local + * class (`a.b.Outer$1`, `a.b.Outer$$anon$1`) or a class nested inside one + * (`a.b.Outer$1$Inner`) - that cannot be narrowed soundly. The JDK backend rewrites + * such a reference to the nearest nameable supertype, which works only while that + * supertype is itself referenceable and offers every member the generated code could + * access. See [[JdkCodeCompiler.referencesUnnarrowableClass]]. */ def active(code: CodeAndComment): CodeCompiler = { - val requested = SQLConf.get.codegenCompiler - if (requested != JANINO && isReplContext) { + // Resolve the configured backend first: when it is already Janino - by configuration or + // because javac is absent - none of the overrides below can change the outcome, and + // their source scans would be pure overhead. + val configured = forBackend(SQLConf.get.codegenCompiler) + if (configured eq JaninoCodeCompiler) { + configured + } else if (isReplContext) { logReplRoutingOnce() JaninoCodeCompiler - } else if (requested != JANINO && requiresJaninoSource(code)) { + } else if (requiresJaninoSource(code)) { logPackageObjectRoutingOnce() JaninoCodeCompiler + } else if (code != null && JdkCodeCompiler.referencesUnnarrowableClass(code.body)) { + logUnnarrowableClassRoutingOnce() + JaninoCodeCompiler } else { - forBackend(requested) + configured } } @@ -193,6 +209,16 @@ object CodeCompiler extends Logging { log"cannot name). This notice is logged once per JVM.") } } + private val unnarrowableClassRoutingLogged = new java.util.concurrent.atomic.AtomicBoolean(false) + private def logUnnarrowableClassRoutingOnce(): Unit = { + if (unnarrowableClassRoutingLogged.compareAndSet(false, true)) { + logInfo(log"Generated code references a class Java cannot name (anonymous, local, or " + + log"nested in one) that cannot be narrowed to a nameable supertype; that unit is " + + log"routed to Janino although " + + log"${MDC(LogKeys.CONFIG, SQLConf.CODEGEN_COMPILER.key)} requests another backend. " + + log"This notice is logged once per JVM.") + } + } // A `package` segment in a qualified/binary class name - a Scala `package object`'s nested // class such as `a.b.package$Inner`. `package` is a Java reserved word the JDK compiler can @@ -731,58 +757,51 @@ object JdkCodeCompiler extends CodeCompiler with Logging { * prefix resolves (e.g. inner classes of the not-yet-compiled generated unit). */ private def rewriteQualifiedName(token: String, classLoader: ClassLoader): String = { + loadLongestPrefix(token, classLoader) match { + case Some((cls, rest)) => + val sourceName = sourceNameOf(nameableSupertype(cls)) + val resolved = if (rest.isEmpty) sourceName else s"$sourceName.$rest" + // `split('.')` drops a trailing empty segment, so a token that ends in `.` + // (member access wrapped onto the next line) must get its dot restored. + if (token.endsWith(".")) resolved + "." else resolved + case None => + InnerClassRefPattern.replaceAllIn(token, ".") + } + } + + /** + * Find the longest dot-delimited prefix of `token` that loads as a class, and return it + * with the remaining member access. Only a prefix containing `$` is tried, since only + * such a prefix can be a binary inner-class name. Returns None when nothing loads (e.g. + * an inner class of the not-yet-compiled generated unit). + */ + private def loadLongestPrefix( + token: String, + classLoader: ClassLoader): Option[(Class[_], String)] = { val parts = token.split('.') var k = parts.length while (k >= 1) { val prefix = parts.iterator.take(k).mkString(".") - // Only a prefix that itself contains `$` can be a binary inner-class name. if (prefix.indexOf('$') >= 0) { - resolveSourceName(prefix, classLoader) match { - case Some(sourceName) => - val rest = parts.iterator.drop(k).mkString(".") - val resolved = if (rest.isEmpty) sourceName else s"$sourceName.$rest" - // `split('.')` drops a trailing empty segment, so a token that ends in `.` - // (member access wrapped onto the next line) must get its dot restored. - return if (token.endsWith(".")) resolved + "." else resolved + loadWithoutInit(prefix, classLoader) match { + case Some(cls) => return Some((cls, parts.iterator.drop(k).mkString("."))) case None => // try a shorter prefix } } k -= 1 } - InnerClassRefPattern.replaceAllIn(token, ".") + None } - /** - * Load `binaryName` without initializing it and return the source name the JDK - * compiler accepts: the canonical name when it is a plain dotted identifier - * name (regular nesting, e.g. `java.util.Map.Entry`), otherwise the binary - * name itself. The binary name is required for classes nested in Scala objects - * and for Scala companion-object classes, whose canonical form carries a - * module `$` that javac cannot resolve, and for Scala operator-named classes - * (e.g. `scala.collection.immutable.::`) whose canonical form is not a valid - * Java identifier - in all those cases the binary name is itself a legal Java - * type reference. Returns None when the name is not a loadable class. - * - * The canonical name is also rejected when it is not a faithful rename of the - * binary name - it must keep the same package. Scala REPL classes (e.g. - * `$line21.$read$$iw$TestCaseClass`) report a misleading `getCanonicalName` that - * drops the package and returns just the simple name (`TestCaseClass`); using it - * would corrupt the reference into an unqualified one javac cannot resolve. - */ - private def resolveSourceName(binaryName: String, classLoader: ClassLoader): Option[String] = { + /** Load `binaryName` without initializing it; None when it is not a loadable class. */ + private def loadWithoutInit(binaryName: String, classLoader: ClassLoader): Option[Class[_]] = { try { // scalastyle:off classforname // Load with the exact loader passed in (the task's context loader), not the Spark // class loader, so the JDK compiler sees what the runtime would; Utils.classForName // cannot target an arbitrary loader. - val loaded = Class.forName(binaryName, false, classLoader) + Some(Class.forName(binaryName, false, classLoader)) // scalastyle:on classforname - val cls = nameableSupertype(loaded) - val canonical = cls.getCanonicalName - val pkg = cls.getPackageName - val usableCanonical = canonical != null && isPlainDottedName(canonical) && - (pkg.isEmpty || canonical.startsWith(pkg + ".")) - Some(if (usableCanonical) canonical else cls.getName) } catch { case _: ClassNotFoundException | _: LinkageError => None case NonFatal(_) => None @@ -790,20 +809,58 @@ object JdkCodeCompiler extends CodeCompiler with Logging { } /** - * Climb to the nearest class that can be named in Java source. Anonymous and local - * classes (e.g. a Scala `new HashMap[..]() {...}` compiled to `Outer$$anon$1`) have no - * source-referenceable name: the JDK compiler rejects a qualified reference to them - * even when the `.class` file is on the classpath, because the Java language forbids - * naming them. Janino does not - it resolves any class by its runtime binary name - - * so this only matters for the JDK backend. The generated code casts an object to - * this type and then invokes methods declared on it; every such method is inherited - * from the supertype, so the nearest nameable supertype is a sound cast target. For an - * anonymous class implementing an interface (`new Comparator() {...}`, whose superclass - * is `Object`), the implemented interface is preferred over `Object`. + * The source name the JDK compiler accepts for `cls`: the canonical name when it is a + * plain dotted identifier name (regular nesting, e.g. `java.util.Map.Entry`), otherwise + * the binary name. The binary name is required for classes nested in Scala objects and + * for Scala companion-object classes, whose canonical form carries a module `$` that + * javac cannot resolve, and for Scala operator-named classes (e.g. + * `scala.collection.immutable.::`) whose canonical form is not a valid Java identifier - + * in all those cases the binary name is itself a legal Java type reference. + * + * The canonical name is also rejected when it is not a faithful rename of the binary + * name - it must keep the same package. Scala REPL classes (e.g. + * `$line21.$read$$iw$TestCaseClass`) report a misleading `getCanonicalName` that drops + * the package and returns just the simple name (`TestCaseClass`); using it would corrupt + * the reference into an unqualified one javac cannot resolve. + * + * Reflection here can raise a `LinkageError` when the class loaded but its enclosing + * class did not (a partial or shaded jar); `NonFatal` does not cover that, so it is + * caught explicitly and the binary name used, matching what an unresolvable prefix + * yields in [[loadLongestPrefix]]. + */ + private def sourceNameOf(cls: Class[_]): String = { + val canonical = + try cls.getCanonicalName + catch { + case _: LinkageError => null + case NonFatal(_) => null + } + val pkg = cls.getPackageName + val usableCanonical = canonical != null && isPlainDottedName(canonical) && + (pkg.isEmpty || canonical.startsWith(pkg + ".")) + if (usableCanonical) canonical else cls.getName + } + + /** + * Climb to the nearest class that can be named in Java source. A class whose + * `getCanonicalName` is null has no source-referenceable name: the JDK compiler rejects + * a qualified reference to it even when the `.class` file is on the classpath, because + * the Java language forbids naming it. That covers anonymous classes (a Scala + * `new HashMap[..]() {...}` compiles to `Outer$$anon$1`), local classes, and classes + * nested inside either of those (`Outer$1$Inner`), which are themselves neither + * anonymous nor local. Janino does not care - it resolves any class by its runtime + * binary name - so this only matters for the JDK backend. For an anonymous class + * implementing an interface (`new Comparator() {...}`, whose superclass is `Object`), + * the implemented interface is preferred over `Object`. + * + * Narrowing a reference this way is only sound while every member the generated code + * could access remains reachable through the replacement type; a unit referencing a + * class for which that does not hold is routed to Janino instead of being rewritten + * (see [[referencesUnnarrowableClass]] and [[CodeCompiler.active]]). */ private def nameableSupertype(start: Class[_]): Class[_] = { var c: Class[_] = start - while (c != null && (c.isAnonymousClass || c.isLocalClass)) { + while (c != null && c.getCanonicalName == null) { val sup: Class[_] = c.getSuperclass c = if (sup != null && (sup ne classOf[Object])) sup @@ -812,6 +869,134 @@ object JdkCodeCompiler extends CodeCompiler with Logging { if (c == null) classOf[Object] else c } + /** + * True when `body` references a class that [[nameableSupertype]] cannot narrow soundly, + * i.e. the class carries a public member that the replacement type does not offer, or + * the replacement type is itself one javac cannot reference. Such a unit must go to + * Janino: rewriting the reference would either drop the member or emit a type name javac + * rejects. + * + * Called from [[CodeCompiler.active]] on every compile, so it is gated behind a scan for + * `$` followed by a digit. Every class the Java language forbids naming carries that in + * its binary name (`Outer$1`, `Outer$1Local`, Scala's `Outer$$anon$1`, and their nested + * members `Outer$1$Inner`), while the other `$` forms the rewrite handles - regular + * nesting (`Map$Entry`), Scala modules (`Foo$`, `Model$Load$Leaf`), package objects + * (`pkg$Inner`), specialized (`Function1$mcII$sp`) and operator-named (`$colon$colon`) + * classes - do not. Lambdas (`Outer$$Lambda$14/0x...`) do carry it and are neither + * anonymous nor local, but they are inert here: the tokenizer stops at `/`, leaving a + * name no loader can resolve. Janino cannot name them either, so a lambda reference + * never reaches generated source in the first place. + * + * The scan adds one linear pass over the body ahead of the compile-cache lookup, behind + * the intrinsified `$`-digit gate that ordinary generated code fails immediately. + * + * The scan reads the raw body, so a `$`-digit sequence inside a string literal or a + * comment can trigger the resolution attempt. That is harmless: an unloadable token is + * ignored, and a loadable one only ever picks Janino, which accepts a superset of what + * javac does. + */ + private[codegen] def referencesUnnarrowableClass(body: String): Boolean = { + if (!containsDollarDigit(body)) return false + val classLoader = Utils.getContextOrSparkClassLoader + val checked = mutable.HashSet.empty[String] + var i = 0 + val n = body.length + while (i < n) { + if (isNameStart(body.charAt(i))) { + val start = i + i += 1 + while (i < n && isNamePart(body.charAt(i))) i += 1 + val token = body.substring(start, i) + if (containsDollarDigit(token) && checked.add(token) && + loadLongestPrefix(token, classLoader).exists { + case (cls, _) => !canNarrowSafely(cls) + }) { + return true + } + } else { + i += 1 + } + } + false + } + + /** True iff `s` holds a `$` immediately followed by an ASCII digit. */ + private[codegen] def containsDollarDigit(s: String): Boolean = { + var i = s.indexOf('$') + while (i >= 0 && i < s.length - 1) { + val next = s.charAt(i + 1) + if (next >= '0' && next <= '9') return true + i = s.indexOf('$', i + 1) + } + false + } + + /** + * True when a reference to `cls` can be replaced by [[nameableSupertype]] without losing + * access to any member. A class that is already nameable needs no narrowing and always + * qualifies. + * + * Otherwise two things must hold. First, the replacement type must be one the generated + * unit can reference: it and every enclosing class must be public. Same-package is NOT + * sufficient even though javac would accept it - the generated class is defined into + * `org.apache.spark.sql.catalyst.expressions` but loaded by [[InMemoryClassLoader]], so + * its runtime package differs from the same-named package on the app loader and a + * package-private access would fail with `IllegalAccessError` at execution time instead + * of at compile time. Second, every public member of the concrete class - including + * inherited ones, since the generated code may access any of them - must be reachable on + * the replacement type. + * + * A member is matched by its exact erased signature, with one allowance for bridges: an + * override of a generic method has a narrower erasure than the supertype declaration it + * implements (`compare(String, String)` against `Comparator.compare(Object, Object)`), + * and the compiler emits a bridge carrying the supertype's signature. Such a method is + * safe to narrow because `invokevirtual` on the supertype signature still dispatches to + * the override. An overload has no bridge, so it is rejected - and it must be, because + * narrowing binds the call to the supertype's method instead: `Invoke` codegen always + * wraps the call in an explicit cast, which would hide the type mismatch from javac and + * silently produce the wrong result rather than fail to compile. + * + * Reflection over the concrete class can raise a `LinkageError` when a member signature + * names a class the loader cannot find (a partial or shaded jar). `NonFatal` does not + * cover that, and an escaping `Error` would bypass the codegen fallbacks, so it is + * caught here and reported as "cannot narrow" - Janino compiles what javac cannot. + */ + private def canNarrowSafely(cls: Class[_]): Boolean = { + val target = nameableSupertype(cls) + if (cls eq target) return true + if (!isPubliclyNameable(target)) return false + try { + val reachable: Seq[Class[_]] = Seq(target, classOf[Object]) + val targetSignatures = reachable.flatMap(_.getMethods).map(erasedSignature).toSet + val targetFields = reachable.flatMap(_.getFields).map(_.getName).toSet + val methods = cls.getMethods + val bridgedTo = methods.iterator + .filter(m => m.isBridge && targetSignatures.contains(erasedSignature(m))) + .map(m => (m.getName, m.getParameterCount)) + .toSet + methods.forall { m => + targetSignatures.contains(erasedSignature(m)) || + bridgedTo.contains((m.getName, m.getParameterCount)) + } && cls.getFields.forall(f => targetFields.contains(f.getName)) + } catch { + case _: LinkageError => false + case NonFatal(_) => false + } + } + + private def erasedSignature(m: Method): (String, Seq[Class[_]]) = + (m.getName, m.getParameterTypes.toSeq) + + /** True iff `cls` and every class enclosing it are public, i.e. javac can name it. */ + private def isPubliclyNameable(cls: Class[_]): Boolean = { + var c = cls + while (c != null) { + if (!Modifier.isPublic(c.getModifiers)) return false + c = c.getEnclosingClass + } + true + } + /** True iff `s` contains only `[A-Za-z0-9_.]` - a dotted Java identifier path. */ private def isPlainDottedName(s: String): Boolean = { var i = 0 diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index 8a0d88346b63c..d966b667f27d4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -2960,9 +2960,11 @@ object SQLConf { "When 'jdk' is requested but javax.tools.JavaCompiler is unavailable " + "(e.g. JRE-only image) Spark falls back to 'janino' with a warning. " + "Regardless of this setting, codegen in REPL / interactive sessions (spark-shell, " + - "Spark Connect session artifacts) and generated code referencing a class nested in " + - "a Scala package object always compile with 'janino', because the JDK compiler " + - "cannot resolve such classes; a one-time INFO log records each such routing.") + "Spark Connect session artifacts), generated code referencing a class nested in " + + "a Scala package object, and generated code referencing an anonymous or local class " + + "that cannot be rewritten to a nameable supertype always compile with 'janino', " + + "because the JDK compiler cannot resolve such classes; a one-time INFO log records " + + "each such routing.") .version("4.3.0") .withBindingPolicy(ConfigBindingPolicy.SESSION) .stringConf diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompilerSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompilerSuite.scala index 8adb537f1110a..616c8a715cff6 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompilerSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeCompilerSuite.scala @@ -726,6 +726,146 @@ class CodeCompilerSuite extends SparkFunSuite with SQLHelper { assert(generated.generate(Array[Any](anon)) === "v") } + // ---------------- unnarrowable anonymous/local classes ---------------- + + test("containsDollarDigit gates the scan on names Java cannot spell") { + // The `$`-digit gate must admit every unnameable shape and reject the nameable `$` + // forms the rewrite handles, or the scan either misses a class or runs needlessly. + for (name <- Seq("Outer$1", "Outer$1Local", "Outer$$anon$1", "Outer$1$Inner", + "a.b.Outer$$anon$12")) { + assert(JdkCodeCompiler.containsDollarDigit(name), s"expected $name to be gated in") + } + for (name <- Seq("", "$", "a$", "java.util.Map$Entry", "Foo$", "Model$SaveLoad$Leaf", + "scala.Function1$mcII$sp", "scala.collection.immutable.$colon$colon", + "pkg.package$Inner", "$line21.$read$$iw$T")) { + assert(!JdkCodeCompiler.containsDollarDigit(name), s"expected $name to be gated out") + } + } + + test("referencesUnnarrowableClass: false when the supertype offers every member") { + // The shapes codegen actually produces: the anonymous or local class only overrides + // methods the supertype already declares, so narrowing the reference loses nothing. + val anonSubclass = new java.util.HashMap[String, String]() { put("k", "v") } + val anonInterface = new java.util.Comparator[String] { + override def compare(a: String, b: String): Int = a.compareTo(b) + } + val narrowable = Seq[Any](anonSubclass, anonInterface, CodeCompilerSuite.plainLocal) + for (o <- narrowable) { + val cls = o.getClass + // Guard the fixture's own precondition: a nameable class would pass the assertion + // below for the wrong reason. + assert(cls.getCanonicalName == null, s"expected an unnameable class, got: ${cls.getName}") + val name = cls.getName + assert(!JdkCodeCompiler.referencesUnnarrowableClass(s"$name v = ($name) references[0];"), + s"expected $name to be narrowable") + } + } + + test("referencesUnnarrowableClass: true when narrowing would lose access") { + // An extra public method, public fields inherited from a second interface, a member on + // a second interface, an overload that shadows nothing, and a local class with an extra + // method: each puts something out of reach of the nearest nameable supertype. + val unnarrowable = Seq[Any]( + CodeCompilerSuite.anonWithExtraMethod, + CodeCompilerSuite.anonWithPublicFields, + CodeCompilerSuite.anonWithSecondInterface, + CodeCompilerSuite.anonWithOverload, + CodeCompilerSuite.localWithExtraMethod) + for (o <- unnarrowable) { + val cls = o.getClass + assert(cls.getCanonicalName == null, s"expected an unnameable class, got: ${cls.getName}") + val name = cls.getName + assert(JdkCodeCompiler.referencesUnnarrowableClass(s"$name v = ($name) references[0];"), + s"expected $name to be rejected") + } + } + + test("referencesUnnarrowableClass: true when the supertype itself cannot be named") { + // Members line up here, but the nearest nameable supertype is a private nested class + // (scala.collection.mutable.HashSet$HashSetIterator), so javac could not write the + // narrowed cast at all. + val cls = scala.collection.mutable.HashSet("a").iterator.getClass + assert(cls.getCanonicalName == null, s"expected an unnameable class, got: ${cls.getName}") + val name = cls.getName + assert(JdkCodeCompiler.referencesUnnarrowableClass(s"$name v = ($name) references[0];"), + s"expected $name to be rejected for an unnameable supertype") + } + + test("rewriteInnerClassRefs: narrows a class nested inside a local class") { + assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available") + // A named class declared inside a local class is neither anonymous nor local, but Java + // cannot name it either: its canonical name is null and the binary name is not a legal + // type reference. It must be narrowed to its nameable supertype, not emitted verbatim. + val cls = CodeCompilerSuite.memberOfLocalClass.getClass + assert(!cls.isAnonymousClass && !cls.isLocalClass && cls.isMemberClass, + s"expected a member class of a local class, got: ${cls.getName}") + assert(cls.getCanonicalName == null, s"expected no canonical name for ${cls.getName}") + val name = cls.getName + assert(rewrite(s"$name v = ($name) references[0];") === + "java.util.ArrayList v = (java.util.ArrayList) references[0];") + } + + test("active(code) routes an unnarrowable class reference to Janino") { + assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available") + val narrowableName = (new java.util.HashMap[String, String]() { put("k", "v") }) + .getClass.getName + val unnarrowableName = CodeCompilerSuite.anonWithExtraMethod.getClass.getName + withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") { + assert(CodeCompiler.active(newCodeAndComment(s"$narrowableName v;")) eq JdkCodeCompiler) + assert(CodeCompiler.active(newCodeAndComment(s"$unnarrowableName v;")) eq JaninoCodeCompiler) + } + withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "janino") { + assert(CodeCompiler.active(newCodeAndComment(s"$unnarrowableName v;")) eq JaninoCodeCompiler) + } + } + + test("JDK backend cannot compile a member access that narrowing drops") { + assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available") + // Both halves of the routing decision: javac rejects the rewritten unit (the reference + // narrows to java.util.HashMap, which has no `extra()`), and `active` therefore hands + // the unit to Janino, which compiles it and produces the right answer. + val anon = CodeCompilerSuite.anonWithExtraMethod + val anonName = anon.getClass.getName + val code = newCodeAndComment( + s""" + |public java.lang.Object generate(Object[] references) { + | $anonName m = ($anonName) references[0]; + | return m.extra(); + |} + """.stripMargin) + intercept[CompileException] { + JdkCodeCompiler.compile(code) + } + withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") { + assert(CodeCompiler.active(code) eq JaninoCodeCompiler) + val (generated, _) = CodeGenerator.compile(code) + assert(generated.generate(Array[Any](anon)) === "extra") + } + } + + test("JDK backend compiles a call that narrowing preserves through a bridge") { + assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available") + // The counterpart to the test above: an override of a generic method erases narrower + // than the interface declares, but the compiler-emitted bridge keeps dispatch correct, + // so this must stay on the JDK backend rather than being routed away. + val anon = new java.util.Comparator[String] { + override def compare(a: String, b: String): Int = a.compareTo(b) + } + val anonName = anon.getClass.getName + val code = newCodeAndComment( + s""" + |public java.lang.Object generate(Object[] references) { + | $anonName c = ($anonName) references[0]; + | return Integer.valueOf(c.compare("a", "b")); + |} + """.stripMargin) + withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") { + assert(CodeCompiler.active(code) eq JdkCodeCompiler) + } + val (generated, _) = JdkCodeCompiler.compile(code) + assert(generated.generate(Array[Any](anon)) === -1) + } + // ---------------- Function1 apply(Object) bridge ---------------- test("stripFunction1ApplyBridges removes the bridge but keeps apply(InternalRow)") { @@ -834,4 +974,63 @@ object CodeCompilerSuite { object SaveLoadV1 { case class Leaf(x: Int) } + + private[codegen] trait Greeter { + def hello(): String + } + + private[codegen] abstract class Converter { + def convert(o: Any): String = "base" + } + + // Anonymous and local classes for the narrowing-soundness tests. They are held in vals + // rather than built inline in the tests because scalac keeps an anonymous class's extra + // members `public` only when the binding's type is inferred as the refined type; giving + // the val an explicit type, or passing the expression as `Any`, makes them private. + val anonWithExtraMethod = new java.util.HashMap[String, String]() { + def extra(): String = "extra" + } + + // Mixing in a Java constants interface is what actually yields public FIELDS: a Scala + // `val` compiles to a private field plus an accessor, which only exercises the method + // check. ObjectStreamConstants contributes 30 public static final fields and no method + // beyond Comparator's, so this isolates the field clause of `canNarrowSafely`. + val anonWithPublicFields = new java.util.Comparator[String] with java.io.ObjectStreamConstants { + override def compare(a: String, b: String): Int = a.compareTo(b) + } + + val anonWithSecondInterface = new java.util.Comparator[String] with Greeter { + override def compare(a: String, b: String): Int = a.compareTo(b) + override def hello(): String = "hi" + } + + // An OVERLOAD, not an override: `convert(String)` does not implement `convert(Any)`, so + // no bridge is emitted. Narrowing would silently bind the call to the supertype's method. + val anonWithOverload = new Converter { + def convert(s: String): String = "anon" + } + + val plainLocal: java.util.ArrayList[String] = { + class PlainLocal extends java.util.ArrayList[String] + new PlainLocal + } + + val localWithExtraMethod = { + class LocalWithExtra extends java.util.ArrayList[String] { + def extra(): String = "extra" + } + new LocalWithExtra + } + + // A named class declared inside a local class: neither anonymous nor local itself + // (`isMemberClass` is true), yet Java cannot name it either. Its supertype offers every + // member, so only the canonical-name test keeps it from being narrowed to a binary name + // javac would reject. + val memberOfLocalClass: Any = { + class Holder { + class Inner extends java.util.ArrayList[String] + def make(): Any = new Inner + } + new Holder().make() + } }