Skip to content

Commit

Permalink
Allow lossless conversions from float into integral types (#3294)
Browse files Browse the repository at this point in the history
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
Some APIs return JSON values that are intended to be represented by
integral types but are returned as floating values e.g `[1.0, -2.0,
25.0]`.

This allows those values to be converted into Integral types.

## Description

This uses a bidirectional conversion to check if a float can be
losslessly converted into a integral type. This can have issues at the
limits of i64::MAX but I think that's probably acceptable. These values
would be represented imprecisely by floats already.

## Testing
Added additional unit tests of the behavior.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
rcoh authored Dec 8, 2023
1 parent bb2c129 commit b09b02f
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 12 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,9 @@ message = "Expose local socket address from ConnectionMetadata."
references = ["aws-sdk-rust#990"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }
author = "declanvk"

[[smithy-rs]]
message = "[`Number`](https://docs.rs/aws-smithy-types/latest/aws_smithy_types/enum.Number.html) `TryInto` implementations now succesfully convert from `f64` to numeric types when no precision is lost. This fixes some deserialization issues where numbers like `25.0` were sent when `Byte` fields were expected."
references = ["smithy-rs#3294"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all" }
author = "rcoh"
53 changes: 41 additions & 12 deletions rust-runtime/aws-smithy-types/src/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ macro_rules! to_unsigned_integer_converter {
Number::NegInt(v) => {
Err(TryFromNumberErrorKind::NegativeToUnsignedLossyConversion(v).into())
}
Number::Float(v) => {
Err(TryFromNumberErrorKind::FloatToIntegerLossyConversion(v).into())
}
Number::Float(v) => attempt_lossless!(v, $typ),
}
}
}
Expand All @@ -102,9 +100,7 @@ macro_rules! to_signed_integer_converter {
match value {
Number::PosInt(v) => Ok(Self::try_from(v)?),
Number::NegInt(v) => Ok(Self::try_from(v)?),
Number::Float(v) => {
Err(TryFromNumberErrorKind::FloatToIntegerLossyConversion(v).into())
}
Number::Float(v) => attempt_lossless!(v, $typ),
}
}
}
Expand All @@ -115,6 +111,17 @@ macro_rules! to_signed_integer_converter {
};
}

macro_rules! attempt_lossless {
($value: expr, $typ: ty) => {{
let converted = $value as $typ;
if (converted as f64 == $value) {
Ok(converted)
} else {
Err(TryFromNumberErrorKind::FloatToIntegerLossyConversion($value).into())
}
}};
}

/// Converts to a `u64`. The conversion fails if it is lossy.
impl TryFrom<Number> for u64 {
type Error = TryFromNumberError;
Expand All @@ -125,9 +132,7 @@ impl TryFrom<Number> for u64 {
Number::NegInt(v) => {
Err(TryFromNumberErrorKind::NegativeToUnsignedLossyConversion(v).into())
}
Number::Float(v) => {
Err(TryFromNumberErrorKind::FloatToIntegerLossyConversion(v).into())
}
Number::Float(v) => attempt_lossless!(v, u64),
}
}
}
Expand All @@ -142,9 +147,7 @@ impl TryFrom<Number> for i64 {
match value {
Number::PosInt(v) => Ok(Self::try_from(v)?),
Number::NegInt(v) => Ok(v),
Number::Float(v) => {
Err(TryFromNumberErrorKind::FloatToIntegerLossyConversion(v).into())
}
Number::Float(v) => attempt_lossless!(v, i64),
}
}
}
Expand Down Expand Up @@ -236,6 +239,7 @@ mod test {
}
));
}
assert_eq!($typ::try_from(Number::Float(25.0)).unwrap(), 25);
};
}

Expand Down Expand Up @@ -302,6 +306,13 @@ mod test {
}
));
}

let range = || ($typ::MIN..=$typ::MAX);

for val in range().take(1024).chain(range().rev().take(1024)) {
assert_eq!(val, $typ::try_from(Number::Float(val as f64)).unwrap());
$typ::try_from(Number::Float((val as f64) + 0.1)).expect_err("not equivalent");
}
};
}

Expand All @@ -318,6 +329,19 @@ mod test {
}
));
}
let range = || (i64::MIN..=i64::MAX);

for val in range().take(1024).chain(range().rev().take(1024)) {
// if we can actually represent the value
if ((val as f64) as i64) == val {
assert_eq!(val, i64::try_from(Number::Float(val as f64)).unwrap());
}
let fval = val as f64;
// at the limits of the range, we don't have this precision
if (fval + 0.1).fract() != 0.0 {
i64::try_from(Number::Float((val as f64) + 0.1)).expect_err("not equivalent");
}
}
}

#[test]
Expand All @@ -333,6 +357,11 @@ mod test {
#[test]
fn to_i8() {
to_signed_converter_tests!(i8);
i8::try_from(Number::Float(-3200000.0)).expect_err("overflow");
i8::try_from(Number::Float(32.1)).expect_err("imprecise");
i8::try_from(Number::Float(i8::MAX as f64 + 0.1)).expect_err("imprecise");
i8::try_from(Number::Float(f64::NAN)).expect_err("nan");
i8::try_from(Number::Float(f64::INFINITY)).expect_err("nan");
}

#[test]
Expand Down

0 comments on commit b09b02f

Please sign in to comment.