rustc: move debug info from LocalDecl and UpvarDecl into a dedicated VarDebugInfo. #56231

eddyb · 2018-11-26T10:31:30Z

This PR introduces a MIR "user variable" debuginfo system, which amounts to mapping a variable name, in some SourceScope, to a Place, so that:

each name can appear multiple times (e.g. due to macro hygiene), even in the same scope
each Place can appear multiple times (e.g. in the future from optimizations like NRVO, which collapse multiple MIR locals into one)
the Places aren't limited to just locals, so they can describe the (right now quite ad-hoc) closure upvars and generator saved state fields, and can be properly transformed by optimizations (e.g. inlining - see src/test/mir-opt/inline-closure-captures.rs)

The main motivation for this was that #48300 and further optimizations were blocked on being able to describe complex debuginfo transformations (see #48300 (comment)).

In the textual representation, the "user variable" debuginfo can be found in each scope, and consists of debug NAME => PLACE; "declarations", e.g. the MIR for let x = ...; let y = ...; ... is now:

    let _1: T;                           // in scope 0 at ...
    scope 1 {
        debug x => _1;                   // in scope 1 at ...
        let _2: T;                       // in scope 1 at ...
        scope 2 {
            debug y => _2;               // in scope 2 at ...
        }
    }

For reference, this is how the information was represented before this PR:
(notably, the scopes in which the variables are visible for debuginfo weren't even shown anywhere, making scope 2 look pointless, and user variable names were part of MIR locals)

    let _1: T;                           // "x" in scope 0 at ...
    scope 1 {
        let _2: T;                       // "y" in scope 1 at ...
        scope 2 {
        }
    }

cc @nikomatsakis @michaelwoerister

rust-highfive · 2018-11-26T10:31:34Z

r? @matthewjasper

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

rust-highfive · 2018-11-26T10:37:58Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (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.

travis_time:end:0292afca:start=1543228378764120239,finish=1543228379805831659,duration=1041711420
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---

[00:02:55] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:02:56] tidy error: /checkout/src/librustc_mir/build/mod.rs:162: line longer than 100 chars
[00:02:56] tidy error: /checkout/src/librustc_mir/build/mod.rs:704: trailing whitespace
[00:02:57] some tidy checks failed
[00:02:57] 
[00:02:57] 
[00:02:57] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:02:57] 
[00:02:57] 
[00:02:57] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:02:57] Build completed unsuccessfully in 0:00:54
[00:02:57] Build completed unsuccessfully in 0:00:54
[00:02:57] Makefile:79: recipe for target 'tidy' failed
[00:02:57] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:19400979
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Mon Nov 26 10:36:06 UTC 2018
---
travis_time:end:141cb790:start=1543228566660476901,finish=1543228566664620771,duration=4143870
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:10f87697
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:07be49e1
travis_time:start:07be49e1
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:2cebc585
$ dmesg | grep -i kill

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)

