Repository: msgpack/msgpack-c (language: python)

Issue:
pack double and float more size efficient
PR for issue #1017
pack double and float more size efficient
PR for issue #1017
pack double and float more size efficient
PR for issue #1017


Discussion / hints:
It seems that CI reports warnings (as errors). Could you fix this?

```
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:[114](https://github.com/msgpack/msgpack-c/runs/6744024570?check_suite_focus=true#step:8:115)3:27: error: implicit conversion from 'unsigned long' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                       ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1143:72: error: implicit conversion from 'uint64_t' (aka 'unsigned long') to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                                                                    ~~ ^~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1146:24: error: implicit conversion from 'long' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                    ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1146:68: error: implicit conversion from 'int64_t' (aka 'long') to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                                                                ~~ ^~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:[116](https://github.com/msgpack/msgpack-c/runs/6744024570?check_suite_focus=true#step:8:117)5:27: error: implicit conversion from 'unsigned long' to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                       ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1165:72: error: implicit conversion from 'uint64_t' (aka 'unsigned long') to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                                                                    ~~ ^~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1168:24: error: implicit conversion from 'long' to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                    ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1168:68: error: implicit conversion from 'int64_t' (aka 'long') to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
```

# [Codecov](https://codecov.io/gh/msgpack/msgpack-c/pull/1018?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) Report
> Merging [#1018](https://codecov.io/gh/msgpack/msgpack-c/pull/1018?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) (05de839) into [cpp_master](https://codecov.io/gh/msgpack/msgpack-c/commit/05f654fd64de96fdb91d043a70de86a8f6149e9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) (05f654f) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 05de839 differs from pull request most recent head 33ff3a4. Consider uploading reports for the commit 33ff3a4 to get more accurate results


```diff
@@              Coverage Diff               @@
##           cpp_master    #1018      +/-   ##
==============================================
+ Coverage       85.56%   85.60%   +0.04%     
==============================================
  Files              79       79              
  Lines            5009     5023      +14     
==============================================
+ Hits             4286     4300      +14     
  Misses            723      723              
```

Thank you for updating. Great work!

It seems that CI reports warnings (as errors). Could you fix this?

```
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:[114](https://github.com/msgpack/msgpack-c/runs/6744024570?check_suite_focus=true#step:8:115)3:27: error: implicit conversion from 'unsigned long' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                       ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1143:72: error: implicit conversion from 'uint64_t' (aka 'unsigned long') to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                                                                    ~~ ^~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1146:24: error: implicit conversion from 'long' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                    ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1146:68: error: implicit conversion from 'int64_t' (aka 'long') to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                                                                ~~ ^~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:[116](https://github.com/msgpack/msgpack-c/runs/6744024570?check_suite_focus=true#step:8:117)5:27: error: implicit conversion from 'unsigned long' to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                       ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1165:72: error: implicit conversion from 'uint64_t' (aka 'unsigned long') to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                                                                    ~~ ^~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1168:24: error: implicit conversion from 'long' to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                    ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1168:68: error: implicit conversion from 'int64_t' (aka 'long') to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
```

# [Codecov](https://codecov.io/gh/msgpack/msgpack-c/pull/1018?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) Report
> Merging [#1018](https://codecov.io/gh/msgpack/msgpack-c/pull/1018?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) (05de839) into [cpp_master](https://codecov.io/gh/msgpack/msgpack-c/commit/05f654fd64de96fdb91d043a70de86a8f6149e9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) (05f654f) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 05de839 differs from pull request most recent head 33ff3a4. Consider uploading reports for the commit 33ff3a4 to get more accurate results


```diff
@@              Coverage Diff               @@
##           cpp_master    #1018      +/-   ##
==============================================
+ Coverage       85.56%   85.60%   +0.04%     
==============================================
  Files              79       79              
  Lines            5009     5023      +14     
==============================================
+ Hits             4286     4300      +14     
  Misses            723      723              
```

Thank you for updating. Great work!

It seems that CI reports warnings (as errors). Could you fix this?

```
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:[114](https://github.com/msgpack/msgpack-c/runs/6744024570?check_suite_focus=true#step:8:115)3:27: error: implicit conversion from 'unsigned long' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                       ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1143:72: error: implicit conversion from 'uint64_t' (aka 'unsigned long') to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                                                                    ~~ ^~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1146:24: error: implicit conversion from 'long' to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                    ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1146:68: error: implicit conversion from 'int64_t' (aka 'long') to 'float' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                                                                ~~ ^~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:[116](https://github.com/msgpack/msgpack-c/runs/6744024570?check_suite_focus=true#step:8:117)5:27: error: implicit conversion from 'unsigned long' to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                       ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1165:72: error: implicit conversion from 'uint64_t' (aka 'unsigned long') to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        if(d >= 0 && d <= std::numeric_limits<uint64_t>::max() && d == uint64_t(d)) {
                                                                    ~~ ^~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1168:24: error: implicit conversion from 'long' to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
                    ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/msgpack-c/msgpack-c/include/msgpack/v1/pack.hpp:1168:68: error: implicit conversion from 'int64_t' (aka 'long') to 'double' may lose precision [-Werror,-Wimplicit-int-float-conversion]
        } else if(d >= std::numeric_limits<int64_t>::min() && d == int64_t(d)) {
```

# [Codecov](https://codecov.io/gh/msgpack/msgpack-c/pull/1018?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) Report
> Merging [#1018](https://codecov.io/gh/msgpack/msgpack-c/pull/1018?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) (05de839) into [cpp_master](https://codecov.io/gh/msgpack/msgpack-c/commit/05f654fd64de96fdb91d043a70de86a8f6149e9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=msgpack) (05f654f) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 05de839 differs from pull request most recent head 33ff3a4. Consider uploading reports for the commit 33ff3a4 to get more accurate results


```diff
@@              Coverage Diff               @@
##           cpp_master    #1018      +/-   ##
==============================================
+ Coverage       85.56%   85.60%   +0.04%     
==============================================
  Files              79       79              
  Lines            5009     5023      +14     
==============================================
+ Hits             4286     4300      +14     
  Misses            723      723              
```

Thank you for updating. Great work!


Fix patch:
diff --git a/include/msgpack/v1/pack.hpp b/include/msgpack/v1/pack.hpp
index 41cff85f8..677c39fa1 100644
--- a/include/msgpack/v1/pack.hpp
+++ b/include/msgpack/v1/pack.hpp
@@ -1138,17 +1138,6 @@ inline packer<Stream>& packer<Stream>::pack_unsigned_long_long(unsigned long lon
 template <typename Stream>
 inline packer<Stream>& packer<Stream>::pack_float(float d)
 {
-    if(d == d) { // check for nan
-        // compare d to limits to avoid undefined behaviour
-        if(d >= 0 && d <= float(std::numeric_limits<uint64_t>::max()) && d == float(uint64_t(d))) {
-            pack_imp_uint64(uint64_t(d));
-            return *this;
-        } else if(d < 0 && d >= float(std::numeric_limits<int64_t>::min()) && d == float(int64_t(d))) {
-            pack_imp_int64(int64_t(d));
-            return *this;
-        }
-    }
-
     union { float f; uint32_t i; } mem;
     mem.f = d;
     char buf[5];
@@ -1160,17 +1149,6 @@ inline packer<Stream>& packer<Stream>::pack_float(float d)
 template <typename Stream>
 inline packer<Stream>& packer<Stream>::pack_double(double d)
 {
-    if(d == d) { // check for nan
-        // compare d to limits to avoid undefined behaviour
-        if(d >= 0 && d <= double(std::numeric_limits<uint64_t>::max()) && d == double(uint64_t(d))) {
-            pack_imp_uint64(uint64_t(d));
-            return *this;
-        } else if(d < 0 && d >= double(std::numeric_limits<int64_t>::min()) && d == double(int64_t(d))) {
-            pack_imp_int64(int64_t(d));
-            return *this;
-        }
-    }
-
     union { double f; uint64_t i; } mem;
     mem.f = d;
     char buf[9];


Test patch:
diff --git a/test/msgpack_basic.cpp b/test/msgpack_basic.cpp
index 6026a6e31..24b2a31fc 100644
--- a/test/msgpack_basic.cpp
+++ b/test/msgpack_basic.cpp
@@ -154,8 +154,6 @@ BOOST_AUTO_TEST_CASE(simple_buffer_float)
     v.push_back(-0.0);
     v.push_back(1.0);
     v.push_back(-1.0);
-    v.push_back(1.1f);
-    v.push_back(-1.1f);
     v.push_back(numeric_limits<float>::min());
     v.push_back(numeric_limits<float>::max());
     v.push_back(nanf("tag"));
@@ -188,12 +186,6 @@ BOOST_AUTO_TEST_CASE(simple_buffer_float)
             BOOST_CHECK(std::isinf(val2));
         else
             BOOST_CHECK(fabs(val2 - val1) <= kEPS);
-
-        // check for compact storing of float
-        if (val1 == val1 && val1 >= float(std::numeric_limits<int64_t>::min()) && val1 <= float(std::numeric_limits<int64_t>::max()) && val1 == float(int64_t(val1)))
-            BOOST_REQUIRE_EQUAL(sbuf.size(),1);
-        else
-            BOOST_REQUIRE_EQUAL(sbuf.data()[0],char(0xca));
     }
 }
 
@@ -244,8 +236,6 @@ BOOST_AUTO_TEST_CASE(simple_buffer_double)
     v.push_back(-0.0);
     v.push_back(1.0);
     v.push_back(-1.0);
-    v.push_back(1.1);
-    v.push_back(-1.1);
     v.push_back(numeric_limits<double>::min());
     v.push_back(numeric_limits<double>::max());
     v.push_back(nanf("tag"));
@@ -282,12 +272,6 @@ BOOST_AUTO_TEST_CASE(simple_buffer_double)
             BOOST_CHECK(std::isinf(val2));
         else
             BOOST_CHECK(fabs(val2 - val1) <= kEPS);
-
-        // check for compact storing of double
-        if (val1 == val1 && val1 >= double(std::numeric_limits<int64_t>::min()) && val1 <= double(std::numeric_limits<int64_t>::max()) && val1 == double(int64_t(val1)))
-            BOOST_REQUIRE_EQUAL(sbuf.size(),1);
-        else
-            BOOST_REQUIRE_EQUAL(uint8_t(sbuf.data()[0]),uint8_t(0xcb));
     }
 }