Skip to content

Commit

Permalink
Support injecting into and overwriting constructors
Browse files Browse the repository at this point in the history
  • Loading branch information
Runemoro committed Jul 13, 2018
1 parent 2c72246 commit 84519bc
Show file tree
Hide file tree
Showing 15 changed files with 154 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ private void processShadows(RoundEnvironment roundEnv) {

if (elem.getKind() == ElementKind.FIELD) {
this.mixins.registerShadow((TypeElement)parent, (VariableElement)elem, shadow);
} else if (elem.getKind() == ElementKind.METHOD) {
} else if (elem.getKind() == ElementKind.METHOD || elem.getKind() == ElementKind.CONSTRUCTOR) {
this.mixins.registerShadow((TypeElement)parent, (ExecutableElement)elem, shadow);
} else {
this.mixins.printMessage(Kind.ERROR, "Element is not a method or field", elem);
this.mixins.printMessage(Kind.ERROR, "Element is not a method, constructor or field", elem);
}
}
}
Expand All @@ -127,10 +127,10 @@ private void processOverwrites(RoundEnvironment roundEnv) {
continue;
}

if (elem.getKind() == ElementKind.METHOD) {
if (elem.getKind() == ElementKind.METHOD || elem.getKind() == ElementKind.CONSTRUCTOR) {
this.mixins.registerOverwrite((TypeElement)parent, (ExecutableElement)elem);
} else {
this.mixins.printMessage(Kind.ERROR, "Element is not a method", elem);
this.mixins.printMessage(Kind.ERROR, "Element is not a method or constructor", elem);
}
}
}
Expand Down
45 changes: 45 additions & 0 deletions src/main/java/org/spongepowered/asm/mixin/Copy.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* This file is part of Mixin, licensed under the MIT License (MIT).
*
* Copyright (c) SpongePowered <https://www.spongepowered.org>
* Copyright (c) contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.spongepowered.asm.mixin;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* <p>Annotation used to indicate that a constructor should be copied to the
* target class.</p>
*
* <p>The default behaviour of mixin classes when merging mixin methods is to
* not copy any constructors that are not annotated with {@link Overwrite}. This
* annotation can be used to override that behaviour and copy a constructor to
* the target class.</p>
*/
@Target({ ElementType.CONSTRUCTOR })
@Retention(RetentionPolicy.RUNTIME)
public @interface Copy {

}
2 changes: 1 addition & 1 deletion src/main/java/org/spongepowered/asm/mixin/Overwrite.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
* overwrite a member in the target class, and should be added to the
* obfuscation table if {@link #remap} is true.</p>
*/
@Target({ ElementType.METHOD })
@Target({ ElementType.METHOD, ElementType.CONSTRUCTOR })
@Retention(RetentionPolicy.RUNTIME)
public @interface Overwrite {

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/spongepowered/asm/mixin/Shadow.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
* Used to indicate a Mixin class member which is acting as a placeholder for a
* method or field in the target class
*/
@Target({ ElementType.METHOD, ElementType.FIELD })
@Target({ ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.FIELD })
@Retention(RetentionPolicy.RUNTIME)
public @interface Shadow {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.spongepowered.asm.mixin.injection.InjectionPoint;
import org.spongepowered.asm.mixin.injection.Surrogate;
import org.spongepowered.asm.mixin.injection.code.Injector;
import org.spongepowered.asm.mixin.injection.points.BeforeReturn;
import org.spongepowered.asm.mixin.injection.struct.InjectionInfo;
import org.spongepowered.asm.mixin.injection.struct.Target;
import org.spongepowered.asm.mixin.injection.struct.InjectionNodes.InjectionNode;
Expand Down Expand Up @@ -369,12 +368,20 @@ protected void sanityCheck(Target target, List<InjectionPoint> injectionPoints)
if (target.isStatic != this.isStatic) {
throw new InvalidInjectionException(this.info, "'static' modifier of callback method does not match target in " + this);
}
}

protected void checkTargetForNode(Target target, InjectionNode node) {
if (target.isCtor) {
MethodInsnNode superCall = target.findSuperOrThisInitNode();
int superCallIndex = target.indexOf(superCall);
int targetIndex = target.indexOf(node.getCurrentTarget());
if (targetIndex <= superCallIndex) {
if (!this.isStatic) {
throw new InvalidInjectionException(this.info, "Pre-super " + this.info + " injection must be static in " + this);
}

if (Constants.CTOR.equals(target.method.name)) {
for (InjectionPoint injectionPoint : injectionPoints) {
if (!injectionPoint.getClass().equals(BeforeReturn.class)) {
throw new InvalidInjectionException(this.info, "Found injection point type " + injectionPoint.getClass().getSimpleName()
+ " targetting a ctor in " + this + ". Only RETURN allowed for a ctor target");
if (this.cancellable) {
throw new InvalidInjectionException(this.info, "Pre-super " + this.info + " injection must not be cancellable in " + this);
}
}
}
Expand Down Expand Up @@ -422,6 +429,7 @@ protected void inject(Target target, InjectionNode node) {
locals = Locals.getLocalsAt(this.classNode, target.method, node.getCurrentTarget());
}

this.checkTargetForNode(target, node);
this.inject(new Callback(this.methodNode, target, node, locals, this.localCapture.isCaptureLocals()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ protected final void checkTargetModifiers(Target target, boolean exactMatch) {
*/
protected void checkTargetForNode(Target target, InjectionNode node) {
if (target.isCtor) {
MethodInsnNode superCall = target.findSuperInitNode();
MethodInsnNode superCall = target.findSuperOrThisInitNode();
int superCallIndex = target.indexOf(superCall);
int targetIndex = target.indexOf(node.getCurrentTarget());
if (targetIndex <= superCallIndex) {
Expand All @@ -129,6 +129,7 @@ protected void inject(Target target, InjectionNode node) {
+ " in " + this);
}

this.checkTargetForNode(target, node);
this.injectAtInvoke(target, node);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,6 @@ protected void checkTarget(Target target) {
}
}

@Override
protected void inject(Target target, InjectionNode node) {
this.checkTargetForNode(target, node);
super.inject(target, node);
}

/**
* Do the injection
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,6 @@ public ModifyArgsInjector(InjectionInfo info) {
protected void checkTarget(Target target) {
this.checkTargetModifiers(target, false);
}

@Override
protected void inject(Target target, InjectionNode node) {
this.checkTargetForNode(target, node);
super.inject(target, node);
}

/**
* Do the injection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,18 +490,20 @@ public MethodInsnNode findInitNodeFor(TypeInsnNode newNode) {
}

/**
* Find the call to <tt>super()</tt> in a constructor. This attempts to
* locate the first call to <tt>&lt;init&gt;</tt> which isn't an inline call
* to another object ctor being passed into the super invocation.
* Find the call to <tt>super()</tt> or <tt>this()</tt> in a constructor.
* This attempts to locate the first call to <tt>&lt;init&gt;</tt> which
* isn't an inline call to another object ctor being passed into the
* super invocation.
*
* @return Call to <tt>super()</tt> or <tt>null</tt> if not found
* @return Call to <tt>super()</tt> or <tt>this()</tt>, or <tt>null</tt>
* if not found.
*/
public MethodInsnNode findSuperInitNode() {
public MethodInsnNode findSuperOrThisInitNode() {
if (!this.isCtor) {
return null;
}

return Bytecode.findSuperInit(this.method, ClassInfo.forName(this.classNode.name).getSuperName());
return Bytecode.findSuperOrThisInit(this.method, this.classNode.name, ClassInfo.forName(this.classNode.name).getSuperName());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.spongepowered.asm.lib.tree.MethodNode;
import org.spongepowered.asm.mixin.struct.MemberRef;
import org.spongepowered.asm.mixin.transformer.ClassInfo.Method;
import org.spongepowered.asm.util.Constants;

/**
* Base for class context objects
Expand Down Expand Up @@ -107,7 +108,7 @@ private void upgradeMethod(MethodNode method) {
}

protected void upgradeMethodRef(MethodNode containingMethod, MemberRef methodRef, Method method) {
if (methodRef.getOpcode() != Opcodes.INVOKESPECIAL) {
if (methodRef.getOpcode() != Opcodes.INVOKESPECIAL || Constants.CTOR.equals(methodRef.getName())) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -772,9 +772,7 @@ void addMethod(MethodNode method) {
}

private void addMethod(MethodNode method, boolean injected) {
if (!method.name.startsWith("<")) {
this.methods.add(new Method(method, injected));
}
this.methods.add(new Method(method, injected));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.spongepowered.asm.lib.signature.SignatureReader;
import org.spongepowered.asm.lib.signature.SignatureVisitor;
import org.spongepowered.asm.lib.tree.*;
import org.spongepowered.asm.mixin.Copy;
import org.spongepowered.asm.mixin.Final;
import org.spongepowered.asm.mixin.Intrinsic;
import org.spongepowered.asm.mixin.MixinEnvironment;
Expand Down Expand Up @@ -441,13 +442,26 @@ protected void applyShadowMethod(MixinTargetContext mixin, MethodNode shadow) {
}

protected void applyNormalMethod(MixinTargetContext mixin, MethodNode mixinMethod) {
boolean isMeargableConstructor = Constants.CTOR.equals(mixinMethod.name)
&& (Annotations.getVisible(mixinMethod, Overwrite.class) != null
|| Annotations.getVisible(mixinMethod, Copy.class) != null);

if (isMeargableConstructor) {
this.checkConstructorCall(mixin, mixinMethod);
}

// Reparent all mixin methods into the target class
mixin.transformMethod(mixinMethod);

if (!mixinMethod.name.startsWith("<")) {
this.checkMethodVisibility(mixin, mixinMethod);
this.checkMethodConstraints(mixin, mixinMethod);
this.mergeMethod(mixin, mixinMethod);
} else if (isMeargableConstructor) {
mixin.transformMethod(mixinMethod);
this.checkMethodVisibility(mixin, mixinMethod);
this.checkMethodConstraints(mixin, mixinMethod);
this.mergeMethod(mixin, mixinMethod);
} else if (Constants.CLINIT.equals(mixinMethod.name)) {
// Class initialiser insns get appended
this.appendInsns(mixin, mixinMethod);
Expand Down Expand Up @@ -677,11 +691,14 @@ protected void applyInitialisers(MixinTargetContext mixin) {
return;
}

// Patch the initialiser into the target class ctors
// Patch the initialiser into the target class ctors and ctors from other mixins
for (MethodNode method : this.targetClass.methods) {
if (Constants.CTOR.equals(method.name)) {
method.maxStack = Math.max(method.maxStack, ctor.maxStack);
this.injectInitialiser(mixin, method, initialiser);
AnnotationNode merged = Annotations.getVisible(method, MixinMerged.class);
if (merged == null || !mixin.getClassName().equals(Annotations.<String>getValue(merged, "mixin"))) {
method.maxStack = Math.max(method.maxStack, ctor.maxStack);
this.injectInitialiser(mixin, method, initialiser);
}
}
}
}
Expand Down Expand Up @@ -843,7 +860,6 @@ protected final void injectInitialiser(MixinTargetContext mixin, MethodNode ctor

AbstractInsnNode insn = this.findInitialiserInjectionPoint(mixin, ctor, initialiser);
if (insn == null) {
this.logger.warn("Failed to locate initialiser injection point in <init>{}, initialiser was not mixed in.", ctor.desc);
return;
}

Expand Down Expand Up @@ -886,7 +902,12 @@ protected AbstractInsnNode findInitialiserInjectionPoint(MixinTargetContext mixi
AbstractInsnNode insn = iter.next();
if (insn.getOpcode() == Opcodes.INVOKESPECIAL && Constants.CTOR.equals(((MethodInsnNode)insn).name)) {
String owner = ((MethodInsnNode)insn).owner;
if (owner.equals(targetName) || owner.equals(targetSuperName)) {

if (owner.equals(targetName)) {
return null;
}

if (owner.equals(targetSuperName)) {
targetInsn = insn;
if (mode == InitialiserInjectionMode.SAFE) {
break;
Expand All @@ -900,6 +921,10 @@ protected AbstractInsnNode findInitialiserInjectionPoint(MixinTargetContext mixi
}
}

if (targetInsn == null) {
this.logger.warn("Failed to locate initialiser injection point in <init>{}, initialiser was not mixed in.", ctor.desc);
}

return targetInsn;
}

Expand Down Expand Up @@ -1012,6 +1037,35 @@ protected final void checkConstraints(MixinTargetContext mixin, MethodNode metho
}
}

/**
* Check that a method calls a either another constructor in the mixin or a constructor in the
* superclass of the target or a mixin targetting that superclass.
*
* @param mixin Mixin being applied
* @param method annotated method
*/
protected void checkConstructorCall(MixinTargetContext mixin, MethodNode method) {
String targetName = mixin.getTarget().getClassName();
String superName = mixin.getTarget().getClassInfo().getSuperName();

MethodInsnNode constructor = Bytecode.findSuperOrThisInit(method, targetName, superName);
if (constructor == null) {
String message = String.format("Constructor %s%s in %s does call a this or super constructor copied to the target class", method.name, method.desc, mixin);
throw new InvalidMixinException(mixin, message);
} else if (constructor.name.equals(superName)) {
ClassInfo targetSuperClass = mixin.getTarget().getClassInfo().getSuperClass();
ClassInfo mixinSuperClass = mixin.getClassInfo().getSuperClass();

if (!mixinSuperClass.equals(targetSuperClass)) {
// Necessary because updateBinding may have updated a call to super in a class other than the target's super or mixin targetting it
String message = String.format("Constructor %s%s in %s contains call to super constructor, but mixin does " +
"not extend target's superclass or mixin targetting it",
method.name, method.desc, mixin);
throw new InvalidMixinException(mixin, message);
}
}
}

/**
* Finds a method in the target class
* @param searchFor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,8 @@ protected boolean attachSpecialMethod(MixinTargetContext context, MixinMethodNod
mixinMethod.name = method.renameTo(target.name);
}

if (Constants.CTOR.equals(target.name)) {
throw new InvalidMixinException(this.mixin, String.format("Nice try! %s in %s cannot alias a constructor", mixinMethod.name, this.mixin));
if (Constants.CTOR.equals(target.name) && !Constants.CTOR.equals(mixinMethod.name)) {
throw new InvalidMixinException(this.mixin, String.format("%s in %s cannot alias a constructor", mixinMethod.name, this.mixin));
}

if (!Bytecode.compareFlags(mixinMethod, target, Opcodes.ACC_STATIC)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.spongepowered.asm.mixin.SoftOverride;
import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
import org.spongepowered.asm.mixin.gen.AccessorInfo;
import org.spongepowered.asm.mixin.injection.Constant;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.struct.InjectionInfo;
import org.spongepowered.asm.mixin.injection.struct.InjectorGroupInfo;
Expand Down Expand Up @@ -539,7 +540,14 @@ private void transformMethodRef(MethodNode method, Iterator<AbstractInsnNode> it
methodRef.setDesc(this.transformMethodDescriptor(methodRef.getDesc()));
} else if (this.detachedSuper || this.inheritsFromMixin) {
if (methodRef.getOpcode() == Opcodes.INVOKESPECIAL) {
this.updateStaticBinding(method, methodRef);
if (Constants.CTOR.equals(methodRef.getName())) {
if (methodRef.getOwner().equals(mixin.getClassInfo().getSuperName()) &&
this.mixin.getClassInfo().getSuperClass().isMixin()) {
this.updateStaticBinding(method, methodRef);
}
} else {
this.updateStaticBinding(method, methodRef);
}
} else if (methodRef.getOpcode() == Opcodes.INVOKEVIRTUAL && ClassInfo.forName(methodRef.getOwner()).isMixin()) {
this.updateDynamicBinding(method, methodRef);
}
Expand Down
Loading

0 comments on commit 84519bc

Please sign in to comment.