blackdrag commented on PR #2736:
URL: https://github.com/apache/groovy/pull/2736#issuecomment-5128475894
[...]
> 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).
which means we add a lot of machinery for a small feature.
```
class A {}
class B extends A {}
class C extends B {}
def call2(x,y){x.m2(y)}
def b = new B()
A.metaClass.m2 = {x -> "A3"}
assert call2(b,0) == "A3" // m2 discovered by findMethodInClassHierarchy
A.metaClass.m2 = {Integer x -> "A4"}
assert call2(b,0) == "A4" // Mutation of parent. More specific m2 discovered
by findMethodInClassHierarchy?
B.metaClass.m2 = {x -> "B3"}
assert call2(b,0) == "B3" // B.m2 *blocks* findMethodInClassHierarchy, more
specific method on A ignored!
assert call2(new C(),0) == "A4" // B.m2 does *not block*
findMethodInClassHierarchy.
```
Sure, considering how this is implemented it makes sense, but from a user
side I think it does not.
> * 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”.
[...]
> 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).
So if class A extends B and A or B have a method foo, and we have a callsite
new B().foo(x), then we do not need to invalidate it if the meta class of A
changes. Only if the metaClass for A changes (new or mutates) then we have to
invalidate the callsite. If this is the case, then why the hierarchy logic? If
I take the above together I get for example something like this on a pure meta
class level:
```
class D{def foo(x){1}}
class E extends D{}
assert new E().foo(0) == 1
D.metaClass.foo = {Integer x -> 2}
assert new E().foo(0) == 1
registry.removeMetaClass(E)
assert new E().foo(0) == 2
registry.removeMetaClass(D)
assert new E().foo(0) == 2
```
Only replacing the meta class of E made the more specific foo in D visible
and it stays visible, even though we removed the meta class of D. Is it
because of the declared method?
```
class F {}
class G extends F {}
F.metaClass.foo = {Integer x -> 1}
F.metaClass.bar = {Integer x -> -1}
assert new F().foo(0) == 1
assert new G().foo(0) == 1 // F.foo visible
G.metaClass.foo = {x -> 2}
assert new G().foo(0) == 1 // F.foo more specific
registry.removeMetaClass(F)
assert new G().foo(0) == 1 // F meta class replaced, EMC(G) still sees the
old F.foo, no switch to G.foo
F.metaClass.foo = {Integer x -> 10}
F.metaClass.bar = {Integer x -> -10}
G.metaClass.bar = {x -> -20}
assert new G().foo(0) == 1 // new EMC(F) with new foo(Integer) method, still
no change for EMC(G)
assert new G().bar(0) == -1 // bar invoked first time, still sees the old
bar method only.
```
No it is not. To sum it up: In what case *must* the mutation/replacement of
a parent meta class invalidate a callsite if the meta class of the current
receiver stays unchanged?
My goal is not to define new meta class logic, my goal is to find out for
what that hierarchy switch point logic is actually needed. It is not clear to
me from the code. So what exactly is it trying to solve?
--
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]
