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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
