SpongePowered / Mixin

Mixin is a trait/mixin and bytecode weaving framework for Java using ASM
MIT License
1.43k stars 193 forks source link

@Instrinsic method causes AbstractMethodError #346

Closed Frontear closed 5 years ago

Frontear commented 5 years ago

Parent

class Parent {
    private final Session = new Session();
    private Renderer = new Renderer();

    public void Session getSession() {
        return session;
    }

    public void Renderer getRenderer() {
        return renderer;
    }
}

Interface

interface IParent {
    Session getSession();
    Renderer getRenderer();
}

Mixin

@Mixin(Parent.class) @Interface(iface = IParent.class, prefix = "wrap$") class MixinParent {
    @Shadow @Final private Session session;
    @Shadow private Renderer renderer;

    @Intrinsic public void Session wrap$getSession() {
        return session;
    }

    @Intrinsic public void Renderer wrap$getRenderer() {
        return renderer;
    }
}

Usage

public static void main(String[] args) {
    IParent parent = (IParent) Parent.getInstance(); // not null, don't worry about that
    parent.getRenderer().something(); // same error
    parent.getSession().otherThing(); // AbstractMethodError, why?
}

I don't understand fundamentally what is different between these two methods, yet they have different results. What might be causing this?

Environment: ForgeGradle 2.1 Mixin Version: 0.7.11

Frontear commented 5 years ago

Incorrect assumption on my part, both methods do not work. I don't understand why the debugging environment works but production doesn't..

Frontear commented 5 years ago

Update

My error was caused by the simple fact that I didn't correctly relay to Mixin that IParent is implemented. Basically, I need to @Implements(@Interface(iface = IParent.class, prefix = "wrap$")).

I don't like that this was completely glossed over in development, as it did not trip any errors, in fact, it worked completely fine in development, however when I tested in production, it obviously failed. I believe there should be some checks in place to prevent this from happening, maybe case an error in development as well so that the developer can fix such a mistake.

Mumfrey commented 5 years ago

Sorry but there's no bug here, you just seem to have some fundamental misunderstandings about the role of @Intrinsic and soft overrides. Since implementing interfaces is one of the most fundamental aspects of Mixin's feature set it should come as not surprise that this is extensively documented.

It is not my fault therefore if you break something by not understanding the code you're writing. I've done my level best to make sure the information is available, digestible and well-presented.

What doesn't help here is that you're using a canned example which itself doesn't make any sense, so you're not doing a good job of articulating your point. Since the return types are invariant there's no need to use soft implementation at all.

As explained in the documentation, there is no problem with merging methods when the overwrite in question is invariant over return type. Soft-implements can help when methods have variant return types because the JVM supports all bindings over the full method signature, it's only Java which does not. With invariant return types, soft overwrites can also help due to intrinsic proxying.

Incorrect assumption on my part, both methods do not work. I don't understand why the debugging environment works but production doesn't..

You don't understand? It's because the target method exists in the deobfuscated development environment (and therefore implements your interface method, regardless of the mixin) and doesn't in production. This is explained in the documentation.


In an effort to prevent this happening again, I will expand the documentation of @Interface and consider putting some checks in place so that if it's used incorrectly a warning is raised. Closing as invalid because I can't fix a failure to RTFM.