Skip to content

debuginfo: Fix NatVis for Rc and Arc with unsized pointees.#98137

Merged
bors merged 2 commits intorust-lang:masterfrom
michaelwoerister:fix-unsized-rc-arc-natvis
Jun 16, 2022
Merged

debuginfo: Fix NatVis for Rc and Arc with unsized pointees.#98137
bors merged 2 commits intorust-lang:masterfrom
michaelwoerister:fix-unsized-rc-arc-natvis

Conversation

@michaelwoerister
Copy link
Member

@michaelwoerister michaelwoerister commented Jun 15, 2022

Currently, the NatVis for Rc<T> and Arc<T> does not support unsized T. For both Rc<T> and Rc<dyn SomeTrait> the visualizers fail:

    [Reference count] : -> must be used on pointers and . on structures
    [Weak reference count] : -> must be used on pointers and . on structures

This PR fixes the visualizers. For slices we can even give show the elements, so one now gets something like:

slice_rc         : { len=3 }
    [Length]         : 3
    [Reference count] : 41
    [Weak reference count] : 2
    [0]              : 1
    [1]              : 2
    [2]              : 3

r? @wesleywiser

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2022
@wesleywiser
Copy link
Member

wesleywiser commented Jun 15, 2022

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 15, 2022

📌 Commit 2b5efa4 has been approved by wesleywiser

@bors 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 Jun 15, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 15, 2022
@GuillaumeGomez
Rollup merge of rust-lang#98137 - michaelwoerister:fix-unsized-rc-arc…
71bc541 
…-natvis, r=wesleywiser

debuginfo: Fix NatVis for Rc and Arc with unsized pointees.

Currently, the NatVis for `Rc<T>` and `Arc<T>` does not support unsized `T`. For both `Rc<T>` and `Rc<dyn SomeTrait>` the visualizers fail:

```txt
    [Reference count] : -> must be used on pointers and . on structures
    [Weak reference count] : -> must be used on pointers and . on structures
```

This PR fixes the visualizers. For slices we can even give show the elements, so one now gets something like:

```txt
slice_rc         : { len=3 }
    [Length]         : 3
    [Reference count] : 41
    [Weak reference count] : 2
    [0]              : 1
    [1]              : 2
    [2]              : 3
```

r? `@wesleywiser`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jun 15, 2022
Closed
@wesleywiser
Copy link
Member

wesleywiser commented Jun 15, 2022

Nominating this for beta-backport. Normally we wouldn't do this but 1.62 is a relatively important release for @michaelwoerister and I's team as it contains all of the work we've done so far on improving the debugging experience on Windows. As such, we've been telling internal users to use this release as a baseline for future work. It would be great if we could have this issue resolved in it as well!

Backport considerations

  • For:

    • Low risk: This change has no effect on any platform other than *-windows-msvc and even then, has no effect on anything except debugger visualizations in WinDbg or VS. As such, there is no risk of changing compiler behavior in any way.
    • Manually tested: In addition to the automated test checked in with this change, this change has also been validated in both WinDbg and Visual Studio manually.

  • Against:

    • We typically wouldn't backport this kind of change and instead allow it to ride the trains.

@wesleywiser wesleywiser added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 15, 2022
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 15, 2022

Failed in rollup #98139.

@bors r-

@bors 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 Jun 15, 2022
@wesleywiser
Copy link
Member

wesleywiser commented Jun 15, 2022

Fixed the test to account for i686.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 15, 2022

📌 Commit 95adaa2 has been approved by wesleywiser

@bors 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 Jun 15, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 16, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 16, 2022
@bors
Auto merge of rust-lang#98161 - matthiaskrgr:rollup-8csenk9, r=matthi…
d40f24e 
…askrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#97757 (Support lint expectations for `--force-warn` lints (RFC 2383))
 - rust-lang#98125 (Entry and_modify doc)
 - rust-lang#98137 (debuginfo: Fix NatVis for Rc and Arc with unsized pointees.)
 - rust-lang#98147 (Make #[cfg(bootstrap)] not error in proc macros on later stages )

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@michaelwoerister
Copy link
Member Author

michaelwoerister commented Jun 16, 2022

debuginfo tests + rollup = 💔 😉

@bors bors merged commit ae58a55 into rust-lang:master Jun 16, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 16, 2022
@apiraino
Copy link
Contributor

apiraino commented Jun 16, 2022

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 16, 2022
ehuss pushed a commit to ehuss/rust that referenced this pull request Jun 24, 2022
@matthiaskrgr @ehuss
Rollup merge of rust-lang#98137 - michaelwoerister:fix-unsized-rc-arc…
fec9ff3 
…-natvis, r=wesleywiser

debuginfo: Fix NatVis for Rc and Arc with unsized pointees.

Currently, the NatVis for `Rc<T>` and `Arc<T>` does not support unsized `T`. For both `Rc<T>` and `Rc<dyn SomeTrait>` the visualizers fail:

```txt
    [Reference count] : -> must be used on pointers and . on structures
    [Weak reference count] : -> must be used on pointers and . on structures
```

This PR fixes the visualizers. For slices we can even give show the elements, so one now gets something like:

```txt
slice_rc         : { len=3 }
    [Length]         : 3
    [Reference count] : 41
    [Weak reference count] : 2
    [0]              : 1
    [1]              : 2
    [2]              : 3
```

r? `@wesleywiser`
@ehuss ehuss mentioned this pull request Jun 24, 2022
Merged
@ehuss ehuss modified the milestones: 1.63.0, 1.62.0 Jun 24, 2022
@ehuss ehuss removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 24, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2022
@bors
Auto merge of rust-lang#98440 - ehuss:update-beta-cargo, r=ehuss
d8ffc1f 
[beta] Beta backports

* Remove the unused-#[doc(hidden)] logic from the unused_attributes lint rust-lang#98336
* debuginfo: Fix NatVis for Rc and Arc with unsized pointees. rust-lang#98137
* Revert "remove num_cpus dependency" in rustc and update cargo rust-lang#97911
* Update LLVM submodule rust-lang#97690
* Revert rust-lang#96682. rust-lang#97636
* don't do Sized and other return type checks on RPIT's real type rust-lang#97431
* Temporarily disable submodule archive downloads. rust-lang#98423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels

beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.62.0

Development

Successfully merging this pull request may close these issues.

8 participants

@michaelwoerister @wesleywiser @bors @GuillaumeGomez @apiraino @ehuss @rust-highfive @rustbot