Stream: git-wasmtime

Topic: wasmtime / issue #3047 `entity_impl!` doesn't work when t...

Wasmtime GitHub notifications bot (Jun 30 2021 at 21:19):

cratelyn opened issue #3047:

Feature

Hi! :wave: I'd like to propose extending cranelift_entity::entity_impl to accept an expression that ought to be used to construct the given entity.

Benefit

In certain corner cases, particularly when using libraries that generate code from .witx defintions, we want to use entity_impl! with handle types that are not defined ourselves. Consequentially, we cannot always construct the handle directly via a statement like $entity(index as u32).

I put together a Rust Playground example demonstrating the problem here, you could also refer to this gist.

:question: Implementation

This part I have some questions, and welcome advice. At a high level, I hoped for this to look something like this...

macro_rules! entity_impl {
     ($entity:ident) => { ... };
     ($entity:ident, $display_prefix:expr) => { ... };
+    ($entity:ident, $display_prefix:expr, $construct:expr) => { ... };
}

...which would allow an invocation shaped like:

entity_impl(ExampleHandle, "example-prefix-", (index as u32).into())

However, when experimenting with this, I ended up running into errors shaped like cannot find value 'index' in this scope. I tried alternative designators like stmt, but didn't have much luck.

I'm happy to carry out the fix for this, but might need a bit of help deciding how to present this change to callers.

Wasmtime GitHub notifications bot (Jun 30 2021 at 21:27):

bjorn3 commented on issue #3047:

You would need something like |$index:ident| $construct:expr and then define the $index variableble instead of index. In rust macros are hygienic, preventing names from flowing in or out of a macro body.

Wasmtime GitHub notifications bot (Jun 30 2021 at 21:49):

cfallin commented on issue #3047:

@cratelyn thanks for filing this issue!

I played with the Rust Playground for a bit and (as @bjorn3 notes above) got something to work by passing in an identifier to work with the hygiene. Something like:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=e798654ac02ac488689666d6d5eed72d

the key bit there is the macro form that takes ($entity:ident, $display_prefix:expr, $arg:ident, $to_expr:expr, $from_expr:expr), with which we can then do something like:

entity_impl!(ExternalHandle, "external-", i, ExternalHandle::from(i), u32::from(i));

Does that seem reasonable? Happy to review a PR if so!

Wasmtime GitHub notifications bot (Jul 02 2021 at 15:17):

cratelyn commented on issue #3047:

That makes sense to me @cfallin, thank you for taking a look! I'll put a patch together for you soon! :adhesive_bandage:

Wasmtime GitHub notifications bot (Jul 02 2021 at 20:27):

cfallin assigned issue #3047:

Feature

Hi! :wave: I'd like to propose extending cranelift_entity::entity_impl to accept an expression that ought to be used to construct the given entity.

Benefit

In certain corner cases, particularly when using libraries that generate code from .witx defintions, we want to use entity_impl! with handle types that are not defined ourselves. Consequentially, we cannot always construct the handle directly via a statement like $entity(index as u32).

I put together a Rust Playground example demonstrating the problem here, you could also refer to this gist.

:question: Implementation

This part I have some questions, and welcome advice. At a high level, I hoped for this to look something like this...

macro_rules! entity_impl {
     ($entity:ident) => { ... };
     ($entity:ident, $display_prefix:expr) => { ... };
+    ($entity:ident, $display_prefix:expr, $construct:expr) => { ... };
}

...which would allow an invocation shaped like:

entity_impl(ExampleHandle, "example-prefix-", (index as u32).into())

However, when experimenting with this, I ended up running into errors shaped like cannot find value 'index' in this scope. I tried alternative designators like stmt, but didn't have much luck.

I'm happy to carry out the fix for this, but might need a bit of help deciding how to present this change to callers.

Wasmtime GitHub notifications bot (Oct 06 2021 at 19:22):

cfallin closed issue #3047 (assigned to cratelyn):

Feature

Hi! :wave: I'd like to propose extending cranelift_entity::entity_impl to accept an expression that ought to be used to construct the given entity.

Benefit

In certain corner cases, particularly when using libraries that generate code from .witx defintions, we want to use entity_impl! with handle types that are not defined ourselves. Consequentially, we cannot always construct the handle directly via a statement like $entity(index as u32).

I put together a Rust Playground example demonstrating the problem here, you could also refer to this gist.

:question: Implementation

This part I have some questions, and welcome advice. At a high level, I hoped for this to look something like this...

macro_rules! entity_impl {
     ($entity:ident) => { ... };
     ($entity:ident, $display_prefix:expr) => { ... };
+    ($entity:ident, $display_prefix:expr, $construct:expr) => { ... };
}

...which would allow an invocation shaped like:

entity_impl(ExampleHandle, "example-prefix-", (index as u32).into())

However, when experimenting with this, I ended up running into errors shaped like cannot find value 'index' in this scope. I tried alternative designators like stmt, but didn't have much luck.

I'm happy to carry out the fix for this, but might need a bit of help deciding how to present this change to callers.

Last updated: Nov 22 2024 at 16:03 UTC