Skip to content

Reordering of attributes passed to rustc_macro_derive #36211

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
dtolnay opened this issue Sep 2, 2016 · 5 comments
Closed

Reordering of attributes passed to rustc_macro_derive #36211

dtolnay opened this issue Sep 2, 2016 · 5 comments

Comments

@dtolnay
Copy link
Member

dtolnay commented Sep 2, 2016

(Using rustc from #35957)

Macros 1.1 is reordering attributes in a way that breaks Syntex. If I have the following:

#[derive(Serialize, Deserialize)]
#[serde(bound(deserialize = "T::Owned: Deserialize"))]
struct CowT<'a, T: ?Sized + 'a + ToOwned>(Cow<'a, T>);

It gets passed to my #[rustc_macro_derive(Serialize)] as:

#[serde(bound(deserialize = "T::Owned: Deserialize"))]
#[derive(Deserialize)]
struct CowT<'a, T: ?Sized + 'a + ToOwned>(Cow<'a, T>);

I would expect the #[derive(Deserialize)] to stay above #[serde(...)].

cc @alexcrichton

@alexcrichton
Copy link
Member

Hm I'm not actually sure how to fix this offhand. When the #[derive] syntax extension is running the #[derive] attribute has already been removed, and then when we call the first Serialize implementation we manually push back on the #[derive(Deserialize)]. We could push it at the front instead of the back but I have a feeling that's probably also not the right place :(

Might require a more intrusive change to #[derive] to not remove the attribute at the beginning.

@nrc
Copy link
Member

nrc commented Sep 2, 2016

We could push it at the front instead of the back but I have a feeling that's probably also not the right place

Why? Putting the remaining derives back where the one we just took out seems like the right thing to do to me.

@alexcrichton
Copy link
Member

Oh right yeah we should put the #[derive] attribute back where we found it, but I don't think that information is available via the kind of syntax extension #[derive] is today?

@dtolnay
Copy link
Member Author

dtolnay commented Sep 27, 2016

This only affected syntex and is no longer required as we move our rustc_macro away from syntex (serde-rs/serde#548).

@dtolnay
Copy link
Member Author

dtolnay commented May 25, 2017

Without syntex this no longer matters.

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

No branches or pull requests

3 participants