rust-highfive · 2018-11-27T11:29:45Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (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.

travis_time:end:07f1bc18:start=1543316636517541716,finish=1543316638760867822,duration=2243326106
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---
[00:21:51]    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:21:56]    Compiling rustc_errors v0.0.0 (/checkout/src/librustc_errors)
[00:23:28]    Compiling proc_macro v0.0.0 (/checkout/src/libproc_macro)
[00:23:37]    Compiling syntax_ext v0.0.0 (/checkout/src/libsyntax_ext)
[00:25:17] error: variable does not need to be mutable
[00:25:17]   --> src/librustc/infer/canonical/query_response.rs:70:14
[00:25:17]    |
[00:25:17] 70 |             |ref infcx, key, canonical_inference_vars| {
[00:25:17]    |              |
[00:25:17]    |              help: remove this `mut`
[00:25:17]    |
[00:25:17]    = note: `-D unused-mut` implied by `-D warnings`
[00:25:17]    = note: `-D unused-mut` implied by `-D warnings`
[00:25:17] 
[00:25:24] error: variable does not need to be mutable
[00:25:24]    --> src/librustc/traits/error_reporting.rs:500:79
[00:25:24]     |
[00:25:24] 500 |         let normalize = |candidate| self.tcx.global_tcx().infer_ctxt().enter(|ref infcx| {
[00:25:24]     |                                                                               |
[00:25:24]     |                                                                               help: remove this `mut`
[00:25:24] 
[00:25:24] error: variable does not need to be mutable
[00:25:24] error: variable does not need to be mutable
[00:25:24]    --> src/librustc/traits/object_safety.rs:560:34
[00:25:24]     |
[00:25:24] 560 |         self.infer_ctxt().enter(|ref infcx| {
[00:25:24]     |                                  |
[00:25:24]     |                                  help: remove this `mut`
[00:25:24] 
[00:25:33] error: aborting due to 3 previous errors
---
[00:25:33] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:25:33] expected success, got: exit code: 101
[00:25:33] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:25:33] Build completed unsuccessfully in 0:22:17
[00:25:33] make: *** [all] Error 1
[00:25:33] Makefile:28: recipe for target 'all' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:192529ae
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue Nov 27 11:29:41 UTC 2018
---
travis_time:end:1d336d6c:start=1543318181619762227,finish=1543318181626161724,duration=6399497
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1cb23ad8
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fo

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 · 2018-12-02T20:51:14Z

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

Dylan-DPC-zz · 2018-12-10T10:21:29Z

ping from triage @eddyb @matthewjasper what's the update on this? is this blocked on anything?

eddyb · 2018-12-10T10:51:06Z

Blocked on #56278. Also, r? @nikomatsakis

cuviper · 2018-12-11T02:00:12Z

when, if ever, can we bump minimum LLVM to 6?

See #56642.

nikomatsakis · 2018-12-17T21:43:03Z

Closing in favor of #56278 for now

@nikomatsakis

…tsakis Future-proof MIR for dedicated debuginfo. This is rust-lang#56231 without the last commit (the one that actually moves to `VarDebuginfo`). Nothing should be broken, but it should no longer depend on debuginfo for anything else. r? @nikomatsakis

@nikomatsakis

…tsakis Future-proof MIR for dedicated debuginfo. This is rust-lang#56231 without the last commit (the one that actually moves to `VarDebuginfo`). Nothing should be broken, but it should no longer depend on debuginfo for anything else. r? @nikomatsakis

src/test/mir-opt/inline-closure-borrows-arg.rs

oli-obk · 2019-04-30T12:09:04Z

r? @oli-obk

src/librustc/mir/mod.rs

src/librustc_codegen_ssa/mir/analyze.rs

oli-obk · 2019-04-30T12:17:01Z

src/librustc_mir/borrow_check/mod.rs

+        if let Place::Base(PlaceBase::Local(local)) = var_debug_info.place {
+            if let Some(prev_name) = local_names[local] {
+                if var_debug_info.name != prev_name {
+                    span_bug!(var_debug_info.source_info.span,


why doesn't this trigger on the reuse of locals? Or can that only happen after optimizations?

Yeah, and borrowck runs before optimizations.
OTOH, this is a complete hack, cc @matthewjasper.

src/librustc_mir/borrow_check/mod.rs

eddyb · 2019-11-27T15:49:53Z

@bors rollup=never

oli-obk

r=me with generator stuff pulled in a function

src/librustc_codegen_llvm/debuginfo/metadata.rs

src/librustc_codegen_ssa/mir/analyze.rs

…VarDebugInfo.

eddyb · 2019-11-27T17:35:47Z

@bors r=oli-obk

bors · 2019-11-27T17:35:48Z

📌 Commit 563ed27 has been approved by oli-obk

bors · 2019-11-27T17:44:55Z

⌛ Testing commit 563ed27 with merge e87a205...

@nikomatsakis

rustc: move debug info from LocalDecl and UpvarDecl into a dedicated VarDebugInfo. This PR introduces a MIR "user variable" debuginfo system, which amounts to mapping a variable name, in some `SourceScope`, to a `Place`, so that: * each name can appear multiple times (e.g. due to macro hygiene), even in the same scope * each `Place` can appear multiple times (e.g. in the future from optimizations like NRVO, which collapse multiple MIR locals into one) * the `Place`s aren't limited to just locals, so they can describe the (right now quite ad-hoc) closure upvars and generator saved state fields, and can be properly transformed by optimizations (e.g. inlining - see `src/test/mir-opt/inline-closure-captures.rs`) The main motivation for this was that #48300 and further optimizations were blocked on being able to describe complex debuginfo transformations (see #48300 (comment)). <hr/> In the textual representation, the "user variable" debuginfo can be found in each scope, and consists of `debug NAME => PLACE;` "declarations", e.g. the MIR for `let x = ...; let y = ...; ...` is now: ```rust let _1: T; // in scope 0 at ... scope 1 { debug x => _1; // in scope 1 at ... let _2: T; // in scope 1 at ... scope 2 { debug y => _2; // in scope 2 at ... } } ``` For reference, this is how the information was represented before this PR: (notably, the scopes in which the variables are visible for debuginfo weren't even shown anywhere, making `scope 2` look pointless, and user variable names were part of MIR locals) ```rust let _1: T; // "x" in scope 0 at ... scope 1 { let _2: T; // "y" in scope 1 at ... scope 2 { } } ``` cc @nikomatsakis @michaelwoerister

eddyb · 2019-11-27T19:36:05Z

Oh, one more thing I shouldn't forget about, especially once this PR lands:
I need to update https://rust-lang.github.io/rustc-guide/mir/index.html to include information about the new "user variable" debuginfo in MIR (similar to PR description, but expanded a bit).

I should also search for // "\w+" in scope because that's likely a MIR snippet with an old-style variable name, so it should be updated (this also will showcase the new debug declarations).

bors · 2019-11-27T21:00:28Z

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing e87a205 to master...

rust-highfive · 2019-11-27T21:01:45Z

📣 Toolstate changed by #56231!

Tested on commit e87a205.
Direct link to PR: #56231

💔 rls on linux: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).

@Xanewok

Tested on commit rust-lang/rust@e87a205. Direct link to PR: <rust-lang/rust#56231> 💔 rls on linux: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).

rust-highfive assigned matthewjasper Nov 26, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2018

eddyb force-pushed the mir-debuginfo branch from 99561dd to d3aa119 Compare November 27, 2018 11:02

eddyb mentioned this pull request Nov 27, 2018

Future-proof MIR for dedicated debuginfo. #56278

Merged

eddyb added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 10, 2018

rust-highfive assigned nikomatsakis and unassigned matthewjasper Dec 10, 2018

nikomatsakis closed this Dec 17, 2018

eddyb reopened this Apr 30, 2019

eddyb force-pushed the mir-debuginfo branch from d3aa119 to bffaaaf Compare April 30, 2019 10:32

This comment has been minimized.

Sign in to view

eddyb commented Apr 30, 2019

View reviewed changes

src/test/mir-opt/inline-closure-borrows-arg.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rust-highfive assigned oli-obk and unassigned nikomatsakis Apr 30, 2019

eddyb force-pushed the mir-debuginfo branch from bffaaaf to 6332fea Compare April 30, 2019 14:11

oli-obk reviewed Apr 30, 2019

View reviewed changes

eddyb mentioned this pull request Nov 6, 2019

[mir-opt] Turn on the ConstProp pass by default #66074

Merged

This comment has been minimized.

Sign in to view

JohnCSimon 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 Nov 16, 2019

This comment has been minimized.

Sign in to view

eddyb force-pushed the mir-debuginfo branch 2 times, most recently from 53a32bb to 6f45e12 Compare November 27, 2019 14:51

oli-obk requested changes Nov 27, 2019

View reviewed changes

src/librustc_codegen_llvm/debuginfo/metadata.rs Show resolved Hide resolved

src/librustc_codegen_ssa/mir/analyze.rs Outdated Show resolved Hide resolved

rustc: move debug info from LocalDecl and UpvarDecl into a dedicated …

563ed27

…VarDebugInfo.

eddyb force-pushed the mir-debuginfo branch from 6f45e12 to 563ed27 Compare November 27, 2019 17:22

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 Nov 27, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 27, 2019

bors mentioned this pull request Nov 27, 2019

Remove interior mutability in mir predecessors cache #64736

Merged

bors merged commit 563ed27 into rust-lang:master Nov 27, 2019

This was referenced Nov 27, 2019

Async fn resume after completion #66321

Merged

rustc: move mir::SourceScopeLocalData to a field of SourceScopeData. #66789

Merged

Reorganize borrow check diagnostic code #66815

Merged

eddyb deleted the mir-debuginfo branch November 28, 2019 13:06

eddyb mentioned this pull request Feb 3, 2020

mir: begin documenting user variable debuginfo. rust-lang/rustc-dev-guide#571

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc: move debug info from LocalDecl and UpvarDecl into a dedicated VarDebugInfo. #56231

rustc: move debug info from LocalDecl and UpvarDecl into a dedicated VarDebugInfo. #56231

eddyb commented Nov 26, 2018 •

edited

Loading

rust-highfive commented Nov 26, 2018

rust-highfive commented Nov 26, 2018

rust-highfive commented Nov 27, 2018

bors commented Dec 2, 2018

Dylan-DPC-zz commented Dec 10, 2018

eddyb commented Dec 10, 2018

cuviper commented Dec 11, 2018

nikomatsakis commented Dec 17, 2018

This comment has been minimized.

This comment has been minimized.

oli-obk commented Apr 30, 2019

oli-obk Apr 30, 2019

eddyb Apr 30, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Nov 27, 2019

oli-obk left a comment

eddyb commented Nov 27, 2019

bors commented Nov 27, 2019

bors commented Nov 27, 2019

eddyb commented Nov 27, 2019

bors commented Nov 27, 2019

rust-highfive commented Nov 27, 2019

rustc: move debug info from LocalDecl and UpvarDecl into a dedicated VarDebugInfo. #56231

rustc: move debug info from LocalDecl and UpvarDecl into a dedicated VarDebugInfo. #56231

Conversation

eddyb commented Nov 26, 2018 • edited Loading

rust-highfive commented Nov 26, 2018

rust-highfive commented Nov 26, 2018

rust-highfive commented Nov 27, 2018

bors commented Dec 2, 2018

Dylan-DPC-zz commented Dec 10, 2018

eddyb commented Dec 10, 2018

cuviper commented Dec 11, 2018

nikomatsakis commented Dec 17, 2018

This comment has been minimized.

This comment has been minimized.

oli-obk commented Apr 30, 2019

oli-obk Apr 30, 2019

Choose a reason for hiding this comment

eddyb Apr 30, 2019

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Nov 27, 2019

oli-obk left a comment

Choose a reason for hiding this comment

eddyb commented Nov 27, 2019

bors commented Nov 27, 2019

bors commented Nov 27, 2019

eddyb commented Nov 27, 2019

bors commented Nov 27, 2019

rust-highfive commented Nov 27, 2019

eddyb commented Nov 26, 2018 •

edited

Loading