MixinPreProcessorStandard incorrectly checks STATIC flag on @Unique fields, a crash happens
Steps to reproduce:
- Create a Mixin like this (1.18.2, Mojang names, Fabric):
import net.minecraft.server.packs.repository.Pack;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Unique;
@Mixin(Pack.class)
public final class TestMixin {
// Matches name of instance field "id" in the target class
@Unique
private static String id;
}
- Launch the client
Actual behavior: crash at start
Caused by: org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException: STATIC modifier of @Shadow field id in ***.TestMixin does not match the target
at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.attachFields(MixinPreProcessorStandard.java:632)
at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.attach(MixinPreProcessorStandard.java:302)
at org.spongepowered.asm.mixin.transformer.MixinPreProcessorStandard.createContextFor(MixinPreProcessorStandard.java:277)
at org.spongepowered.asm.mixin.transformer.MixinInfo.createContextFor(MixinInfo.java:1289)
at org.spongepowered.asm.mixin.transformer.MixinApplicatorStandard.apply(MixinApplicatorStandard.java:292)
at org.spongepowered.asm.mixin.transformer.TargetClassContext.apply(TargetClassContext.java:421)
at org.spongepowered.asm.mixin.transformer.TargetClassContext.applyMixins(TargetClassContext.java:403)
at org.spongepowered.asm.mixin.transformer.MixinProcessor.applyMixins(MixinProcessor.java:363)
... 16 more
Expected behavior: the static field id gets renamed to something unuque and launch proceeds
This is a rare edge case (it occured when I was obfuscating a mod with some custom techiques), but I beleive that this is an incorrect behavior -- what's the point in checking static modifier of a field, if it gets renamed to something unuqie anyway and will not conflict with anything?
I propose that compareFlags should be called only for @Shadow fields, like this:
if (isShadow && !Bytecode.compareFlags(mixinField, target, Opcodes.ACC_STATIC)) {
throw new InvalidMixinException(this.mixin, String.format("STATIC modifier of @Shadow field %s in %s does not match the target",
mixinField.name, this.mixin));
}
Or, at least the message could be more clear -- the field is not @Shadow after all. Something like "STATIC modifier of @Unique fields should match modifier of the shadowed field in the target class". After writing it down it sounds really strange :)
Yeah the entire point of @Unique is "I am adding this and don't intend to accidentall clobber an existing member, so needing it to match doesn't make sense, it's just a bug plain and simple.