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

rustc: Tweak expansion order of custom derive #36782

Merged
merged 1 commit into from
Sep 29, 2016

Conversation

alexcrichton
Copy link
Member

This commit alters the expansion order of custom macros-1.1 style #[derive]
modes. Instead of left-to-right the expansion now happens in three categories,
each of which is internally left-to-right:

  • Old-style custom derive (#[derive_Foo]) is expanded
  • New-style custom derive (macros 1.1) is expanded
  • Built in derive modes are expanded

This gives built in derive modes maximal knowledge about the struct that's being
expanded and also avoids pesky issues like exposing #[structural_match] or
#[rustc_copy_clone_marker].

cc #35900

This commit alters the expansion order of custom macros-1.1 style `#[derive]`
modes. Instead of left-to-right the expansion now happens in three categories,
each of which is internally left-to-right:

* Old-style custom derive (`#[derive_Foo]`) is expanded
* New-style custom derive (macros 1.1) is expanded
* Built in derive modes are expanded

This gives built in derive modes maximal knowledge about the struct that's being
expanded and also avoids pesky issues like exposing `#[structural_match]` or
`#[rustc_copy_clone_marker]`.

cc rust-lang#35900
@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

r? @nrc

the problem: #35900 (comment)
proposed solution: #35900 (comment)

@rust-highfive rust-highfive assigned nrc and unassigned aturon Sep 27, 2016
@nrc
Copy link
Member

nrc commented Sep 27, 2016

While reviewing this it occurred to me that we should deprecate the old style custom derive sooner rather than later. It is unstable, so I don't think we should wait for macros 1.1 to stabilise.

@nrc
Copy link
Member

nrc commented Sep 27, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 27, 2016

📌 Commit e5e7021 has been approved by nrc

@bors
Copy link
Contributor

bors commented Sep 28, 2016

⌛ Testing commit e5e7021 with merge 98772ac...

@bors
Copy link
Contributor

bors commented Sep 28, 2016

💔 Test failed - auto-win-gnu-64-opt

@alexcrichton
Copy link
Member Author

@bors: retry

On Tue, Sep 27, 2016 at 7:50 PM, bors [email protected] wrote:

💔 Test failed - auto-win-gnu-64-opt
https://buildbot.rust-lang.org/builders/auto-win-gnu-64-opt/builds/5725


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#36782 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95L6BlbTBEOQ42lmWtFh1v2kF9RbRks5qudX7gaJpZM4KIEpt
.

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Sep 28, 2016
…er, r=nrc

rustc: Tweak expansion order of custom derive

This commit alters the expansion order of custom macros-1.1 style `#[derive]`
modes. Instead of left-to-right the expansion now happens in three categories,
each of which is internally left-to-right:

* Old-style custom derive (`#[derive_Foo]`) is expanded
* New-style custom derive (macros 1.1) is expanded
* Built in derive modes are expanded

This gives built in derive modes maximal knowledge about the struct that's being
expanded and also avoids pesky issues like exposing `#[structural_match]` or
`#[rustc_copy_clone_marker]`.

cc rust-lang#35900
bors added a commit that referenced this pull request Sep 28, 2016
Rollup of 11 pull requests

- Successful merges: #36376, #36672, #36740, #36757, #36765, #36769, #36782, #36783, #36784, #36795, #36796
- Failed merges:
@bors bors merged commit e5e7021 into rust-lang:master Sep 29, 2016
@alexcrichton alexcrichton deleted the rustc-macro-expand-order branch October 6, 2016 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants