INF and NAN support everywhere #62
Conversation
| #[inline] | ||
| pub fn write_characters_f32(&mut self, value: f32) -> Result<(), NewEncodeError> { | ||
| self.write_characters_f64(value as f64) | ||
| } |
There was a problem hiding this comment.
I'm kind of nervous about casting NaN between f32 and f64. Can we duplicate the logic using the associated consts for f32?
There was a problem hiding this comment.
Casting f32 to f64 is lossless, so nothing should go wrong.
| self.write_characters("NAN") | ||
| } else { | ||
| self.write_characters(value) | ||
| } |
There was a problem hiding this comment.
This might be cleaner as a match statement. For the NaN case, we might be able to use a match guard like x if x.is_nan() => { ... }
There was a problem hiding this comment.
Didn't know that was a feature 👀
There was a problem hiding this comment.
warning: floating-point types cannot be used in patterns
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #41620 <https://github.com/rust-lang/rust/issues/41620>
rbx_xml/test-files/inf-and-nan.rbxmx
Outdated
| @@ -7,25 +7,125 @@ | |||
| <string name="Name">Folder</string> | |||
| <BinaryString name="Tags"></BinaryString> | |||
| </Properties> | |||
| <Item class="NumberValue" referent="RBX3B3D9D3DB43D4E6793B190B081E0A886"> | |||
| <Item class="Folder" referent="RBX84B85A2C500048E79846CD63A6127BA3"> | |||
There was a problem hiding this comment.
Can we avoid introducing a grouping for just the NumberValue instances and keep everything flat?
|
Travis CI build failure is my fault, I'll get it fixed in master. |
|
@LPGhatguy Flattened the rbxmx file and responded to your other comments. |
|
Build failure doesn't look like your fault, I'll fix it post-merge. |
Closes #39
I believe I got everything.