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 tracing for NonZero* #38

Closed
wants to merge 1 commit into from
Closed

Fix tracing for NonZero* #38

wants to merge 1 commit into from

Conversation

juntyr
Copy link

@juntyr juntyr commented Jun 3, 2024

Summary

NonZero* is a fundamentally useful datatype in Rust, which serde models as a newtype around the inner numerical type. That makes tracing any datatype that contains any NonZero* difficult, as the tracer by default always passes zero numeric values, which NonZero* rejects. While giving example data can solve this issue, it is impractical for deeply nested NonZero*s.

This PR fixes this by checking if the traced integer-looking-value is one of the NonZero* types. While the test is ... not pretty ..., it should be optimised out during monomorphisation.

Test Plan

I'm happy to add a few tests for cases you suggest :)

@ma2bd
Copy link
Contributor

ma2bd commented Jun 3, 2024

Thanks @juntyr for raising the issue. I'm inclined to propose a more basic solution (#39) that doesn't introduce any breaking changes.

@juntyr
Copy link
Author

juntyr commented Jun 4, 2024

@ma2bd Thanks for the quick reply! #39 would also likely solve the issue (and is much more applicable and general so thumbs up for that in any case) though I wonder if there is any data structure that works better with a zero-based field …

This reminds me of the std::mem::uninitialized implementation which just sets each byte to 0x01 to protects against non-zero-like types, so setting a globally-applicable value seems to be an acceptable solution.

Thus, I think that #39 is a better idea, perhaps with an extra doc comment or helped method to explain that all default integer values can be set to 1 to allow non-zero values to be successfully traced.

@juntyr juntyr closed this Jun 4, 2024
@juntyr juntyr deleted the nonzero branch June 6, 2024 10:52
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.

2 participants