Opaque builtin derive macros #63462

matthewjasper · 2019-08-11T13:44:50Z

Buiilt-in derives are now opaque macros
- This required limiting the visibility of some previously unexposed functions in core.
- This also required the change to Ident serialization.
All gensyms are replaced with hygienic identifiers
Use hygiene to avoid most other name-resolution issues with buiilt-in derives.
- As far as I know the only remaining case that breaks is an ADT that has the same name as one of its parameters. Fixing this completely seemed to be more effort than it's worth.
Remove gensym in Ident::decode, which lead to linker errors due to inline being gensymmed.
- Identnow panics if incremental compilation tries to serialize it (it currently doesn't).
- Ident no longer uses gensym to emulate cross-crate hygiene. It only applied to reexports.
- SyntaxContext is no longer serializable.
- The long-term fix for this is to properly implement cross-crate hygiene, but this seemed to be acceptable for now.
Move type/const parameter shadowing checks to resolve
- This was previously split between resolve and type checking. The type checking pass compared InternedStrings, not Identifiers.
Removed the SyntaxContext from {ast, hir}::{InlineAsm, GlobalAsm}

petrochenkov · 2019-08-11T14:59:21Z

bors · 2019-08-11T14:59:22Z

📌 Commit 9dbfc67696a3832e19458700ee503b6103df3450 has been approved by petrochenkov

rust-highfive · 2019-08-11T15:26:48Z

The job x86_64-gnu-llvm-6.0 of your PR failed (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-08-11T14:13:25.3162210Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-08-11T14:13:25.3341033Z ##[command]git config gc.auto 0
2019-08-11T14:13:25.3419024Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-08-11T14:13:25.3483648Z ##[command]git config --get-all http.proxy
2019-08-11T14:13:25.3623251Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63462/merge:refs/remotes/pull/63462/merge
---
2019-08-11T14:13:59.9067129Z do so (now or later) by using -b with the checkout command again. Example:
2019-08-11T14:13:59.9067170Z 
2019-08-11T14:13:59.9067694Z   git checkout -b <new-branch-name>
2019-08-11T14:13:59.9067725Z 
2019-08-11T14:13:59.9067775Z HEAD is now at 036dc5877 Merge 9dbfc67696a3832e19458700ee503b6103df3450 into 2b78e10ac1454d2d4190c575f6ece03f484ac398
2019-08-11T14:13:59.9221795Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-08-11T14:13:59.9224107Z ==============================================================================
2019-08-11T14:13:59.9224173Z Task         : Bash
2019-08-11T14:13:59.9224210Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-08-11T15:15:30.9229144Z .................................................................................................... 1300/8873
2019-08-11T15:15:37.6661774Z .................................................................................................... 1400/8873
2019-08-11T15:15:43.9765109Z .................................................................................................... 1500/8873
2019-08-11T15:15:54.7275220Z ....................................................................................i............... 1600/8873
2019-08-11T15:16:02.5400008Z i................................................................................................... 1700/8873
2019-08-11T15:16:09.0692375Z ................F...........................................................iiiii................... 1800/8873
2019-08-11T15:16:31.4679062Z .................................................................................................... 2000/8873
2019-08-11T15:16:33.9691223Z .................................................................................................... 2100/8873
2019-08-11T15:16:36.7184180Z .................................................................................................... 2200/8873
2019-08-11T15:16:44.5691425Z .................................................................................................... 2300/8873
---
2019-08-11T15:20:42.1272416Z .................................................................................................... 5300/8873
2019-08-11T15:20:49.4393879Z ........i........................................................................................... 5400/8873
2019-08-11T15:20:54.9181069Z .................................................................................................... 5500/8873
2019-08-11T15:21:07.3578276Z .................................................................................................... 5600/8873
2019-08-11T15:21:21.7115566Z ...ii...i..ii...........i........................................................................... 5700/8873
2019-08-11T15:21:36.9436608Z .................................................................................................... 5900/8873
2019-08-11T15:21:41.6597020Z .................................................................................................... 6000/8873
2019-08-11T15:21:41.6597020Z .................................................................................................... 6000/8873
2019-08-11T15:21:56.1110945Z ....i..ii........................................................................................... 6100/8873
2019-08-11T15:22:15.1078460Z ...............................................i.................................................... 6300/8873
2019-08-11T15:22:17.2010417Z .................................................................................................... 6400/8873
2019-08-11T15:22:19.6678702Z ...................i................................................................................ 6500/8873
2019-08-11T15:22:24.2111184Z .................................................................................................... 6600/8873
---
2019-08-11T15:26:24.9887319Z failures:
2019-08-11T15:26:24.9914907Z 
2019-08-11T15:26:24.9915344Z ---- [ui] ui/deriving/deriving-hash.rs stdout ----
2019-08-11T15:26:24.9915586Z 
2019-08-11T15:26:24.9915839Z error: test compilation failed although it shouldn't!
2019-08-11T15:26:24.9915884Z status: exit code: 1
2019-08-11T15:26:24.9916537Z command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/deriving/deriving-hash.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/deriving/deriving-hash/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/deriving/deriving-hash/auxiliary" "-A" "unused"
2019-08-11T15:26:24.9916835Z ------------------------------------------
2019-08-11T15:26:24.9916866Z 
2019-08-11T15:26:24.9917054Z ------------------------------------------
2019-08-11T15:26:24.9917111Z stderr:
2019-08-11T15:26:24.9917111Z stderr:
2019-08-11T15:26:24.9917306Z ------------------------------------------
2019-08-11T15:26:24.9917354Z error[E0194]: type parameter `__H` shadows another type parameter of the same name
2019-08-11T15:26:24.9917641Z    |
2019-08-11T15:26:24.9917641Z    |
2019-08-11T15:26:24.9917683Z LL | #[derive(Hash)] enum Collision<__H> { __H { __H__H: __H } }
2019-08-11T15:26:24.9917955Z    |          |
2019-08-11T15:26:24.9917995Z    |          shadows another type parameter
2019-08-11T15:26:24.9917995Z    |          shadows another type parameter
2019-08-11T15:26:24.9918036Z    |          first `__H` declared here
2019-08-11T15:26:24.9918510Z error: aborting due to previous error
2019-08-11T15:26:24.9918540Z 
2019-08-11T15:26:24.9918835Z For more information about this error, try `rustc --explain E0194`.
2019-08-11T15:26:24.9918890Z 
---
2019-08-11T15:26:24.9950644Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:536:22
2019-08-11T15:26:24.9950728Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
2019-08-11T15:26:24.9974129Z 
2019-08-11T15:26:24.9974206Z 
2019-08-11T15:26:24.9977302Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2019-08-11T15:26:24.9977600Z 
2019-08-11T15:26:24.9977630Z 
2019-08-11T15:26:24.9981143Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2019-08-11T15:26:24.9981237Z Build completed unsuccessfully in 1:06:05
2019-08-11T15:26:24.9981237Z Build completed unsuccessfully in 1:06:05
2019-08-11T15:26:25.8104202Z ##[error]Bash exited with code '1'.
2019-08-11T15:26:25.8147538Z ##[section]Starting: Checkout
2019-08-11T15:26:25.8181223Z ==============================================================================
2019-08-11T15:26:25.8181303Z Task         : Get sources
2019-08-11T15:26:25.8181355Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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)

Centril · 2019-08-11T15:31:51Z

Failed in PR builder; @bors r-

petrochenkov · 2019-08-15T12:50:33Z

@bors r+

bors · 2019-08-15T12:50:35Z

📌 Commit 01587b1 has been approved by petrochenkov

@petrochenkov

…ves, r=petrochenkov Opaque builtin derive macros * Buiilt-in derives are now opaque macros * This required limiting the visibility of some previously unexposed functions in `core`. * This also required the change to `Ident` serialization. * All gensyms are replaced with hygienic identifiers * Use hygiene to avoid most other name-resolution issues with buiilt-in derives. * As far as I know the only remaining case that breaks is an ADT that has the same name as one of its parameters. Fixing this completely seemed to be more effort than it's worth. * Remove gensym in `Ident::decode`, which lead to linker errors due to `inline` being gensymmed. * `Ident`now panics if incremental compilation tries to serialize it (it currently doesn't). * `Ident` no longer uses `gensym` to emulate cross-crate hygiene. It only applied to reexports. * `SyntaxContext` is no longer serializable. * The long-term fix for this is to properly implement cross-crate hygiene, but this seemed to be acceptable for now. * Move type/const parameter shadowing checks to `resolve` * This was previously split between resolve and type checking. The type checking pass compared `InternedString`s, not Identifiers. * Removed the `SyntaxContext` from `{ast, hir}::{InlineAsm, GlobalAsm}` cc rust-lang#60869 r? @petrochenkov

