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

Temporarily prohibit proc macro attributes placed after derives #54277

Merged
merged 1 commit into from
Sep 17, 2018

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Sep 16, 2018

... and also proc macro attributes used together with #[test]/#[bench].

Addresses item 6 from #50911 (comment).

The end goal is straightforward predictable left-to-right expansion order for attributes.
Right now derives are expanded last regardless of their relative ordering with macro attributes and right now it's simpler to temporarily prohibit macro attributes placed after derives than changing the expansion order.
I'm not sure whether the new beta is already released or not, but if it's released, then this patch needs to be backported, so the solution needs to be minimal.

How to fix broken code (derives):

  • Move macro attributes above derives. This won't change expansion order, they are expanded before derives anyway.

Using attribute macros on same items with #[test] and #[bench] is prohibited for similar expansion order reasons, but this one is going to be reverted much sooner than restrictions on derives (UPDATE: reverted in #54336).

How to fix broken code (test/bench):

  • Enable #![feature(plugin)] (don't ask why).

r? @ghost

... and also proc macro attributes used together with test/bench.
@petrochenkov
Copy link
Contributor Author

r? @alexcrichton

@petrochenkov petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 17, 2018
@alexcrichton
Copy link
Member

@bors: r+ p=1

@bors
Copy link
Contributor

bors commented Sep 17, 2018

📌 Commit 229df02 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2018
@bors
Copy link
Contributor

bors commented Sep 17, 2018

⌛ Testing commit 229df02 with merge 354a29a...

bors added a commit that referenced this pull request Sep 17, 2018
Temporarily prohibit proc macro attributes placed after derives

... and also proc macro attributes used together with `#[test]`/`#[bench]`.

Addresses item 6 from #50911 (comment).

The end goal is straightforward predictable left-to-right expansion order for attributes.
Right now derives are expanded last regardless of their relative ordering with macro attributes and right now it's simpler to temporarily prohibit macro attributes placed after derives than changing the expansion order.
I'm not sure whether the new beta is already released or not, but if it's released, then this patch needs to be backported, so the solution needs to be minimal.

How to fix broken code (derives):
- Move macro attributes above derives. This won't change expansion order, they are expanded before derives anyway.

Using attribute macros on same items with `#[test]` and `#[bench]` is prohibited for similar expansion order reasons, but this one is going to be reverted much sooner than restrictions on derives.

How to fix broken code (test/bench):
- Enable `#![feature(plugin)]` (don't ask why).

r? @ghost
@bors
Copy link
Contributor

bors commented Sep 17, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 354a29a to master...

@bors bors merged commit 229df02 into rust-lang:master Sep 17, 2018
petrochenkov added a commit to petrochenkov/rust that referenced this pull request Oct 5, 2018
The restrictions were introduced in rust-lang#54277 and no longer necessary now because legacy plugins are now expanded in usual left-to-right order
fn deref_mut(&mut self) -> &mut T {
&mut self.ptr
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentionally not implemented, but I'm considering ways in which we can make ASTs fully immutable (and interned, I suppose).

@petrochenkov petrochenkov deleted the afterder branch June 5, 2019 16:15
rust-cloud-vms bot pushed a commit to aDotInTheVoid/rust that referenced this pull request Apr 2, 2024
`P<T>` has implemented `DerefMut` since rust-lang#54277. While this was lamented
at the time [1], rustc now relies on it extensively via the many
implementors of MutVisitor.

Updates the docs to reflect that `P<T>` is fundamentally mutable, and a
few other cleanups to make them nicer to browse.

[1]: rust-lang#54277 (comment)
[2]: https://doc.rust-lang.org/1.77.1/nightly-rustc/rustc_ast/mut_visit/trait.MutVisitor.html#implementors
rust-cloud-vms bot pushed a commit to aDotInTheVoid/rust that referenced this pull request Apr 2, 2024
`P<T>` has implemented `DerefMut` since rust-lang#54277. While this was lamented
at the time [1], rustc now relies on it extensively via the many
implementors of MutVisitor [2].

Updates the docs to reflect that `P<T>` is fundamentally mutable, and a
few other cleanups to make them nicer to browse.

[1]: rust-lang#54277 (comment)
[2]: https://doc.rust-lang.org/1.77.1/nightly-rustc/rustc_ast/mut_visit/trait.MutVisitor.html#implementors
rust-cloud-vms bot pushed a commit to aDotInTheVoid/rust that referenced this pull request Apr 3, 2024
`P<T>` has implemented `DerefMut` since rust-lang#54277. While this was lamented
at the time [1], rustc now relies on it extensively via the many
implementors of MutVisitor [2].

Updates the docs to reflect that `P<T>` is fundamentally mutable, and a
few other cleanups to make them nicer to browse.

[1]: rust-lang#54277 (comment)
[2]: https://doc.rust-lang.org/1.77.1/nightly-rustc/rustc_ast/mut_visit/trait.MutVisitor.html#implementors
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 3, 2024
rustc_ast: Update `P<T>` docs to reflect mutable status.

`P<T>` has implemented `DerefMut` since rust-lang#54277. While this was lamented at the time [1], rustc now relies on it extensively via the many implementors of MutVisitor [2].

Updates the docs to reflect that `P<T>` is fundamentally mutable, and a few other cleanups to make them nicer to browse.

[1]: rust-lang#54277 (comment)
[2]: https://doc.rust-lang.org/1.77.1/nightly-rustc/rustc_ast/mut_visit/trait.MutVisitor.html#implementors

r? `@Nilstrieb`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
Rollup merge of rust-lang#123393 - aDotInTheVoid:ast-p-docs, r=Nilstrieb

rustc_ast: Update `P<T>` docs to reflect mutable status.

`P<T>` has implemented `DerefMut` since rust-lang#54277. While this was lamented at the time [1], rustc now relies on it extensively via the many implementors of MutVisitor [2].

Updates the docs to reflect that `P<T>` is fundamentally mutable, and a few other cleanups to make them nicer to browse.

[1]: rust-lang#54277 (comment)
[2]: https://doc.rust-lang.org/1.77.1/nightly-rustc/rustc_ast/mut_visit/trait.MutVisitor.html#implementors

r? `@Nilstrieb`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants