Skip to content

improved error reporting for UnwrapThrowExt for Result#4035

Merged
daxpedda merged 2 commits intowasm-bindgen:mainfrom
sokorototo:main
Aug 1, 2024
Merged

improved error reporting for UnwrapThrowExt for Result#4035
daxpedda merged 2 commits intowasm-bindgen:mainfrom
sokorototo:main

Conversation

@sokorototo
Copy link
Copy Markdown
Contributor

Since Result<T, E: fmt::Debug> has access to E: fmt::Debug, this PR modifies the default implementation of UnwrapThrowExt::unwrap_throw() to include this extra information. This is my first PR, and I'm open to criticism

Copy link
Copy Markdown
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, nice!
Thank you!

@daxpedda
Copy link
Copy Markdown
Member

daxpedda commented Aug 1, 2024

I will add a changelog entry.

@daxpedda daxpedda merged commit 5490ed2 into wasm-bindgen:main Aug 1, 2024
Comment thread src/lib.rs
Comment on lines +1386 to +1389
if cfg!(all(
target_arch = "wasm32",
not(any(target_os = "emscripten", target_os = "wasi"))
)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be gated behind cfg(debug_assertions), the same as the default unwrap_throw implementation; see #2995 (review).

(The cfg(feature = "std") might not be needed anymore after #4005.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
Addressed in #4042.

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.

3 participants