-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Attributes are reordered before they are passed to a proc-macro #67839
Comments
I agree that the relative order of inert attributes should be kept. I plan to fix this during the upcoming macro invocation collector rewrite (#54727 (comment)), unless someone gets to it earlier (this should be doable without rewriting the invocation collector entirely). cc #63221 |
I think this change can wait, until the new macro system is implemented, becuse this would be a breaking change either way. (for example the code given above would have different output, if a macro relies on the current order of attributes) |
Would this issue also include the proposal of ordering from rust-lang/reference#565 (comment)? If I currently write, #[foo]
#[derive(Bar)] As far as I observed: |
Fixed in #82419. @pksunkara |
@petrochenkov Really appreciate your hard work in fixing both the issues. I have reworked some of my APIs to not run into this, but I am happy that I can now pick the best ergonomics for my proc-macro libraries instead of trying to navigate these issues. |
expand: Preserve order of inert attributes during expansion Fixes rust-lang#67839 Fixes rust-lang#81871 r? `@Aaron1011`
expand: Preserve order of inert attributes during expansion Fixes rust-lang#67839 Fixes rust-lang#81871 r? ``@Aaron1011``
expand: Preserve order of inert attributes during expansion Fixes rust-lang#67839 Fixes rust-lang#81871 r? ```@Aaron1011```
expand: Preserve order of inert attributes during expansion Fixes rust-lang#67839 Fixes rust-lang#81871 r? ````@Aaron1011````
This way test macros following `#[rstest]` can decide whether or not to generate test macro to avoid duplicate test runs. It is an attempt to improve capabilities among test macros. Currently, following test from [googletest](https://github.com/google/googletest-rust/blob/21f2948684847922a416252b8118e3eada8e29d6/integration_tests/src/google_test_with_rstest.rs#L52-L57)(`main` branch at 2025-01-16) will run twice. ```rust #[rstest] #[case(1)] #[gtest] fn paramterised_test_should_work_with_rstest_first(#[case] value: u32) -> Result<()> { verify_that!(value, eq(value)) } ``` See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.
This way test macros following `#[rstest]` can decide whether or not to generate test macro to avoid duplicate test runs. It is an attempt to improve capabilities among test macros. Currently, following test from [googletest](https://github.com/google/googletest-rust/blob/21f2948684847922a416252b8118e3eada8e29d6/integration_tests/src/google_test_with_rstest.rs#L52-L57)(`main` branch at 2025-01-16) will run twice. ```rust #[rstest] #[case(1)] #[gtest] fn paramterised_test_should_work_with_rstest_first(#[case] value: u32) -> Result<()> { verify_that!(value, eq(value)) } ``` See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.
This way test macros following `#[rstest]` can decide whether or not to generate test macro to avoid duplicate test runs. It is an attempt to improve capabilities among test macros. Currently, following test from [googletest](https://github.com/google/googletest-rust/blob/21f2948684847922a416252b8118e3eada8e29d6/integration_tests/src/google_test_with_rstest.rs#L52-L57)(`main` branch at 2025-01-16) will run twice. ```rust #[rstest] #[case(1)] #[gtest] fn paramterised_test_should_work_with_rstest_first(#[case] value: u32) -> Result<()> { verify_that!(value, eq(value)) } ``` See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.
This way test macros following `#[rstest]` can decide whether or not to generate test macro to avoid duplicate test runs. It is an attempt to improve capabilities among test macros. Currently, following test from [googletest](https://github.com/google/googletest-rust/blob/21f2948684847922a416252b8118e3eada8e29d6/integration_tests/src/google_test_with_rstest.rs#L52-L57)(`main` branch at 2025-01-16) will run twice. ```rust #[rstest] #[case(1)] #[gtest] fn paramterised_test_should_work_with_rstest_first(#[case] value: u32) -> Result<()> { verify_that!(value, eq(value)) } ``` See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.
`#[gtest]` will benefit from la10736/rstest#291, we could also benefit other test macros. It is an attempt to improve capabilities among test macros to avoid duplicated test runs which is rare to be aware of. The rationale is simple: procedure of attribute macro see only attributes following it. Macros next to processing macro will not see generated macro if it is generated in-place. So, instead of generating test macro in-place, appending generated test macro will let macros next to processing macro have chance to react, for example, not generate test macro if there is already one. We could deprecate `#[googletest::test]` oneday after rust-lang/rust#82419 released. See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, la10736/rstest#291, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.
`#[gtest]` will benefit from la10736/rstest#291, we could also benefit other test macros. It is an attempt to improve capabilities among test macros to avoid duplicated test runs which is rare to be aware of. The rationale is simple: procedure of attribute macro see only attributes following it. Macros next to processing macro will not see generated macro if it is generated in-place. So, instead of generating test macro in-place, appending generated test macro will let macros next to processing macro have chance to react, for example, not generate test macro if there is already one. We could deprecate `#[googletest::test]` oneday after la10736/rstest#291 released. See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, la10736/rstest#291, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.
`#[gtest]` will benefit from la10736/rstest#291, we could also benefit other test macros. This is an attempt to improve capabilities among test macros to avoid duplicated test runs which is rare to be aware of. The rationale is simple: procedure of attribute macro see only attributes following it. Macros next to processing macro will not see generated macro if it is generated in-place. So, instead of generating test macro in-place, appending generated test macro will let macros next to processing macro have chance to react, for example, not generate test macro if there is already one. We could deprecate `#[googletest::test]` oneday after la10736/rstest#291 released. See: tokio-rs/tokio#6497, d-e-s-o/test-log#46, frondeus/test-case#143, la10736/rstest#291, kezhuw/stuck#53. Refs: rust-lang/rust#67839, rust-lang/rust#82419.
I am working on a proc-macro, where I would like to give the user the option, to decide, which attributes (and doc comments) should be forwarded.
For example
as you can see in the above case, the order of the attributes is very important.
Sadly the rust compiler (or the
proc_macro
crate) does reorder the attributes, before passing them to the function as aTokenStream
.(this is the output of
cargo expand
, the proc-macro gets a similarTokenStream
, where the order is not preserved either)I think this should not happen and the order should be preserved! Are there any good reasons for reordering them?
related to #36211
The text was updated successfully, but these errors were encountered: