You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ya...@apache.org on 2026/08/10 14:21:39 UTC
(spark) branch branch-4.3 updated: [SPARK-57370][SQL][FOLLOWUP] Route codegen referencing an unnameable class to Janino when narrowing is unsound
This is an automated email from the ASF dual-hosted git repository.
LuciferYang pushed a commit to branch branch-4.3
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.3 by this push:
new 3e39b4ac372c [SPARK-57370][SQL][FOLLOWUP] Route codegen referencing an unnameable class to Janino when narrowing is unsound
3e39b4ac372c is described below
commit 3e39b4ac372c4ee41f97f93419448aee51256497
Author: YangJie <ya...@baidu.com>
AuthorDate: Mon Aug 10 22:21:02 2026 +0800
[SPARK-57370][SQL][FOLLOWUP] Route codegen referencing an unnameable class to Janino when narrowing is unsound
### What changes were proposed in this pull request?
Follow-up to #56430, addressing review comments from cloud-fan.
The JDK backend rewrote a reference to a class Java cannot name (an anonymous or local class, or a class nested inside one) into its nearest nameable supertype unconditionally. That is only sound when the supertype is itself referenceable and offers every member the generated code could access, and nothing enforced either condition. Such a unit is now routed to Janino instead, joining the two existing deterministic-routing cases (REPL contexts and Scala package-object classes).
`narrowingVerdict` decides this, in three values. `Narrowable(target)` carries the replacement type, so the decision and the name emitted for it come from one evaluation. `Unnarrowable` is positive evidence that narrowing loses access and routes the unit to Janino. `Unknown(cause)` means reflection could not answer, which is no evidence either way: the unit stays on the configured backend and the reference keeps its binary name, so javac rejects it rather than compiling into a wrong answer.
A member is matched by its exact erased signature, with an allowance for bridges: an override of a generic method erases narrower than the supertype declaration it implements (`compare(String, String)` against `Comparator.compare(Object, Object)`) and carries a bridge with the supertype's signature, so narrowing keeps dispatching to the override. An overload has no bridge and is rejected, and it must be, because `Invoke` codegen wraps every call in an explicit cast, which would hide t [...]
Fields and static methods get no such allowance. Both are bound statically, with no counterpart to `invokevirtual`'s re-dispatch, so they are matched by declaring class rather than by signature: a class that redeclares an inherited public field, or hides an inherited public static method, exposes it under a name the replacement type also answers to, and the narrowed reference would silently reach the replacement type's member. `getFields`/`getMethods` report the hiding and the hidden [...]
A synthetic member is exempt from those checks, because javac's source-level lookup cannot see one ("cannot find symbol") and so no generated reference can reach it. scalac emits a `public static final $anonfun$...` on an anonymous class for every closure in its body, and a public `$outer` field and accessor on an inner class; without the exemption every closure-carrying anonymous class would route to Janino. The exemption is withheld when the target answers to the same name and kind, [...]
A class that needs no narrowing is checked only for run-time reachability, using its own modifier rather than the enclosing chain: for a nested class `getModifiers` reports the source-level modifier from the InnerClasses attribute, while the JVM checks the class file's own `ACC_PUBLIC`, and a public class nested in a package-private one has it and is reachable across loaders. The enclosing-chain walk remains the right test for a climbed target, where rejecting means routing to Janino [...]
`nameableSupertype` now climbs while `getCanonicalName` is null rather than while the class is anonymous or local, which additionally covers a named class nested inside one of those, neither anonymous nor local itself, yet equally unnameable.
Reflection in this path can raise a `LinkageError` when a class loads but a type in its signature does not (a partial or shaded jar). `NonFatal` does not cover that and an escaping `Error` would bypass the codegen fallbacks, so `narrowingVerdict` catches it and reports `Unknown`, which neither routes the unit nor lets the rewrite narrow it. A non-`LinkageError` failure is caught the same way, matching what `sourceNameOf` already does for the same `getCanonicalName` call and what the ` [...]
Also two comment wordings in the `CodeCompiler.active` scaladoc, and the `spark.sql.codegen.compiler` config doc, which enumerated only two always-Janino cases.
### Why are the changes needed?
Fixes the unchecked assumption. Without it, an anonymous type whose accessed member is absent from the supertype either fails to compile or, for a same-arity overload, a shadowed field, a hidden static, or an instance method clashing with a static, compiles clean and returns the supertype's answer.
### Does this PR introduce _any_ user-facing change?
No. The default backend is Janino, and for `spark.sql.codegen.compiler=jdk` this only moves units that javac would have mishandled onto the working path.
### How was this patch tested?
Thirteen cases in `CodeCompilerSuite` covering the `$`-digit gate, narrowable and unnarrowable shapes, an unnameable supertype, a class nested in a local class, synthetic members on both sides of the exemption, reflection failing at the climb and at member enumeration, and the routing decision under both backend settings.
Three of them are end-to-end: they compile and execute a body that reads the hazardous member, assert the routed result is the class's own value rather than the supertype's, and assert the counterfactual that the narrowed name answers differently, so the test would still fail if routing stopped carrying the correctness. Each production check was mutated in turn and the corresponding test fails without it; the three end-to-end tests are independent discriminators, so reverting one fix [...]
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code(Opus 5)
Closes #57768 from LuciferYang/spark-57370-narrowing-followup.
Authored-by: YangJie <ya...@baidu.com>
Signed-off-by: yangjie01 <ya...@baidu.com>
(cherry picked from commit b3c0402a9c8da97c655a52c791cc59d6f8ea9789)
Signed-off-by: yangjie01 <ya...@baidu.com>
---
.../expressions/codegen/CodeCompiler.scala | 493 +++++++++++--
.../org/apache/spark/sql/internal/SQLConf.scala | 9 +-
.../expressions/codegen/StaticClashBase.java | 31 +
.../expressions/codegen/CodeCompilerSuite.scala | 807 ++++++++++++++++++++-
4 files changed, 1276 insertions(+), 64 deletions(-)
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 3c7f605b806b..2a64f5d00713 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.{Member, 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,34 @@ 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, i.e. 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 is shown to be unnarrowable. 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; this arm covers the classes for which reflection positively reports it does
+ * not. A class whose verdict reflection cannot determine is not routed: a reference to
+ * it in code position keeps its binary name, which javac rejects for these shapes.
+ * 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 +212,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
@@ -513,6 +542,10 @@ object JdkCodeCompiler extends CodeCompiler with Logging {
* backend compiled it. `classLoader` resolves the candidate inner-class
* references for the `$`-rewrite (see [[rewriteInnerClassRefs]]); it must be the
* same loader the compile resolves classes through.
+ *
+ * Hoisted imports are deliberately left un-rewritten: an `import` requires a canonical
+ * name, so a binary inner-class name there is a javac error whether or not it is
+ * narrowed.
*/
private[codegen] def wrapAsCompilationUnit(body: String, classLoader: ClassLoader): String = {
val (extraImports, cleanedBody) = extractLeadingImports(body)
@@ -731,58 +764,76 @@ 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 = narrowedSourceName(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, ".")
+ }
+ }
+
+ /**
+ * The source name to emit for `cls`: the name of the nearest class Java can name, in the
+ * form javac accepts.
+ *
+ * Narrowing happens only on a positive [[Narrowable]] verdict; anything else keeps the
+ * binary name. Consulting the verdict is the load-bearing part, because the reflective
+ * failures are not symmetric: the supertype climb can succeed while member enumeration
+ * throws, so climbing here without it would narrow a class whose members were never
+ * checked, to a public supertype javac happily accepts. Taking the target from the verdict
+ * rather than re-deriving it costs one climb instead of two. Keeping the binary name of a
+ * class Java cannot name yields a name javac rejects, so the unit fails to compile rather
+ * than compiling into a wrong answer.
+ */
+ private def narrowedSourceName(cls: Class[_]): String = narrowingVerdict(cls) match {
+ case Narrowable(target) => sourceNameOf(target)
+ case Unnarrowable => cls.getName
+ case Unknown(cause) =>
+ // Logged from here, not from the verdict: this is the one call site that knows the
+ // token really is a type reference in code position, so the message's claim that the
+ // unit will fail to compile actually holds. The routing scan reads the raw body and
+ // can evaluate a token inside a literal, where the same verdict costs nothing.
+ logUnevaluableClassOnce(cls, cause)
+ cls.getName
+ }
+
+ /**
+ * 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 +841,61 @@ 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 reflection shows that does not hold is routed to Janino instead of being
+ * rewritten. When reflection cannot tell, the unit stays on the JDK backend and the
+ * reference keeps its binary name, which javac rejects, so the unit fails to compile
+ * rather than compiling into a wrong answer (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 +904,301 @@ 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`). The other `$` forms the rewrite handles do not: regular
+ * nesting (`Map$Entry`), Scala modules (`Foo$`, `Model$Load$Leaf`), package objects
+ * (`pkg$Inner`), specialized (`Function1$mcII$sp`) and operator-named (`$colon$colon`)
+ * classes. Runtime-synthesized classes need not carry it at all: a lambda is
+ * `Outer$$Lambda$14/0x...` on JDK 17 but `Outer$$Lambda/0x...` on 21 and later, and a
+ * hidden class is `Host$Named/0x...`. Neither routes on its own name: the tokenizer stops
+ * at `/`, and the truncated remainder either resolves to nothing (a lambda) or to the
+ * ordinary class the hidden class was defined from, whose own verdict is then the one that
+ * answers. On JDK 17 the lambda shape does pass the gate, at the cost of one failed load.
+ *
+ * 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 outside code position (inside a
+ * string literal, say a regex the optimizer folded in, or inside 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) &&
+ routesOnToken(token, classLoader)) {
+ return true
+ }
+ } else {
+ i += 1
+ }
+ }
+ false
+ }
+
+ /**
+ * True when `token` resolves to a class whose reference has to go to Janino, i.e. one
+ * [[narrowingVerdict]] positively reports as [[Unnarrowable]].
+ *
+ * An [[Unknown]] verdict does not route: it is no evidence that narrowing is unsafe, only
+ * that reflection could not tell. The token then contributes nothing to the decision, so
+ * unless some other token in the same body is [[Unnarrowable]] the unit stays on the
+ * configured backend, where a reference in code position keeps its binary name and javac
+ * rejects it, so the unit fails to compile rather than compiling into a wrong answer.
+ * Routing it to Janino would compile it instead, which is the better outcome for this one
+ * unit, but it would also let a truncated classpath quietly move work off the configured
+ * backend. The routing arms exist for source Spark knows javac cannot express, not for a
+ * broken deployment.
+ */
+ private def routesOnToken(token: String, classLoader: ClassLoader): Boolean = {
+ loadLongestPrefix(token, classLoader).exists {
+ case (cls, _) => narrowingVerdict(cls) match {
+ case Unnarrowable => true
+ case Narrowable(_) | Unknown(_) => 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
+ }
+
+ /**
+ * Whether a reference to a class can be replaced by the nearest class Java can name.
+ *
+ * [[Narrowable]] carries the replacement type, so the decision and the name emitted for it
+ * come from one evaluation. The other two both keep the JDK backend from rewriting a
+ * reference, but they differ in what a caller may conclude: [[Unnarrowable]] is positive
+ * evidence that narrowing loses access, so the unit is routed to Janino, while [[Unknown]]
+ * means reflection could not answer (a partial or shaded jar) and is no evidence either
+ * way.
+ */
+ private sealed trait NarrowingVerdict
+ private case class Narrowable(target: Class[_]) extends NarrowingVerdict
+ private case object Unnarrowable extends NarrowingVerdict
+ private case class Unknown(cause: Throwable) extends NarrowingVerdict
+
+ /**
+ * The verdict for replacing a reference to `cls` with [[nameableSupertype]]. A class that
+ * is already nameable needs no narrowing, and only has to pass the accessibility check
+ * below on itself.
+ *
+ * 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 of its own, 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.
+ *
+ * Telling the two apart needs the bridge to be matched to the specific method it forwards
+ * to, not merely to some method of the same name and arity. A class can hold both shapes
+ * at once - an anonymous `Comparator[String]` that also declares `compare(int, int)` has
+ * a bridge for the override and an unrelated overload sharing its name and arity - and
+ * excusing the whole name/arity group would let the overload through. So a bridge covers
+ * a method only when their parameter types line up (boxing counts as equivalent, since a
+ * specialized primitive override such as `apply(int)` is reached through an
+ * `apply(Object)` bridge), and only when it is the group's sole non-bridge method: with
+ * two of them the bridge cannot forward to both, so at least one becomes unreachable
+ * after narrowing.
+ *
+ * Fields and static methods get no such allowance. Both are bound statically, so they are
+ * matched by declaring class rather than by signature: a class that redeclares an inherited
+ * public field, or hides an inherited public static method, exposes it under a name the
+ * replacement type also answers to, and the narrowed reference would silently reach the
+ * replacement type's member instead. `getFields`/`getMethods` report the hiding and the
+ * hidden member both, so a name- or signature-based check would accept the pair. There is
+ * no counterpart to `invokevirtual`'s re-dispatch here, so the only safe such member is one
+ * the replacement type declares or inherits itself.
+ *
+ * Reflection over either class can raise a `LinkageError` when a signature or an
+ * enclosing class names something the loader cannot find (a partial or shaded jar), and
+ * the failure is not symmetric: the climb can succeed while member enumeration throws, so
+ * a verdict of [[Narrowable]] would let the rewrite emit a supertype name javac accepts
+ * for a class whose members were never checked. Such a class is reported [[Unknown]]
+ * instead, which keeps both the routing decision and the rewrite from acting on it.
+ *
+ * A non-`LinkageError` failure is caught the same way, because this file already treats
+ * this reflection as fallible in both directions: [[sourceNameOf]] guards
+ * `getCanonicalName` with `NonFatal`, as did the `resolveSourceName` that it and
+ * [[loadWithoutInit]] were split out of, over its whole load-climb-name sequence. Catching
+ * both is also what lets the routing scan read the raw body safely. A token there need not
+ * be a type reference in this unit at all: a class name embedded in a generated
+ * error-message literal reaches this method exactly as a real reference does, and an
+ * escaping exception would cost the unit its compile over one, at the routing step before
+ * any compile is attempted (`CodeGenerator.compile` only unwraps cache exceptions).
+ */
+ private def narrowingVerdict(cls: Class[_]): NarrowingVerdict = {
+ try {
+ // An array class cannot arrive from the tokenizer: `javaType` renders an array as
+ // `component[]` and neither `[` nor `;` is a name character, so `foo.Bar$1[]` yields
+ // the component token, which is the one that needs the verdict. A future caller could
+ // still pass one, and it would narrow silently: an array of an unnameable component
+ // type has a null canonical name and only Object's members, so the climb lands on
+ // `Cloneable` and the member check passes. Reporting it unnarrowable routes the unit
+ // to Janino, which needs no rewrite for the component name it would find there.
+ if (cls.isArray) return Unnarrowable
+ val target = nameableSupertype(cls)
+ // A nameable class needs no narrowing, only a check that the generated unit can reach
+ // it at run time. That is the class's own modifier, NOT [[isPubliclyNameable]]'s walk
+ // over the enclosing chain: for a nested class `getModifiers` reports the source-level
+ // modifier from the InnerClasses attribute, while the JVM checks the class file's own
+ // `ACC_PUBLIC`, and a public class nested in a package-private one has it and is
+ // reachable across loaders. The walk is the right test for the climbed target below,
+ // where rejecting means routing to Janino. Here it would mean emitting this class's own
+ // binary name, which javac resolves for no nested class at all, turning a reference
+ // that compiled and ran into a compile error. The two arms answer different questions
+ // and disagree on this shape by design.
+ if (cls eq target) {
+ return if (Modifier.isPublic(cls.getModifiers)) Narrowable(cls) else Unnarrowable
+ }
+ // The climbed target is emitted by canonical name, so here nameability is what matters:
+ // a target javac cannot name from the generated unit's package must not be narrowed to.
+ if (!isPubliclyNameable(target)) return Unnarrowable
+ val reachable: Seq[Class[_]] = Seq(target, classOf[Object])
+ // Statics are excluded: they are matched by declaring class below, and letting one
+ // satisfy the signature set would accept an INSTANCE method of `cls` whose only
+ // counterpart on the target is static, a call the narrowed reference would bind
+ // statically to the target's method. scalac produces that pair, since it does not
+ // treat a Java static as an inherited member.
+ val targetSignatures = reachable.flatMap(_.getMethods)
+ .filterNot(m => Modifier.isStatic(m.getModifiers)).map(erasedSignature).toSet
+ val targetStaticSignatures = reachable.flatMap(_.getMethods)
+ .filter(m => Modifier.isStatic(m.getModifiers)).map(erasedSignature).toSet
+ val methods = cls.getMethods
+ val bridges = methods.filter(m => m.isBridge && targetSignatures.contains(erasedSignature(m)))
+ val nonBridgeCount = methods.iterator.filterNot(_.isBridge)
+ .foldLeft(Map.empty[(String, Int), Int]) { (counts, m) =>
+ val key = (m.getName, m.getParameterCount)
+ counts.updated(key, counts.getOrElse(key, 0) + 1)
+ }
+ def coveredByBridge(m: Method): Boolean =
+ nonBridgeCount.getOrElse((m.getName, m.getParameterCount), 0) <= 1 &&
+ bridges.exists(b => forwardsTo(b, m))
+ def declaredOnTarget(m: Member): Boolean = m.getDeclaringClass.isAssignableFrom(target)
+ val targetFieldNames = Seq(target, classOf[Object]).flatMap(_.getFields).map(_.getName).toSet
+ // A synthetic member is invisible to javac's source-level lookup: referencing one is
+ // "cannot find symbol" even though `getMethods`/`getFields` report it, so no generated
+ // reference can reach it and its loss to narrowing costs nothing. scalac emits several:
+ // a lambda body in the class becomes a `public static final $anonfun$...`, and an inner
+ // class carries a public `$outer` field and accessor. The exemption is withheld when the
+ // target answers to the same name and kind, because the JVM ignores `ACC_SYNTHETIC` when
+ // it resolves a statically-bound member. There the synthetic one shadows the target's,
+ // and narrowing would change which is read.
+ def exemptSynthetic(m: Method): Boolean = m.isSynthetic && {
+ val shadowed =
+ if (Modifier.isStatic(m.getModifiers)) targetStaticSignatures else targetSignatures
+ !shadowed.contains(erasedSignature(m))
+ }
+ val membersLineUp = methods.forall { m =>
+ if (Modifier.isStatic(m.getModifiers)) declaredOnTarget(m) || exemptSynthetic(m)
+ else targetSignatures.contains(erasedSignature(m)) || coveredByBridge(m) ||
+ exemptSynthetic(m)
+ } && cls.getFields.forall { f =>
+ declaredOnTarget(f) || (f.isSynthetic && !targetFieldNames.contains(f.getName))
+ }
+ if (membersLineUp) Narrowable(target) else Unnarrowable
+ } catch {
+ case e: LinkageError => Unknown(e)
+ case NonFatal(e) => Unknown(e)
+ }
+ }
+
+ private val unevaluableClassLogged = new java.util.concurrent.atomic.AtomicBoolean(false)
+ // The one outcome an operator cannot diagnose from the routing logs: reflection over a
+ // referenced class threw, so Spark cannot tell whether narrowing it is safe. The unit stays
+ // on the configured backend with the reference spelled as a binary name javac rejects, which
+ // surfaces as a compile error that looks like a codegen bug rather than a classpath one, and
+ // repeats, since the compile cache does not retain failures. Called only from
+ // [[narrowedSourceName]], so the once-per-JVM budget is spent on a real type reference.
+ private def logUnevaluableClassOnce(cls: Class[_], e: Throwable): Unit = {
+ if (unevaluableClassLogged.compareAndSet(false, true)) {
+ logWarning(log"Reflection over ${MDC(LogKeys.CLASS_NAME, cls.getName)} failed, so " +
+ log"Spark cannot tell whether a reference to it can be narrowed to a name the JDK " +
+ log"compiler accepts; the reference keeps its binary name, which that compiler " +
+ log"rejects. This usually means a partial or shaded jar on the classpath. Setting " +
+ log"${MDC(LogKeys.CONFIG, SQLConf.CODEGEN_COMPILER.key)} to 'janino' compiles such " +
+ log"units anyway. This notice is logged once per JVM.", e)
+ }
+ }
+
+ /**
+ * True when `bridge` can be the bridge the compiler emitted for `impl`: same name and
+ * arity, and every bridge parameter accepts the corresponding one of `impl`. Boxing is
+ * treated as equivalence so that a specialized primitive override (`apply(int)` reached
+ * through an `apply(Object)` bridge) matches.
+ */
+ private def forwardsTo(bridge: Method, impl: Method): Boolean =
+ bridge.getName == impl.getName &&
+ bridge.getParameterCount == impl.getParameterCount &&
+ bridge.getParameterTypes.zip(impl.getParameterTypes).forall {
+ case (bridgeParam, implParam) => boxed(bridgeParam).isAssignableFrom(boxed(implParam))
+ }
+
+ /** The wrapper type for a primitive, or `cls` itself when it is already a reference type. */
+ private def boxed(cls: Class[_]): Class[_] = cls match {
+ case Integer.TYPE => classOf[java.lang.Integer]
+ case java.lang.Long.TYPE => classOf[java.lang.Long]
+ case java.lang.Double.TYPE => classOf[java.lang.Double]
+ case java.lang.Float.TYPE => classOf[java.lang.Float]
+ case java.lang.Short.TYPE => classOf[java.lang.Short]
+ case java.lang.Byte.TYPE => classOf[java.lang.Byte]
+ case Character.TYPE => classOf[java.lang.Character]
+ case java.lang.Boolean.TYPE => classOf[java.lang.Boolean]
+ case other => other
+ }
+
+ 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 1f3987371e42..574816094bd8 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,12 @@ 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 Spark determines cannot be soundly rewritten to a nameable supertype (that " +
+ "supertype, or a class enclosing it, is not public, or it does not offer a member the " +
+ "class exposes) always compile with 'janino'. A one-time INFO log records each such " +
+ "routing.")
.version("4.3.0")
.withBindingPolicy(ConfigBindingPolicy.SESSION)
.stringConf
diff --git a/sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/expressions/codegen/StaticClashBase.java b/sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/expressions/codegen/StaticClashBase.java
new file mode 100644
index 000000000000..661bc374d834
--- /dev/null
+++ b/sql/catalyst/src/test/java/org/apache/spark/sql/catalyst/expressions/codegen/StaticClashBase.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions.codegen;
+
+/**
+ * A narrowing-soundness fixture for {@code CodeCompilerSuite}: a public class declaring a
+ * public STATIC method. A Scala anonymous subclass can declare an INSTANCE method of the same
+ * erased signature, because scalac does not treat a Java static as an inherited member, while
+ * javac rejects the pair outright. That combination has to be written in Java to exist at all.
+ *
+ * Narrowing a reference to such a subclass is unsound: a static call is bound statically, so
+ * {@code ((StaticClashBase) ref).value()} reaches this method, not the subclass's.
+ */
+public class StaticClashBase {
+ public static int value() { return 1; }
+}
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 8adb537f1110..95692428349a 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
@@ -18,10 +18,13 @@
package org.apache.spark.sql.catalyst.expressions.codegen
import java.io.File
+import java.lang.reflect.Modifier
import java.net.{URI, URL, URLClassLoader}
import java.util.Collections
import javax.tools.{JavaFileObject, SimpleJavaFileObject, StandardLocation, ToolProvider}
+import scala.jdk.CollectionConverters._
+
import org.codehaus.commons.compiler.CompileException
import org.mockito.Mockito.{mock, when}
@@ -428,24 +431,265 @@ class CodeCompilerSuite extends SparkFunSuite with SQLHelper {
// Compile `dyn.DynHelper` into a fresh temp dir using the system Java compiler, so
// the class exists only under that dir (never on java.class.path).
- private def compileDynHelper(): File = {
+ test("reflection that raises a LinkageError does not route the unit or escape") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // A partial or shaded jar can leave a class loadable while its enclosing class is not.
+ // `getCanonicalName` and `getEnclosingClass` both throw NoClassDefFoundError then, and
+ // `NonFatal` does not cover a LinkageError, so an escaping Error would bypass the
+ // codegen fallbacks. The token cannot be evaluated, which is no evidence that narrowing
+ // it is unsafe, so the unit must stay on the configured backend rather than gain a
+ // permanent Janino arm.
+ val dir = compileLinkageFixture()
+ // Drop the enclosing class, keeping the anonymous one that references it.
+ assert(new File(dir, "lnk/Holder$Mid.class").delete(), "fixture setup: enclosing class")
+ val loader = new DirClassLoader(dir, getClass.getClassLoader)
+ val anon = loader.loadClass("lnk.Holder$Mid$1")
+ // Precondition: the reflective calls the predicate makes really do throw here.
+ intercept[LinkageError](anon.getCanonicalName)
+ intercept[LinkageError](anon.getEnclosingClass)
+
+ val body = s"${anon.getName} v = (${anon.getName}) references[0];"
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(loader)
+ assert(!JdkCodeCompiler.referencesUnnarrowableClass(body),
+ "an unevaluable token must not route the unit to Janino")
+ withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") {
+ assert(CodeCompiler.active(newCodeAndComment(body)) eq JdkCodeCompiler)
+ }
+ // The rewrite degrades to the binary name instead of propagating the Error.
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(body, loader) === body)
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ }
+
+ test("reflection that fails only on member enumeration does not narrow the reference") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The dangerous asymmetry: a class can be missing a type named in one of its method
+ // signatures, so the supertype climb succeeds while `getMethods` throws. Narrowing then
+ // looks safe, since the emitted supertype name compiles, even though no member was ever
+ // checked, and an overload collision would bind to the supertype's method. The verdict
+ // has to keep the binary name instead, which javac rejects and Spark falls back from.
+ val dir = compileMemberLinkageFixture()
+ val loader = new DirClassLoader(dir, getClass.getClassLoader)
+ val anon = loader.loadClass("lnk2.Holder$1")
+ val body = s"${anon.getName} v = (${anon.getName}) references[0];"
+
+ // Positive control: while the classpath is complete the reference IS narrowed, which
+ // proves the token reaches the verdict rather than being inert in this body.
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(body, loader) ===
+ "lnk2.Foo v = (lnk2.Foo) references[0];",
+ "with a complete classpath the reference must narrow to the interface")
+
+ // Now break only the method signature's parameter type. A fresh loader is needed because
+ // the one above has already resolved members for this class.
+ assert(new File(dir, "lnk2/Missing.class").delete(), "fixture setup: signature type")
+ val brokenLoader = new DirClassLoader(dir, getClass.getClassLoader)
+ val brokenAnon = brokenLoader.loadClass("lnk2.Holder$1")
+ // Preconditions: the climb reads fine, only member enumeration throws.
+ assert(brokenAnon.getCanonicalName == null, "fixture must be unnameable")
+ assert(brokenAnon.getInterfaces.map(_.getName).contains("lnk2.Foo"), "climb input must read")
+ intercept[LinkageError](brokenAnon.getMethods)
+
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(brokenLoader)
+ // Not routed: an unevaluable class is no evidence that narrowing is unsafe.
+ assert(!JdkCodeCompiler.referencesUnnarrowableClass(body),
+ "an unevaluable token must not route the unit to Janino")
+ // And not narrowed either: narrowing to lnk2.Foo would compile and hide the risk.
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(body, brokenLoader) === body,
+ "an unevaluable class must keep its binary name rather than be narrowed")
+ withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") {
+ assert(CodeCompiler.active(newCodeAndComment(body)) eq JdkCodeCompiler)
+ }
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ }
+
+ test("reflection that fails with a non-LinkageError does not narrow the reference") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The other half of the same failure: a classloader is user code, so it may reject a
+ // class with an ordinary RuntimeException instead of ClassNotFoundException, as a
+ // relocating or artifact loader can, and the JVM propagates that out of `getMethods`
+ // unwrapped rather than as a LinkageError. The verdict has to treat it the same way,
+ // which is why its catch does not stop at LinkageError.
+ val dir = compileMemberLinkageFixture()
+ val loader = new DirClassLoader(dir, getClass.getClassLoader) {
+ override def findClass(name: String): Class[_] =
+ if (name == "lnk2.Missing") throw new IllegalStateException("relocated away")
+ else super.findClass(name)
+ }
+ val anon = loader.loadClass("lnk2.Holder$1")
+ // Preconditions. What matters is not the exact type but that it is NOT a LinkageError,
+ // since that is the property the verdict's `NonFatal` arm exists for; the enumeration
+ // that throws is the one over `lnk2.Foo`, which declares the missing parameter type.
+ assert(anon.getCanonicalName == null, "fixture must be unnameable")
+ assert(anon.getInterfaces.map(_.getName).contains("lnk2.Foo"), "climb input must read")
+ intercept[IllegalStateException](anon.getMethods)
+
+ val body = s"${anon.getName} v = (${anon.getName}) references[0];"
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(loader)
+ assert(!JdkCodeCompiler.referencesUnnarrowableClass(body),
+ "an unevaluable token must not route the unit to Janino")
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(body, loader) === body,
+ "an unevaluable class must keep its binary name rather than be narrowed")
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ }
+
+ /**
+ * Compile `nrw.Holder`, whose local class holds a named inner class extending `ArrayList`.
+ * `Holder$<n>Local$Inner` is a member class of a local class: neither anonymous nor local
+ * itself, with a null canonical name. javac's outer reference `this$0` is package-private
+ * and comes with no accessor method, so `getFields`/`getMethods` report nothing beyond
+ * `ArrayList`'s and the class is narrowable.
+ */
+ private def compileNestedLocalFixture(): File = {
val dir = Utils.createTempDir()
val src =
- "package dyn; public class DynHelper { public static int magic() { return 4242; } }"
+ """package nrw;
+ |public class Holder {
+ | public static Object make() {
+ | class Local {
+ | public class Inner extends java.util.ArrayList<String> { }
+ | }
+ | return new Local().new Inner();
+ | }
+ |}
+ |""".stripMargin
+ compileJavaFixtures(dir, Seq("nrw/Holder.java" -> src))
+ dir
+ }
+
+ /**
+ * Compile `shd.Holder`, whose anonymous subclass redeclares its supertype's public field.
+ * `getFields` reports both, so a field check that compares names rather than declaring
+ * classes accepts the pair, and narrowing then reads the supertype's value, since field
+ * access is resolved statically. Java, not Scala: a Scala `val` compiles to a private
+ * field plus an accessor, so it cannot shadow a public field.
+ *
+ * `makePublic` carries the same shadowing pair on a member class of a local class. javac
+ * strips `ACC_PUBLIC` from an anonymous class, which Janino then refuses to reference from
+ * the generated unit's package, so an end-to-end test needs this shape instead, for the same
+ * reason [[compileStaticHiderFixture]] uses one.
+ */
+ private def compileShadowedFieldFixture(): File = {
+ val dir = Utils.createTempDir()
+ val src =
+ """package shd;
+ |public class Holder {
+ | public static class Base { public int shadowed = 1; }
+ | public static Object make() { return new Base() { public int shadowed = 99; }; }
+ | public static Object makePublic() {
+ | class Local {
+ | public class Inner extends Base { public int shadowed = 99; }
+ | }
+ | return new Local().new Inner();
+ | }
+ |}
+ |""".stripMargin
+ compileJavaFixtures(dir, Seq("shd/Holder.java" -> src))
+ dir
+ }
+
+ /**
+ * Compile `sth.Holder`, whose member-of-local class hides its supertype's public static
+ * method. `getMethods` reports both, with identical erased signatures, so a signature-based
+ * check accepts the pair, and a narrowed call binds the supertype's method, since a static
+ * call is bound statically. A member class of a local class is used rather than an anonymous
+ * one because only the former keeps `ACC_PUBLIC`, which the Janino path needs.
+ */
+ private def compileStaticHiderFixture(): File = {
+ val dir = Utils.createTempDir()
+ val src =
+ """package sth;
+ |public class Holder {
+ | public static class Base { public static int hidden() { return 1; } }
+ | public static Object make() {
+ | class Local {
+ | public class Inner extends Base { public static int hidden() { return 99; } }
+ | }
+ | return new Local().new Inner();
+ | }
+ | public static Object makePlain() {
+ | class Local2 { public class Inner extends Base { } }
+ | return new Local2().new Inner();
+ | }
+ |}
+ |""".stripMargin
+ compileJavaFixtures(dir, Seq("sth/Holder.java" -> src))
+ dir
+ }
+
+ /**
+ * Compile `lnk2.Holder`, whose anonymous `Foo` declares a method taking `lnk2.Missing`.
+ * Deleting `Missing.class` afterwards leaves the anonymous class loadable with a readable
+ * supertype while `getMethods` throws: the asymmetric partial-jar shape.
+ */
+ private def compileMemberLinkageFixture(): File = {
+ val dir = Utils.createTempDir()
+ compileJavaFixtures(dir, Seq(
+ "lnk2/Missing.java" -> "package lnk2; public class Missing {}",
+ "lnk2/Foo.java" -> "package lnk2; public interface Foo { String extra(Missing m); }",
+ "lnk2/Holder.java" ->
+ """package lnk2;
+ |public class Holder {
+ | public static Object make() {
+ | return new Foo() { public String extra(Missing m) { return "e"; } };
+ | }
+ |}
+ |""".stripMargin))
+ dir
+ }
+
+ /** Compile Java source strings, given as (path, source) pairs, into `dir`. */
+ private def compileJavaFixtures(dir: File, sources: Seq[(String, String)]): Unit = {
val compiler = ToolProvider.getSystemJavaCompiler
val fm = compiler.getStandardFileManager(null, null, null)
try {
fm.setLocation(StandardLocation.CLASS_OUTPUT, Collections.singletonList(dir))
- val srcFile = new SimpleJavaFileObject(
- URI.create("string:///dyn/DynHelper.java"), JavaFileObject.Kind.SOURCE) {
- override def getCharContent(ignoreEncodingErrors: Boolean): CharSequence = src
+ val files = sources.map { case (path, src) =>
+ new SimpleJavaFileObject(URI.create(s"string:///$path"), JavaFileObject.Kind.SOURCE) {
+ override def getCharContent(ignoreEncodingErrors: Boolean): CharSequence = src
+ }.asInstanceOf[JavaFileObject]
}
- assert(
- compiler.getTask(null, fm, null, null, null, Collections.singletonList(srcFile)).call(),
- "failed to compile dyn.DynHelper fixture")
+ assert(compiler.getTask(null, fm, null, null, null, files.asJava).call(),
+ s"failed to compile fixture: ${sources.map(_._1).mkString(", ")}")
} finally {
fm.close()
}
+ }
+
+ /**
+ * Compile `lnk.Holder`, whose nested `Mid` holds an anonymous `Runnable`. Deleting
+ * `Holder$Mid.class` afterwards leaves `Holder$Mid$1` loadable but makes reflection over
+ * its enclosing class throw, which is the partial-jar shape the guards have to survive.
+ */
+ private def compileLinkageFixture(): File = {
+ val dir = Utils.createTempDir()
+ val src =
+ """package lnk;
+ |public class Holder {
+ | public static class Mid {
+ | public static Object make() { return new Runnable() { public void run() {} }; }
+ | }
+ |}
+ |""".stripMargin
+ compileJavaFixtures(dir, Seq("lnk/Holder.java" -> src))
+ dir
+ }
+
+ private def compileDynHelper(): File = {
+ val dir = Utils.createTempDir()
+ val src =
+ "package dyn; public class DynHelper { public static int magic() { return 4242; } }"
+ compileJavaFixtures(dir, Seq("dyn/DynHelper.java" -> src))
dir
}
@@ -726,6 +970,437 @@ 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.
+ // `specializedPrimitiveOverride` is the boxing case - scalac specializes the override to
+ // `apply(int)` while the bridge that reaches it takes `Object`. `anonWithLambdaBody` is
+ // the synthetic-static case: a closure in the body adds a `public static final
+ // $anonfun$...` helper javac cannot name, which must not make the class unnarrowable.
+ 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,
+ CodeCompilerSuite.specializedPrimitiveOverride, CodeCompilerSuite.anonWithLambdaBody)
+ 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: a synthetic static does not make a class unnarrowable") {
+ // Pins the fixture's own shape, so the case above cannot pass by accident if a future
+ // scalac stops emitting the helper as a public static on the anonymous class.
+ val cls = CodeCompilerSuite.anonWithLambdaBody.getClass
+ val statics = cls.getMethods.filter(m => Modifier.isStatic(m.getModifiers))
+ assert(statics.nonEmpty, s"fixture must carry a public static, got none on ${cls.getName}")
+ assert(statics.forall(_.isSynthetic),
+ s"fixture's statics must all be synthetic, got: ${statics.mkString(", ")}")
+ assert(statics.exists(_.getDeclaringClass eq cls),
+ s"at least one static must be declared by the anonymous class itself, got: " +
+ statics.map(_.getDeclaringClass.getName).mkString(", "))
+ }
+
+ 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, a local class with an extra
+ // method, a bridged override sharing its name and arity with an unrelated overload, and
+ // an instance method whose only counterpart on the supertype is static: 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,
+ CodeCompilerSuite.anonWithBridgeAndPrimitiveOverload,
+ CodeCompilerSuite.anonWithBridgeAndReferenceOverload,
+ CodeCompilerSuite.anonOverStaticClash)
+ 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: a bridge covers only the method it forwards to") {
+ // Guards the fixture shapes behind the two bridge tests: both classes carry a genuine
+ // bridge for the generic override, so a check that excused the whole name/arity group
+ // would let the unrelated overload ride along on it.
+ for (o <- Seq[Any](CodeCompilerSuite.anonWithBridgeAndPrimitiveOverload,
+ CodeCompilerSuite.anonWithBridgeAndReferenceOverload)) {
+ val compares = o.getClass.getMethods.filter(_.getName == "compare")
+ assert(compares.exists(_.isBridge), "fixture precondition: expected a bridge method")
+ assert(compares.count(m => !m.isBridge && m.getParameterCount == 2) === 2,
+ "fixture precondition: expected the override and the overload to share name and arity")
+ }
+ }
+
+ 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("referencesUnnarrowableClass: a field shadowing the supertype's cannot be narrowed") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The one silent-wrong-answer shape that methods do not have. `getFields` reports both
+ // the shadowing and the shadowed field, so comparing names would accept the pair, but
+ // field access is resolved statically: the narrowed reference would read the supertype's
+ // 1 instead of the anonymous class's 99, compiling cleanly the whole way.
+ val dir = compileShadowedFieldFixture()
+ val loader = new DirClassLoader(dir, getClass.getClassLoader)
+ val anon = loader.loadClass("shd.Holder").getMethod("make").invoke(null).getClass
+ assert(anon.getCanonicalName == null, s"expected an unnameable class, got: ${anon.getName}")
+ // Fixture preconditions: the shadowing pair is exactly what a name check would miss.
+ val vs = anon.getFields.filter(_.getName == "shadowed")
+ assert(vs.length === 2, s"expected two public `shadowed` fields: ${vs.mkString(", ")}")
+ assert(vs.exists(_.getDeclaringClass eq anon),
+ "one `shadowed` must be declared by the anonymous class")
+
+ val body = s"${anon.getName} v = (${anon.getName}) references[0];"
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(loader)
+ assert(JdkCodeCompiler.referencesUnnarrowableClass(body),
+ "a shadowed field must make the class unnarrowable")
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(body, loader) === body,
+ "an unnarrowable class must keep its binary name")
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ }
+
+ test("referencesUnnarrowableClass: a static method hiding the supertype's cannot be narrowed") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The method counterpart of the field case: `hidden()` erases identically on both
+ // classes, so a signature check accepts the pair, but a static call is bound statically:
+ // the narrowed reference would call the supertype's `hidden()` for 1 instead of 99, and
+ // Java permits the instance-qualified form so even a plain cast rebinds.
+ val dir = compileStaticHiderFixture()
+ val loader = new DirClassLoader(dir, getClass.getClassLoader)
+ val inner = loader.loadClass("sth.Holder").getMethod("make").invoke(null).getClass
+ assert(inner.getCanonicalName == null, s"expected an unnameable class, got: ${inner.getName}")
+ // Fixture preconditions: `f` is static and declared here, while the target declares an
+ // identically-erased static of its own: the pair a declaring-class check must reject and
+ // a signature check would accept. (`getMethods` reports only the hiding one; a static is
+ // hidden, not inherited.)
+ val fs = inner.getMethods.filter(_.getName == "hidden")
+ assert(fs.length === 1, s"expected one visible `f`, got: ${fs.mkString(", ")}")
+ assert((fs.head.getDeclaringClass eq inner) && Modifier.isStatic(fs.head.getModifiers),
+ s"`f` must be a static declared by the member class, got: ${fs.head}")
+ val base = loader.loadClass("sth.Holder$Base")
+ assert(base.getMethods.exists(m =>
+ m.getName == "hidden" && m.getParameterCount == 0 && Modifier.isStatic(m.getModifiers)),
+ "the target must declare the same static signature, or nothing would be hidden")
+ assert(inner.getSuperclass eq base, s"expected Base as superclass, got ${inner.getSuperclass}")
+ // And the complement, so both branches of the declaring-class check are pinned: a static
+ // the class merely INHERITS from the target stays narrowable.
+ val plain = loader.loadClass("sth.Holder").getMethod("makePlain").invoke(null).getClass
+ assert(plain.getCanonicalName == null, s"expected an unnameable class, got: ${plain.getName}")
+ assert(plain.getMethods.exists(m =>
+ m.getName == "hidden" && Modifier.isStatic(m.getModifiers) &&
+ (m.getDeclaringClass eq base)),
+ "the complement fixture must inherit the target's static rather than hide it")
+
+ val body = s"${inner.getName} v = (${inner.getName}) references[0];"
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(loader)
+ assert(JdkCodeCompiler.referencesUnnarrowableClass(body),
+ "a hidden static method must make the class unnarrowable")
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(body, loader) === body,
+ "an unnarrowable class must keep its binary name")
+ val plainBody = s"${plain.getName} v = (${plain.getName}) references[0];"
+ assert(!JdkCodeCompiler.referencesUnnarrowableClass(plainBody),
+ "merely inheriting the target's static must stay narrowable")
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(plainBody, loader) ===
+ s"sth.Holder.Base v = (sth.Holder.Base) references[0];",
+ "the complement fixture must narrow to the target")
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ }
+
+ 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, so the
+ // climb has to key off `getCanonicalName == null` to catch it. Its outer reference is
+ // reachable from neither side: javac's `this$0` is package-private with no accessor, and
+ // scalac's `$outer` pair is synthetic, so the member check sees nothing beyond
+ // ArrayList's either way (the two tests below cover the Scala shape).
+ val dir = compileNestedLocalFixture()
+ val loader = new DirClassLoader(dir, getClass.getClassLoader)
+ // Derive the binary name rather than hardcoding it: the JLS fixes the shape
+ // (`Holder$<digits>Local$Inner`) but leaves the digit sequence to the compiler.
+ val cls = loader.loadClass("nrw.Holder")
+ .getMethod("make").invoke(null).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
+ val body = s"$name v = ($name) references[0];"
+ // Fixture guard: this shape must stay on the JDK backend, or the rewrite is moot. The
+ // context loader has to be swapped because the fixture lives only in a temp dir.
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(loader)
+ assert(!JdkCodeCompiler.referencesUnnarrowableClass(body),
+ "fixture must be narrowable, otherwise it would route to Janino instead")
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ assert(JdkCodeCompiler.rewriteInnerClassRefs(body, loader) ===
+ "java.util.ArrayList v = (java.util.ArrayList) references[0];")
+ }
+
+ test("rewriteInnerClassRefs: keeps the binary name of an unnarrowable nested local class") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The complement of the test above: the same shape plus a non-synthetic public method
+ // `ArrayList` does not have, so narrowing would put it out of reach. The rewrite refuses
+ // rather than emitting a name that compiles but drops the member. The `$outer` field and
+ // accessor are NOT what makes this unnarrowable: being synthetic, they are unreachable
+ // from generated source either way. No context-loader swap is needed here: this fixture
+ // is on the suite's own classpath, unlike the temp-dir one above.
+ val cls = CodeCompilerSuite.memberOfLocalClassWithExtra.getClass
+ assert(cls.getCanonicalName == null, s"expected no canonical name for ${cls.getName}")
+ assert(cls.getMethods.exists(m => m.getName == "extra" && !m.isSynthetic),
+ "fixture must carry a non-synthetic public method absent from ArrayList")
+ val name = cls.getName
+ val body = s"$name v = ($name) references[0];"
+ assert(JdkCodeCompiler.referencesUnnarrowableClass(body),
+ "a member class with an extra public method must be reported unnarrowable")
+ assert(rewrite(body) === body, "an unnarrowable class must keep its binary name")
+ }
+
+ test("referencesUnnarrowableClass: synthetic outer accessors do not block narrowing") {
+ // `memberOfLocalClass` carries scalac's public `$outer` field and accessor, both synthetic
+ // and therefore invisible to javac's source lookup ("cannot find symbol"), so nothing a
+ // generator emits can reach them and narrowing to `ArrayList` loses nothing.
+ val cls = CodeCompilerSuite.memberOfLocalClass.getClass
+ assert(cls.getCanonicalName == null, s"expected no canonical name for ${cls.getName}")
+ val outerFields = cls.getFields.filter(_.getName.contains("outer"))
+ assert(outerFields.nonEmpty && outerFields.forall(_.isSynthetic),
+ s"fixture must carry a synthetic public outer field, got: ${outerFields.mkString(", ")}")
+ val extraMethods = cls.getMethods.filterNot(m =>
+ classOf[java.util.ArrayList[_]].getMethods.exists(_.getName == m.getName))
+ assert(extraMethods.nonEmpty && extraMethods.forall(_.isSynthetic),
+ s"fixture's extra methods must all be synthetic, got: ${extraMethods.mkString(", ")}")
+ val name = cls.getName
+ assert(!JdkCodeCompiler.referencesUnnarrowableClass(s"$name v = ($name) references[0];"),
+ "synthetic-only extras must not make a class unnarrowable")
+ }
+
+ 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)
+ }
+
+ test("end-to-end: a shadowing field reads the class's own value, not the target's") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The verdict tests pin the decision; this pins the outcome. `Inner` redeclares `Base`'s
+ // public `shadowed`, and a field read is bound statically, so the two names answer
+ // differently at run time: 99 through the class, 1 through the supertype. Routing keeps 99.
+ val dir = compileShadowedFieldFixture()
+ val loader = new DirClassLoader(dir, getClass.getClassLoader)
+ val value = loader.loadClass("shd.Holder").getMethod("makePublic").invoke(null)
+ val name = value.getClass.getName
+ assert(value.getClass.getCanonicalName == null, s"expected an unnameable class: $name")
+ val hazard = newCodeAndComment(
+ s"""
+ |public java.lang.Object generate(Object[] references) {
+ | $name v = ($name) references[0];
+ | return Integer.valueOf(v.shadowed);
+ |}
+ """.stripMargin)
+ // The counterfactual: the same read spelled with the name narrowing would emit. Janino
+ // compiles it too, and it answers 1, which is exactly why it may not be emitted.
+ val narrowed = newCodeAndComment(
+ """
+ |public java.lang.Object generate(Object[] references) {
+ | shd.Holder.Base v = (shd.Holder.Base) references[0];
+ | return Integer.valueOf(v.shadowed);
+ |}
+ """.stripMargin)
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(loader)
+ // Janino names the class itself, so it binds the class's own field.
+ assert(JaninoCodeCompiler.compile(hazard)._1.generate(Array[Any](value)) === 99)
+ withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") {
+ assert(CodeCompiler.active(hazard) eq JaninoCodeCompiler)
+ val (generated, _) = CodeGenerator.compile(hazard)
+ assert(generated.generate(Array[Any](value)) === 99,
+ "routing must preserve the shadowing field's value")
+ }
+ assert(JaninoCodeCompiler.compile(narrowed)._1.generate(Array[Any](value)) === 1,
+ "counterfactual: the narrowed name reads the supertype's field")
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ }
+
+ test("end-to-end: a hidden static call reaches the class's own method, not the target's") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The method counterpart. `Inner.hidden()` hides `Base.hidden()` with an identical
+ // erasure, and a static call is bound statically even in the instance-qualified form, so
+ // the narrowed reference would answer 1 where the class answers 99.
+ val dir = compileStaticHiderFixture()
+ val loader = new DirClassLoader(dir, getClass.getClassLoader)
+ val value = loader.loadClass("sth.Holder").getMethod("make").invoke(null)
+ val name = value.getClass.getName
+ assert(value.getClass.getCanonicalName == null, s"expected an unnameable class: $name")
+ val hazard = newCodeAndComment(
+ s"""
+ |public java.lang.Object generate(Object[] references) {
+ | $name v = ($name) references[0];
+ | return Integer.valueOf(v.hidden());
+ |}
+ """.stripMargin)
+ val narrowed = newCodeAndComment(
+ """
+ |public java.lang.Object generate(Object[] references) {
+ | sth.Holder.Base v = (sth.Holder.Base) references[0];
+ | return Integer.valueOf(v.hidden());
+ |}
+ """.stripMargin)
+ val prev = Thread.currentThread().getContextClassLoader
+ try {
+ Thread.currentThread().setContextClassLoader(loader)
+ assert(JaninoCodeCompiler.compile(hazard)._1.generate(Array[Any](value)) === 99)
+ withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") {
+ assert(CodeCompiler.active(hazard) eq JaninoCodeCompiler)
+ val (generated, _) = CodeGenerator.compile(hazard)
+ assert(generated.generate(Array[Any](value)) === 99,
+ "routing must preserve the hiding static's value")
+ }
+ assert(JaninoCodeCompiler.compile(narrowed)._1.generate(Array[Any](value)) === 1,
+ "counterfactual: the narrowed name calls the supertype's static")
+ } finally {
+ Thread.currentThread().setContextClassLoader(prev)
+ }
+ }
+
+ test("end-to-end: an instance method clashing with a static keeps the class's value") {
+ assume(JdkCodeCompiler.isAvailable, "javax.tools.JavaCompiler not available")
+ // The third shape: `value()` is an INSTANCE method on the anonymous class whose only
+ // counterpart on `StaticClashBase` is STATIC. Java permits the instance-qualified form
+ // for a static, so the narrowed call compiles and binds the target's `value()`, giving 1
+ // instead of 7. Both fixtures are on the suite's own classpath, so no loader swap here.
+ val anon = CodeCompilerSuite.anonOverStaticClash
+ val name = anon.getClass.getName
+ assert(anon.getClass.getCanonicalName == null, s"expected an unnameable class: $name")
+ val target = classOf[StaticClashBase].getName
+ val hazard = newCodeAndComment(
+ s"""
+ |public java.lang.Object generate(Object[] references) {
+ | $name v = ($name) references[0];
+ | return Integer.valueOf(v.value());
+ |}
+ """.stripMargin)
+ val narrowed = newCodeAndComment(
+ s"""
+ |public java.lang.Object generate(Object[] references) {
+ | $target v = ($target) references[0];
+ | return Integer.valueOf(v.value());
+ |}
+ """.stripMargin)
+ assert(JaninoCodeCompiler.compile(hazard)._1.generate(Array[Any](anon)) === 7)
+ withSQLConf(SQLConf.CODEGEN_COMPILER.key -> "jdk") {
+ assert(CodeCompiler.active(hazard) eq JaninoCodeCompiler)
+ val (generated, _) = CodeGenerator.compile(hazard)
+ assert(generated.generate(Array[Any](anon)) === 7,
+ "routing must preserve the instance method's value")
+ }
+ assert(JdkCodeCompiler.compile(narrowed)._1.generate(Array[Any](anon)) === 1,
+ "counterfactual: the narrowed name calls the target's static")
+ }
+
// ---------------- Function1 apply(Object) bridge ----------------
test("stripFunction1ApplyBridges removes the bridge but keeps apply(InternalRow)") {
@@ -834,4 +1509,120 @@ 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 `narrowingVerdict`.
+ 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 INSTANCE method whose only counterpart on the supertype is STATIC. scalac allows it,
+ // since it does not treat a Java static as an inherited member, so `value()` is not an
+ // override; javac rejects the pair outright, which is why `StaticClashBase` is written in
+ // Java. A narrowed call binds the supertype's static `value()`, returning 1 rather than 7.
+ val anonOverStaticClash = new StaticClashBase {
+ def value(): Int = 7
+ }
+
+ // 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"
+ }
+
+ // A bridged generic override alongside an unrelated overload of the same name and arity.
+ // `compare(String, String)` is reached through a `compare(Object, Object)` bridge, but
+ // `compare(Int, Int)` has no bridge of its own: after narrowing to `Comparator`, an
+ // integer call would bind to `compare(Object, Object)` and land in the String-casting
+ // bridge instead of the overload.
+ val anonWithBridgeAndPrimitiveOverload = new java.util.Comparator[String] {
+ override def compare(a: String, b: String): Int = a.compareTo(b)
+ def compare(a: Int, b: Int): Int = a - b
+ }
+
+ // The same collision with a reference-typed overload. Boxing-blind parameter matching
+ // would accept this one, since the bridge's `Object` parameters do accept `Integer`.
+ val anonWithBridgeAndReferenceOverload = new java.util.Comparator[String] {
+ override def compare(a: String, b: String): Int = a.compareTo(b)
+ def compare(a: Integer, b: Integer): Int = a - b
+ }
+
+ // Sound counterpart of the two above, and the reason bridge matching treats boxing as
+ // equivalence: scalac specializes this override to `apply(int)`, which is reached through
+ // an `apply(Object)` bridge. Requiring the bridge parameter to accept the override's
+ // parameter without boxing would reject it, since `Object` is not assignable from `int`.
+ val specializedPrimitiveOverride = new scala.runtime.AbstractFunction1[Int, Boolean] {
+ def apply(i: Int): Boolean = i > 0
+ }
+
+ // A lambda in the body makes scalac emit a `public static final $anonfun$...` helper on the
+ // anonymous class. It is synthetic, so javac cannot name it ("cannot find symbol") and no
+ // generated reference can reach it, so narrowing loses nothing and the static check has to
+ // excuse it or every closure-carrying anonymous class would route to Janino.
+ val anonWithLambdaBody = new java.util.Comparator[String] {
+ override def compare(a: String, b: String): Int = {
+ val len: String => Int = _.length
+ len(a) - len(b)
+ }
+ }
+
+ 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. The `$outer` field and accessor scalac adds are synthetic, hence
+ // unreachable from generated source and no obstacle to narrowing.
+ val memberOfLocalClass: Any = {
+ class Holder {
+ class Inner extends java.util.ArrayList[String]
+ def make(): Any = new Inner
+ }
+ new Holder().make()
+ }
+
+ // The same shape plus one NON-synthetic public method, which is what actually puts a member
+ // out of reach of `ArrayList`. Used where the test needs an unnarrowable member-of-local.
+ val memberOfLocalClassWithExtra: Any = {
+ class Holder {
+ class Inner extends java.util.ArrayList[String] {
+ def extra(): String = "e"
+ }
+ def make(): Any = new Inner
+ }
+ new Holder().make()
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org