Skip to content

feat: WIP enums implementation #598

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

feat: WIP enums implementation #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f3858c
feat: WIP enums implementation
yossydev Mar 15, 2025
7cad2ba
wip
yossydev Mar 20, 2025
2b3311a
wip
yossydev Mar 20, 2025
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
20 changes: 20 additions & 0 deletions index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
enum CompassDirection {
North,
East,
South,
West
}

globalThis.__enumCheck = {
northValue: CompassDirection.North === 0,
eastValue: CompassDirection.East === 1,
southValue: CompassDirection.South === 2,
westValue: CompassDirection.West === 3,

zeroName: CompassDirection[0] === "North",
oneName: CompassDirection[1] === "East",
twoName: CompassDirection[2] === "South",
threeName: CompassDirection[3] === "West",

keys: Object.keys(CompassDirection)
};
Comment on lines +1 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: A good idea is probably to write unit tests for these TypeScript specific features. It's of course quite hard to keep conformance with TypeScript behaviour without dragging in the TypeScript test-suite which I don't believe we want to bother ourselves with.

Thought: There are a few tsconfig options which alter the behaviour of TypeScript const enums: preserveConstEnums, isolatedModules and erasableSyntaxOnly. We will probably need to somehow configure these for TypeScript compatability, but that is a future issue either way when it comes to compatability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suggestion: A good idea is probably to write unit tests for these TypeScript specific features. It's of course quite hard to keep conformance with TypeScript behaviour without dragging in the TypeScript test-suite which I don't believe we want to bother ourselves with.

thx!
https://github.com/oxc-project/oxc/tree/main/tasks/transform_conformance/tests/babel-plugin-transform-typescript/test/fixtures oxc's Would a transpiler test be helpful? 👀

Copy link
Member

Choose a reason for hiding this comment

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

Regarding tsconfig options, I think we'd choose one behaviour and stick with it. For enums, my personal choice would be:

  1. non-const enums are actually compiled into JS objects in the way TS implements them, so two-directional objects.
  2. const enums are fully inlined into all use-sites: Note that this means adding data into Modules if a const enum is exported, so that they can be imported and used.

Note that the latter point is kind of an interesting and magical choice. What it would mean is that code like this would work:

if (value ==== ConstEnum.Variant) {}
// becomes eg.
if (value ==== 1) {}

but this would not:

if (Object.keys(ConstEnum).includes(value)) {}
// becomes eg.
if (Object.keys((() => { throw new Error("const enum object cannot be used") })()).includes(value)) {}

45 changes: 29 additions & 16 deletions nova_vm/src/ecmascript/syntax_directed_operations/scope_analysis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,15 +1107,18 @@ impl<'a> TopLevelLexicallyDeclaredNames<'a> for Statement<'a> {
Statement::ImportDeclaration(decl) => decl.bound_names(f),
Statement::ExportNamedDeclaration(decl) => decl.bound_names(f),
#[cfg(feature = "typescript")]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {}
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {}
#[cfg(not(feature = "typescript"))]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {
unreachable!()
}
// Note: No bounds names for export all and export default declarations.
Statement::ExportAllDeclaration(_) | Statement::ExportDefaultDeclaration(_) => {}
Statement::TSEnumDeclaration(_)
| Statement::TSModuleDeclaration(_)
Statement::TSModuleDeclaration(_)
| Statement::TSImportEqualsDeclaration(_)
| Statement::TSExportAssignment(_)
| Statement::TSNamespaceExportDeclaration(_) => unreachable!(),
Expand Down Expand Up @@ -1189,13 +1192,16 @@ impl<'a> TopLevelLexicallyScopedDeclarations<'a> for Statement<'a> {
}
Statement::ClassDeclaration(decl) => f(LexicallyScopedDeclaration::Class(decl)),
#[cfg(feature = "typescript")]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {}
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {}
#[cfg(not(feature = "typescript"))]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {
unreachable!()
}
Statement::TSEnumDeclaration(_)
| Statement::TSExportAssignment(_)
Statement::TSExportAssignment(_)
| Statement::TSImportEqualsDeclaration(_)
| Statement::TSModuleDeclaration(_)
| Statement::TSNamespaceExportDeclaration(_) => unreachable!(),
Expand Down Expand Up @@ -1275,13 +1281,16 @@ impl<'a> TopLevelVarDeclaredNames<'a> for Statement<'a> {
| Statement::WhileStatement(_)
| Statement::WithStatement(_) => self.var_declared_names(f),
#[cfg(feature = "typescript")]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {}
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {}
#[cfg(not(feature = "typescript"))]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {
unreachable!()
}
Statement::TSEnumDeclaration(_)
| Statement::TSExportAssignment(_)
Statement::TSExportAssignment(_)
| Statement::TSImportEqualsDeclaration(_)
| Statement::TSModuleDeclaration(_)
| Statement::TSNamespaceExportDeclaration(_) => unreachable!(),
Expand Down Expand Up @@ -1404,13 +1413,17 @@ impl<'a> TopLevelVarScopedDeclarations<'a> for Statement<'a> {
// 2. Return a new empty List.
}
#[cfg(feature = "typescript")]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {}
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {}
#[cfg(not(feature = "typescript"))]
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {
Statement::TSTypeAliasDeclaration(_)
| Statement::TSInterfaceDeclaration(_)
| Statement::TSEnumDeclaration(_) => {
unreachable!()
}
Statement::TSEnumDeclaration(_)
| Statement::TSExportAssignment(_)

Statement::TSExportAssignment(_)
| Statement::TSImportEqualsDeclaration(_)
| Statement::TSModuleDeclaration(_)
| Statement::TSNamespaceExportDeclaration(_) => unreachable!(),
Expand Down
65 changes: 63 additions & 2 deletions nova_vm/src/engine/bytecode/bytecode_compiler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ pub(crate) struct CompileContext<'agent, 'gc, 'scope> {
/// In a `(a?.b)?.()` chain the evaluation of `(a?.b)` must be considered a
/// reference.
is_call_optional_chain_this: bool,
current_value: Option<Value<'gc>>,
}

impl<'a, 'gc, 'scope> CompileContext<'a, 'gc, 'scope> {
Expand All @@ -98,6 +99,7 @@ impl<'a, 'gc, 'scope> CompileContext<'a, 'gc, 'scope> {
current_break: None,
optional_chains: None,
is_call_optional_chain_this: false,
current_value: None,
}
}

Expand Down Expand Up @@ -358,6 +360,7 @@ impl<'a, 'gc, 'scope> CompileContext<'a, 'gc, 'scope> {
) {
debug_assert_eq!(instruction.argument_count(), 1);
debug_assert!(instruction.has_identifier_index());

self._push_instruction(instruction);
let identifier = self.add_identifier(identifier);
self.add_index(identifier);
Expand Down Expand Up @@ -507,6 +510,7 @@ impl CompileEvaluation for ast::NumericLiteral<'_> {
fn compile(&self, ctx: &mut CompileContext) {
let constant = ctx.agent.heap.create(self.value);
ctx.add_instruction_with_constant(Instruction::StoreConstant, constant);
ctx.current_value = Some(constant.into_value());
}
}

Expand Down Expand Up @@ -2935,6 +2939,62 @@ impl CompileEvaluation for ast::ContinueStatement<'_> {
}
}

#[cfg(feature = "typescript")]
impl CompileEvaluation for ast::TSEnumDeclaration<'_> {
fn compile<'gc>(&self, ctx: &mut CompileContext<'_, 'gc, '_>) {
let is_const = self.r#const;
if is_const {
return;
}
let enum_name = self.id.name;
println!("{:?}", enum_name);

// TODO: implement var Foo

ctx.add_instruction(Instruction::ObjectCreate);
ctx.add_instruction(Instruction::Store);
ctx.add_instruction(Instruction::LoadCopy);
ctx.add_instruction(Instruction::Load);
let mut next_value = 0.0;
for prop in self.members.iter() {
let key_str = match &prop.id {
ast::TSEnumMemberName::Identifier(ident) => ident.name.as_str(),
ast::TSEnumMemberName::String(string) => string.value.as_str(),
};
let key = String::from_str(ctx.agent, key_str, ctx.gc);
let value: Value<'gc>;
if let Some(expr) = &prop.initializer {
expr.compile(ctx);
if is_reference(expr) {
ctx.add_instruction(Instruction::GetValue);
}
value = ctx
.current_value
.take()
.unwrap_or(Value::from_f64(ctx.agent, next_value, ctx.gc));

if value.is_number() {
next_value += 1.0;
} else {
next_value += 1.0;
}
} else {
value = Value::from_f64(ctx.agent, next_value, ctx.gc);
next_value += 1.0;
}
ctx.add_instruction_with_constant(Instruction::LoadConstant, key.into_value());
ctx.add_instruction_with_constant(Instruction::StoreConstant, value);
ctx.add_instruction(Instruction::ObjectDefineProperty);
if value.is_number() {
ctx.add_instruction_with_constant(Instruction::LoadConstant, value);
ctx.add_instruction_with_constant(Instruction::StoreConstant, key.into_value());
ctx.add_instruction(Instruction::ObjectDefineProperty);
}
}
ctx.add_instruction(Instruction::Store);
}
}

impl CompileEvaluation for ast::Statement<'_> {
fn compile(&self, ctx: &mut CompileContext) {
match self {
Expand Down Expand Up @@ -2972,8 +3032,9 @@ impl CompileEvaluation for ast::Statement<'_> {
Statement::TSTypeAliasDeclaration(_) | Statement::TSInterfaceDeclaration(_) => {
unreachable!()
}
Statement::TSEnumDeclaration(_)
| Statement::TSExportAssignment(_)
#[cfg(feature = "typescript")]
Statement::TSEnumDeclaration(x) => x.compile(ctx),
Statement::TSExportAssignment(_)
| Statement::TSImportEqualsDeclaration(_)
| Statement::TSModuleDeclaration(_)
| Statement::TSNamespaceExportDeclaration(_) => unreachable!(),
Expand Down
Loading