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

Use :ident matcher instead of :ty matcher #17

Merged
merged 1 commit into from
Aug 22, 2020

Conversation

Aaron1011
Copy link
Contributor

When a :ty matcher is used in a macro_rules! macro, the captured
argument can no longer be used to match a literal in a new
macro_rules! invocation. For example, the following code:

macro_rules! inner {
    (my_val) => {}
}

macro_rules! outer {
    ($val:ty) => {
        inner!($val);
    }
}

outer!(my_val);

produces the following error:

error: no rules expected the token `my_val`
  --> src/lib.rs:7:16
   |
1  | macro_rules! inner {
   | ------------------ when calling this macro
...
7  |         inner!($val);
   |                ^^^^ no rules expected this token in macro call
...
11 | outer!(my_val);
   | --------------- in this macro invocation
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

That is, once my_val is captured as a :ty, it will not match a my_val literal
in the inner! invocation.

Due to a bug in rustc, code like this was accepted by the compiler
when a proc-macro attribute was applied to the macro invocation.
This affects fourier-algorithms due to the following code:

#[target_cfg(target = "[x86|x86_64]+avx")]
crate::avx_vector! { $type };

The avx_vector! macro is expecting either f32 or f64
as an ordinary token. However, the $type metavariable uses a
:ty matcher, which will prevent it from matching against f32
or f64 (even though the captured type is always f32 or f64).

Due to the target_cfg attribute proc-macro, this code was incorrectly
allowed to compile. When the underlying bug in rustc is fixed
in rust-lang/rust#73084, fourier-algorithms
will stop compiling.

This commit changes two :ty matchers to :ident matchers, which can
be used as arguments to a macro_rules! macro expecting a plain f32
or f64 token. This will still compile on existing compile versions,
and will allow fourier-algorithms to continue to compile once
rust-lang/rust#73084 is merged into rustc.

When a `:ty` matcher is used in a `macro_rules!` macro, the captured
argument can no longer be used to match a literal in a new
`macro_rules!` invocation. For example, the following code:

```rust
macro_rules! inner {
    (my_val) => {}
}

macro_rules! outer {
    ($val:ty) => {
        inner!($val);
    }
}

outer!(my_val);
```

produces the following error:

```
error: no rules expected the token `my_val`
  --> src/lib.rs:7:16
   |
1  | macro_rules! inner {
   | ------------------ when calling this macro
...
7  |         inner!($val);
   |                ^^^^ no rules expected this token in macro call
...
11 | outer!(my_val);
   | --------------- in this macro invocation
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
```

That is, once `my_val` is captured as a `:ty`, it will not match a `my_val` literal
in the `inner!` invocation.

Due to a bug in rustc, code like this was accepted by the compiler
when a proc-macro attribute was applied to the macro invocation.
This affects `fourier-algorithms` due to the following code:

```rust
#[target_cfg(target = "[x86|x86_64]+avx")]
crate::avx_vector! { $type };
```

The `avx_vector!` macro is expecting either `f32` or `f64`
as an ordinary token. However, the `$type` metavariable uses a
`:ty` matcher, which will prevent it from matching against `f32`
or `f64` (even though the captured type is always `f32` or `f64`).

Due to the `target_cfg` attribute proc-macro, this code was incorrectly
allowed to compile. When the underlying bug in rustc is fixed
in rust-lang/rust#73084, `fourier-algorithms`
will stop compiling.

This commit changes two `:ty` matchers to `:ident` matchers, which can
be used as arguments to a `macro_rules!` macro expecting a plain `f32`
or `f64` token. This will still compile on existing compile versions,
and will allow `fourier-algorithms` to continue to compile once
rust-lang/rust#73084 is merged into rustc.
@calebzulawski
Copy link
Owner

Thanks! Looks good.

@calebzulawski calebzulawski merged commit a56be4b into calebzulawski:master Aug 22, 2020
@Aaron1011
Copy link
Contributor Author

@calebzulawski: Would you mind making a new point release? I'm planning to merge rust-lang/rust#73084 soon, and it would be nice for there to be an upgrade path for all downstream crates affected by it.

@calebzulawski
Copy link
Owner

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants