Skip to content

Feature/enum extensions#199

Open
LlamaLad7 wants to merge 9 commits intomainfrom
feature/enum-extensions
Open

Feature/enum extensions#199
LlamaLad7 wants to merge 9 commits intomainfrom
feature/enum-extensions

Conversation

@LlamaLad7
Copy link
Copy Markdown
Member

@LlamaLad7 LlamaLad7 commented Mar 18, 2026

Closes #190
Commits best consumed individually.

Requires https://github.com/LlamaLad7/fabric-loader/tree/temp/enum-extensions-testing

This adds enum extensions, which follow the general shape of:

@Mixin(TargetEnum.class)
public enum EnumMixin {
    ADDED_CONSTANT_1("a"),
    ADDED_CONSTANT_2("b");

    @Shadow
    private EnumMixin(String example) {
    }
}

They require the as-yet-unreleased compat level of 0.17.1, partially to encourage migrating but also because they make use of the new <clinit> logic.

Notable features include:

  • Support for abstract enums (most of the work is done by the existing inner class handling)
  • The MixinIntrinsics.currentEnumOrdinal() method which can be used in enum initializers to access the relevant constant's final ordinal
  • Detection and helpful error for accidental switching on the ordinal of an enum mixin, which can cause incorrect jump tables to be generated.
  • Special handling of a custom field attribute which can be set by ClassTweaker to indicate stub enum constants
  • Helpful error messages for duplicate constant declarations either in 2 mixins or in 1 mixin and the target class (stubs don't count)
  • Deterministic application order for enum extensions in a given priority (sorted lexicographically by first added constant)
  • Various bug fixes
  • New support for @Shadow on constructors. Declared constructors can already implicitly shadow, but @Shadow has the benefit of failing fast if a matching constructor is not found. All constructors in enum extensions are assumed to have @Shadow implicitly, unlike constructors in normal mixins which must be assumed to be there only to appease the compiler

We make some (relatively minor) assumptions about the structure of enum classes, throwing informative errors if any of them are violated.
For both mixin enums and target enums we assume:

  • There will be a values array which is the only field marked static, synthetic, and with the type of ThisClass[]
  • There will be 1 assignment to this array
  • There will be 1 assignment to each enum constant

For mixin enums we additionally assume:

  • In <clinit>, the first loaded int constant will be the ordinal of the first enum constant, and then after that enum constant is assigned, the next loaded int constant will be the ordinal of the second enum constant, etc

These fast paths make no sense, it should be perfectly sensible to reference the mixin class anywhere.
@LlamaLad7 LlamaLad7 force-pushed the feature/enum-extensions branch 8 times, most recently from 9d4238f to 7be28cf Compare March 26, 2026 13:03
@LlamaLad7 LlamaLad7 requested a review from modmuss50 March 26, 2026 13:43
@LlamaLad7 LlamaLad7 force-pushed the feature/enum-extensions branch from 7be28cf to 46833ce Compare March 26, 2026 22:33
Copy link
Copy Markdown

@Earthcomputer Earthcomputer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't spot any other issues with this.

I assume the new shadow constructors are a feature applicable more widely than enum extensions? Like, now you could do:

@Mixin(TargetClass.class)
class MixinClass {
   @Shadow
   private MixinClass() {}

   void foo() {
      TargetClass newTarget = (TargetClass) (Object) new MixinClass();
   }
}

to create a new instance of TargetClass if the constructor is private (as an alternative to using an invoker)?

@LlamaLad7
Copy link
Copy Markdown
Member Author

Yes. You can already do this just without @Shadow, the new @Shadow support is simply there to fail fast.

NeoForge wants this so they can disable extending `IExtensibleEnum`s with Mixin.
@LlamaLad7 LlamaLad7 force-pushed the feature/enum-extensions branch from 46833ce to 6f5407d Compare March 26, 2026 23:04
@LlamaLad7 LlamaLad7 force-pushed the feature/enum-extensions branch from 6f5407d to e8bd94a Compare March 26, 2026 23:08
Copy link
Copy Markdown
Member

@modmuss50 modmuss50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, I havent done an indepth review of the logic as I dont think its worth my time and I am not that familar with it. But the high level design looks solid alongside the CT changes that have already gone in.

As discussed automated testing can be handled as a seperate thing. We will also want to decide how/where to formally document this.

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.

Enum extensions

3 participants