Skip to content

Fix ResolverSyntheticInstanceof incorrectly converting non-boolean ModifyExpressionValue to ModifyInstanceofValue#10

Open
SavageVictor wants to merge 1 commit into
Sinytra:1.21.xfrom
SavageVictor:fix/modify-expression-value-instanceof-return-type
Open

Fix ResolverSyntheticInstanceof incorrectly converting non-boolean ModifyExpressionValue to ModifyInstanceofValue#10
SavageVictor wants to merge 1 commit into
Sinytra:1.21.xfrom
SavageVictor:fix/modify-expression-value-instanceof-return-type

Conversation

@SavageVictor
Copy link
Copy Markdown

Problem

When a @ModifyExpressionValue mixin targets a method call whose result is used in a nearby instanceof check, ResolverSyntheticInstanceof (via the skipInsnComparison path) would unconditionally convert the annotation to @ModifyInstanceofValue.

This is only correct when the handler returns boolean (i.e. it truly modifies the boolean result of the instanceof check). If the handler returns a non-boolean type — because it modifies the value fed into the instanceof check — the conversion produces an invalid method signature like (LBlock;)LBlock; where (Z)Z is required, crashing at mixin apply time with:

InvalidInjectionException: @ModifyInstanceofValue ... has an invalid signature.
Found unexpected return type Block, expected boolean.
Handler signature: (LBlock;)LBlock; Expected signature: (Z)Z

Reproduction

BigGlobe's Biome_DontFreezeRiverWater mixin uses @ModifyExpressionValue with handler signature (Block)Block targeting BlockState.getBlock(), whose result is then compared via instanceof FluidBlock. The converter sees the nearby instanceof and incorrectly rewrites the mixin as @ModifyInstanceofValue, crashing the game on bootstrap.

Fix

Before converting, check the handler's return type via Type.getReturnType(context.methodNode().desc). If it is not boolean, skip the conversion and return pass().

Testing

Minecraft: 1.21.1
NeoForge: 21.1.172

Mod Version
Connector 2.0.0-beta.14+1.21.1
ConnectorExtras 1.12.1+1.21.1
Forgified Fabric API 0.116.7+2.2.4+1.21.1
Big Globe 5.3.1-MC1.21.1
Iris (NeoForge) 1.8.12+mc1.21.1
Sodium (NeoForge) 0.6.13+mc1.21.1

With the fix applied, Biome_DontFreezeRiverWater is correctly left as @ModifyExpressionValue and the game boots successfully.

…difyExpressionValue to ModifyInstanceofValue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant