Remove `macro_defs` map #72284

Aaron1011 · 2020-05-16T22:16:46Z

We now store the DefId directly in ExpnKind::Macro. This will allow
us to serialize ExpnData in PR #72121 without needing to manage a side
table.

rust-highfive · 2020-05-16T22:16:49Z

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

Aaron1011 · 2020-05-16T22:17:23Z

r? @petrochenkov

Aaron1011 · 2020-05-16T22:17:41Z

Let's see if this causes any of the performance issues in #72121:

@bors try @rust-timer queue

rust-timer · 2020-05-16T22:18:00Z

Awaiting bors try build completion

bors · 2020-05-16T22:18:10Z

⌛ Trying commit 4b808ee4c1f4d16a72114889038c4ea2ebb613ee with merge d97eb6fdac44fb15b9a3996f1de01e8265e85007...

Aaron1011 · 2020-05-17T01:18:20Z

@bors try @rust-timer queue

rust-timer · 2020-05-17T01:18:22Z

Awaiting bors try build completion

bors · 2020-05-17T01:18:35Z

⌛ Trying commit 89e00da2ff53c32ea1753737f8556f8b77ef5d10 with merge 244e1f853fa4115575ec8a4b64f257ae7cbcaed3...

rust-highfive · 2020-05-17T02:21:52Z

The job x86_64-gnu-llvm-8 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.

##[section]Starting: Linux x86_64-gnu-llvm-8
##[section]Starting: Initialize job
Agent name: 'Azure Pipelines 4'
Agent machine name: 'fv-az578'
Current agent version: '2.168.2'
##[group]Operating System
16.04.6
LTS
LTS
##[endgroup]
##[group]Virtual Environment
Environment: ubuntu-16.04
Version: 20200430.2
Included Software: https://github.com/actions/virtual-environments/blob/ubuntu16/20200430.2/images/linux/Ubuntu1604-README.md
##[endgroup]
Agent running as: 'vsts'
Prepare build directory.
Set build variables.
Download all required tasks.
Download all required tasks.
Downloading task: Bash (3.163.2)
Checking job knob settings.
   Knob: AgentToolsDirectory = /opt/hostedtoolcache Source: ${AGENT_TOOLSDIRECTORY} 
   Knob: AgentPerflog = /home/vsts/perflog Source: ${VSTS_AGENT_PERFLOG} 
Start tracking orphan processes.
##[section]Finishing: Initialize job
##[section]Starting: Configure Job Name
==============================================================================
---
========================== Starting Command Output ===========================
[command]/bin/bash --noprofile --norc /home/vsts/work/_temp/54f81b26-ae68-4aab-b291-ed6a8731bfad.sh

##[section]Finishing: Disable git automatic line ending conversion
##[section]Starting: Checkout rust-lang/rust@refs/pull/72284/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
---
##[command]git remote add origin https://github.com/rust-lang/rust
##[command]git config gc.auto 0
##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
##[command]git config --get-all http.proxy
##[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/72284/merge:refs/remotes/pull/72284/merge
---
 ---> cb2676f08729
Step 5/8 : ENV RUST_CONFIGURE_ARGS       --build=x86_64-unknown-linux-gnu       --llvm-root=/usr/lib/llvm-8       --enable-llvm-link-shared       --set rust.thin-lto-import-instr-limit=10
 ---> Using cache
 ---> df25ce111862
Step 6/8 : ENV SCRIPT python2.7 ../x.py test --exclude src/tools/tidy &&            python2.7 ../x.py test src/test/mir-opt --pass=build                                   --target=armv5te-unknown-linux-gnueabi &&            python2.7 ../x.py test src/tools/tidy
 ---> 599b9ac96b27
Step 7/8 : ENV NO_DEBUG_ASSERTIONS=1
 ---> Using cache
 ---> 091087e35a36
---
   Compiling fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
   Compiling chalk-rust-ir v0.10.0
   Compiling rustc_ast_pretty v0.0.0 (/checkout/src/librustc_ast_pretty)
   Compiling rustc_hir v0.0.0 (/checkout/src/librustc_hir)
   Compiling rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
   Compiling chalk-solve v0.10.0
   Compiling rustc_hir_pretty v0.0.0 (/checkout/src/librustc_hir_pretty)
   Compiling rustc_parse v0.0.0 (/checkout/src/librustc_parse)
   Compiling rustc_ast_lowering v0.0.0 (/checkout/src/librustc_ast_lowering)
---
   Compiling fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
   Compiling chalk-rust-ir v0.10.0
   Compiling rustc_ast_pretty v0.0.0 (/checkout/src/librustc_ast_pretty)
   Compiling rustc_hir v0.0.0 (/checkout/src/librustc_hir)
   Compiling rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
   Compiling chalk-solve v0.10.0
   Compiling rustc_hir_pretty v0.0.0 (/checkout/src/librustc_hir_pretty)
   Compiling rustc_parse v0.0.0 (/checkout/src/librustc_parse)
   Compiling rustc_ast_lowering v0.0.0 (/checkout/src/librustc_ast_lowering)
---
.......................................................i............................................ 1800/10169
.................................................................................................... 1900/10169
..........................................................................i..i...................... 2000/10169
.................................................................................................... 2100/10169
................................................................iiiii............................... 2200/10169
.................................................................................................... 2400/10169
.................................................................................................... 2500/10169
.................................................................................................... 2600/10169
.................................................................................................... 2700/10169
---
.................................................................................................... 5200/10169
.................................................................................................... 5300/10169
...........................i........................................................................ 5400/10169
....................i............................................................................... 5500/10169
............................ii.ii........i...i...................................................... 5600/10169
..............................................................................i..................... 5800/10169
.................................................................................................... 5900/10169
.........................ii.....................................i................................... 6000/10169
.................................................................................................... 6100/10169
.................................................................................................... 6100/10169
.................................................................................................... 6200/10169
......................................................................................ii...i..ii.... 6300/10169
.................................................................................................... 6500/10169
.................................................................................................... 6600/10169
.................................................................................................... 6700/10169
.................................................................................................... 6700/10169
...................i..ii............................................................................ 6800/10169
.................................................................................................... 7000/10169
.........................................................................i.......................... 7100/10169
.................................................................................................... 7200/10169
.................................................................................................... 7300/10169
---
.................................................................................................... 8100/10169
.................................................................................................... 8200/10169
...........................................................................................i........ 8300/10169
.................................................................................................... 8400/10169
.............................................iiiiii.iiiii.i......................................... 8500/10169
..................................................................................................i. 8600/10169
.................................................................................................... 8800/10169
.................................................................................................... 8900/10169
.................................................................................................... 9000/10169
.................................................................................................... 9100/10169
---
diff of stdout:

17 /*
18 Expansions:
19 0: parent: ExpnId(0), call_site_ctxt: #0, kind: Root
- 1: parent: ExpnId(0), call_site_ctxt: #0, kind: Macro(Bang, "foo")
+ 1: parent: ExpnId(0), call_site_ctxt: #0, kind: Macro { kind: Bang, name: "foo", macro_def_id: Some(DefId(0:1 ~ unpretty_debug[317d]::foo[0])) }
22 SyntaxContexts:
22 SyntaxContexts:
23 #0: parent: #0, outer_mark: (ExpnId(0), Opaque)

The actual stdout differed from the expected stdout.
Actual stdout saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/hygiene/unpretty-debug/unpretty-debug.stdout
Actual stdout saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/hygiene/unpretty-debug/unpretty-debug.stdout
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args hygiene/unpretty-debug.rs`
error: 1 errors occurred comparing output.
status: exit code: 0
status: exit code: 0
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/hygiene/unpretty-debug.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/hygiene/unpretty-debug" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Zunpretty=expanded,hygiene" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/hygiene/unpretty-debug/auxiliary"
------------------------------------------
// check-pass
// compile-flags: -Zunpretty=expanded,hygiene


// Don't break whenever Symbol numbering changes
// normalize-stdout-test "\d+#" -> "0#"
// minimal junk
// minimal junk
#![feature /* 274#0 */(no_core)]
#![no_core /* 440#0 */]

macro_rules! foo /* 806#0 */ { ($ x : ident) => { y + $ x } }

fn bar /* 809#0 */() { let x /* 807#0 */ = 1; y /* 808#1 */ + x /* 807#0 */ }

fn y /* 808#0 */() { }
/*
Expansions:
Expansions:
0: parent: ExpnId(0), call_site_ctxt: #0, kind: Root
1: parent: ExpnId(0), call_site_ctxt: #0, kind: Macro { kind: Bang, name: "foo", macro_def_id: Some(DefId(0:1 ~ unpretty_debug[317d]::foo[0])) }
SyntaxContexts:
SyntaxContexts:
#0: parent: #0, outer_mark: (ExpnId(0), Opaque)
#1: parent: #0, outer_mark: (ExpnId(1), SemiTransparent)

------------------------------------------
stderr:
------------------------------------------
---
thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:348:22
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


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-8/bin/FileCheck" "--nodejs" "/usr/bin/node" "--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" "8.0.0" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test --exclude src/tools/tidy
Build completed unsuccessfully in 0:59:33
Build completed unsuccessfully in 0:59:33
== clock drift check ==
  local time: Sun May 17 02:21:47 UTC 2020
  network time: Sun, 17 May 2020 02:21:47 GMT
== end clock drift check ==

##[error]Bash exited with code '1'.
##[section]Finishing: Run build
##[section]Starting: Checkout rust-lang/rust@refs/pull/72284/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
Author       : Microsoft
Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
==============================================================================
Cleaning any cached credential from repository: rust-lang/rust (GitHub)
##[section]Finishing: Checkout rust-lang/rust@refs/pull/72284/merge to s
Cleaning up task key
Start cleaning up orphan processes.
Terminate orphan process: pid (3365) (python)
##[section]Finishing: Finalize Job

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 @rust-lang/infra. (Feature Requests)

bors · 2020-05-17T04:00:38Z

☀️ Try build successful - checks-azure
Build commit: 244e1f853fa4115575ec8a4b64f257ae7cbcaed3 (244e1f853fa4115575ec8a4b64f257ae7cbcaed3)

rust-timer · 2020-05-17T04:00:39Z

Queued 244e1f853fa4115575ec8a4b64f257ae7cbcaed3 with parent dd927a5, future comparison URL.

rust-timer · 2020-05-17T07:00:05Z

Finished benchmarking try commit 244e1f853fa4115575ec8a4b64f257ae7cbcaed3, comparison URL.

Aaron1011 · 2020-05-17T07:33:13Z

The coercions-debug-incr-patched: println benchmark is giving a weird result. I suspect that it's spurious, since it's marked as high variance, and the slower queries (LLVM-codegen related) seem completely unrelated to this PR.

petrochenkov · 2020-05-17T10:09:51Z

Keeping something in global data is a last resort, struct ExpnData has a comment about it.
Sometimes you don't have access to any context (like in ident.edition() or in token pretty-printing), so you have to access global data through some locks.

Which place in #72121 doesn't have access to resolver to get the DefId from it?

Aaron1011 · 2020-05-17T19:28:53Z

@petrochenkov: I think it should be possible to access macro_defs from the new places that need it (hygiene serialization and deserialization).

I made this change to simplify #72121. Instead of needing to deserialize from a separate side table into the resolver when we deserialize an ExpnData, all of the data we need is present in ExpnData.

Are you worried that there could be a performance impact that's not revealed by the perf run?

marmeladema · 2020-05-22T08:38:04Z

Removing that map might help fix #71820 actually. Is there a version of the code that works that i could try out?

petrochenkov · 2020-05-22T20:30:23Z

@Aaron1011

Instead of needing to deserialize from a separate side table into the resolver when we deserialize an ExpnData, all of the data we need is present in ExpnData.

I see, so it needs to be (de)serialized for fn macro_def_scope to work correctly for foreign expansions.
I agree that it makes sense to put it into ExpnData then.

Could you place it directly into struct ExpnData perhaps to avoid churn?
It's optional anyway regardless of the expansion kind.

src/librustc_resolve/lib.rs

src/librustc_resolve/macros.rs

We store store the `DefId` directly in `ExpnData`. This will allow us to serialize `ExpnData` in PR rust-lang#72121 without needing to manage a side table.

Aaron1011 · 2020-05-22T20:59:41Z

@petrochenkov: Updated

petrochenkov · 2020-05-22T21:22:07Z

@bors try @rust-timer queue
(To check whether regression in #72284 (comment) was spurious.)

r=me otherwise

rust-timer · 2020-05-22T21:22:09Z

Awaiting bors try build completion

bors · 2020-05-22T21:22:18Z

⌛ Trying commit d277904 with merge 3cf94ad0dc89d67b40807889bdfcd0552afbc4db...

bors · 2020-05-23T00:12:05Z

☀️ Try build successful - checks-azure
Build commit: 3cf94ad0dc89d67b40807889bdfcd0552afbc4db (3cf94ad0dc89d67b40807889bdfcd0552afbc4db)

rust-timer · 2020-05-23T00:12:07Z

Queued 3cf94ad0dc89d67b40807889bdfcd0552afbc4db with parent 215f2d3, future comparison URL.

rust-timer · 2020-05-24T00:16:46Z

Finished benchmarking try commit 3cf94ad0dc89d67b40807889bdfcd0552afbc4db, comparison URL.

Aaron1011 · 2020-05-24T00:23:42Z

@petrochenkov: It looks like the regression was spurious

petrochenkov · 2020-05-24T07:46:53Z

@bors r+

bors · 2020-05-24T07:46:54Z

📌 Commit d277904 has been approved by petrochenkov

@ghost

Rollup of 3 pull requests Successful merges: - rust-lang#72284 (Remove `macro_defs` map) - rust-lang#72393 (Rewrite `Parser::collect_tokens`) - rust-lang#72528 (Fix typo in doc comment.) Failed merges: r? @ghost

rust-highfive assigned matthewjasper May 16, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 16, 2020

rust-highfive assigned petrochenkov and unassigned matthewjasper May 16, 2020

This comment has been minimized.

Sign in to view

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-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2020

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 May 17, 2020

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 May 17, 2020

petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 18, 2020

Aaron1011 mentioned this pull request May 20, 2020

Serialize span hygiene data #72121

Merged

6 tasks

petrochenkov reviewed May 22, 2020

View reviewed changes

src/librustc_resolve/lib.rs Outdated Show resolved Hide resolved

src/librustc_resolve/macros.rs Show resolved Hide resolved

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 May 22, 2020

Remove macro_defs map

d277904

We store store the `DefId` directly in `ExpnData`. This will allow us to serialize `ExpnData` in PR rust-lang#72121 without needing to manage a side table.

Aaron1011 force-pushed the feature/inline-macro-did branch from 89e00da to d277904 Compare May 22, 2020 20:58

petrochenkov added S-waiting-on-perf Status: Waiting on a perf run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 22, 2020

Aaron1011 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 24, 2020

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 May 24, 2020

RalfJung mentioned this pull request May 24, 2020

Rollup of 3 pull requests #72529

Merged

bors merged commit 94d96b1 into rust-lang:master May 24, 2020

marmeladema mentioned this pull request May 24, 2020

Do not remove closures during ReplaceBodyWithLoop pass #72088

Closed

SimonSapin mentioned this pull request May 26, 2020

Regression: spurious "error: expected ," in serde::Serialize on macro_rules-variable path enum discriminant #72608

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `macro_defs` map #72284

Remove `macro_defs` map #72284

Aaron1011 commented May 16, 2020

rust-highfive commented May 16, 2020

Aaron1011 commented May 16, 2020

Aaron1011 commented May 16, 2020 •

edited

Loading

rust-timer commented May 16, 2020

bors commented May 16, 2020

This comment has been minimized.

This comment has been minimized.

Aaron1011 commented May 17, 2020

rust-timer commented May 17, 2020

bors commented May 17, 2020

rust-highfive commented May 17, 2020

bors commented May 17, 2020

rust-timer commented May 17, 2020

rust-timer commented May 17, 2020

Aaron1011 commented May 17, 2020

petrochenkov commented May 17, 2020

Aaron1011 commented May 17, 2020

marmeladema commented May 22, 2020

petrochenkov commented May 22, 2020

Aaron1011 commented May 22, 2020

petrochenkov commented May 22, 2020

rust-timer commented May 22, 2020

bors commented May 22, 2020

bors commented May 23, 2020

rust-timer commented May 23, 2020

rust-timer commented May 24, 2020

Aaron1011 commented May 24, 2020

petrochenkov commented May 24, 2020

bors commented May 24, 2020

Remove macro_defs map #72284

Remove macro_defs map #72284

Conversation

Aaron1011 commented May 16, 2020

rust-highfive commented May 16, 2020

Aaron1011 commented May 16, 2020

Aaron1011 commented May 16, 2020 • edited Loading

rust-timer commented May 16, 2020

bors commented May 16, 2020

This comment has been minimized.

This comment has been minimized.

Aaron1011 commented May 17, 2020

rust-timer commented May 17, 2020

bors commented May 17, 2020

rust-highfive commented May 17, 2020

bors commented May 17, 2020

rust-timer commented May 17, 2020

rust-timer commented May 17, 2020

Aaron1011 commented May 17, 2020

petrochenkov commented May 17, 2020

Aaron1011 commented May 17, 2020

marmeladema commented May 22, 2020

petrochenkov commented May 22, 2020

Aaron1011 commented May 22, 2020

petrochenkov commented May 22, 2020

rust-timer commented May 22, 2020

bors commented May 22, 2020

bors commented May 23, 2020

rust-timer commented May 23, 2020

rust-timer commented May 24, 2020

Aaron1011 commented May 24, 2020

petrochenkov commented May 24, 2020

bors commented May 24, 2020

Remove `macro_defs` map #72284

Remove `macro_defs` map #72284

Aaron1011 commented May 16, 2020 •

edited

Loading