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.

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

Improvements to NatVis support #80311

Merged
merged 1 commit into from
Dec 30, 2020
Merged

Improvements to NatVis support #80311

merged 1 commit into from
Dec 30, 2020

Conversation

sivadeilra
Copy link

NatVis files describe how to display types in some Windows debuggers,
such as Visual Studio, WinDbg, and VS Code.

This commit makes several improvements:

  • Adds visualizers for Rc, Weak, and Arc.

  • Changes [size] to [len], for consistency with the Rust API.
    Visualizers often use [size] to mirror the size() method on C++ STL
    collections.

  • Several visualizers used the PVOID and ULONG typedefs. These are part
    of the Windows API; they are not guaranteed to always be defined in a
    pure Rust DLL/EXE. I converted PVOID to void* and ULONG to
    unsigned long.

  • Cosmetic change: Removed {} braces around the visualized display
    for Option types. They now display simply as Some(value) or
    None, which reflects what is written in source code.

  • The visualizer for alloc::string::String makes assumptions about
    the layout of String (it casts String* to another type), rather
    than using symbolic expressions. This commit changes the visualizer
    so that it simply uses symbolic expressions to access the string
    data and string length.

  • The visualizers for str and String now place the character data
    array under a synthetic [chars] node. When expanding a String
    node, users rarely want to see an array of characters. This just places
    them behind one expansion node / level.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 22, 2020
src/etc/natvis/libcore.natvis Outdated Show resolved Hide resolved
src/etc/natvis/libcore.natvis Outdated Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

Have you tried to run ./x.py test src/test/debuginfo locally?
Some test outputs may change and need updating.
IIRC, CI that is run on PR submission doesn't run Windows-specific tests like this.

@petrochenkov 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 Dec 22, 2020
@sivadeilra
Copy link
Author

Have you tried to run ./x.py test src/test/debuginfo locally?

Yes, I've run ./x.py test src/test/debuginfo locally. All green.

By the way, I'm working on better support for Option and for enums generally in NatVis. I've encountered at least one bug in the NatVis interpreter itself, and I'm working with the NatVis team on how to get that fixed. So I'll have more NatVis-related PRs in the (hopefully near) future.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 23, 2020

📌 Commit 1a30823 has been approved by petrochenkov

@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 Dec 23, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 24, 2020
Improvements to NatVis support

NatVis files describe how to display types in some Windows debuggers,
such as Visual Studio, WinDbg, and VS Code.

This commit makes several improvements:

* Adds visualizers for Rc<T>, Weak<T>, and Arc<T>.

* Changes [size] to [len], for consistency with the Rust API.
  Visualizers often use [size] to mirror the size() method on C++ STL
  collections.

* Several visualizers used the PVOID and ULONG typedefs. These are part
  of the Windows API; they are not guaranteed to always be defined in a
  pure Rust DLL/EXE. I converted PVOID to `void*` and `ULONG` to
  `unsigned long`.

* Cosmetic change: Removed {} braces around the visualized display
  for `Option` types. They now display simply as `Some(value)` or
  `None`, which reflects what is written in source code.

* The visualizer for `alloc::string::String` makes assumptions about
  the layout of `String` (it casts `String*` to another type), rather
  than using symbolic expressions. This commit changes the visualizer
  so that it simply uses symbolic expressions to access the string
  data and string length.

* The visualizers for `str` and `String` now place the character data
  array under a synthetic `[chars]` node. When expanding a `String`
  node, users rarely want to see an array of characters. This just places
  them behind one expansion node / level.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 24, 2020
Improvements to NatVis support

NatVis files describe how to display types in some Windows debuggers,
such as Visual Studio, WinDbg, and VS Code.

This commit makes several improvements:

* Adds visualizers for Rc<T>, Weak<T>, and Arc<T>.

* Changes [size] to [len], for consistency with the Rust API.
  Visualizers often use [size] to mirror the size() method on C++ STL
  collections.

* Several visualizers used the PVOID and ULONG typedefs. These are part
  of the Windows API; they are not guaranteed to always be defined in a
  pure Rust DLL/EXE. I converted PVOID to `void*` and `ULONG` to
  `unsigned long`.

* Cosmetic change: Removed {} braces around the visualized display
  for `Option` types. They now display simply as `Some(value)` or
  `None`, which reflects what is written in source code.

* The visualizer for `alloc::string::String` makes assumptions about
  the layout of `String` (it casts `String*` to another type), rather
  than using symbolic expressions. This commit changes the visualizer
  so that it simply uses symbolic expressions to access the string
  data and string length.

* The visualizers for `str` and `String` now place the character data
  array under a synthetic `[chars]` node. When expanding a `String`
  node, users rarely want to see an array of characters. This just places
  them behind one expansion node / level.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 24, 2020
Improvements to NatVis support

NatVis files describe how to display types in some Windows debuggers,
such as Visual Studio, WinDbg, and VS Code.

This commit makes several improvements:

* Adds visualizers for Rc<T>, Weak<T>, and Arc<T>.

* Changes [size] to [len], for consistency with the Rust API.
  Visualizers often use [size] to mirror the size() method on C++ STL
  collections.

