Skip to content
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

Support matching interpolated types in macro invocation syntax #6659

Closed
zargony opened this issue May 21, 2013 · 5 comments
Closed

Support matching interpolated types in macro invocation syntax #6659

zargony opened this issue May 21, 2013 · 5 comments
Labels
A-syntaxext Area: Syntax extensions

Comments

@zargony
Copy link
Contributor

zargony commented May 21, 2013

It would be nice if macros could be invoked using an interpolated type from a transcription of another macro.

This is my macro to convert C typed values from an external library to Rust types:

macro_rules! c_to_rust(
  ($v:ident c_int) => ($v);
  ($v:ident *c_char) => (str::raw::from_c_str($v));
)

It works fine when invoked like c_to_rust!(i: c_int), but it can't be used from within another macro using an interpolated type like c_to_rust!($v: $ty) (where the macro's invocation contains $v:ident, $ty:ty) - probably because interpolated types cannot be matched literally in the invocation syntax.

Here's my other macro which fails to use the above one. For bindings to an external library, I need to provide a lot of callback functions. So I'm using a macro to define them, since they only differ in name and parameter types. But I want the forwarding function to convert the parameter to Rust types before passing them along, which unfortunately doesn't work. (Any hints on how to work around this are welcome, I didn't find a suitable way without repeating a lot of code)

macro_rules! forward_callback(
  ($fn_name:ident, $op_name:ident, $($param_name:ident: $param_type:ty),*) => (
    extern fn $fn_name (userdata: *c_void, $($param_name: $param_type),*) {
      unsafe {
        let session: &Session = cast::transmute(userdata);
        session.$op_name($(c_to_rust!($param_name: $param_type)),*);
                                                    ^~~~~~~~~ FAILS
      }
    }
  );
)

// Intended use:
forward_callback!(function1, p1: c_int)
forward_callback!(function2, p1: c_int, p2: *c_char)
@Aatch
Copy link
Contributor

Aatch commented May 24, 2013

Hmm, I'm not sure if this is actually a error. Rust macros work on token-trees, not text. Since c_int is just a literal token for the string c_int, it won't match against a type token, which is what your forward_callback! produces.

As for a workaround, you could add another clause to c_to_rust! that matches any type.

The only thing I could see is maybe adding a bit of logic to allow specialisation on tokens. So you could say, "match a type token that is c_int".

@zargony
Copy link
Contributor Author

zargony commented May 24, 2013

Yes, this is more like a feature request than a bug report :). In my case, it would have been nice to be able to match on types. I guess I tried to add some kind of polymorphism to the c_to_rust! macro with it (which I'd find useful to be supported)

Adding a clause that matches any type wouldn't help since I needed different types to be handled differently. The only way I found to work around it is to define a trait ToRust and implement a to_rust function for every desired type - which ended way more lengthy than I thought it would be.

@pnkfelix
Copy link
Member

@Aatch you raise an interesting point there: it seems like this is a place where the macro system is falling down.

In your suggestion to add logic for specialization on tokens, was there any particular syntax you were thinking of? Off the top of my head, I was thinking perhaps something like:

macro_rules! c_to_rust(
  ($v:ident c_int:ty) => ($v);
  ($v:ident $(*c_char):ty) => (str::raw::from_c_str($v));
)

where the trailing : $ty is meant to be analogous to how the pattern bindings are assigned types, but now instead of having a pattern binding before the colon, it is instead a type expression that must be matched. Though it seems like this particular idea might be fraught with usability perils, in terms of how far the ":ty" might occur from the beginning of the left-hand-side...

Another idea for a syntax is:

macro_rules! c_to_rust(
  ($v:ident $t:ty) if $t match c_int => ($v);
  ($v:ident $t:ty) if $t match *c_char => (str::raw::from_c_str($v));
)

which might be an easier syntax to both use and also implement.

@catamorphism
Copy link
Contributor

(bug triage) No updates here.

@steveklabnik
Copy link
Member

Triage: macro stuff has been stabilized, anything more would need an RFC. If this is worth porting over to an RFC repo issue, please let me know.

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 3, 2021
Fix let_and_return false positive

The issue:

See this Rust playground link: https://play.rust-lang.org/?edition=2018&gist=12cb5d1e7527f8c37743b87fc4a53748

Run the above with clippy to see the following warning:

```
warning: returning the result of a `let` binding from a block
  --> src/main.rs:24:5
   |
23 |     let value = Foo::new(&x).value();
   |     --------------------------------- unnecessary `let` binding
24 |     value
   |     ^^^^^
   |
   = note: `#[warn(clippy::let_and_return)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
   |
23 |
24 |     Foo::new(&x).value()
   |
```

Implementing the suggested fix, removing the temporary let binding,
yields a compiler error:

```
error[E0597]: `x` does not live long enough
  --> src/main.rs:23:14
   |
23 |     Foo::new(&x).value()
   |     ---------^^-
   |     |        |
   |     |        borrowed value does not live long enough
   |     a temporary with access to the borrow is created here ...
24 | }
   | -
   | |
   | `x` dropped here while still borrowed
   | ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `Foo`
   |
   = note: the temporary is part of an expression at the end of a block;
           consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
   |
23 |     let x = Foo::new(&x).value(); x
   |     ^^^^^^^                     ^^^
```

The fix:

Of course, clippy looks like it should already handle this edge case;
however, it appears `utils::fn_def_id` is not returning a `DefId` for
`Foo::new`. Changing the `qpath_res` lookup to use the child Path
`hir_id` instead of the parent Call `hir_id` fixes the issue.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-syntaxext Area: Syntax extensions
Projects
None yet
Development

No branches or pull requests

5 participants