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

expand: Simplify expansion of derives #65252

Merged
merged 4 commits into from Oct 19, 2019

Conversation

@petrochenkov
Copy link
Contributor

petrochenkov commented Oct 9, 2019

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

This comment has been minimized.

Copy link
Contributor Author

petrochenkov commented Oct 9, 2019

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.
Copy link
Member

mark-i-m left a comment

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 Outdated Show resolved Hide resolved
for placeholder in extra_placeholders {
def_collector.visit_macro_invoc(*placeholder);
}

let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope };
fragment.visit_with(&mut visitor);

This comment has been minimized.

Copy link
@mark-i-m

mark-i-m Oct 10, 2019

Member

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

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Oct 10, 2019

Author Contributor

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

@petrochenkov petrochenkov force-pushed the petrochenkov:deriveholders2 branch from ffaa87e to 1270140 Oct 10, 2019
@matthewjasper

This comment has been minimized.

Copy link
Contributor

matthewjasper commented Oct 10, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 10, 2019

📌 Commit 1270140 has been approved by matthewjasper

tmandry added a commit to tmandry/rust that referenced this pull request Oct 11, 2019
…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 added a commit that referenced this pull request Oct 11, 2019
Rollup of 16 pull requests

Successful merges:

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

Failed merges:

r? @ghost
tmandry added a commit to tmandry/rust that referenced this pull request Oct 11, 2019
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
Centril added a commit to Centril/rust that referenced this pull request Oct 11, 2019
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
Centril added a commit to Centril/rust that referenced this pull request Oct 13, 2019
…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 added a commit that referenced this pull request Oct 13, 2019
Rollup of 12 pull requests

Successful merges:

 - #65214 (Split non-CAS atomic support off into target_has_atomic_load_store)
 - #65246 (vxWorks: implement get_path() and get_mode() for File fmt::Debug)
 - #65252 (expand: Simplify expansion of derives)
 - #65312 (improve performance of signed saturating_mul)
 - #65336 (Fix typo in task::Waker)
 - #65346 (nounwind tests and cleanup)
 - #65347 (Fix #[unwind(abort)] with Rust ABI)
 - #65362 (syntax: consolidate function parsing in item.rs)
 - #65366 (Implement Error::source on IntoStringError + Remove superfluous cause impls)
 - #65369 (Don't discard value names when using address or memory sanitizer)
 - #65370 (Add `dyn` to `Any` documentation)
 - #65373 (Fix typo in docs for `Rc`)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Oct 14, 2019
…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 added a commit that referenced this pull request Oct 14, 2019
Rollup of 4 pull requests

Successful merges:

 - #64987 (Compute the layout of uninhabited structs)
 - #65252 (expand: Simplify expansion of derives)
 - #65260 (Optimize `LexicalResolve::expansion`.)
 - #65261 (Remove `Option` from `TokenStream`)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Oct 14, 2019
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 #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

This comment has been minimized.

Copy link
Contributor

bors commented Oct 14, 2019

⌛️ Testing commit 1270140 with merge 9f86b18...

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 14, 2019

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

This comment has been minimized.

Copy link
Contributor

bors commented Oct 14, 2019

💔 Test failed - checks-azure

@pietroalbini

This comment has been minimized.

Copy link
Member

pietroalbini commented Oct 14, 2019

@bors retry azure network error

@Centril

This comment has been minimized.

Copy link
Member

Centril commented Oct 14, 2019

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

bors added a commit that referenced this pull request Oct 15, 2019
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 #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

This comment has been minimized.

Copy link
Contributor

bors commented Oct 15, 2019

💔 Test failed - checks-azure

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 15, 2019

