[
https://issues.apache.org/jira/browse/GROOVY-12610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136987#comment-18136987
]
ASF GitHub Bot commented on GROOVY-12610:
-----------------------------------------
paulk-asert commented on PR #3169:
URL: https://github.com/apache/groovy/pull/3169#issuecomment-6595793782
This spike explores the opposite direction to #3137. Instead of rejecting
nest-mate private access under `@TypeChecked`, it makes dynamic dispatch reach
a private field or method of a superclass from a subclass nested with it, as
statically compiled code already does.
The fallback in `MetaClassImpl` runs only when the caller's own view has no
such member, and only if all of these hold:
- the caller (for a closure, its enclosing class) is a subclass of the
declaring class, and the receiver is an instance of the caller;
- the caller and the declaring class are nest-mates;
- the caller is not an inner class whose chain of enclosing instances leads
to the declaring class. That case keeps resolving to the enclosing instance
(GROOVY-12533).
So a nest-mate outside the hierarchy still can't reach the member through a
subclass instance, which keeps GROOVY-10198's "private members aren't
inherited". A per-metaclass flag keeps the `methodMissing` / `propertyMissing`
miss path at a field read; I measured no difference from master. The selected
suites pass: `groovy.lang`, `o.c.g.runtime`, STC/SC, `gls`, `bugs`,
`groovy.util`, `o.c.g.transform` and `vmplugin`, 14,232 tests in all.
**Known gap:** the sender-less property entry points name the receiver's
class as the caller. Those are `getProperty(Object, String)`,
`getAttribute(Object, String)` and `InvokerHelper.getProperty`. So a caller
outside the nest can still reach the field through them. Closing that properly
needs the caller (ideally its `Lookup`) carried through the MOP, which fits
with GEP-31's lexical, call-site-carried realm context.
`Base` declares `private String z` and `private String pm()`. Java results
are from `javac` (lambdas stand in for closures). "runtime failure" means
`MissingPropertyException`, `MissingMethodException` or `MissingFieldException`.
| Access | Java | master (dyn / TC) | spike (dyn / TC) | CS |
|---|---|---|---|---|
| **nested `static class Sub extends Base`** | | | | |
| `this.z` | ✗ private access | runtime failure | ✓ | ✓ |
| bare `z` | ✗ static context | runtime failure | ✓ | ✓ |
| `super.z` | ✓ | runtime failure | ✓ | ✓ |
| `((Base) this).z` | ✓ | runtime failure | ✓ | ✓ |
| `that.z` (`Sub that`) | ✗ | runtime failure | ✓ | ✓ |
| closure/lambda `this.z` / `super.z` | ✗ / ✓ | runtime failure | ✓ | ✓ |
| write `this.z = 'y'` | ✗ | runtime failure | ✓ | ✓ |
| `this.@z` | n/a | ✓ | ✓ | ✓ |
| `this.pm()` | ✗ cannot find symbol | runtime failure | ✓ | ✓ |
| `super.pm()` (also in a closure/lambda) | ✓ | runtime failure | ✓ | ✓ |
| bare `pm()` in a closure/lambda | ✗ static context | runtime failure | ✓ |
✓ |
| **`SubSub extends Sub`** | | | | |
| `this.z` / `((Base) this).z` | ✗ / ✓ | runtime failure | ✓ | ✓ |
| **`Other`, a nest-mate but not a subclass** | | | | |
| `sub.z` / `sub.pm()` / `sub.@z` | ✗ | runtime failure | runtime failure |
✓ |
| `((Base) sub).z` / `((Base) sub).pm()` | ✓ | runtime failure | runtime
failure | ✓ |
| **class outside the nest** | | | | |
| `sub.z` | ✗ | runtime failure | runtime failure | compile error |
| **`Sub` declares its own `getZ()` / `propertyMissing`** | n/a | `getZ` /
`missing z` | `getZ` / `x` | `getZ` / `x` |
| **inner (non-static) `Sub`** | | | | |
| bare `z` | ✓ the enclosing instance's | enclosing instance | enclosing
instance | enclosing instance |
| `this.z` | ✗ | enclosing instance | enclosing instance | enclosing
instance |
Java decides by the static type of the qualifying expression: `super`, or a
cast to `Base`, is allowed; a subclass-typed receiver is not. Dynamic dispatch
only sees the runtime class and the caller, so it can't follow that rule
exactly. Within the hierarchy the spike is more permissive than Java. The
`Other` + cast rows are the one place it's stricter, and there only
compile-time resolution (CS, or a direct-invocation tier in GEP-31) could give
Java's answer.
**Behaviour change to note:** a nested subclass with `propertyMissing` now
reads its nest-mate superclass's private field instead of getting the
`propertyMissing` call. That matches what CS does.
**Possible paths forward**
1. **6.0.x:** keep the current runtime behaviour, and have type checking
report the access instead of letting it fail at run time. That's #3137, ideally
narrowed later to match this spike's rule (reject only callers outside the
receiver's hierarchy) and extended to private methods. No dynamic semantics
change in a patch line.
2. **Groovy 7, now:** turn this spike into a real change. Add the matrix as
a regression test, close the property gap by giving the sender-less property
entry points the same treatment as `invokeMethod(Object, String, Object[])`,
and add a COMPATIBILITY.md entry for the `propertyMissing` precedence change.
Then drop the #3137 rule on master, since TC's nest-mate leniency becomes
correct.
3. **Groovy 7, via GEP-31:** leave dynamic semantics alone for now. Instead,
propose that realm-aware call sites carry the caller's `Lookup` alongside the
realm token. That gives a reliable caller identity, and nest-mate private
access through the JVM's own access checks, without `setAccessible`.
Direct-invocation tiers would also cover the cast cases dynamic dispatch can't
see. The matrix above would be the acceptance table, and `GROOVY_DYN` would
inherit whichever rule we settle on.
My preference: (1) for 6.0.x regardless. For 7, (2) is a self-contained
improvement available now, but it changes `GROOVY_DYN` semantics, which (3)
might then revise again. I'd only take (2) if GEP-31 is unlikely to land in the
7 timeframe.
> STC accepts access to a private superclass field through a nested subclass
> under @TypeChecked, which fails at run time
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12610
> URL: https://issues.apache.org/jira/browse/GROOVY-12610
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 4.0.33, 5.0.8, 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> {code:groovy}
> class Base {
> private String z = 'x'
> @groovy.transform.TypeChecked
> static class Sub extends Base {
> def m() { this.z } // also: z, super.z, closures, lambdas, writes,
> that.z
> }
> }
> new Base.Sub().m()
> {code}
> Actual: the code compiles, then fails with MissingPropertyException: No such
> property: z for class: Base$Sub. Dynamic Groovy fails the same way.
> Expected: under @TypeChecked, a compile error "No such property: z for class:
> Base$Sub", as already reported when Base and Sub are both nested in another
> class. @CompileStatic keeps accepting the code, because it accesses the field
> directly from a nest-mate.
> Cause: the type checker uses the nest-mate accessibility rule of static
> compilation for @TypeChecked code too. Type-checked code is dispatched
> dynamically, and the metaclass of a subclass does not list a private field of
> its superclass.
> Seen in 4.0.33, 5.0.8 and 6.0.0.
> PR: https://github.com/apache/groovy/pull/3137
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
