[
https://issues.apache.org/jira/browse/GROOVY-12303?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109254#comment-18109254
]
ASF GitHub Bot commented on GROOVY-12303:
-----------------------------------------
daniellansun commented on PR #2834:
URL: https://github.com/apache/groovy/pull/2834#issuecomment-5454483959
@blackdrag
Thank you — the missing definition was the real problem, and the previous
recovery path was mixing the two strategies.
The contract is now on `ClassNodeResolver` (the four-mode table in the class
javadoc). `asmResolving` and `classLoaderResolving` are independent searches,
not each other’s fallback:
| Mode | ASM | Class loader | Lookup | ASM format error | `loadClass` CNFE |
`loadClass` NCDFE | Script |
|---|---|---|---|---|---|---|---|
| default | on | on | ASM first; `loadClass` only if ASM has no match |
swallowed, then `loadClass` | `tryAsScript(null)` | wrap and rethrow, unless
ASM already classified a bytecode-name mismatch | replace a found class only
with `cls.getClassLoader() != loader` (or, on an ASM match, parent-visible
`.class`) and a newer source |
| ASM-only | on | off | ASM only | thrown | n/a | n/a | no match →
`tryAsScript(null)` |
| loader-only | off | on | `loadClass` only | n/a | `tryAsScript(null)` |
wrap and rethrow; **no decompile** | success path only (`cls.getClassLoader()
!= loader`) |
| neither | off | off | groovy source only | n/a | n/a | n/a |
`tryAsScript(null)` (`java.*` / `$` skipped) |
`ClassHelper.make(name).isResolved()` in the ASM branch is the interned-type
table (`String`, primitives, …), not `loadClass`. ASM-only never calls
`loadClass`. Loader-only never decompiles.
GROOVY-12303 for unlinkable bytecode is the **default** (and ASM-only) path:
ASM runs first, so a missing superclass never reaches `loadClass`. Loader-only
with unlinkable bytes is unrecoverable by design — that is where the mix used
to live.
On your two code points:
1. The `LookupResult` constructor tests are gone. Coverage is `resolveName`
/ `findClassNode` across all four modes (`@MethodSource('modes')`), plus
`CompilationUnit.compile` for unlinkable types (default succeeds; loader-only
throws the wrapped `NoClassDefFoundError`).
2. Agreed: `parent.getResource` is not `cls.getClassLoader() != loader`, and
on NCDFE there is no `Class`. We do not treat a parent resource as proof that
the failed class was defined by the parent, and we do not add a
`parent.loadClass` probe (`GroovyClassLoader`’s protected `loadClass` would
compile scripts). NCDFE therefore does not replace a class with a script,
except an ASM bytecode-name mismatch (the requested name never existed). The
live-`Class` check is unchanged on the success path.
Thank you again for insisting on the matrix before more recovery patches.
> ClassNodeResolver: NoClassDefFoundError during class-loader lookup aborts
> resolution
> ------------------------------------------------------------------------------------
>
> Key: GROOVY-12303
> URL: https://issues.apache.org/jira/browse/GROOVY-12303
> Project: Groovy
> Issue Type: Bug
> Reporter: Daniel Sun
> Priority: Major
>
> When class-loader lookup is used ({{{}asmResolving{}}} off, or the type
> exists only in memory), {{ClassNodeResolver}} calls
> {{{}GroovyClassLoader.loadClass{}}}. If the requested class *exists* but
> cannot be linked (missing superclass or interface), the JVM throws
> {{{}NoClassDefFoundError{}}}.
> That error used to escape resolution. Compilation aborted with an {{Error}}
> that named the {*}missing dependency{*}, not the type being resolved.
> {{resolveName}} could also cache the name as a miss ({{{}NO_CLASS{}}}), so a
> later successful compile of the dependency would not be retried.
> A TODO in {{findByClassLoading}} has noted this since the 2012 split out of
> {{{}ResolveVisitor{}}}.
> h3. Expected
> * If bytecode for the requested name is still on the class path, decompile
> it (ASM does not link) and continue.
> * Else if a groovy source of the same name is available, add it to the
> compilation queue.
> * Else if a {{.class}} resource exists for that path but declares a
> different binary name (JVM {{defineClass}} name check; also case-insensitive
> filesystems), treat the lookup as a miss. Detect this from the bytecode name,
> not from {{NoClassDefFoundError}} text (HotSpot's {{wrong name}} phrase is
> English-only; OpenJ9 does not use it).
> * Otherwise rethrow {{NoClassDefFoundError}} with the looked-up name in the
> message, and do not cache {{{}NO_CLASS{}}}.
> h3. Actual
> {{NoClassDefFoundError}} propagated out of
> {{{}ClassNodeResolver.findByClassLoading{}}}.
> h3. Reproducer
> Put {{HasDep.class}} (extends a type that is not loadable) on the class path,
> disable ASM resolving, and compile:
> {code:groovy}
> HasDep x = null
> {code}
> This fails with {{NoClassDefFoundError}} for the missing super-type instead
> of resolving {{{}HasDep{}}}.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
