Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic interfaces cannot be soft-implemented #169

Open
gabizou opened this issue Mar 10, 2017 · 4 comments
Open

Generic interfaces cannot be soft-implemented #169

gabizou opened this issue Mar 10, 2017 · 4 comments

Comments

@gabizou
Copy link
Member

gabizou commented Mar 10, 2017

Take the following interfaces:

public interface Foo<A extends Foo> {

    public A bar();

}

public interface Baz extends Foo<Baz> {

}

And with the following Mixin:

@Mixin(Bam.class)
@Implements(@Interface(iface = Baz.class, prefix = "baz$"))
public abstract class MixinBam {

   public Baz baz$bar() {
     return this;
   }

}

Mixin will fail to locate the desired generic method and as a result fail.

I haven't tested whether a workaround of not prefixing the generic method and having MixinBam implement Baz will work (of course it compiles, but I haven't checked if at runtime it applies cleanly).

@Mumfrey
Copy link
Member

Mumfrey commented Mar 10, 2017

I'll take a look at this on the 17th. This kind of implementatio needs special attention because of the need to handle synthetic bridges.

@Mumfrey Mumfrey self-assigned this Mar 10, 2017
@Mumfrey
Copy link
Member

Mumfrey commented Mar 21, 2017

Just to provide some explanation as to why this is non-trivial. When implementing a generic interface this way, where a type argument appears in a signature of an interface method, the compiler generates "bridge methods" which allow dynamic binding against both the derived interface and the superinterface. To take your example, if this wasn't a Mixin then the following code:

public class Bam implements Baz {
    public Baz bar() {
        return this;
    }
}

would actually result in the following in the compiled class bytecode:

public class Bam implements Baz {
    public Baz bar() {
        return this;
    }

    // Synthetic bridge method with signature matching the interface in
    // which the method was declared
    public Foo bar() {
        return this.bar(); // calls the method above
    }
}

Of course there's no way to explicitly define this type of bridge method in java code, since methods with signatures which differ only by return type are not allowed in Java, though they are of course perfectly legal in bytecode.

There is another type of bridge method which is used when a method parameter is generic:

interface Damagable<T extends Number> {
    void applyDamage(T amount);
}

interface PartialDamagable extends Damagable<Float> {
}

class Gavin implements PartialDamagable {
    public void applyDamage(Float amount) {
        // stuff
    }
}

In this case, the bridge method which takes the supertype will have to cast down the argument to the correct type:

// synthetic bridge
public void applyDamage(Number amount) {
    this.applyDamage((Float)amount);
}

These two scenarios can be combined of course. It also gets more complicated if there are multiple type arguments in the interface signature.

Normally when you implement an interface "for real" in a Mixin, the bridges are generated by the compiler and Mixin just has to apply the generated methods. However in this case Mixin itself is going to have to generate the correct bridges, which is a nontrivial task.

To achieve this, the following will have to be implemented:

  • Resolving the correct generic type arguments for the interface in question. For interfaces with no generic arguments this is not too hard, but for generic interfaces themselves it will require some way of specifying the type arguments in the @Interface annotation, which will of course need to be parsed and verified.
  • Working out, based on the superinterface hierarchy, what synthetic bridges need to be generated
  • Generating the necessary bridges at application time, this isn't too tricky since bridges themselves have a simple, predictable structure.

Whilst I would normally mark this as enhancement because it's an upgrade to a pre-existing feature, in this situation I am going to apply the feature tag because this represents quite a large jump from the existing functionality which effectively represents a new feature in Mixin's feature set.

@Mumfrey
Copy link
Member

Mumfrey commented Mar 21, 2017

Added the annotation processor tag as well since support for this feature will need to be integrated into the AP.

@Mumfrey Mumfrey changed the title Cannot Implement prefix generic methods Generic interfaces cannot be soft-implemented Mar 21, 2017
@gabizou
Copy link
Member Author

gabizou commented Mar 16, 2022

Not to be necro'ing this issue, but just ran into somewhat of a similar problem with a semi related issue: SpongePowered/Sponge#3647

In short, as I suspected was the reasoning case for what's happening in the issue above:

MinecraftServerMixin_API is generically extending ReentrantBlockableEventLoop as the target class is, but the byte code output still includes the synthetic bridge method tell:

     public bridge synthetic tell(java.lang.Object arg0) { //(Ljava/lang/Object;)V
         <localVar:index=0 , name=this , desc=Lorg/spongepowered/common/mixin/api/minecraft/server/MinecraftServerMixin_API;, sig=null, start=L1, end=L2>

         L1 {
             aload0 // reference to self
             aload1
             checkcast java/lang/Runnable
             invokespecial net/minecraft/util/thread/ReentrantBlockableEventLoop.tell(Ljava/lang/Runnable;)V
             return
         }
         L2 {
         }
     }

And this is the crux of the issue, this method is inherited by the extending class ReentrantBlockableEventLoop<Runnable>, and no real way to make this not happen except by extending ReentrantBlockableEventLoop with a raw type...

Unless a hack workaround like @Shadow might work out in this case to explicitly define the tell as a Runnable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants