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

Fix .natvis visualizers. #60687

Merged
merged 1 commit into from
May 17, 2019
Merged

Conversation

MaulingMonkey
Copy link
Contributor

Updated to handle these changes:

  • core::ptr::* lost their __0 elements and are just plain pointers
  • core::ptr::* probably shouldn't dereference in DisplayString s
  • VecDeque probably should dereference it's buf pointer to display individual items.
  • VecDeque and Vec use core::ptr::* s
  • VecDeque and LinkedList moved modules again.

Retested - still working fine, left alone:

  • String, &str, Option

Side Chatter

  • Props to Alex for pointing out this was broken in the #ides-and-editors Discord channel
  • It'd be nice if there was a sane way to automate unit testing these visualizers.
    (I assume COM automation of Visual Studio would be a no go on the build servers, and probably really incredibly painful to write too! Suggestions welcome...)

Updated to handle these changes:
  - `core::ptr::*` lost their `__0` elements and are just plain pointers
  - `core::ptr::*` probably shouldn't dereference in `DisplayString` s
  - `VecDeque` and `Vec` use `core::ptr::*` s
  - `VecDeque` and `LinkedList` moved modules again.

Retested - still working fine, left alone:
  - `String`, `&str`, `Option`
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 9, 2019
@nikomatsakis
Copy link
Contributor

r? @alexcrichton -- I have no idea what this code is, but it sounds like you might.

@alexcrichton
Copy link
Member

@bors: r+

I also don't really know what it is, but I trust those modifying it :)

@bors
Copy link
Contributor

bors commented May 16, 2019

📌 Commit 7c55b48 has been approved by alexcrichton

@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 May 16, 2019
@bors
Copy link
Contributor

bors commented May 17, 2019

⌛ Testing commit 7c55b48 with merge 6aee7fc0f64b6824205578ff85d1ed08e6cd0b71...

Centril added a commit to Centril/rust that referenced this pull request May 17, 2019
…r=alexcrichton

Fix .natvis visualizers.

### Updated to handle these changes:
  - `core::ptr::*` lost their `__0` elements and are just plain pointers
  - `core::ptr::*` probably shouldn't dereference in `DisplayString` s
  - `VecDeque` probably *should* dereference it's buf pointer to display individual items.
  - `VecDeque` and `Vec` use `core::ptr::*` s
  - `VecDeque` and `LinkedList` moved modules again.

### Retested - still working fine, left alone:
  - `String`, `&str`, `Option`

### Side Chatter
  - Props to Alex for pointing out this was broken in the `#ides-and-editors` Discord channel
  - It'd be nice if there was a sane way to automate unit testing these visualizers.
    (I assume COM automation of Visual Studio would be a no go on the build servers, and probably really incredibly painful to write too!  Suggestions welcome...)
@Centril
Copy link
Contributor

Centril commented May 17, 2019

@bors retry

bors added a commit that referenced this pull request May 17, 2019
Rollup of 6 pull requests

Successful merges:

 - #60685 (Switch to SPDX 2.1 license expression)
 - #60687 (Fix .natvis visualizers.)
 - #60805 (remove compiletest's dependency on `filetime`)
 - #60862 (Get ty from local_decls instead of using Place)
 - #60873 (Parse alternative incorrect uses of await and recover)
 - #60894 (Add entry-like methods to HashSet)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented May 17, 2019

⌛ Testing commit 7c55b48 with merge 1bbb135...

@bors bors merged commit 7c55b48 into rust-lang:master May 17, 2019
@dgrunwald
Copy link
Contributor

It'd be nice if there was a sane way to automate unit testing these visualizers.
(I assume COM automation of Visual Studio would be a no go on the build servers, and probably really incredibly painful to write too! Suggestions welcome...)

Visual Studio isn't the only debugger with natvis support; there's also WinDbg (.nvload/dx commands).
So it's possible that this could be tested by talking to cdb.exe via stdin/stdout.

