Apply attr proc macros before cfg processing

[tmnilsson]

Fixes #39336.
r? @jseyfried

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jseyfried (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[aidanhs]

Thanks for the PR @tmnilsson, we'll check in now and again to make sure @jseyfried (or another reviewer) gets to this soon!

[carols10cents]

review ping @jseyfried! pinging you on IRC too.... or not because either I don't know your nick or you're not online :) sneaky!

[jseyfried]

nit: } else { is more idiomatic

[jseyfried]

nit: !attr.is_some() -> attr.is_none()

[jseyfried]

@tmnilsson looks good so far!

Could you test that
#[derive(Foo)] #[foo] struct S { #[cfg(any())] x: i32 } gets fully configured (i.e. x is removed), given

#[proc_macro_derive(Foo, attributes(foo))]
fn derive_foo(input: TokenStream) -> TokenStream { input }

The issue here is that when collecting #[derive(Foo)] #[foo] struct S { ... }, we don't know whether #[foo] is an attribute proc-macro or just an "inert" attribute used to give information to #[derive(Foo)], so we collect is as an attribute proc macro first (i.e. your code won't fully configure).

If we learn that #[foo] is not in fact a proc macro, we re-collect into a derive invocation here: https://github.com/tmnilsson/rust/blob/835992d4b14800cac0c450f893c050fdedd18dca/src/libsyntax/ext/expand.rs#L285.

I believe you'll need to fully_configure! the item at this line to ensure that we don't regress examples like this that compile today.

[tmnilsson]

Thanks for the feedback! That example would not get fully configured, as you expected. I wasn't even aware of inert attributes, actually. Anyway, I moved the full configure as you suggested, and that seems to make it work.

[jseyfried]

nit: item.map(|item| cfg.fold_trait_item(item).pop().unwrap()) is more idiomatic and efficient here (i.e. with P). Same for ImplItem.

[tmnilsson]

Got it, fixed.

[jseyfried]

@tmnilsson Excellent, thanks!
r=me modulo optional nit.

Also FYI we usually rebase PR branch over newer master and force push instead of merging master into PR branch.

[jseyfried]

@bors r+

[bors]

📌 Commit 0f97b6b has been approved by jseyfried

[bors]

⌛ Testing commit 0f97b6b with merge 46ef620...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: jseyfried
Pushing 46ef620 to master...