Create floats feature to remove float operations

[webmaster128]

Updated version of Maksym's work from nearprotocol#1

Kudos, Max!

[ethanfrey]

I'm digging in a bit to try to figure out where this float issue could be caused by.
Do you have a simple test case to see if there is float output (eg. sample code to compile to wasm and wasm2wat + grep script to check it)? I can try some changes and see if they hit anything.

[ethanfrey]

Don't we want to immediately error on these two cases if cfg(not(feature = "floats"))?
It may make sense to just provide two implementations for parse_number (with and without floats)

[webmaster128]

Right. At the moment those cases fall into the fallback implementation which is made for digits only, which cuts the number at the . or e.

[ethanfrey]

One difference I notice from my fork from serde-json-core is that there are f64 arguments involved here, even if ignored. In number.rs:

            #[inline]
            #[cfg(feature = "floats")]
            fn visit_f64<E>(self, value: f64) -> Result<Number, E>
            where
                E: de::Error,
            {
                Number::from_f64(value).ok_or_else(|| de::Error::custom("not a JSON number"))
            }

            #[inline]
            #[cfg(not(feature = "floats"))]
            fn visit_f64<E>(self, _value: f64) -> Result<Number, E>
            where
                E: de::Error,
            {
                unreachable!();
            }

However, in my fork of serde-json-core, the visit_f64 method doesn't even exist:

    fn deserialize_f64<V>(self, _visitor: V) -> Result<V::Value, Self::Error>
    where
        V: Visitor<'de>,
    {
        unreachable!()
    }

Basically, the float-free code is marking f64 as unreachable in the Deserializer, while your current fork marks it as unreachable in the Visitor. This means at some point this may interpret a f64 in the code?

I do see the Deserializer also marks it as un-implemented.

    #[cfg(feature = "floats")]
    deserialize_number!(deserialize_f64 => visit_f64);
    #[cfg(not(feature = "floats"))]
    fn deserialize_f64<V: de::Visitor<'de>>(self, _: V) -> Result<V::Value, Error> {
        unimplemented!()
    }

[ethanfrey]

Thinking a bit more, this seems to have to do with Display, and the biggest Display use is Error.

This line might be causing the issue (by calling Display implementations from other code):

serde_json_wasm/src/error.rs

Line 291 in d5f19b5

ErrorCode::Io(ref err) => Display::fmt(err, f),

Try replacing with a constant string to test (thus no code path calls into the Error::Display implementation of some random IoError we don't control). Maybe this helps 🙏

[webmaster128]

This means at some point this may interpret a f64 in the code?

The Visitor trait has a default implementation that is alomst the same as the one in here:

    /// The input contains an `f32`.
    ///
    /// The default implementation forwards to [`visit_f64`].
    ///
    /// [`visit_f64`]: #method.visit_f64
    fn visit_f32<E>(self, v: f32) -> Result<Self::Value, E>
    where
        E: Error,
    {
        self.visit_f64(v as f64)
    }

    /// The input contains an `f64`.
    ///
    /// The default implementation fails with a type error.
    fn visit_f64<E>(self, v: f64) -> Result<Self::Value, E>
    where
        E: Error,
    {
        Err(Error::invalid_type(Unexpected::Float(v), &self))
    }

I think the main difference is, that serde-json-wasm (mind the separators) does not implement a custom Visitor trait at all.

[webmaster128]

This line might be causing the issue (by calling Display implementations from other code):

I just tried this one but it was not sufficient.