@petrochenkov

…ves, r=petrochenkov Opaque builtin derive macros * Buiilt-in derives are now opaque macros * This required limiting the visibility of some previously unexposed functions in `core`. * This also required the change to `Ident` serialization. * All gensyms are replaced with hygienic identifiers * Use hygiene to avoid most other name-resolution issues with buiilt-in derives. * As far as I know the only remaining case that breaks is an ADT that has the same name as one of its parameters. Fixing this completely seemed to be more effort than it's worth. * Remove gensym in `Ident::decode`, which lead to linker errors due to `inline` being gensymmed. * `Ident`now panics if incremental compilation tries to serialize it (it currently doesn't). * `Ident` no longer uses `gensym` to emulate cross-crate hygiene. It only applied to reexports. * `SyntaxContext` is no longer serializable. * The long-term fix for this is to properly implement cross-crate hygiene, but this seemed to be acceptable for now. * Move type/const parameter shadowing checks to `resolve` * This was previously split between resolve and type checking. The type checking pass compared `InternedString`s, not Identifiers. * Removed the `SyntaxContext` from `{ast, hir}::{InlineAsm, GlobalAsm}` cc rust-lang#60869 r? @petrochenkov

Mark-Simulacrum · 2019-08-15T20:01:48Z

@bors r- PR build failed (and multiple rollups)