MaulingMonkey added a commit to MaulingMonkey/rust that referenced this pull request May 20, 2019
…tvis` regressions, like those fixed in rust-lang#60687.

Several Microsoft debuggers (VS, VS Code, WinDbg, CDB, ...) consume the `*.natvis` files we embed into rust `*.pdb` files.
While this only tests CDB, that test coverage should help for all of them.

CHANGES

src\bootstrap
  - test.rs:  Run CDB debuginfo tests on MSVC targets

src\test\debuginfo
  - issue-13213.rs:  CDB has trouble with this, skip for now (newly discovered regression?)
  - pretty-std.rs:  Was ignored, re-enable for CDB only to start with, add CDB tests.
  - should-fail.rs:  Add CDB tests.

src\tools\compiletest:
  - Added "-cdb" option
  - Added Mode::DebugInfoCdb ("debuginfo-cdb")
  - Added run_debuginfo_cdb_test[_no_opt]
  - Renamed Mode::DebugInfoBoth -> DebugInfoGdbLldb ("debuginfo-gdb+lldb") since it's no longer clear what "Both" means.
  - Find CDB at the default Win10 SDK install path "C:\Program Files (x86)\Windows Kits\10\Debugger\*\cdb.exe"
  - Ignore CDB tests if CDB not found.

ISSUES

  - `compute_stamp_hash`: not sure if there's any point in hashing `%ProgramFiles(x86)%`
  - `OsString` lacks any `*.natvis` entries (would be nice to add in a followup changelist)
  - DSTs (array/string slices) which work in VS & VS Code fail in CDB.
  - I've avoided `Mode::DebugInfoAll` as 3 debuggers leads to pow(2,3)=8 possible combinations.

REFERENCE

CDB is not part of the base Visual Studio install, but can be added via the Windows 10 SDK:
  https://developer.microsoft.com/en-us/windows/downloads/windows-10-sdk
Installing just "Debugging Tools for Windows" is sufficient.

CDB appears to already be installed on appveyor CI, where this changelist can find it, based on it's use here:
  https://github.com/rust-lang/rust/blob/0ffc57311030a1930edfa721fe57d0000a063af4/appveyor.yml#L227

CDB commands and command line reference:
  https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-reference
Centril added a commit to Centril/rust that referenced this pull request May 22, 2019
…pport, r=alexcrichton

Add basic CDB support to debuginfo compiletest s, to help catch `*.natvis` regressions, like those fixed in rust-lang#60687.

First draft, feedback welcome.

Several Microsoft debuggers (VS, VS Code, WinDbg, CDB, ...) consume the `*.natvis` files we embed into rust `*.pdb` files.  While this only tests CDB, that test coverage should help for all of them.

# Changes

## src\bootstrap
  - test.rs:  Run CDB debuginfo tests on MSVC targets

## src\test\debuginfo
  - issue-13213.rs:  CDB has trouble with this, skip for now (newly discovered regression?)
  - pretty-std.rs:  Was ignored, re-enable for CDB only to start with, add CDB tests.
  - should-fail.rs:  Add CDB tests.

## src\tools\compiletest:
  - Added "-cdb" option
  - Added Mode::DebugInfoCdb ("debuginfo-cdb")
  - Added run_debuginfo_cdb_test[_no_opt]
  - Renamed Mode::DebugInfoBoth -> DebugInfoGdbLldb ("debuginfo-gdb+lldb") since it's no longer clear what "Both" means.
  - Find CDB at the default Win10 SDK install path "C:\Program Files (x86)\Windows Kits\10\Debugger\\*\cdb.exe"
  - Ignore CDB tests if CDB not found.

# Issues

  - `compute_stamp_hash`: not sure if there's any point in hashing `%ProgramFiles(x86)%`
  - `OsString` lacks any `*.natvis` entries (would be nice to add in a followup changelist)
  - DSTs (array/string slices) which work in VS & VS Code fail in CDB.
  - I've avoided `Mode::DebugInfoAll` as 3 debuggers leads to pow(2,3)=8 possible combinations.

# Reference

CDB is not part of the base Visual Studio install, but can be added via the Windows 10 SDK:
  https://developer.microsoft.com/en-us/windows/downloads/windows-10-sdk
Installing just "Debugging Tools for Windows" is sufficient.

CDB appears to already be installed on appveyor CI, where this changelist can find it, based on it's use here:
  https://github.com/rust-lang/rust/blob/0ffc57311030a1930edfa721fe57d0000a063af4/appveyor.yml#L227

CDB commands and command line reference:
  https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-reference
bors added a commit that referenced this pull request May 23, 2019
…excrichton

Add basic CDB support to debuginfo compiletest s, to help catch `*.natvis` regressions, like those fixed in #60687.

First draft, feedback welcome.

Several Microsoft debuggers (VS, VS Code, WinDbg, CDB, ...) consume the `*.natvis` files we embed into rust `*.pdb` files.  While this only tests CDB, that test coverage should help for all of them.

# Changes

## src\bootstrap
  - test.rs:  Run CDB debuginfo tests on MSVC targets

## src\test\debuginfo
  - issue-13213.rs:  CDB has trouble with this, skip for now (newly discovered regression?)
  - pretty-std.rs:  Was ignored, re-enable for CDB only to start with, add CDB tests.
  - should-fail.rs:  Add CDB tests.

## src\tools\compiletest:
  - Added "-cdb" option
  - Added Mode::DebugInfoCdb ("debuginfo-cdb")
  - Added run_debuginfo_cdb_test[_no_opt]
  - Renamed Mode::DebugInfoBoth -> DebugInfoGdbLldb ("debuginfo-gdb+lldb") since it's no longer clear what "Both" means.
  - Find CDB at the default Win10 SDK install path "C:\Program Files (x86)\Windows Kits\10\Debugger\\*\cdb.exe"
  - Ignore CDB tests if CDB not found.

# Issues

  - `compute_stamp_hash`: not sure if there's any point in hashing `%ProgramFiles(x86)%`
  - `OsString` lacks any `*.natvis` entries (would be nice to add in a followup changelist)
  - DSTs (array/string slices) which work in VS & VS Code fail in CDB.
  - I've avoided `Mode::DebugInfoAll` as 3 debuggers leads to pow(2,3)=8 possible combinations.

# Reference

CDB is not part of the base Visual Studio install, but can be added via the Windows 10 SDK:
  https://developer.microsoft.com/en-us/windows/downloads/windows-10-sdk
Installing just "Debugging Tools for Windows" is sufficient.

CDB appears to already be installed on appveyor CI, where this changelist can find it, based on it's use here:
  https://github.com/rust-lang/rust/blob/0ffc57311030a1930edfa721fe57d0000a063af4/appveyor.yml#L227

CDB commands and command line reference:
  https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-reference
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.

8 participants