Merged
Conversation
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`
Contributor
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Contributor
|
r? @alexcrichton -- I have no idea what this code is, but it sounds like you might. |
Member
|
@bors: r+ I also don't really know what it is, but I trust those modifying it :) |
Collaborator
|
📌 Commit 7c55b48 has been approved by |
bors
added
S-waiting-on-bors
Collaborator
|
⌛ 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...)
Contributor
|
@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
Collaborator
Contributor
Visual Studio isn't the only debugger with natvis support; there's also WinDbg ( |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updated to handle these changes:
core::ptr::*lost their__0elements and are just plain pointerscore::ptr::*probably shouldn't dereference inDisplayStringsVecDequeprobably should dereference it's buf pointer to display individual items.VecDequeandVecusecore::ptr::*sVecDequeandLinkedListmoved modules again.Retested - still working fine, left alone:
String,&str,OptionSide Chatter
#ides-and-editorsDiscord channel(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...)