Add foundations for :truncation_risk target
#459
Merged
Conversation
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
This adds one (templated) type for each of various kinds of truncation risk that we might have. Each type has a `would_value_truncate(T)` member, which assesses _any individual value_ of type `T` to see if it would truncate. Future PRs will associate each operation with one of these. This set isn't perfect. In particular, I'm pretty suspicious as to whether `ValueTimesRatioIsNotInteger` is the right solution. But in my local client, this suite seems to be able to handle all the cases I checked. I think **the most important thing is the structure**. If we land a suite that passes all known test cases, then we can clean up the particular details later on, and trust in our expanding set of test cases to be sure we end up in a better place. Helps #349. Technically, that's only about overflow, but we can't actually _use_ the new overflow implementations without moving to this operation based approach, which means we need to be able to handle truncation too. Besides, this will ultimately lead to more efficient truncation checkers as well.
hoffbrinkle
approved these changes
Jul 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds one (templated) type for each of various kinds of truncation
risk that we might have. Each type has a
would_value_truncate(T)member, which assesses any individual value of type
Tto see if itwould truncate. Future PRs will associate each operation with one of
these.
This set isn't perfect. In particular, I'm pretty suspicious as to
whether
ValueTimesRatioIsNotIntegeris the right solution. But in mylocal client, this suite seems to be able to handle all the cases I
checked. I think the most important thing is the structure. If we
land a suite that passes all known test cases, then we can clean up the
particular details later on, and trust in our expanding set of test
cases to be sure we end up in a better place.
Helps #349. Technically, that's only about overflow, but we can't
actually use the new overflow implementations without moving to this
operation based approach, which means we need to be able to handle
truncation too. Besides, this will ultimately lead to more efficient
truncation checkers as well.