Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update from upstream #6

Merged
merged 10 commits into from
Oct 14, 2024
Merged

Update from upstream #6

merged 10 commits into from
Oct 14, 2024

Conversation

mitchej123
Copy link

Fixes the inline shadowed variable initialization bug.

LlamaLad7 and others added 10 commits July 19, 2024 18:40
Bafflingly, Mixin fails to match a `NEW` instruction whose `<init>` call is outside the slice range, *iff the user specified a descriptor*.
I'm even more surprised that a mod relied on this.
Fixes FabricMC#152.
…sers.

These will never match because the LHS is the mixin name and the RHS is the target name.
This restores the 0.8.5 behaviour which does not check the owner at all.
We have never supported assignment expressions in initialisers until now anyway, and even if we check the owner we cannot determine whether the instruction is operating on the current *instance* without proper static analysis, so IMO there's not much point bothering.
The old one did not properly account for discontinuities in the list, leading to double-width types clobbering meaningful locals and causing mysterious VerifyErrors.
`InjectionInfo.parse` can be very expensive and in the case of some MixinExtras features is not pure.
The correct approach would be to use `InjectionInfo.getInjectorAnnotation`, but the override is no longer needed anyway since we only support Java 8+ so Mixin will already make injector methods private and synthetic.
Except, Mixin does it based on the current `COMPATIBILITY_LEVEL`, which is wrong, because we could be on JAVA_17 and yet a class compiled with JAVA_8 had no choice but to use public methods.
The original implementation was also incomplete because it forgot about default methods.
@mitchej123 mitchej123 merged commit 020f925 into main Oct 14, 2024
4 checks passed
@mitchej123 mitchej123 deleted the update-from-upstream branch October 14, 2024 18:17
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.

3 participants