feat: apply #[cfg] to proc macro inputs

[wyatt-herkamp]

This will attempt to process cfg attributes and cfg_attr attributes for proc macro expansion.

Closes #8434 , #11657, and #13904

[wyatt-herkamp]

I do have a few questions for the team that works on Rust Analyzer.

1. Do you want me to throw this behind a feature flag as an unstable feature while it is tested?
2. The changes to fixup.rs I changed the type for remove from SyntaxNode to SyntaxElement because I need to be able to remove the comma as well. Is this the best choice. Or can I remove the comma another way?
3. Are there edge cases when I need to pass the cfg and cfg_attr attribute to the actual proc macro?
4. Can this match statement be simplified down?

[wyatt-herkamp]

#[cfg_attr(..,..)] Is parsing

[rustbot]

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

• 33282c7
• 8f08bbe
• 574e23e

[Veykril]

Annoying we have to do this on the syntax tree but there is no way around that. note that

struct S {
    field: fn(#[cfg(FALSE)] ()) -> ()
}

here we also need to configure the param away. It's fine to not support this right now though, I think we should probably have this infra here traverse the token tree in an untyped manner and just look for attributes and then handle things accordingly? Unsure.

Anyways with the few things I've raised I'm fine with merging this for now, since you said you were planning on tackling full cfg_attr supports, cleaning up and what not can be done afterwards (since the changes are all tied closely to a single module).

[Veykril]

Forgot to answer your questions:

Do you want me to throw this behind a feature flag as an unstable feature while it is tested?

If this is easily doable (which it looks like it should be), then yes let's do that for now so we can test it a bit.

The changes to fixup.rs I changed the type for remove from SyntaxNode to SyntaxElement because I need to be able to remove the comma as well. Is this the best choice. Or can I remove the comma another way?

Seems right to do this

Are there edge cases when I need to pass the cfg and cfg_attr attribute to the actual proc macro?

I don't think so

Can this match statement be simplified down?

Not really

[wyatt-herkamp]

If this is easily doable (which it looks like it should be), then yes let's do that for now so we can test it a bit.

How do I get the settings from the expand db?

I don't think so

Actually you already provided one
field: fn(#[cfg(FALSE)] ()) -> ()

[Veykril]

How do I get the settings from the expand db?

We'd have to thread that through the database which is kind of annoying actually. Should be fine to not put this behind a config then. It shouldn't really break anything anyways (or rather make it worse than the status quo).

Also would be nice to add a test that this works as expected for our builtin derives in crates\hir-def\src\macro_expansion_tests\builtin_derive_macro.rs

[Veykril]

Are there edge cases when I need to pass the cfg and cfg_attr attribute to the actual proc macro?

Actually there is another one, #[cfg_attr(foo, cfg_attr(bar, baz))], that is we need to recurse on this

[wyatt-herkamp]

Are there edge cases when I need to pass the cfg and cfg_attr attribute to the actual proc macro?

Actually there is another one, #[cfg_attr(foo, cfg_attr(bar, baz))], that is we need to recurse on this

Do you want that here? Or should that be done when we handle multiple attributes?

[Veykril]

that is fine to skip for now, its a very rare occurence

[wyatt-herkamp]

that is fine to skip for now, its a very rare occurence

Awesome. I think it should be good now.

Comparing against output from cargo expand is good.

[Veykril]

Alright, lgtm 👍 (a few things I'd clean up slightly, but I think that can wait until your follow up and I might just do that myself, don't wanna nitpick and block a PR that way)

[Veykril]

Very nice to see this, this has been a pain point for a lot of folks, especially diesel I think cc @weiznich :)
Thanks!
@bors r+

[bors]

📌 Commit 447de3d has been approved by Veykril

It is now in the queue for this repository.

[bors]

⌛ Testing commit 447de3d with merge d3e6fea...

[bors]

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing d3e6fea to master...