Expand derive macros in the MacroExpander

[keeperofdakeys]

This removes the expand_derives function, and sprinkles the functionality throughout the Invocation Collector, Expander and Resolver.

Fixes #39326

r? @jseyfried

[keeperofdakeys]

r? @jseyfried

[jseyfried]

Can you just the original path here directly?
Eventually we'll allow e.g. #![derive(foo::bar)].
path.segments[0].identifier.name would also work for now, but I see no reason to collect a path: Vec<_>.

[jseyfried]

Also, how about "cannot find derive macro {} in this scope"?
c.f. this error

[jseyfried]

Could we switch the order here (i.e. ProcMacro first, then add_derived_markers/Builtin) for back-compat?

Since we want the input of proc macro derives to be fully expanded, we'll want to move in the direction of not including builtin derives, but I'd like to do that all at once with a Crater run.

[keeperofdakeys]

So this PR has removed derive from the macro resolver, which has an interesting consequence. The first piece of code works on stable today, the second piece doesn't (but should with this change as it is currently). Obviously derive as an attribute macro is not going to work. Should anything be changed here?

// Works today
macro_rules! derive {
    () => { println!("HI!"); }
}

fn main() {
    derive!();
}

macro_rules! derive {
    () => { println!("HI!"); }
}

// Doesn't work today, except if this appears before the macro_rules!.
#[derive(Default)]
struct A;

fn main() {
    derive!();
}

[jseyfried]

@keeperofdakeys
I don't think anything needs changing. I see this is a minor improvement, but since we're already fully special-casing #[derive(...)] I don't think it matters that much so long as we don't break things :)

[jseyfried]

@bors r+

[bors]

📌 Commit b117bee has been approved by jseyfried

[frewsxcv]

This PR might have resulted in this error during the rollup:

https://travis-ci.org/rust-lang/rust/jobs/198341670

#39537

---- [pretty] pretty/attr-variant-data.rs stdout ----

	

error: pretty-printed source does not typecheck

status: exit code: 101

command: /checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc - -Zno-trans --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/pretty/attr-variant-data.pretty-out --target=x86_64-unknown-linux-gnu -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/pretty -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/pretty/attr-variant-data.stage2-x86_64-unknown-linux-gnu.pretty.libaux -Crpath -O -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers

stdout:

------------------------------------------

------------------------------------------

stderr:

------------------------------------------

error: cannot find derive macro `Serialize` in this scope

  --> <anon>:46:10

   |

46 | #[derive(Serialize, Deserialize)]

   |          ^^^^^^^^^

error: cannot find derive macro `Serialize` in this scope

  --> <anon>:30:10

   |

30 | #[derive(Serialize, Deserialize)]

   |          ^^^^^^^^^

error: cannot find derive macro `Serialize` in this scope

  --> <anon>:22:10

   |

22 | #[derive(Serialize, Deserialize)]

   |          ^^^^^^^^^

error: cannot find derive macro `Serialize` in this scope

  --> <anon>:19:10

   |

19 | #[derive(Serialize, Deserialize)]

   |          ^^^^^^^^^

error: aborting due to 4 previous errors

[keeperofdakeys]

@jseyfried So it looks like travis didn't run the pretty tests, one of which I'm failing (which I've confirmed locally). It involves parsing and pretty printing derive attributes? I'm not sure how the pretty printer is linked to the invocation collector though. Any thoughts?

[keeperofdakeys]

Oh I think I understand now. With the custom_derive feature active, unknown derive macros were previously a warning. So the pretty print didn't bail out - however now it's an error.

The simple fix would be to remove the derives, or change them to builtin variants. Then maybe rename the serde attributes to something nonsensical, because it no longer makes semantic sense if the derive macros are no longer serde's.

[keeperofdakeys]

I deleted the failing test, since it didn't seem to add much after removing the custom derive parts (other tests test custom attributes). I also added a new test using a #[proc_macro_derive(Foo, attributes(Bar))], which should cover as much as the old test and proc macro derive attributes.

[jseyfried]

@keeperofdakeys Great, thanks!
@bors r+

[bors]

📌 Commit a201348 has been approved by jseyfried