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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Jun 16, 2022
Merged

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

merged 2 commits into from
Jun 16, 2022

Conversation

michaelwoerister
Copy link
Member

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

@bors r+ rollup

@bors
Copy link
Contributor

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

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

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

Fixed the test to account for i686.

@bors r+ rollup

@bors
Copy link
Contributor

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

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

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