Skip to content

Add open enums and enum methods with tests #631

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.

Sign up for GitHub

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

Open
eliotmoss wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add open enums and enum methods with tests #631

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9165555
Add open enums and enum methods with tests
eliotmoss Apr 1, 2026
1d2cc22
Restructuring to eliminate VstEnum in favor of VstClass and to proces…
eliotmoss Apr 7, 2026
56864a0
Fix -O3 CI failure
eliotmoss Apr 7, 2026
9855b59
Fix was not quite right
eliotmoss Apr 8, 2026
f77dd61
Need to set facts to suppress null check on enum method calls, to avo…
eliotmoss Apr 8, 2026
9daafee
Seems some null checks on enums were still getting through
eliotmoss Apr 8, 2026
70acb9b
Remove a dead field and code related to it. Add O3 tests to my CI Ma…
eliotmoss Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions aeneas/src/core/Eval.v3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ component Eval {
return b == null;
}
CLASS_QUERY, VARIANT_QUERY => {
if (EnumType.?(tt)) {

@titzer titzer Apr 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

From this it's clear to me that enums need to be desugared to variants earlier in a more general way. Claude has just smeared special cases from front to middle to backend. There shouldn't be any need to alter the evaluation of operators like this.

@eliotmoss eliotmoss Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm willing to work on simplifying such things. I'm not 100% clear on the desugaring you're hoping for, though. I can see that enums are kind of like variants that have no variant fields. Of course enums have their own "fields", that are implemented as arrays indexed by tag. If the two are to be unified into a single whole, we need some place to hang both kinds of fields. Also while enum class structure mirrors variant class structure (in the case of no generic parameters), enums themselves are single values (or have unique single values associated with them) while variant cases are types (unless they have no fields, in which case they have the same "ambiguity" as enums). If you have a little more guidance I would be more confident revising code ...

@titzer titzer Apr 2, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ah, forgive me. It used to be that enums were immediately desugared to a single class declaration. At some point I split off the implementation entirely. If open enums were desugared into open variants instead (i.e. adding a class per enum case), then enum method dispatch would just be variant dispatch. Enum fields would still be implemented via field arrays and use VariantGetTag as the index. Unboxing will just eliminate the overhead of boxing enum values.

@eliotmoss eliotmoss Apr 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I get the idea but am foggy as to the particulars. Specifically I wonder about the field arrays, and maybe also enumsets. Is the desugaring proposed to happen in VstSsaGen, so that enums are mostly washed away once we have SSA, but the distinction is more visible in parsing (of course) and verification? This would make sense in that, once verified, enums could be mapped down to other structures (variants, field arrays) and marked to be unboxed (always). I'd have to get more deeply into it to see where issues might come up with enumsets, but again, suitable use of VariantGetTag would get the raw numeric values required. So, I could try to combine processing in verification (and maybe parts of parsing), but strive to eliminate separate processing once in SSA.

// enum subtype query: check integer tag is in [enumTagLo, enumTagHi]
var decl = EnumType.!(tt).enumDecl;
var tagVal = Int.unbox(val);
return tagVal >= decl.enumTagLo && tagVal <= decl.enumTagHi;
}
if (val == null) return false;
if (tt.open()) return false;
return Record.?(val) && TypeSystem.isSubtype(Record.!(val).rtype, tt);
Expand Down Expand Up @@ -811,19 +817,34 @@ def evalOp(op: Operator, args: Arguments) -> Result {
return if(object != null, object.values[field.index]);
}
VariantGetMethod(method) => {
var ta = args.getTypeArgs();
if (EnumType.?(ta[0])) {
var spec = lookupEnumVirtual(args, method);
return Closure.new(args.vals[0], spec);
}
var object = args.r(0);
var spec = args.getClosedIrSpec(method);
if (spec == null) return args.notFoldable(null);
return Closure.new(object, spec);
}
VariantGetVirtual(method) => {
var ta = args.getTypeArgs();
if (EnumType.?(ta[0])) {
var spec = lookupEnumVirtual(args, method);
return Closure.new(args.vals[0], spec);
}
var object = getRecordReceiver(args);
var spec = args.getClosedIrSpec(method);
if (spec == null) return args.notFoldable(null);
spec = lookupVariantVirtual(args, object, spec);
return Closure.new(object, spec);
}
VariantGetSelector(selector) => {
var ta = args.getTypeArgs();
if (EnumType.?(ta[0])) {
var spec = lookupEnumVirtual(args, selector);
return FuncVal.new(spec);
}
var object = getRecordReceiver(args);
var spec = args.getClosedIrSpec(selector);
if (spec == null) return args.notFoldable(null);
Expand Down Expand Up @@ -915,13 +936,23 @@ def evalOp(op: Operator, args: Arguments) -> Result {
return args.tailCall(spec, object, 1, args.vals.length);
}
CallVariantVirtual(method) => {
var ta = args.getTypeArgs();
if (EnumType.?(ta[0])) {
var spec = lookupEnumVirtual(args, method);
return args.tailCall(spec, args.vals[0], 1, args.vals.length);
}
var object = getRecordReceiver(args);
var spec = args.getClosedIrSpec(method);
if (spec == null) return args.notFoldable(null);
spec = lookupVariantVirtual(args, object, spec);
return args.tailCall(spec, object, 1, args.vals.length);
}
CallVariantSelector(selector) => {
var ta = args.getTypeArgs();
if (EnumType.?(ta[0])) {
var spec = lookupEnumVirtual(args, selector);
return args.tailCall(spec, args.vals[0], 1, args.vals.length);
}
var object = getRecordReceiver(args);
var spec = args.getClosedIrSpec(selector);
if (spec == null) return args.notFoldable(null);
Expand Down Expand Up @@ -1148,6 +1179,19 @@ def lookupClassVirtual(args: Arguments, object: Record, spec: IrSpec) -> IrSpec
args.throw(V3Exception.NullCheck, null);
return null;
}
def lookupEnumVirtual(args: Arguments, member: IrMember) -> IrSpec {
var ta = args.getTypeArgs();
var baseSpec = IrSpec.new(ta[0], ta, member);
var tag = Int.unbox(args.vals[0]);
if (IrMethod.?(member)) {
var rootVst = IrMethod.!(member).source;
if (rootVst != null && rootVst.enumCaseIrs != null && tag < rootVst.enumCaseIrs.length) {
var overrideIr = rootVst.enumCaseIrs[tag];
if (overrideIr != null) return IrSpec.new(ta[0], ta, overrideIr);
}
}
return baseSpec;
}
def lookupVariantVirtual(args: Arguments, object: Record, spec: IrSpec) -> IrSpec {
if (object != null) return args.getProgram().ir.resolveMethodImpl(object.rtype, spec);
return args.getProgram().ir.resolveVariantDefaultMethodImpl(spec);
Expand Down
6 changes: 6 additions & 0 deletions aeneas/src/core/Operator.v3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,12 @@ component V3Op {
var paramTypes = Arrays.prepend(ftype, Function.getParamTypeArray(ftype));
return newOp0(Opcode.CallFunction, [ftype], paramTypes, Function.getReturnType(ftype));
}
def newCallFunctionDirect(ftype: Type) -> Operator {
ftype = Function.funcRefType(ftype);
if (ftype.typeCon.kind != Kind.FUNCREF) return V3.fail("only function types allowed");
var paramTypes = Arrays.prepend(ftype, Function.getParamTypeArray(ftype));
return newOp0(Opcode.CallFunction, [ftype], paramTypes, Function.getReturnType(ftype));
}
def newCreateClosure(methodRef: IrSpec, closure: Type) -> Operator {
var typeArgs = methodRef.typeArgs;
return newOp0(Opcode.CreateClosure(methodRef.asMethod()), typeArgs, [closure], methodRef.getBoundType());
Expand Down
4 changes: 3 additions & 1 deletion aeneas/src/ir/Ir.v3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,9 @@ class IrModule {
return ic;
}
def newIrClass(ctype: Type, superClass: IrClass, decl: VstCompound) -> IrClass {
var ic = IrBuilder.new(ctype, superClass).buildClass(decl);
var builder = IrBuilder.new(ctype, superClass);

@titzer titzer Apr 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't know why it feels the need to add the IR module to the builder now.

@eliotmoss eliotmoss Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apparently the field and this code are dead, so I'm removing it.

builder.irModule = this;
var ic = builder.buildClass(decl);
classes.put(ic);
return ic;
}
Expand Down
39 changes: 39 additions & 0 deletions aeneas/src/ir/Normalization.v3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ class ReachabilityNormalizer(config: NormalizerConfig, ra: ReachabilityAnalyzer)
if (rc.isUnboxed()) {
// move flattened data type receiver to function sig
ftype = Function.prependParamTypes(rc.variantNorm.sub, ftype);
} else if (EnumType.?(rc.oldType)) {

@titzer titzer Apr 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If enums are properly desugared to variants, this will just fall out of the normal unboxing.

@eliotmoss eliotmoss Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

isUnboxed means that normalization did the unboxing - but enums are implicitly already unboxed, so the test doesn't work for them. Unifying these more would be a bigger change. Is that how you would prefer to go?

// enum: prepend tag type so dispatch table CallFunction matches CallMethod
var tagType = V3.getVariantTagType(rc.oldType);
ftype = Function.prependParamType(tagType, ftype);
}
rm.funcNorm = FuncNorm.!(norm(ftype));
var typeParams = if(rm.spec != null, rm.spec.getTypes().methodTypeArgs);
Expand Down Expand Up @@ -588,11 +592,15 @@ class ReachabilityNormalizer(config: NormalizerConfig, ra: ReachabilityAnalyzer)
rm.norm.flags |= IrFlag.M_OVERRIDE;
sm.norm.flags |= IrFlag.M_OVERRIDDEN;
}
// For enum methods, M_OVERRIDDEN must be transferred from original

@titzer titzer Apr 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't know why it thinks this.

@eliotmoss eliotmoss Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ultimately this appears necessary because enums don't use regular vtables so their overrides are processed differently. That don't use regular vtables because individual enum cases are not types and thus don't have a corresponding RaClass, etc. Again, is this deeper change one you would prefer to happen?

// because enum case overrides don't share vtable slots.
if (m.flags.M_OVERRIDDEN) rm.norm.flags |= IrFlag.M_OVERRIDDEN;
if (rm.virtual != null) virtuals = List.new(rm.virtual, virtuals);
}
def layoutMtable(rv: RaVirtual) {
if (rv.mtable != null) return;
var rm = rv.raMethod, rc = ra.getClass(rm.receiver);
if (EnumType.?(rc.oldType)) return layoutEnumMtable(rv, rm, rc);
var size = rc.maxClassId - rc.minClassId;
if (ra.compiler.RaDevirtualize && size == 1) return; // no need for an mtable
var table = Array<IrMethod>.new(size), mtable = IrMtable.new(rm.norm, rc.minClassId, table);
Expand Down Expand Up @@ -630,6 +638,37 @@ class ReachabilityNormalizer(config: NormalizerConfig, ra: ReachabilityAnalyzer)
setMtable(l.head, rv);
}
}
def layoutEnumMtable(rv: RaVirtual, rm: RaMethod, rc: RaClass) {

@titzer titzer Apr 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

More replications with special cases from our AI friends. As this is very tricky, we definitely don't want to do this again.

@eliotmoss eliotmoss Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Part of the same effects from enum cases not having an RaClass and thus not lining up with the existing dispatch mechanisms.

var enumDecl = EnumType.!(rc.oldType).enumDecl;
var numCases = enumDecl.enumTagHi + 1;
var table = Array<IrMethod>.new(numCases);
var mtable = IrMtable.new(rm.norm, 0, table);
rv.mtable = mtable;

var ft = Function.funcRefType(rm.norm.getMethodType());
mtable.record = ra.prog.newRecord(V3Array.newType(ft), numCases);

// Fill all slots with the default implementation.
var defaultSpec = IrSpec.new(rm.norm.receiver, [rm.norm.receiver], rm.norm);
for (i < numCases) {
table[i] = rm.norm;
mtable.record.values[i] = FuncVal.new(defaultSpec);
}

// Fill override slots.
var rootVst = rm.orig.source;
if (rootVst != null && rootVst.enumCaseIrs != null) {
for (i < rootVst.enumCaseIrs.length) {
var overrideIr = rootVst.enumCaseIrs[i];
if (overrideIr == null) continue;
var overrideRm = overrideIr.raMethod;
if (overrideRm == null || !overrideRm.raFacts.RM_LIVE) continue;
table[i] = overrideRm.norm;
var ta = Arrays.replace(overrideRm.getSpec().typeArgs, 0, overrideRm.norm.receiver);
mtable.record.values[i] = FuncVal.new(IrSpec.new(ta[0], ta, overrideRm.norm));
}
}
}
def resolveMethodImpl(rc: RaClass, rm: RaMethod) -> RaMethod {
var sm: RaMethod;
for (sc = rc; sc != null; sc = sc.parent) { // find super method, if any
Expand Down
24 changes: 24 additions & 0 deletions aeneas/src/ir/Reachability.v3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,31 @@ class ReachabilityAnalyzer(compilation: Compilation) {
if (rm.isVirtual()) return;
rm.virtual = RaVirtual.new(rm);
var rc = makeClass(rm.receiver);
if (EnumType.?(rc.oldType)) {
getEnumVirtual(rm, rc);
return;
}
for (l = rc.subtypes; l != null; l = l.tail) {
analyzeVirtual(l.head, rm);
}
}
def getEnumVirtual(rm: RaMethod, rc: RaClass) {
// Add root implementation and mark it live so its SSA is generated.
var rv = rm.virtual;
rv.addImpl(rm);
getMethod(null, rm);
// Add per-case override implementations.
var rootVst = rm.orig.source;
if (rootVst != null && rootVst.enumCaseIrs != null) {
for (ir in rootVst.enumCaseIrs) {
if (ir == null) continue;
var overrideRm = makeMethod([rm.receiver], ir, null);
ir.raMethod = overrideRm;
rv.addImpl(overrideRm);
getMethod(null, overrideRm);
}
}
}
def getMethod(op: SsaApplyOp, rm: RaMethod) {
if (rm.setFact(RaFact.RM_LIVE)) return;
liveMethods.put(rm);
Expand Down Expand Up @@ -728,6 +749,9 @@ class ReachabilityAnalyzer(compilation: Compilation) {
if (ic == null) ic = IrClass.new(t, null, null, [], []);
raType = newRaClass(t, ic, parent);
}
ENUM => {
raType = newRaClass(t, oldIr.makeIrClass(t), null);
}
_ => {
for (l = t.nested; l != null; l = l.tail) makeType(l.head);
raType = RaType.new(t);
Expand Down
46 changes: 46 additions & 0 deletions aeneas/src/ir/SsaNormalizer.v3
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class SsaRaNormalizer extends SsaRebuilder {
newParams.put(SsaParam.new(newParams.length, newIrType));
start++; // skip synthesized receiver
}
// enum: receiver maps directly to the prepended tag param (no synthesized receiver)
}
if (tn.size == 1) {
// common case; simple normalization
Expand Down Expand Up @@ -253,6 +254,7 @@ class SsaRaNormalizer extends SsaRebuilder {
ai_new = Arrays.prepend(newGraph.nullReceiver(), ai_new);
}
}
// enum: no receiver prepend needed (tag is already in normalized args)
normCall(i_old, funcNorm, newOp, ai_new);
}
CallClassMethod(method) => {
Expand Down Expand Up @@ -287,6 +289,9 @@ class SsaRaNormalizer extends SsaRebuilder {
CallVariantVirtual(method) => {
// devirtualize methods that are not overridden
var rc = norm.ra.getClass(op.typeArgs[0]);
if (EnumType.?(rc.oldType)) {
return normEnumVirtualCall(i_old, orig, method);
}
var t = extractVirtualRef(orig, method), funcNorm = t.0, m = t.1;
var ai_new = normArgs(funcNorm, genRefs(i_old.inputs));
if (t.2) { // still a virtual dispatch
Expand Down Expand Up @@ -854,6 +859,30 @@ class SsaRaNormalizer extends SsaRebuilder {
ifc.endCase(newGraph.falseConst());
return ifc.finish();
}
def normEnumVirtualCall(i_old: SsaApplyOp, orig: Operator, method: IrMethod) {
var t = extractVirtualRef(orig, method), funcNorm = t.0, m = t.1;
var ai_new = normArgs(funcNorm, genRefs(i_old.inputs));
var tagType = V3.getVariantTagType(EnumType.!(norm.ra.getClass(orig.typeArgs[0]).oldType));
if (t.2) { // still a virtual dispatch
// enum value IS the tag; use it to index into the dispatch table
var tag = ai_new[0];
var record = IrSelector.!(m.member).mtable.record;
var table = newGraph.valConst(record.rtype, record);
var func = curBlock.opArrayGetElem(record.rtype, tagType, Facts.O_SAFE_BOUNDS, table, tag);
if (norm.config.NonRefClosureReceiver) {
// Native/wasm: no Oop prepend
ai_new = Arrays.prepend(func, ai_new);
normCall(i_old, funcNorm, V3Op.newCallFunctionDirect(funcNorm.sub[0]), ai_new);
} else {
// JVM: prepend null Oop for closure compatibility
ai_new = Arrays.concat([func, newGraph.nullReceiver()], ai_new);
normCall(i_old, funcNorm, V3Op.newCallFunction(funcNorm.sub[0]), ai_new);
}
} else {
// devirtualized: no receiver prepend (tag is already in normalized args)
normCall(i_old, funcNorm, V3Op.newCallMethod(m), ai_new);
}
}
def normVariantGetTag(vn: VariantNorm, args: Range<SsaInstr>) -> SsaInstr {
if (vn == null) return null;
if (vn.hasNoTag()) {
Expand Down Expand Up @@ -1086,6 +1115,23 @@ class SsaRaNormalizer extends SsaRebuilder {
}
return opAnd(left, check);
}
if (EnumType.?(atn.oldType) && EnumType.?(rtn.oldType)) {
// Enum subtype range test: lo <= tag <= hi.
var aDecl = EnumType.!(atn.oldType).enumDecl;
var rDecl = EnumType.!(rtn.oldType).enumDecl;
var actualTag = ai_old[offset]; // enum value IS its tag integer
var tagType = IntType.!(aDecl.tagType);
var lo = rDecl.enumTagLo, hi = rDecl.enumTagHi;
var check: SsaInstr;
if (lo == hi) {
check = curBlock.pure(V3Op.newIntEq(tagType), [actualTag, newGraph.intConst(lo)]);
} else {
var chkLo = curBlock.pure(tagType.opLtEq(), [newGraph.intConst(lo), actualTag]);
var chkHi = curBlock.pure(tagType.opLtEq(), [actualTag, newGraph.intConst(hi)]);
check = curBlock.opBoolAnd0(chkLo, chkHi);
}
return opAnd(left, check);
}
// break
}
_ => ; // break
Expand Down
Loading
Loading