MixinExtras icon indicating copy to clipboard operation
MixinExtras copied to clipboard

Adds more support for changing 'return' and 'throw' statements

Open Papierkorb2292 opened this issue 3 years ago • 2 comments

  • @WrapWithCondition is now able to wrap method exiting instructions, if they are safe to remove. Method exiting instructions are safe to remove, if they're always followed by another method exiting instruction and all variables, that are later used and not set, are initialized.
  • Added the @InitVariable injector, which can set variables, but, unlike @ModifyVariable, it doesn't provide the previous value and only variables from the next frame instead of the current frame are available.
  • Added the BeforeThrow (@At("MIXINEXTRAS:THROW")) injection point that targets ATHROW instructions and also can optionally accept an ordinal.
  • Added @RedirectExit injector, which can replace ATHROW and return instructions.
  • Changed @ModifyReturnValue to support replacement of the target instruction by checking the opcode of the original target instead of the current target
  • Added @ModifyThrowValue to change Throwables about to be thrown.

Methods for the required bytecode validation are in InjectorUtils. My fork contains possible wiki entries representing the changes.

Papierkorb2292 avatar Oct 15 '22 16:10 Papierkorb2292

My initial instinct is that @InitVariable and support for wrapping/replacing method exits is too niche and confusing to be added to the main repository right now. The verification logic is also insanely complex, and while it seems you've certainly had a good go at it, I'm hesitant to expose something that finnicky to the masses, especially when as far as I can remember you're the only person I've encountered who's ever needed to wrap a method exit. Modifying the thrown exception may be more useful, and is certainly less complicated, so for the time being my recommendation is that you split that into a separate PR and keep the complex stuff in your own submodule / library for now, though I may re-evaluate at some point in the future.

LlamaLad7 avatar Oct 23 '22 23:10 LlamaLad7

My initial instinct is that @InitVariable and support for wrapping/replacing method exits is too niche and confusing to be added to the main repository right now. The verification logic is also insanely complex, and while it seems you've certainly had a good go at it, I'm hesitant to expose something that finnicky to the masses, especially when as far as I can remember you're the only person I've encountered who's ever needed to wrap a method exit. Modifying the thrown exception may be more useful, and is certainly less complicated, so for the time being my recommendation is that you split that into a separate PR and keep the complex stuff in your own submodule / library for now, though I may re-evaluate at some point in the future.

Should the BeforeThrow injection point stay in the more complex PR? I only added it in case a local variable is thrown, so it's probably not that useful and the implementation of the registration might be a bit messy.

Papierkorb2292 avatar Oct 24 '22 14:10 Papierkorb2292

Should the BeforeThrow injection point stay in the more complex PR? I only added it in case a local variable is thrown, so it's probably not that useful and the implementation of the registration might be a bit messy.

Could BeforeThrow be PR? Here is niche case that I encounter :rip:

if (!(pipe.pipe instanceof PipeLogisticsChassis)) {
   throw new TargetNotFoundException("Couldn't find " + clazz.getName() + ", pipe wasn't a chassi pipe", this);
}

KorewaLidesu avatar Nov 24 '22 09:11 KorewaLidesu

Could BeforeThrow be PR? Here is niche case that I encounter :rip:

if (!(pipe.pipe instanceof PipeLogisticsChassis)) {
   throw new TargetNotFoundException("Couldn't find " + clazz.getName() + ", pipe wasn't a chassi pipe", this);
}

In such cases the constructor of the exception can be targeted with a shift of AFTER.

Papierkorb2292 avatar Nov 24 '22 10:11 Papierkorb2292

In such cases the constructor of the exception can be targeted with a shift of AFTER.

AHHHHHHH! Thanks for the solutions. ~~i'm brainded now~~ Luv u <3

KorewaLidesu avatar Nov 24 '22 13:11 KorewaLidesu

Going to close for now. May revisit this in the future but I think it's too finicky atm. Targeting throws will be addressed by an upcoming feature anyway.

LlamaLad7 avatar Jan 13 '24 19:01 LlamaLad7