jdaugherty commented on PR #3086:
URL: https://github.com/apache/groovy/pull/3086#issuecomment-6716162143
@paulk-asert Thanks for the thorough probe. Pushed 08b2089eec:
1. Cold tier: `ColdReflectiveMethodHandleWrapper.tryBuild` now also keeps
`selector.metaClassSwitchPoint`, and `isValidFor` rejects the wrapper once it
is invalidated.
2. Tests: `IndyForeignMetaClassSwitchPointTest` now runs each scenario for a
subclass given the type's EMC (Spock `Mock()`), a subclass answering the type's
registry metaclass (GORM proxy), and a `DelegatingMetaClass` over the EMC as a
control. The scenarios are the method replaced twice, and the real method
called first, then replaced, each for a method call and for `+=`. The cold tier
runs in a fresh JVM with `-Dgroovy.indy.cold.reflection=true`
(`IndyForeignMetaClassColdTierProbe`, the same pattern as `IndyLoggingProbe`),
and `IndyScopedSwitchPointTest` has a unit test for the new `isValidFor` check.
Without the `isValidFor` check, only the cold-tier test fails
(`SHARED_METACLASS`: expected `replaced`, was `real`). On the PR's base, every
proxy-shape case fails and the `DelegatingMetaClass` controls pass.
3. Filled in the missing cells in the table above. I ran the GORM-proxy and
`DelegatingMetaClass` rows against the released 5.1.3 and 6.0.0 jars.
The `DelegatingMetaClass` control covers method calls only. While adding it
I found that a compound-assignment site does not guard on the receiver's
metaclass instance. `IndyCompoundAssign` guards on the receiver and argument
classes and the SwitchPoints, while a method call also gets `Selector`'s
metaclass-identity guard. Two instances of one Groovy class with different
metaclasses therefore share whichever `plus` was linked first:
```groovy
class Plain { def plus(int n) { 'real' } }
def addOne(v) { def r = v; r += 1; r }
def a1 = new Plain(); a1.metaClass.plus = { int n -> 'x' }
assert addOne(a1) == 'x'
assert addOne(new Plain()) == 'real' // 'x' on 6.0.0 and master, with or
without this PR; 'real' on 5.1.3
```
It is independent of this change, so I left it out of this PR; it should get
its own issue.
The 6.0.x backport branch
[`GROOVY-12558-6_0_X`](https://github.com/jdaugherty/groovy/tree/GROOVY-12558-6_0_X)
has the same commit (f03c0d2e1a). The indy, `ClassInfo` and
`ColdReflectionParityTest` suites pass on both branches.
--
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]