* Several visualizers used the PVOID and ULONG typedefs. These are part
  of the Windows API; they are not guaranteed to always be defined in a
  pure Rust DLL/EXE. I converted PVOID to `void*` and `ULONG` to
  `unsigned long`.

* Cosmetic change: Removed {} braces around the visualized display
  for `Option` types. They now display simply as `Some(value)` or
  `None`, which reflects what is written in source code.

* The visualizer for `alloc::string::String` makes assumptions about
  the layout of `String` (it casts `String*` to another type), rather
  than using symbolic expressions. This commit changes the visualizer
  so that it simply uses symbolic expressions to access the string
  data and string length.

* The visualizers for `str` and `String` now place the character data
  array under a synthetic `[chars]` node. When expanding a `String`
  node, users rarely want to see an array of characters. This just places
  them behind one expansion node / level.
@bors
Copy link
Contributor

bors commented Dec 26, 2020

⌛ Testing commit 1a30823 with merge 4e90fe905fd5f202aad0197b01f6c248dc932254...

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 26, 2020
@petrochenkov
Copy link
Contributor

Looks like some tests do need an update.

In the worst case min-cdb-version can be used to conditionally ignore some test cases (example: #76390).

@petrochenkov 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 Dec 27, 2020
@sivadeilra
Copy link
Author

Yeah, I'm updating those tests now.

I did run python x.py src\test\debuginfo, as suggested, but apparently this does not actually run the tests. I saw test result: ok and assumed everything was fine, but I should have looked more closely because it actually says:

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

I've tried using both \\ and / path separators, but that made no difference. Any hints on how to get these tests to actually run, locally?

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 28, 2020

The tests should be run if cdb is found, and the logic for finding cdb is in fn find_cdb in src\tools\compiletest\src\main.rs.
Looks like the only requirement is that the target should be *-pc-windows-msvc and cdb.exe should live in one of the known locations.

I did run these tests in the past without any additional setup, but perhaps I had all the necessary components accidentally installed for other reasons.

NatVis files describe how to display types in some Windows debuggers,
such as Visual Studio, WinDbg, and VS Code.

This commit makes several improvements:

* Adds visualizers for Rc<T>, Weak<T>, and Arc<T>.

* Changes [size] to [len], for consistency with the Rust API.
  Visualizers often use [size] to mirror the size() method on C++ STL
  collections.

* Several visualizers used the PVOID and ULONG typedefs. These are part
  of the Windows API; they are not guaranteed to always be defined in a
  pure Rust DLL/EXE. I converted PVOID to `void*` and `ULONG` to
  `unsigned long`.

* Cosmetic change: Removed {} braces around the visualized display
  for `Option` types. They now display simply as `Some(value)` or
  `None`, which reflects what is written in source code.

* The visualizer for `alloc::string::String` makes assumptions about
  the layout of `String` (it casts `String*` to another type), rather
  than using symbolic expressions. This commit changes the visualizer
  so that it simply uses symbolic expressions to access the string
  data and string length.
@sivadeilra
Copy link
Author

Thanks @petrochenkov , that was exactly the issue. I made sure that find_cdb found my debugger, repro'd the failures, fixed them, and updated the PR.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 28, 2020

📌 Commit 2f58422 has been approved by petrochenkov

@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 Dec 28, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 30, 2020
Improvements to NatVis support

NatVis files describe how to display types in some Windows debuggers,
such as Visual Studio, WinDbg, and VS Code.

This commit makes several improvements:

* Adds visualizers for Rc<T>, Weak<T>, and Arc<T>.

* Changes [size] to [len], for consistency with the Rust API.
  Visualizers often use [size] to mirror the size() method on C++ STL
  collections.

* Several visualizers used the PVOID and ULONG typedefs. These are part
  of the Windows API; they are not guaranteed to always be defined in a
  pure Rust DLL/EXE. I converted PVOID to `void*` and `ULONG` to
  `unsigned long`.

* Cosmetic change: Removed {} braces around the visualized display
  for `Option` types. They now display simply as `Some(value)` or
  `None`, which reflects what is written in source code.

* The visualizer for `alloc::string::String` makes assumptions about
  the layout of `String` (it casts `String*` to another type), rather
  than using symbolic expressions. This commit changes the visualizer
  so that it simply uses symbolic expressions to access the string
  data and string length.

* The visualizers for `str` and `String` now place the character data
  array under a synthetic `[chars]` node. When expanding a `String`
  node, users rarely want to see an array of characters. This just places
  them behind one expansion node / level.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 30, 2020
Rollup of 7 pull requests

Successful merges:

 - rust-lang#80185 (Fix ICE when pointing at multi bytes character)
 - rust-lang#80260 (slightly more typed interface to panic implementation)
 - rust-lang#80311 (Improvements to NatVis support)
 - rust-lang#80337 (Use `desc` as a doc-comment for queries if there are no doc comments)
 - rust-lang#80381 (Revert "Cleanup markdown span handling")
 - rust-lang#80492 (remove empty wraps, don't return Results from from infallible functions)
 - rust-lang#80509 (where possible, pass slices instead of &Vec or &String (clippy::ptr_arg))

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 88b198b into rust-lang:master Dec 30, 2020
@rustbot rustbot added this to the 1.51.0 milestone Dec 30, 2020
@sivadeilra sivadeilra deleted the natvis branch December 31, 2020 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants