Skip to content

Commit 92911cf

Browse files
committed
refactor(encoding): remove as many as casts as possible
There was a bug in the `impl EncodeGaugeValue for u64` implementation due to the `as i64` cast. The implementation only checked if `self == u64::MAX`, this is wrong since `u64::MAX` is not the only value that cannot be represented by an `i64`. The range of values is actually `self > i64::MAX`. `u64::MAX == 2^64 - 1` whereas `i64::MAX == 2^63 - 1`, this means there are `2^63` `u64` values that are not representable by an `i64`, not just `i64::MAX`. Delegating the checks to `i64::try_from(self)` ensures the logic is right. For this reason I also switched the rest of the implementations to use `N::from` instead of `as N` since the `N::from` function is only implemented for types whose conversion is infallible, this guarantees no such issues will ever happen. The only `as` cast left is `u64 as f64` in `EncodeExemplarValue`, this is not possible to remove since there is no `impl TryFrom<u64> for f64`. Signed-off-by: Jalil David Salamé Messina <jalil.salame@gmail.com>
1 parent 7844d86 commit 92911cf

File tree

1 file changed

+8
-12
lines changed

1 file changed

+8
-12
lines changed

src/encoding.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -602,12 +602,8 @@ impl EncodeGaugeValue for u64 {
602602
fn encode(&self, encoder: &mut GaugeValueEncoder) -> Result<(), std::fmt::Error> {
603603
// Between forcing end users to do endless as i64 for things that are
604604
// clearly u64 and having one error case for rarely used protobuf when
605-
// a gauge is set to u64::MAX, the latter seems like the right choice.
606-
if *self == u64::MAX {
607-
return Err(std::fmt::Error);
608-
}
609-
610-
encoder.encode_i64(*self as i64)
605+
// a gauge is set to >i64::MAX, the latter seems like the right choice.
606+
encoder.encode_i64(i64::try_from(*self).map_err(|_err| std::fmt::Error)?)
611607
}
612608
}
613609

@@ -619,13 +615,13 @@ impl EncodeGaugeValue for f64 {
619615

620616
impl EncodeGaugeValue for i32 {
621617
fn encode(&self, encoder: &mut GaugeValueEncoder) -> Result<(), std::fmt::Error> {
622-
encoder.encode_i64(*self as i64)
618+
encoder.encode_i64(i64::from(*self))
623619
}
624620
}
625621

626622
impl EncodeGaugeValue for f32 {
627623
fn encode(&self, encoder: &mut GaugeValueEncoder) -> Result<(), std::fmt::Error> {
628-
encoder.encode_f64(*self as f64)
624+
encoder.encode_f64(f64::from(*self))
629625
}
630626
}
631627

@@ -687,13 +683,13 @@ impl EncodeCounterValue for f64 {
687683

688684
impl EncodeCounterValue for u32 {
689685
fn encode(&self, encoder: &mut CounterValueEncoder) -> Result<(), std::fmt::Error> {
690-
encoder.encode_u64(*self as u64)
686+
encoder.encode_u64(u64::from(*self))
691687
}
692688
}
693689

694690
impl EncodeCounterValue for f32 {
695691
fn encode(&self, encoder: &mut CounterValueEncoder) -> Result<(), std::fmt::Error> {
696-
encoder.encode_f64(*self as f64)
692+
encoder.encode_f64(f64::from(*self))
697693
}
698694
}
699695

@@ -751,13 +747,13 @@ impl EncodeExemplarValue for u64 {
751747

752748
impl EncodeExemplarValue for f32 {
753749
fn encode(&self, mut encoder: ExemplarValueEncoder) -> Result<(), std::fmt::Error> {
754-
encoder.encode(*self as f64)
750+
encoder.encode(f64::from(*self))
755751
}
756752
}
757753

758754
impl EncodeExemplarValue for u32 {
759755
fn encode(&self, mut encoder: ExemplarValueEncoder) -> Result<(), std::fmt::Error> {
760-
encoder.encode(*self as f64)
756+
encoder.encode(f64::from(*self))
761757
}
762758
}
763759

0 commit comments

Comments
 (0)