Compile-time variant of shim_sig_arg #4806
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
src/shims/sig.rs
Outdated
| /// | ||
| /// Groups tokens into comma-separated chunks and calls the provided macro on them. | ||
| #[macro_export] | ||
| macro_rules! _comma_sep { |
There was a problem hiding this comment.
This thing is a classic tt-muncher, which you can read up on elsewhere. But for a quick explanation here: It basically consumes the first token of the input, appends some tokens to a 'collector' argument as relevant, then at the end emits the collected tokens as its output.
You need the collector since a macro can't expand to an invalid expression, so you can't have [foo!() bar!()] where foo expands to foo, and bar expands to bar,.
A tt-muncher can have not great performance implications, since it's deeply recursive, but that's not likely to be an issue here since we're doing a linear scan and operating on relatively small chunks of tokens, all told. On the other hand, they're capable of basically arbitrary parsing operations since they are both stateful and not subject to the follow-rule and grouping limitations of syntactical matchers. They're basically the step in between a macro_rules and a proc macro.
There was a problem hiding this comment.
Could you extend the doc comment to explain the usage of the macro, and have an internal comment explaining the grammar of the intermediate states (the @ $mac $this [...] [...] ... part)?
There was a problem hiding this comment.
Also, with its $this argument and the output and output being [...] lists, I think it's not worth using a very generic name... please just call it shim_sig_args_sep or so.
|
Very neat, thanks :) |
src/shims/sig.rs
Outdated
| /// ``` | ||
| /// shim_sig_args_sep!(this, [*const _, i32, libc::off64_t], shim_sig_arg) | ||
| /// // expands to: | ||
| /// [this.machine.layouts.const_raw_ptr.ty, this.tcx.types.i32, helpers::path_ty_layout(this, &["libc", "off64_t").ty] |
There was a problem hiding this comment.
| /// [this.machine.layouts.const_raw_ptr.ty, this.tcx.types.i32, helpers::path_ty_layout(this, &["libc", "off64_t").ty] | |
| /// [shim_sig_arg!(*const _), shim_sig_arg!(i32), shim_sig_arg!(libc::off64_t)] |
src/shims/sig.rs
Outdated
| /// ``` | ||
| #[macro_export] | ||
| macro_rules! shim_sig_args_sep { | ||
| ($this:ident, [$($tt:tt)*], $mac:ident) => { |
There was a problem hiding this comment.
Since mac is always shim_sig_arg, is there even a reason to make it a parameter?
|
CI is also red. |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
☔ The latest upstream changes (possibly a5e85ae) made this pull request unmergeable. Please resolve the merge conflicts. |
|
This looks great, thanks! Please rebase to resolve the conflicts and squash the commits. You can squash manually if there are multiple independent commits you want to preserve, or use @rustbot author |
3 participants
Tried to keep things as readable as possible by splitting out the macro magic into its own helper.