expand: Simplify expansion of derives #65252

petrochenkov · 2019-10-09T22:45:22Z

And make it more uniform with other macros.
This is done by merging placeholders for future derives' outputs into the derive container's output fragment early (addressing FIXMEs from #63667).

Also, macros with names starting with _ are no longer reported as unused, in accordance with the usual behavior of unused lints.

r? @matthewjasper or @mark-i-m

petrochenkov · 2019-10-09T22:57:53Z

FWIW, the end goal here is attempting to turn derive into a regular attribute macro, but there's still quite a bit of steps until that:

Expanding derives immediately with their container instead of enqueueing them for later expansion. Maybe I'll do that in this PR if it stays unmerged for a few days.
Implementing left-to-right attribute expansion (cc @CAD97). Right now derives (perhaps from multiple derive containers) are always expanded after attributes, regardless of their order in source code, that's unnatural and is a source of some hacks. We should try to fix it, thankfully there is some future proofing in place.

mark-i-m

Overall, it looks good to me and makes sense, but I don't feel confident enough in my understanding of derive macros to give a r=me...

src/libsyntax/ext/expand.rs

src/librustc_resolve/build_reduced_graph.rs

mark-i-m · 2019-10-10T04:04:12Z

src/librustc_resolve/build_reduced_graph.rs

-        for placeholder in extra_placeholders {
-            def_collector.visit_macro_invoc(*placeholder);
-        }
-
        let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope };
        fragment.visit_with(&mut visitor);


Just to test my understanding, this visit now takes care of the derive placeholders, right?

Yes, the placeholders for derives are now inside the fragment, like placeholders for other kinds of macros.

matthewjasper · 2019-10-10T19:45:45Z

@bors r+

bors · 2019-10-10T19:45:46Z

📌 Commit 1270140 has been approved by matthewjasper

@matthewjasper