bors · 2019-08-16T10:43:19Z

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

matthewjasper · 2019-08-16T18:57:26Z

@bors rollup=never p=1

Most `Ident`s are serialized as `InternedString`s the exceptions are: * Reexports * Attributes * Idents in macro definitions Using gensyms helped reexports emulate hygiene. However, the actual item wouldn't have a gensymmed name so would be usable cross-crate. So removing this case until we have proper cross-crate hygiene seems sensible. Codegen attributes (`inline`, `export_name`) are resolved by their `Symbol`. This meant that opaque macro-expanded codegen attributes could cause linker errors. This prevented making built-in derives hygienic.

Also make them generally more hygienic with name resolution.

For some reason type checking did this. Further it didn't consider hygiene.

We now store it in the `Span` of the expression or item.

The implementations were wrong and unused.

matthewjasper · 2019-08-17T10:32:52Z

@bors r=petrochenkov

bors · 2019-08-17T10:32:53Z

📌 Commit 1c0a546 has been approved by petrochenkov

bors · 2019-08-17T12:54:01Z

⌛ Testing commit 1c0a546 with merge d65e272...

@petrochenkov

…ochenkov Opaque builtin derive macros * Buiilt-in derives are now opaque macros * This required limiting the visibility of some previously unexposed functions in `core`. * This also required the change to `Ident` serialization. * All gensyms are replaced with hygienic identifiers * Use hygiene to avoid most other name-resolution issues with buiilt-in derives. * As far as I know the only remaining case that breaks is an ADT that has the same name as one of its parameters. Fixing this completely seemed to be more effort than it's worth. * Remove gensym in `Ident::decode`, which lead to linker errors due to `inline` being gensymmed. * `Ident`now panics if incremental compilation tries to serialize it (it currently doesn't). * `Ident` no longer uses `gensym` to emulate cross-crate hygiene. It only applied to reexports. * `SyntaxContext` is no longer serializable. * The long-term fix for this is to properly implement cross-crate hygiene, but this seemed to be acceptable for now. * Move type/const parameter shadowing checks to `resolve` * This was previously split between resolve and type checking. The type checking pass compared `InternedString`s, not Identifiers. * Removed the `SyntaxContext` from `{ast, hir}::{InlineAsm, GlobalAsm}` cc #60869 r? @petrochenkov

