This is an automated email from the ASF dual-hosted git repository.
LuciferYang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new b3c0402a9c8d [SPARK-57370][SQL][FOLLOWUP] Route codegen referencing an
unnameable class to Janino when narrowing is unsound
b3c0402a9c8d is described below
commit b3c0402a9c8da97c655a52c791cc59d6f8ea9789
Author: YangJie <[email protected]>
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 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
---
.../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 8d4856a9c6ce..38bf897d9ba9 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
@@ -2973,9 +2973,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: [email protected]
For additional commands, e-mail: [email protected]
