daniellansun commented on PR #2736: URL: https://github.com/apache/groovy/pull/2736#issuecomment-5120228130
@blackdrag Thank you for the correction and for the detailed parent→child matrix — that is exactly the right level of precision. ## 1. Hierarchy direction Understood: the scenario of interest is **superclass MetaClass changes observed from a child receiver**, not the reverse. Sorry the earlier matrix inverted that ladder. The tests now follow your corrected direction (`A` parent → `B`/`C`/`D` children). ## 2. Missing-method-only walk (and the C/D gap) Agreed on the important refinement: - `MetaClassImpl.findMethodInClassHierarchy` is a **missing-method** path. It only opens a real walk when some strong MetaClass in the hierarchy is a **modified** `MutableMetaClass` (EMC is the usual case). - A method that already exists on the child keeps winning for applicable arguments after hierarchy re-link; SwitchPoint retirement does **not** rebuild MetaClass tables. - The **C vs D** divergence (same structure, different MetaClass construction time relative to parent EMC) is pre-existing MOP behaviour: construction-time snapshot / inheritance of ancestor expando methods into a new `MetaClassImpl`, not something class-domain SwitchPoints can honestly “fix”. **6.0 decision for this PR:** preserve current MOP visibility rules; scope invalidation to match those rules. Making `MetaClassImpl` immune to other MetaClasses (and removing the hierarchy walk from it) is a separate semantic redesign — see follow-ups below. Documented on `IndyInvalidation`, `ClassHierarchyIndex`, `package-info`, and `ClassInfo.incVersion`. ## 3. Callsite caching vs hierarchy version Agreed that **most of your second script is driven by MetaClass change on the receiver**, not by a hierarchy version: | Step | What retires the site | Runtime | |------|----------------------|---------| | `C` becomes EMC | exact-class domain (receiver MC changed) | `call(c,0) → -2` | | restore class-level `MetaClassImpl` | exact-class | `→ 40` | | `removeMetaClass(C)` + fresh `MetaClassImpl` while `A` has EMC | exact-class | `→ -2` (construction snapshot) | | `A.metaClass = null` | hierarchy fan-out (parent EMC remove) | still `→ -2` (snapshot retained) | Hierarchy SwitchPoint is only there so **already-linked miss** sites on subtypes re-select when an ancestor becomes / stops being a modified mutable MetaClass (e.g. linked miss → parent adds method; linked hit → parent EMC removed). Measured note on the last line of your draft (`c.metaClass = emc` after `A` is cleared): the draft expected a “clueless” EMC → `40`, but the EMC created while `A` still had methods **retains** `ClosureMetaMethod`s for `m2(Integer)` / `m2(String)`, so resolution stays `-2`. Same retention idea as the `MetaClassImpl` snapshot — not live super lookup after `A` is gone. Captured in `Groovy12191.blackdragScenario_callsiteCaching_metaClassChangeNotHierarchyVersion`. Pure Java inheritance matches the miss path (`LinkedHashMap` / `HashMap`). ## 4. Policy (your +1) Unchanged and reaffirmed: - pure unmodified `MetaClassImpl` ↔ `MetaClassImpl` (or null) → **exact class only** - EMC install / remove / in-place update, global EMC, interface / array → **class + hierarchy** - per-instance MetaClass → **exact class only** - category / unscoped → bulk ## 5. MetaClass-owned SwitchPoint — follow-up Agreed, and we will file follow-up JIRAs (not landing in this PR): 1. **MetaClass-owned SwitchPoints** for indy MOP invalidation (natural home for MC-specific update/replace, true per-instance domains, custom MetaClass kinds). 2. **`MetaClassImpl` hierarchy immunity** — decide whether `MetaClassImpl` should stop observing other MetaClasses (construction-time snapshot + `findMethodInClassHierarchy`), so C/D timing is no longer load-order dependent. I will link the issue keys in `IndyInvalidation` / `package-info` once they exist. ## 6. Regression coverage added - `blackdragScenario_missingMethodOnly_andMetaClassImplTiming` - `blackdragScenario_callsiteCaching_metaClassChangeNotHierarchyVersion` - `parentEmcAfterLinkedMiss_childCallSiteSeesNewMethod` - `parentEmcRemoveAfterLinkedHit_childCallSiteMissesAgain` - `parentEmc_doesNotReplacePresentChildMethod_afterHierarchyRelink` - `javaSubtype_seesParentEmc_andHierarchyFanOut` Happy to adjust further if any row of the matrix still disagrees with the behaviour you want for 6.0. -- 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]