bors · 2019-08-17T16:27:32Z

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing d65e272 to master...

rust-highfive assigned petrochenkov Aug 11, 2019

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

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the hygienic-builtin-derives branch from dea327c to 9dbfc67 Compare August 11, 2019 14:12

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 Aug 11, 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 Aug 11, 2019

matthewjasper force-pushed the hygienic-builtin-derives branch from 9dbfc67 to 1c8d77c Compare August 11, 2019 17:10

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the hygienic-builtin-derives branch from 1c8d77c to 2e8fae4 Compare August 11, 2019 17:58

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the hygienic-builtin-derives branch 2 times, most recently from e748673 to 6f00e1e Compare August 12, 2019 21:13

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 13, 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 Aug 15, 2019

Centril mentioned this pull request Aug 15, 2019

Rollup of 5 pull requests #63602

Closed

Centril mentioned this pull request Aug 15, 2019

Rollup of 8 pull requests #63603

Closed

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 Aug 15, 2019

matthewjasper force-pushed the hygienic-builtin-derives branch from 01587b1 to 39db90f Compare August 15, 2019 19:23

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the hygienic-builtin-derives branch from 39db90f to 4749e7e Compare August 15, 2019 20:04

matthewjasper force-pushed the hygienic-builtin-derives branch from 4749e7e to b375f58 Compare August 16, 2019 18:56

matthewjasper force-pushed the hygienic-builtin-derives branch from b375f58 to a175920 Compare August 16, 2019 20:30

This comment has been minimized.

Sign in to view

matthewjasper added 7 commits August 17, 2019 08:59

Make built-in derives opaque macros

6ee60af

Make fmt-internal functions private

0280e8f

Remove gensyms from built-in derives

3296d0e

Also make them generally more hygienic with name resolution.

Move type parameter shadowing errors to resolve

f70c90c

For some reason type checking did this. Further it didn't consider hygiene.

Remove SyntaxContext from {ast, hir}::{GlobalAsm, InlineAsm}

d04af19

We now store it in the `Span` of the expression or item.

Remove unused SyntaxContext serialization impls

1c0a546

The implementations were wrong and unused.

matthewjasper force-pushed the hygienic-builtin-derives branch from a175920 to 1c0a546 Compare August 17, 2019 08:14

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 Aug 17, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 17, 2019

bors merged commit 1c0a546 into rust-lang:master Aug 17, 2019

matthewjasper deleted the hygienic-builtin-derives branch August 17, 2019 16:28

petrochenkov mentioned this pull request Aug 24, 2019

Crater runs for Rust 1.38.0 #63628

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opaque builtin derive macros #63462

Opaque builtin derive macros #63462

matthewjasper commented Aug 11, 2019 •

edited

This comment has been minimized.

petrochenkov commented Aug 11, 2019

bors commented Aug 11, 2019

rust-highfive commented Aug 11, 2019

Centril commented Aug 11, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Aug 15, 2019

bors commented Aug 15, 2019

This comment has been minimized.

Mark-Simulacrum commented Aug 15, 2019

bors commented Aug 16, 2019

matthewjasper commented Aug 16, 2019

This comment has been minimized.

matthewjasper commented Aug 17, 2019

bors commented Aug 17, 2019

bors commented Aug 17, 2019

bors commented Aug 17, 2019

Opaque builtin derive macros #63462

Opaque builtin derive macros #63462

Conversation

matthewjasper commented Aug 11, 2019 • edited

This comment has been minimized.

petrochenkov commented Aug 11, 2019

bors commented Aug 11, 2019

rust-highfive commented Aug 11, 2019

Centril commented Aug 11, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Aug 15, 2019

bors commented Aug 15, 2019

This comment has been minimized.

Mark-Simulacrum commented Aug 15, 2019

bors commented Aug 16, 2019

matthewjasper commented Aug 16, 2019

This comment has been minimized.

matthewjasper commented Aug 17, 2019

bors commented Aug 17, 2019

bors commented Aug 17, 2019

bors commented Aug 17, 2019

matthewjasper commented Aug 11, 2019 •

edited