Skip to content

Add windows-result crate#2847

Merged
kennykerr merged 9 commits intomasterfrom
result
Feb 15, 2024
Merged

Add windows-result crate#2847
kennykerr merged 9 commits intomasterfrom
result

Conversation

@kennykerr
Copy link
Collaborator

This update pulls out the error handling implementation from the windows-core crate and making it available directly without a dependency on the windows or windows-core crates. Windows error handling can be tricky. This should make it simpler for library and component authors to use and provide Windows error handling and propagation in more scenarios. Notably:

  • These types are still included in the windows-core and windows crates as a dependency so it should not affect existing code too much.

  • This depends on the low-level vtable generation provided by Add "vtbl" option for low-level interface vtable generation #2845 and provides a simple example of how one might go about using that.

  • The Error and HRESULT types have been simplified a little. Mainly they no longer force you to convert to and from HSTRING types. You can use a str to create a new Error value and you can retrieve the error messages from an HRESULT or Error value directly as a String.

@kennykerr
Copy link
Collaborator Author

@ridwanabdillahi I moved some types from windows-core to windows-result - does the new crate needs its own natvis? Haven't had much success debugging the test failure.

@tim-weis
Copy link
Contributor

The respective parts of the .natvis implementation need to move and have their <Type> elements updated appropriately. The .natvis also needs to be wired up, which currently happens here:

#![cfg_attr(windows_debugger_visualizer, debugger_visualizer(natvis_file = "../windows.natvis"))]

The debugger_visualizer tests need to be updated as well. (I can't prepare a PR right now being on a rather useless internet connection unfortunately.)

@ridwanabdillahi
Copy link
Contributor

Sorry for the delay but Tim is correct. Since the crate housing certain types have changed, the Natvis definitions for those types need to be updated to specify the correct fully qualified name of these types. For instance windows_core::error::Error will need to change to windows_result::error::Error.

@kennykerr
Copy link
Collaborator Author

Thanks for the quick fix @riverar!

@tim-weis
Copy link
Contributor

The .natvis file still needs to be split in two and associated with each crate. The test currently succeeds by coincidence. A test that only references windows-result would indeed fail, as no debugger visualizers are compiled into the windows-result PDB.

While at it I would also suggest renaming the .natvis file following the crate name (i.e., windows-core.natvis and windows-result.natvis). This makes it a lot easier to spot when multiple .natvis files are compiled into a PDB using the .nvlist debugger command.

@kennykerr
Copy link
Collaborator Author

Thanks Tim, yes it makes sense to split them up and have per-crate natvis files. I'd also like to greatly simplify the testing as its currently too difficult to figure out what's wrong and how to fix it. Anyway, I'll leave that for another PR. Feel free to jump in to help if you would like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants