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

Nightly rejects some macro-generated doc attribute values accepted by stable #85432

Closed
Erutuon opened this issue May 18, 2021 · 14 comments
Closed
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Milestone

Comments

@Erutuon
Copy link

Erutuon commented May 18, 2021

A version of my crate parse-mediawiki-sql with macro-generated docs builds on stable, but errors out on nightly, where the error suggests that I fix the error by enabling #![feature(extended_key_value_attributes)].

Stable is cargo 1.52.0 (rust-lang/cargo@69767412a 2021-04-21), rustc 1.52.1 (9bc8c42 2021-05-09), nightly is cargo 1.54.0-nightly (rust-lang/cargo@070e459c2 2021-05-11), rustc 1.54.0-nightly (3e99439 2021-05-17).

The error occurs in a macro that generates a struct declaration with an implementation of a trait and calls another macro to take a string generated by another set of macros and put it into a doc attribute above the struct. Here are the lines containing the macro declarations.

The error looks like this:

error[E0658]: arbitrary expressions in key-value attributes are unstable
   --> src/schemas.rs:152:13
    |
152 |               database_table_doc!($table_name $(, $page)?),
    |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
274 | / impl_row_from_sql! {
275 | |     image
276 | |     Image<'input> {
277 | |         name: PageTitle,
...   |
293 | |     }
294 | | }
    | |_- in this macro invocation
    |
    = note: see issue #78835 <https://github.com/rust-lang/rust/issues/78835> for more information
    = help: add `#![feature(extended_key_value_attributes)]` to the crate attributes to enable
    = note: this error originates in the macro `impl_row_from_sql` (in Nightly builds, run with -Z macro-backtrace for more info)

There are 22 calls to the macro, but only 13 errors, for the calls those whose contents start with $table_name:ident $output_type:ident<$life:lifetime>. I tried to generate a minimal example by copying the macros and one of the failing calls, but failed: this reduced version compiled on both stable and nightly in the playground.

Somehow this sequence of tokens causes the error in the crate but not in the smaller playground example. I don't know much about the internals of the compiler here so have no idea how to change the playground example to generate the error.

@Erutuon Erutuon added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels May 18, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 18, 2021
@hameerabbasi hameerabbasi added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label May 18, 2021
@hameerabbasi
Copy link
Contributor

@rustbot ping cleanup

@rustbot
Copy link
Collaborator

rustbot commented May 18, 2021

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label May 18, 2021
@hellow554
Copy link
Contributor

hellow554 commented May 18, 2021

While compiling with nightly another (maybe (un-)related error) pops up, which isn't present on stable:

[...]
error[E0308]: mismatched types
    --> /home/marcel/.cargo/registry/src/github.com-1ecc6299db9ec823/lexical-core-0.7.4/src/atof/algorithm/math.rs:2065:27
     |
2065 |     let rs = Limb::BITS - s;
     |                           ^ expected `u32`, found `usize`

error[E0277]: cannot subtract `usize` from `u32`
    --> /home/marcel/.cargo/registry/src/github.com-1ecc6299db9ec823/lexical-core-0.7.4/src/atof/algorithm/math.rs:2065:25
     |
2065 |     let rs = Limb::BITS - s;
     |                         ^ no implementation for `u32 - usize`
     |
     = help: the trait `Sub<usize>` is not implemented for `u32`

Never mind, seems to be an issue with the 0.7.4 version of lexical-core. cargo update fixed that by updating to 0.7.6 and now I can see OPs error

@hellow554
Copy link
Contributor

hellow554 commented May 18, 2021

This code errors on nightly, but not on stable:

macro_rules! with_doc_comment {
    ($comment:expr, $item:item) => {
        #[doc = $comment]
        $item
    };
}

macro_rules! database_table_doc {
    () => {
        ""
    };
}

with_doc_comment! {
    database_table_doc!(),
    #[derive(Debug)]
    struct Image {
        #[cfg(test)]
        _f: (),
    }

}

fn main() {}

I had a similar piece of code (I try to post it in the next post) that fails with the exact message on stable as well.

But here's the regression:

searched nightlies: from nightly-2019-10-01 to nightly-2021-05-01
regressed nightly: nightly-2021-04-12
searched commits: from a836d9b to a866124
regressed commit: ba6275b

cc #82608 @Aaron1011

@hellow554
Copy link
Contributor

hellow554 commented May 18, 2021

This code fails on stable with the same message as on nightly:

macro_rules! database_table_doc {
    () => {
        ""
    };
}

#[doc = database_table_doc!()]
#[derive(Debug)]
struct Image {
    #[cfg(test)]
    _f: (),
}

