-
Notifications
You must be signed in to change notification settings - Fork 110
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
Switch Rust codegen to use new ModuleDef #1675
Conversation
d8d1485
to
dfe601f
Compare
0da5569
to
2dffb7c
Compare
2dffb7c
to
f6d2e77
Compare
f6d2e77
to
7a3fe5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nits otherwise I'm happy with this. I'm less familiar with how the codegen works in the nitty gritty but everything here makes sense to me.
|
||
Ok(files) | ||
pub fn generate(module: RawModuleDef, lang: Language, namespace: &str) -> anyhow::Result<Vec<(String, String)>> { | ||
let module = match module { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this whole function looks good, this was exactly the structure I thought would make sense here while we port
} | ||
write_type(ctx, out, &elem.algebraic_type) | ||
})?; | ||
AlgebraicTypeUse::Unit => write!(out, "()")?, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with this patch :)
extract_descriptions(module.path()).expect("failed to extract module descriptions"); | ||
let RawModuleDef::V8BackCompat(raw_module_def) = raw_module_def else { | ||
panic!("no more v8") | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So both C# and rust are patched to produce v9 now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not yet - the panic!()
will happen when v8 is no longer being produced.
Description of Changes
Based on #1661. The way I did this also allows for incremental porting of the typescript & csharp codegens - obviously it should happen soon, but this is nice cause it allows avoiding conflicts (unless Ingvar's already started working on the csharp one).
Expected complexity level and risk
2
Testing
Snapshot tests all pass.