The job dist-x86_64-linux of 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.
2019-10-15T06:36:38.9477629Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
2019-10-15T06:36:39.0512108Z error: Could not document `syntax`.
2019-10-15T06:36:39.0517653Z 
2019-10-15T06:36:39.0517779Z Caused by:
2019-10-15T06:36:39.0530499Z   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustdoc --edition=2018 --crate-type lib --crate-name syntax src/libsyntax/lib.rs --target x86_64-unknown-linux-gnu -o /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/doc --color always -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/release/deps --extern bitflags=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/libbitflags-28fa941764cd0b5d.rmeta --extern lazy_static=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/liblazy_static-25e195a008dc650e.rmeta --extern log=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/liblog-6a39080e603a7540.rmeta --extern rustc_data_structures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_data_structures-8fe38918b9a2a938.rmeta --extern errors=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_errors-f5bdf664f2735d32.rmeta --extern rustc_index=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_index-da81bc8f818c54ab.rmeta --extern rustc_lexer=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_lexer-96f153401da80175.rmeta --extern rustc_target=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_target-e6bb297dc731315e.rmeta --extern scoped_tls=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/libscoped_tls-7c3a2b01c74539dd.rmeta --extern rustc_serialize=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-95bcadfb6805bd96.rmeta --extern smallvec=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/libsmallvec-b7c252bfcc11690e.rmeta --extern syntax_pos=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax_pos-b59e470af0b21954.rmeta --document-private-items --passes strip-hidden` (exit code: 1)
2019-10-15T06:36:45.1756638Z [RUSTC-TIMING] syntax test:false 10.807
2019-10-15T06:36:45.1892085Z error: build failed
2019-10-15T06:36:45.1914488Z 
2019-10-15T06:36:45.1915587Z 
2019-10-15T06:36:45.1915587Z 
2019-10-15T06:36:45.1918195Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "doc" "-Zconfig-profile" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--locked" "--color" "always" "--features" "jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--no-deps" "-p" "rustc" "-p" "syntax" "-p" "syntax_pos" "-p" "rustc_save_analysis" "-p" "rustc_plugin" "-p" "rustc_lint" "-p" "graphviz" "-p" "rustc_codegen_utils" "-p" "build_helper" "-p" "rustc_llvm" "-p" "rustc_metadata" "-p" "rustc_index" "-p" "serialize" "-p" "arena" "-p" "rustc_macros" "-p" "fmt_macros" "-p" "rustc_plugin_impl" "-p" "rustc_passes" "-p" "rustc_apfloat" "-p" "rustc_privacy" "-p" "rustc_mir" "-p" "rustc_lexer" "-p" "rustc_errors" "-p" "rustc_incremental" "-p" "syntax_ext" "-p" "rustc_resolve" "-p" "rustc_codegen_llvm" "-p" "rustc_target" "-p" "rustc_typeck" "-p" "rustc_interface" "-p" "rustc_data_structures" "-p" "rustc_codegen_ssa" "-p" "rustc_fs_util" "-p" "rustc_driver" "-p" "rustc_traits"
2019-10-15T06:36:45.1918820Z 
2019-10-15T06:36:45.1918865Z 
2019-10-15T06:36:45.1933358Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
2019-10-15T06:36:45.1933496Z Build completed unsuccessfully in 1:31:35
2019-10-15T06:36:45.1933496Z Build completed unsuccessfully in 1:31:35
2019-10-15T06:36:45.1999935Z == clock drift check ==
2019-10-15T06:36:45.2019773Z   local time: Tue Oct 15 06:36:45 UTC 2019
2019-10-15T06:36:45.4473188Z   network time: Tue, 15 Oct 2019 06:36:45 GMT
2019-10-15T06:36:45.4473545Z == end clock drift check ==
2019-10-15T06:36:46.8921931Z ##[error]Bash exited with code '1'.
2019-10-15T06:36:46.8959790Z ##[section]Starting: Upload CPU usage statistics
2019-10-15T06:36:46.8968136Z ==============================================================================
2019-10-15T06:36:46.8968255Z Task         : Bash
2019-10-15T06:36:46.8968347Z Description  : Run a Bash script on macOS, Linux, or Windows

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)

@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

petrochenkov commented Oct 15, 2019

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 petrochenkov force-pushed the petrochenkov:deriveholders2 branch from 6ae4143 to 487e3c1 Oct 15, 2019
@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

petrochenkov commented Oct 15, 2019

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

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 15, 2019

📌 Commit 487e3c1 has been approved by matthewjasper

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 17, 2019

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

petrochenkov added 4 commits Oct 9, 2019
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 petrochenkov force-pushed the petrochenkov:deriveholders2 branch from 487e3c1 to 7f89f04 Oct 18, 2019
@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

petrochenkov commented Oct 18, 2019

@bors r=matthewjasper

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 18, 2019

📌 Commit 7f89f04 has been approved by matthewjasper

@petrochenkov

This comment has been minimized.

Copy link
Contributor Author

petrochenkov commented Oct 18, 2019

@bors rollup=maybe

Centril added a commit to Centril/rust that referenced this pull request Oct 19, 2019
…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 added a commit that referenced this pull request Oct 19, 2019
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
@bors bors merged commit 7f89f04 into rust-lang:master Oct 19, 2019
4 checks passed
4 checks passed
pr Build #20191018.68 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.