-
Notifications
You must be signed in to change notification settings - Fork 314
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
Rust: Attribute-style procedural macro to generate EVMC FFI code with user-defined data #262
Conversation
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.
Need to add the new creates into .bumpversion.cfg
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.
Can you order the builder helpers the same way they ordered in evmc_declare_vm
?
It is not macro-parameterized, so we can just stick the entire thing in
evmc-vm's lib.rs.
…On Tue, May 7, 2019, 20:21 Alex Beregszaszi ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In bindings/rust/evmc-declare/src/lib.rs
<#262 (comment)>:
> + let static_version_ident = Ident::new(&concatenated_version, name_allcaps.span());
+
+ // Turn the stylized VM name and version into string literals.
+ // FIXME: Not sure if the span of name.as_str() is the same as that of name.
+ let stylized_name_literal = LitStr::new(name_stylized.as_str(), name_stylized.as_str().span());
+ let version_literal = LitStr::new(version, version.span());
+
+ quote! {
+ static #static_name_ident: &'static str = #stylized_name_literal;
+ static #static_version_ident: &'static str = #version_literal;
+ }
+}
+
+/// Generates a definition and impl for a struct which contains the EVMC instance needed by FFI,
+/// and the user-defined VM.
+// TODO: Move this struct and impl into evmc_vm.
How would you do that?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#262 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABYRBBZBIEPNGN6FK3WLFUTPUIMJJANCNFSM4HKXUJHQ>
.
|
I switched back in order to get this done more quickly. To do the field
manipulation (i had this done in earlier commits), we also need to be able
to properly initialize arbitrary fields in evmc_create_vm
…On Tue, May 7, 2019, 20:21 Alex Beregszaszi ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In bindings/rust/evmc-declare/src/lib.rs
<#262 (comment)>:
> + let version_literal = LitStr::new(version, version.span());
+
+ quote! {
+ static #static_name_ident: &'static str = #stylized_name_literal;
+ static #static_version_ident: &'static str = #version_literal;
+ }
+}
+
+/// Generates a definition and impl for a struct which contains the EVMC instance needed by FFI,
+/// and the user-defined VM.
+// TODO: Move this struct and impl into evmc_vm.
+fn build_vm_container() -> proc_macro2::TokenStream {
+ quote! {
+ struct __EvmcContainer<T: ::evmc_vm::EvmcVm + Sized> {
+ instance: ::evmc_sys::evmc_instance,
+ vm: T,
So we're still doing the same trick here 😉
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#262 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABYRBBYBEWFMT2TPPMYMYXLPUIMG3ANCNFSM4HKXUJHQ>
.
|
Also we need to change the examplevm to use this in order to test it. |
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.
Need to rebase.
d078d6c
to
420f384
Compare
71685ca
to
9c905e8
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.
Please rebase.
Why? Can it be done in a separate PR then? |
Have you tested that the bumpversion config works? |
How do I test this? |
71c2165
to
6d6d249
Compare
#[macro_use] | ||
use evmc_declare::evmc_declare_vm; | ||
|
||
#[evmc_declare_vm("Foo VM", "ewasm")] |
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.
Since this is a comprehensive test, I'd say we need to VMs declared here:
- supports single capability and takes version from crate
- supports multiple capabilities and takes version from declare macro
35b17c3
to
d873408
Compare
Closes #168.
Closes #223.
Part of #308.