fn main() {}

I just "inlined" the with_doc_comment macro.


So in 58d2bad the error message changed from

error: unexpected token: `database_table_doc`
 --> src/main.rs:7:9
  |
7 | #[doc = database_table_doc!()]
  |         ^^^^^^^^^^^^^^^^^^

to

error[E0658]: arbitrary expressions in key-value attributes are unstable
 --> src/main.rs:7:9
  |
7 | #[doc = database_table_doc!()]
  |         ^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #78835 <https://github.com/rust-lang/rust/issues/78835> for more information
  = help: add `#![feature(extended_key_value_attributes)]` to the crate attributes to enable

(that's what we have today)

cc #78837 #78835 @petrochenkov

I think the commit message makes it very clear why the possiblity was added to use macros there

[...] External doc strings (#[doc = include_str!("my_doc.md")], [...]

What to do on this, is above my paygrade ;)

@rustbot modify labels: -E-needs-mcve

@rustbot
Copy link
Collaborator

rustbot commented May 18, 2021

Error: Label ICEBreaker-Cleanup-Crew can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot rustbot removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label May 18, 2021
@hellow554
Copy link
Contributor

Funny enough, in the tracking issue #78835 @alexschrod had the same issue as in this post, so this error is somewhat known. In the paste crate it was reported at dtolnay/paste#29 and fixed in dtolnay/paste#30, so maybe that's something for you @Erutuon if you need a fix right now.

@hameerabbasi
Copy link
Contributor

@rustbot modify labels -E-needs-mcve

@hameerabbasi
Copy link
Contributor

Sorry for the spam!

@rustbot modify labels -ICEBreaker-Cleanup-Crew

@rustbot rustbot removed the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label May 18, 2021
@hellow554
Copy link
Contributor

@hameerabbasi you can edit your post and use the rustbot command again without creating a new post ;)

@Aaron1011 Aaron1011 added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label May 18, 2021
@Aaron1011 Aaron1011 self-assigned this May 18, 2021
@Erutuon
Copy link
Author

Erutuon commented May 18, 2021

@hellow554

This code fails on stable with the same message as on nightly:

macro_rules! database_table_doc {
    () => {
        ""
    };
}

#[doc = database_table_doc!()]
#[derive(Debug)]
struct Image {
    #[cfg(test)]
    _f: (),
}

fn main() {}

I just "inlined" the with_doc_comment macro.

Right, that's the reason for the with_doc_comment macro. On stable, taking a macro-generated string followed by an item and passing them to another macro and having that macro add the string to a doc attribute above the item seems to convince the compiler (somehow) that the doc attribute value is acceptable, even though it's not really a string literal.

Funny enough, in the tracking issue #78835 @alexschrod had the same issue as in this post, so this error is somewhat known. In the paste crate it was reported at dtolnay/paste#29 and fixed in dtolnay/paste#30, so maybe that's something for you @Erutuon if you need a fix right now.

It looks like I'm using the same technique as the regression test, but on nightly now it doesn't compile in 13 out of the 22 macro calls in my code.

@Aaron1011
Copy link
Member

This will be fixed by #85445

Aaron1011 added a commit to Aaron1011/rust that referenced this issue May 18, 2021
Fixes rust-lang#85432

When processing a `#[derive]` or `#[cfg_eval]` attribute, we need to
re-parse our attribute target, which requires flattenting all
`Nonterminals`. However, this caused us to incorrectly gate on a
(flattented) nonterminal in a key-value attribute, which is supposed to
be allowed.

Since we already perform this gating during the initial parse, we
suppress it in `capture_cfg` mode.
@jyn514 jyn514 added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels May 19, 2021
@jyn514 jyn514 added this to the 1.53.0 milestone May 19, 2021
@hameerabbasi
Copy link
Contributor

Assigning P-critical as discussed in the WG-prioritization discussion on Zulip and removing I-prioritize.

@rustbot modify labels +P-critical -I-prioritize

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 19, 2021
Mark-Simulacrum pushed a commit to Mark-Simulacrum/rust that referenced this issue Jun 11, 2021
Fixes rust-lang#85432

When processing a `#[derive]` or `#[cfg_eval]` attribute, we need to
re-parse our attribute target, which requires flattenting all
`Nonterminals`. However, this caused us to incorrectly gate on a
(flattented) nonterminal in a key-value attribute, which is supposed to
be allowed.

Since we already perform this gating during the initial parse, we
suppress it in `capture_cfg` mode.
@pietroalbini
Copy link
Member

Fixed on 1.54 by #85445 and fixed on 1.53 by #86225.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants