Skip to content

Commit

Permalink
Allow interface Mixins (#51)
Browse files Browse the repository at this point in the history
* Allow mixining interfaces

Reapplies #13
Fixes #34
Includes (a form of) SpongePowered#413 and SpongePowered#415

* Fix mods using static vagueness

Whilst SpongePowered#415 is generating things more correctly, there are mods (such as Better End) which rely on the current behaviour

* Fix lambdas in interface Mixins

Fixes #43
  • Loading branch information
Chocohead authored Jun 28, 2021
1 parent cf6c9a2 commit 38825f7
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,6 @@ public void notifyRemapped() {
}

public void registerInjector(AnnotatedElementInjector elem) {
if (this.mixin.isInterface()) {
this.ap.printMessage(Kind.ERROR, "Injector in interface is unsupported", elem.getElement());
}

for (String reference : elem.getAnnotation().<String>getList("method")) {
ITargetSelector targetSelector = TargetSelector.parse(reference);

Expand Down Expand Up @@ -260,10 +256,6 @@ private boolean registerInjector(AnnotatedElementInjector elem, String reference
* and process the references
*/
public void registerInjectionPoint(AnnotatedElementInjectionPoint elem, String format) {
if (this.mixin.isInterface()) {
this.ap.printMessage(Kind.ERROR, "Injector in interface is unsupported", elem.getElement());
}

if (!elem.shouldRemap()) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@

import java.util.ArrayList;

import org.apache.logging.log4j.LogManager;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.tree.AnnotationNode;
import org.objectweb.asm.tree.MethodNode;
import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException;
import org.spongepowered.asm.mixin.refmap.IMixinContext;
import org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException;
import org.spongepowered.asm.util.asm.ASM;

/**
Expand All @@ -54,10 +54,12 @@ public AccessorGenerator(AccessorInfo info, boolean isStatic) {
}

protected void checkModifiers() {
if (this.info.isStatic() && !this.targetIsStatic) {
IMixinContext context = this.info.getContext();
throw new InvalidInjectionException(context, String.format("%s is invalid. Accessor method is%s static but the target is not.",
this.info, this.info.isStatic() ? "" : " not"));
if (this.info.isStatic() != this.targetIsStatic) {
if (!this.targetIsStatic) {
throw new InvalidAccessorException(this.info, String.format("%s is invalid. Accessor method is static but the target is not.", this.info));
} else {
LogManager.getLogger("mixin").info("{} should be static as its target is", this.info);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.objectweb.asm.tree.MethodNode;
import org.objectweb.asm.tree.VarInsnNode;
import org.spongepowered.asm.mixin.MixinEnvironment.Option;
import org.spongepowered.asm.mixin.gen.throwables.InvalidAccessorException;
import org.spongepowered.asm.mixin.Mutable;
import org.spongepowered.asm.mixin.transformer.ClassInfo.Method;
import org.spongepowered.asm.mixin.transformer.MixinTargetContext;
Expand All @@ -52,6 +53,12 @@ public AccessorGeneratorFieldSetter(AccessorInfo info) {

@Override
public void validate() {
if (Bytecode.hasFlag(this.info.getClassNode(), Opcodes.ACC_INTERFACE)) {
//This will result in a ClassFormatError when the class is verified
throw new InvalidAccessorException(info, String.format("%s tried to change interface field %s::%s",
this.info, this.info.getClassNode().name, this.targetField.name));
}

super.validate();

Method method = this.info.getClassInfo().findMethod(this.info.getMethod());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,10 @@ public MethodNode generate() {
method.instructions.add(new VarInsnNode(Opcodes.ALOAD, 0));
}
Bytecode.loadArgs(this.argTypes, method.instructions, this.targetIsStatic ? 0 : 1);
boolean isInterface = Bytecode.hasFlag(this.info.getClassNode(), Opcodes.ACC_INTERFACE);
boolean isPrivate = Bytecode.hasFlag(this.targetMethod, Opcodes.ACC_PRIVATE);
int opcode = this.targetIsStatic ? Opcodes.INVOKESTATIC : (isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL);
method.instructions.add(new MethodInsnNode(opcode, this.info.getClassNode().name, this.targetMethod.name, this.targetMethod.desc, false));
int opcode = this.targetIsStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : (isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL);
method.instructions.add(new MethodInsnNode(opcode, this.info.getClassNode().name, this.targetMethod.name, this.targetMethod.desc, isInterface));
method.instructions.add(new InsnNode(this.returnType.getOpcode(Opcodes.IRETURN)));
return method;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,10 @@ protected AbstractInsnNode invokeHandler(InsnList insns) {
* @return injected insn node
*/
protected AbstractInsnNode invokeHandler(InsnList insns, MethodNode handler) {
boolean isInterface = Bytecode.hasFlag(classNode, Opcodes.ACC_INTERFACE);
boolean isPrivate = (handler.access & Opcodes.ACC_PRIVATE) != 0;
int invokeOpcode = this.isStatic ? Opcodes.INVOKESTATIC : isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL;
MethodInsnNode insn = new MethodInsnNode(invokeOpcode, this.classNode.name, handler.name, handler.desc, false);
int invokeOpcode = this.isStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : isPrivate ? Opcodes.INVOKESPECIAL : Opcodes.INVOKEVIRTUAL;
MethodInsnNode insn = new MethodInsnNode(invokeOpcode, this.classNode.name, handler.name, handler.desc, isInterface);
insns.add(insn);
this.info.addCallbackInvocation(handler);
return insn;
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/org/spongepowered/asm/mixin/struct/MemberRef.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.tree.FieldInsnNode;
import org.objectweb.asm.tree.MethodInsnNode;
import org.spongepowered.asm.mixin.transformer.ClassInfo;
import org.spongepowered.asm.mixin.transformer.throwables.MixinTransformerError;
import org.spongepowered.asm.util.Bytecode;

Expand Down Expand Up @@ -241,7 +242,7 @@ public void setOpcode(int opcode) {
if (tag == 0) {
throw new MixinTransformerError("Invalid opcode " + Bytecode.getOpcodeName(opcode) + " for method handle " + this.handle + ".");
}
boolean itf = tag == Opcodes.H_INVOKEINTERFACE;
boolean itf = this.handle.isInterface();
this.handle = new org.objectweb.asm.Handle(tag, this.handle.getOwner(), this.handle.getName(), this.handle.getDesc(), itf);
}

Expand All @@ -252,7 +253,8 @@ public String getOwner() {

@Override
public void setOwner(String owner) {
boolean itf = this.handle.getTag() == Opcodes.H_INVOKEINTERFACE;
boolean itf = this.handle.isInterface();
assert ClassInfo.forName(owner).isInterface() == itf;
this.handle = new org.objectweb.asm.Handle(this.handle.getTag(), owner, this.handle.getName(), this.handle.getDesc(), itf);
}

Expand All @@ -263,7 +265,7 @@ public String getName() {

@Override
public void setName(String name) {
boolean itf = this.handle.getTag() == Opcodes.H_INVOKEINTERFACE;
boolean itf = this.handle.isInterface();
this.handle = new org.objectweb.asm.Handle(this.handle.getTag(), this.handle.getOwner(), name, this.handle.getDesc(), itf);
}

Expand All @@ -274,7 +276,7 @@ public String getDesc() {

@Override
public void setDesc(String desc) {
boolean itf = this.handle.getTag() == Opcodes.H_INVOKEINTERFACE;
boolean itf = this.handle.isInterface();
this.handle = new org.objectweb.asm.Handle(this.handle.getTag(), this.handle.getOwner(), this.handle.getName(), desc, itf);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@
*/
package org.spongepowered.asm.mixin.transformer;

import java.lang.reflect.Modifier;
import java.util.Map.Entry;

import org.objectweb.asm.tree.FieldNode;
import org.objectweb.asm.tree.MethodNode;
import org.spongepowered.asm.mixin.MixinEnvironment;
import org.spongepowered.asm.mixin.MixinEnvironment.CompatibilityLevel.LanguageFeature;
import org.spongepowered.asm.mixin.injection.struct.InjectionInfo;
import org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException;
import org.spongepowered.asm.mixin.transformer.ClassInfo.Field;
Expand Down Expand Up @@ -96,28 +99,34 @@ protected void applyInitialisers(MixinTargetContext mixin) {
*/
@Override
protected void prepareInjections(MixinTargetContext mixin) {
// disabled for interface mixins
for (MethodNode method : this.targetClass.methods) {
try {
InjectionInfo injectInfo = InjectionInfo.parse(mixin, method);
if (injectInfo != null) {
throw new InvalidInterfaceMixinException(mixin, injectInfo + " is not supported on interface mixin method " + method.name);
//Make sure we're running on a Java version which supports interfaces having method bodies
if (!MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.METHODS_IN_INTERFACES)) {
throw new InvalidInterfaceMixinException(mixin, injectInfo + " is not supported on interface mixin method " + method.name);
}
}
} catch (InvalidInjectionException ex) {
String description = ex.getInjectionInfo() != null ? ex.getInjectionInfo().toString() : "Injection";
throw new InvalidInterfaceMixinException(mixin, description + " is not supported in interface mixin");
}
}

super.prepareInjections(mixin);
}

/* (non-Javadoc)
* @see org.spongepowered.asm.mixin.transformer.MixinApplicator
* #applyInjections(
* org.spongepowered.asm.mixin.transformer.MixinTargetContext)
*/
@Override
protected void applyInjections(MixinTargetContext mixin) {
// Do nothing
}
protected void checkMethodVisibility(MixinTargetContext mixin, MethodNode mixinMethod) {
//Allow injecting into static interface methods where it isn't possible to control the access of the injection method
if (Modifier.isStatic(mixinMethod.access) && !MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.PRIVATE_METHODS_IN_INTERFACES)) {
InjectionInfo injectInfo = InjectionInfo.parse(mixin, mixinMethod);
if (injectInfo != null) {
return;
}
}

super.checkMethodVisibility(mixin, mixinMethod);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ private static void createProxy(MethodNode methodNode, ClassInfo targetClass, Me
Type[] args = Type.getArgumentTypes(methodNode.desc);
Type returnType = Type.getReturnType(methodNode.desc);
Bytecode.loadArgs(args, methodNode.instructions, 0);
methodNode.instructions.add(new MethodInsnNode(Opcodes.INVOKESTATIC, targetClass.getName(), method.getName(), methodNode.desc, false));
methodNode.instructions.add(new MethodInsnNode(Opcodes.INVOKESTATIC, targetClass.getName(), method.getName(), methodNode.desc, targetClass.isInterface()));
methodNode.instructions.add(new InsnNode(returnType.getOpcode(Opcodes.IRETURN)));
methodNode.maxStack = Bytecode.getFirstNonArgLocalIndex(args, false);
methodNode.maxLocals = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.tree.AnnotationNode;
import org.objectweb.asm.tree.FieldNode;
import org.spongepowered.asm.mixin.MixinEnvironment;
import org.spongepowered.asm.mixin.MixinEnvironment.CompatibilityLevel.LanguageFeature;
import org.spongepowered.asm.mixin.transformer.ClassInfo.Method;
import org.spongepowered.asm.mixin.transformer.MixinInfo.MixinClassNode;
import org.spongepowered.asm.mixin.transformer.MixinInfo.MixinMethodNode;
Expand Down Expand Up @@ -58,8 +60,11 @@ class MixinPreProcessorInterface extends MixinPreProcessorStandard {
protected void prepareMethod(MixinMethodNode mixinMethod, Method method) {
// Userland interfaces should not have non-public methods except for lambda bodies
if (!Bytecode.hasFlag(mixinMethod, Opcodes.ACC_PUBLIC) && !Bytecode.hasFlag(mixinMethod, Opcodes.ACC_SYNTHETIC)) {
throw new InvalidInterfaceMixinException(this.mixin, "Interface mixin contains a non-public method! Found " + method + " in "
+ this.mixin);
//On versions that support it private methods are also allowed
if (!Bytecode.hasFlag(mixinMethod, Opcodes.ACC_PRIVATE) || !MixinEnvironment.getCompatibilityLevel().supports(LanguageFeature.PRIVATE_METHODS_IN_INTERFACES)) {
throw new InvalidInterfaceMixinException(this.mixin, "Interface mixin contains a non-public method! Found " + method + " in "
+ this.mixin);
}
}

super.prepareMethod(mixinMethod, method);
Expand Down

1 comment on commit 38825f7

@modmuss50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh, this does not merge nicely into 0.8.3

Please sign in to comment.