transpile: Use try_from to detect overflow builtin result truncation#1879
Open
thedataking wants to merge 1 commit into
Open
transpile: Use try_from to detect overflow builtin result truncation#1879thedataking wants to merge 1 commit into
thedataking wants to merge 1 commit into
Conversation
Emitting result_ty::try_from(result).is_err() states the intent more clearly in the generated code than casting the narrowed result back up to i128 and comparing.
ahomescu
reviewed
Jul 2, 2026
| mut out: *mut ::core::ffi::c_int, | ||
| ) -> ::core::ffi::c_int { | ||
| let (c2rust_result, c2rust_overflowed) = (a as i128).overflowing_mul(b as i128); | ||
| let c2rust_result_narrow = c2rust_result as ::core::ffi::c_int; |
Contributor
There was a problem hiding this comment.
If you're using try_from below, you don't need this cast here because Ok(x) is what you want.
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
2 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.
Follow-up to #1863 addressing @ahomescu's review feedback: emit
result_ty::try_from(result).is_err()instead of casting the narrowed result back up toi128and comparing. The checks are equivalent;try_fromreads better in the generated code:The
try_frompath is derived from the already-converted result type, so crate-local aliases (size_t) and absolute paths (::core::ffi::c_int) both come out right. A newoverflow_mixed.csnapshot pins the generated idiom.Based on #1863; will retarget to master when that merges.