…hewjasper expand: Simplify expansion of derives And make it more uniform with other macros. This is done by merging placeholders for future derives' outputs into the derive container's output fragment early (addressing FIXMEs from rust-lang#63667). Also, macros with names starting with `_` are no longer reported as unused, in accordance with the usual behavior of `unused` lints. r? @matthewjasper or @mark-i-m

@ghost

Rollup of 16 pull requests Successful merges: - rust-lang#64337 (libstd: Fix typos in doc) - rust-lang#64986 (Function pointers as const generic arguments) - rust-lang#65048 (Added doc about behavior of extend on HashMap) - rust-lang#65191 (Add some regression tests) - rust-lang#65200 (Add ?Sized bound to a supertrait listing in E0038 error documentation) - rust-lang#65205 (Add long error explanation for E0568) - rust-lang#65240 (self-profiling: Add events for metadata loading (plus a small dep-tracking optimization)) - rust-lang#65248 (Suggest `if let` on `let` refutable binding) - rust-lang#65252 (expand: Simplify expansion of derives) - rust-lang#65263 (Deduplicate is_{freeze,copy,sized}_raw) - rust-lang#65265 (Cleanup librustc mir err codes) - rust-lang#65266 (Mark Path::join as must_use) - rust-lang#65276 (Don't cc rust-lang/compiler for toolstate changes) - rust-lang#65277 (Query generator kind for error reporting) - rust-lang#65283 (stability: Do not use `buffer_lint` after lowering to HIR) - rust-lang#65289 (Fix suggested bound addition diagnostic) Failed merges: r? @ghost

@ghost

Rollup of 16 pull requests Successful merges: - rust-lang#64337 (libstd: Fix typos in doc) - rust-lang#64986 (Function pointers as const generic arguments) - rust-lang#65048 (Added doc about behavior of extend on HashMap) - rust-lang#65191 (Add some regression tests) - rust-lang#65200 (Add ?Sized bound to a supertrait listing in E0038 error documentation) - rust-lang#65205 (Add long error explanation for E0568) - rust-lang#65240 (self-profiling: Add events for metadata loading (plus a small dep-tracking optimization)) - rust-lang#65248 (Suggest `if let` on `let` refutable binding) - rust-lang#65252 (expand: Simplify expansion of derives) - rust-lang#65263 (Deduplicate is_{freeze,copy,sized}_raw) - rust-lang#65265 (Cleanup librustc mir err codes) - rust-lang#65266 (Mark Path::join as must_use) - rust-lang#65276 (Don't cc rust-lang/compiler for toolstate changes) - rust-lang#65277 (Query generator kind for error reporting) - rust-lang#65283 (stability: Do not use `buffer_lint` after lowering to HIR) - rust-lang#65289 (Fix suggested bound addition diagnostic) Failed merges: r? @ghost

@matthewjasper

…hewjasper expand: Simplify expansion of derives And make it more uniform with other macros. This is done by merging placeholders for future derives' outputs into the derive container's output fragment early (addressing FIXMEs from rust-lang#63667). Also, macros with names starting with `_` are no longer reported as unused, in accordance with the usual behavior of `unused` lints. r? @matthewjasper or @mark-i-m

@matthewjasper

…hewjasper expand: Simplify expansion of derives And make it more uniform with other macros. This is done by merging placeholders for future derives' outputs into the derive container's output fragment early (addressing FIXMEs from rust-lang#63667). Also, macros with names starting with `_` are no longer reported as unused, in accordance with the usual behavior of `unused` lints. r? @matthewjasper or @mark-i-m

bors · 2019-10-14T15:49:33Z

⌛ Testing commit 1270140 with merge 9f86b1832c885b9d61a34d977eb8ea2fc499faf9...

rust-highfive · 2019-10-14T15:52:36Z

Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

bors · 2019-10-14T15:52:46Z

💔 Test failed - checks-azure

pietroalbini · 2019-10-14T16:05:57Z

@bors retry azure network error

Centril · 2019-10-14T16:38:52Z

@bors rollup=never (possible cause of failure in two rollups)

petrochenkov · 2019-10-15T17:29:06Z

rustdoc doesn't like macro items inside functions, which is weird because it was supposed to be fixed by #63624, I'll check further.

petrochenkov · 2019-10-15T21:12:49Z

Removed one more unwrap from the function that #63624 previously fixed.
@bors r=matthewjasper

bors · 2019-10-15T21:12:50Z

📌 Commit 487e3c1ca90d0bdc73ff479b4fda24d888b3cf54 has been approved by matthewjasper

bors · 2019-10-17T22:40:52Z

☔ The latest upstream changes (presumably #65495) made this pull request unmergeable. Please resolve the merge conflicts.

And make it more uniform with other macros. By merging placeholders for future derives' outputs into the derive container's output fragment early.

The issue is rustdoc-specific because its root cause if the `everybody_loops` pass makes some def-ids to not have local hir-ids

petrochenkov · 2019-10-18T21:55:08Z

@bors r=matthewjasper

bors · 2019-10-18T21:55:09Z

📌 Commit 7f89f04 has been approved by matthewjasper

petrochenkov · 2019-10-18T21:55:47Z

@bors rollup=maybe

@matthewjasper

…hewjasper expand: Simplify expansion of derives And make it more uniform with other macros. This is done by merging placeholders for future derives' outputs into the derive container's output fragment early (addressing FIXMEs from rust-lang#63667). Also, macros with names starting with `_` are no longer reported as unused, in accordance with the usual behavior of `unused` lints. r? @matthewjasper or @mark-i-m

@ghost

Rollup of 6 pull requests Successful merges: - #65174 (Fix zero-size uninitialized boxes) - #65252 (expand: Simplify expansion of derives) - #65485 (Suppress ICE when validators disagree on `LiveDrop`s in presence of `&mut`) - #65542 (Refer to "associated functions" instead of "static methods") - #65545 (More symbol cleanups) - #65576 (Don't add `argc` and `argv` arguments to `main` on WASI.) Failed merges: r? @ghost

expand/resolve: Turn `#[derive]` into a regular macro attribute This PR turns `#[derive]` into a regular attribute macro declared in libcore and defined in `rustc_builtin_macros`, like it was previously done with other "active" attributes in rust-lang#62086, rust-lang#62735 and other PRs. This PR is also a continuation of rust-lang#65252, rust-lang#69870 and other PRs linked from them, which layed the ground for converting `#[derive]` specifically. `#[derive]` still asks `rustc_resolve` to resolve paths inside `derive(...)`, and `rustc_expand` gets those resolution results through some backdoor (which I'll try to address later), but otherwise `#[derive]` is treated as any other macro attributes, which simplifies the resolution-expansion infra pretty significantly. The change has several observable effects on language and library. Some of the language changes are **feature-gated** by [`feature(macro_attributes_in_derive_output)`](rust-lang#81119). #### Library - `derive` is now available through standard library as `{core,std}::prelude::v1::derive`. #### Language - `derive` now goes through name resolution, so it can now be renamed - `use derive as my_derive; #[my_derive(Debug)] struct S;`. - `derive` now goes through name resolution, so this resolution can fail in corner cases. Crater found one such regression, where import `use foo as derive` goes into a cycle with `#[derive(Something)]`. - **[feature-gated]** `#[derive]` is now expanded as any other attributes in left-to-right order. This allows to remove the restriction on other macro attributes following `#[derive]` (rust-lang/reference#566). The following macro attributes become a part of the derive's input (this is not a change, non-macro attributes following `#[derive]` were treated in the same way previously). - `#[derive]` is now expanded as any other attributes in left-to-right order. This means two derive attributes `#[derive(Foo)] #[derive(Bar)]` are now expanded separately rather than together. It doesn't generally make difference, except for esoteric cases. For example `#[derive(Foo)]` can now produce an import bringing `Bar` into scope, but previously both `Foo` and `Bar` were required to be resolved before expanding any of them. - **[feature-gated]** `#[derive()]` (with empty list in parentheses) actually becomes useful. For historical reasons `#[derive]` *fully configures* its input, eagerly evaluating `cfg` everywhere in its target, for example on fields. Expansion infra doesn't do that for other attributes, but now when macro attributes attributes are allowed to be written after `#[derive]`, it means that derive can *fully configure* items for them. ```rust #[derive()] #[my_attr] struct S { #[cfg(FALSE)] // this field in removed by `#[derive()]` and not observed by `#[my_attr]` field: u8 } ``` - `#[derive]` on some non-item targets is now prohibited. This was accidentally allowed as noop in the past, but was warned about since early 2018 (rust-lang#50092), despite that crater found a few such cases in unmaintained crates. - Derive helper attributes used before their introduction are now reported with a deprecation lint. This change is long overdue (since macro modularization, rust-lang#52226 (comment)), but it was hard to do without fixing expansion order for derives. The deprecation is tracked by rust-lang#79202. ```rust #[trait_helper] // warning: derive helper attribute is used before it is introduced #[derive(Trait)] struct S {} ``` Crater analysis: rust-lang#79078 (comment)

rust-highfive assigned matthewjasper Oct 9, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2019

mark-i-m reviewed Oct 10, 2019

View reviewed changes

petrochenkov force-pushed the deriveholders2 branch from ffaa87e to 1270140 Compare October 10, 2019 18:40

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 Oct 10, 2019

tmandry mentioned this pull request Oct 11, 2019

Rollup of 16 pull requests #65297

Closed

Centril mentioned this pull request Oct 13, 2019

Rollup of 12 pull requests #65382

Closed

Centril mentioned this pull request Oct 14, 2019

Rollup of 4 pull requests #65406

Closed

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 14, 2019

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 Oct 14, 2019

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2019

petrochenkov force-pushed the deriveholders2 branch from 6ae4143 to 487e3c1 Compare October 15, 2019 21:10

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2019

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 17, 2019

petrochenkov added 4 commits October 19, 2019 00:23

resolve: Mark macros starting with an underscore as used

2d3c17a

expand: Simplify expansion of derives

7ce85f2

And make it more uniform with other macros. By merging placeholders for future derives' outputs into the derive container's output fragment early.

privacy: Avoid one more unwrap causing an ICE in rustdoc

25cc99f

The issue is rustdoc-specific because its root cause if the `everybody_loops` pass makes some def-ids to not have local hir-ids

Fix rebase

7f89f04

petrochenkov force-pushed the deriveholders2 branch from 487e3c1 to 7f89f04 Compare October 18, 2019 21:54

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 18, 2019

Centril mentioned this pull request Oct 19, 2019

Rollup of 6 pull requests #65588

Merged

bors merged commit 7f89f04 into rust-lang:master Oct 19, 2019

petrochenkov mentioned this pull request Nov 15, 2020

expand/resolve: Turn #[derive] into a regular macro attribute #79078

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expand: Simplify expansion of derives #65252

expand: Simplify expansion of derives #65252

petrochenkov commented Oct 9, 2019

petrochenkov commented Oct 9, 2019

mark-i-m left a comment

mark-i-m Oct 10, 2019

petrochenkov Oct 10, 2019

matthewjasper commented Oct 10, 2019

bors commented Oct 10, 2019

bors commented Oct 14, 2019

rust-highfive commented Oct 14, 2019

bors commented Oct 14, 2019

pietroalbini commented Oct 14, 2019

Centril commented Oct 14, 2019

petrochenkov commented Oct 15, 2019

petrochenkov commented Oct 15, 2019

bors commented Oct 15, 2019

bors commented Oct 17, 2019

petrochenkov commented Oct 18, 2019

bors commented Oct 18, 2019

petrochenkov commented Oct 18, 2019

expand: Simplify expansion of derives #65252

expand: Simplify expansion of derives #65252

Conversation

petrochenkov commented Oct 9, 2019

petrochenkov commented Oct 9, 2019

mark-i-m left a comment

Choose a reason for hiding this comment

mark-i-m Oct 10, 2019

Choose a reason for hiding this comment

petrochenkov Oct 10, 2019

Choose a reason for hiding this comment

matthewjasper commented Oct 10, 2019

bors commented Oct 10, 2019

bors commented Oct 14, 2019

rust-highfive commented Oct 14, 2019

bors commented Oct 14, 2019

pietroalbini commented Oct 14, 2019

Centril commented Oct 14, 2019

petrochenkov commented Oct 15, 2019

petrochenkov commented Oct 15, 2019

bors commented Oct 15, 2019

bors commented Oct 17, 2019

petrochenkov commented Oct 18, 2019

bors commented Oct 18, 2019

petrochenkov commented Oct 18, 2019