Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standalone Fixes #403

Merged
merged 27 commits into from
Dec 21, 2021
Merged

Standalone Fixes #403

merged 27 commits into from
Dec 21, 2021

Conversation

mborland
Copy link
Member

Moves the discussion from the now merged #386

@ckormanyos
Copy link
Member

Hi @mborland thanks for creating this PR for further progress.

In our previous thread, you made a comment regarding using a multiprecision type for __int128. I think this could be potentially problematic (although I'm not sure). Problems may arise because the particular type unsigned __int128 is being actually needed as cpp_int's class-internal double-limb-type.

So you might end up using a class that has not even yet been synthesized in order to build up class-internals?

I could be wrong on this one though since i'm not exactly sure how the intent is that you had mentioned.

@ckormanyos
Copy link
Member

ckormanyos commented Dec 11, 2021

Hi @mborland and @jzmaddock

The noexcept culprit seems to have been found. Thanks John!

From our previous chat, @jzmaddock mentions...

perhaps you could test making that method noexcept only when BOOST_NO_CXX17_IF_CONSTEXPR is not defined?

The modification around line 1272 in cpp_int.hpp fixes the noexcept warnings here.

There are a handful of other little changes in my branch for warning fixes on conversion and sign conversion (these are optional). My CI in the unrelated project ran green.

@mborland
Copy link
Member Author

@ckormanyos if you look at the latest commit boost::multiprecision::int128_type is just an alias to __int128. Let me know if it suppresses the warnings for you like it should.

@ckormanyos
Copy link
Member

look at the latest commit boost::multiprecision::int128_type is just an alias to __int128. Let me know if it suppresses the warnings for you like it should

Hi @mborland. Thank you.

Your recent commits eliminate almost all the warnings. For lack of a better communication method, I simply included full warnings under subjection to -Wall -Wextra -pedantic below.

The two cases are:

  • develop branch
  • your standalone branch.

Almost all warnings are gone from your branch with a couple remaining, that I did not, yet at the moment, analyze.

DEVELOP BRANCH

chris@DESKTOP-Q6SS99B:/mnt/c/Users/User/Documents/Ks/PC_Software/Test$ g++ -Wall -Wextra -pedantic -Os -march=native -std=c++11 -I/mnt/c/boost/modular_boost/boost/libs/multiprecision/include -I/mnt/c/boost/modular_boost/boost/libs/math/include -I/mnt/c/boost/modular_boost/boost/libs/config/include ./test.cpp -o test.exe
In file included from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/number_base.hpp:18,
                 from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/is_variable_precision.hpp:9,
                 from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/precision.hpp:9,
                 from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/number.hpp:11,
                 from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/cpp_dec_float.hpp:29,
                 from ./test.cpp:61:
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:31:18: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   31 | struct is_signed<__int128> : public std::integral_constant<bool, true> {};
      |                  ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:33:27: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   33 | struct is_signed<unsigned __int128> : public std::integral_constant<bool, false> {};
      |                           ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:35:20: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   35 | struct is_unsigned<__int128> : public std::integral_constant<bool, false> {};
      |                    ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:37:29: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   37 | struct is_unsigned<unsigned __int128> : public std::integral_constant<bool, true> {};
      |                             ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:39:20: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   39 | struct is_integral<__int128> : public std::integral_constant<bool, true> {};
      |                    ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:41:29: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   41 | struct is_integral<unsigned __int128> : public std::integral_constant<bool, true> {};
      |                             ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:43:22: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   43 | struct is_arithmetic<__int128> : public std::integral_constant<bool, true> {};
      |                      ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:45:31: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   45 | struct is_arithmetic<unsigned __int128> : public std::integral_constant<bool, true> {};
      |                               ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:47:22: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   47 | struct make_unsigned<__int128>
      |                      ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:49:26: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   49 |    using type = unsigned __int128;
      |                          ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:52:31: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   52 | struct make_unsigned<unsigned __int128>
      |                               ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:54:26: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   54 |    using type = unsigned __int128;
      |                          ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:57:20: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   57 | struct make_signed<__int128>
      |                    ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:59:17: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   59 |    using type = __int128;
      |                 ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:62:29: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   62 | struct make_signed<unsigned __int128>
      |                             ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/traits/std_integer_traits.hpp:64:17: warning:
ISO C++ does not support__int128fortype name’ [-Wpedantic]
   64 |    using type = __int128;
      |                 ^~~~~~~~
In file included from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/generic_interconvert.hpp:9,
                 from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/number.hpp:12,
                 from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/cpp_dec_float.hpp:29,
                 from ./test.cpp:61:
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/default_ops.hpp: In functionvoid boost::multiprecision::default_ops::eval_karatsuba_sqrt(Backend&, const Backend&, Backend&, Backend&, size_t)’:
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/default_ops.hpp:1691:25: warning: ISO C++ does not support__int128fora’ [-Wpedantic]
 1691 |       unsigned __int128 a{}, b{}, c{};
      |                         ^
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/default_ops.hpp:1691:30: warning: ISO C++ does not support__int128forb’ [-Wpedantic]
 1691 |       unsigned __int128 a{}, b{}, c{};
      |                              ^
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/default_ops.hpp:1691:35: warning: ISO C++ does not support__int128forc’ [-Wpedantic]
 1691 |       unsigned __int128 a{}, b{}, c{};
      |                                   ^
In file included from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/number.hpp:16,
                 from /mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/cpp_dec_float.hpp:29,
                 from ./test.cpp:61:
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/hash.hpp: At global scope:
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/hash.hpp:23:49: warning: ISO C++ does not support__int128forval’ [-Wpedantic]
   23 | std::size_t hash_value(const unsigned __int128& val);
      |                                                 ^~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/hash.hpp:25:47: warning: ISO C++ does not support__int128forval’ [-Wpedantic]
   25 | inline std::size_t hash_value(const __int128& val)
      |                                               ^~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/hash.hpp: In functionstd::size_t boost::multiprecision::detail::hash_value(const __int128&)’:
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/hash.hpp:27:43: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   27 |    return hash_value(static_cast<unsigned __int128>(val));
      |                                           ^~~~~~~~
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/hash.hpp: At global scope:
/mnt/c/boost/modular_boost/boost/libs/multiprecision/include/boost/multiprecision/detail/hash.hpp:44:56: warning: ISO C++ does not support__int128forval’ [-Wpedantic]
   44 | inline std::size_t hash_value(const unsigned __int128& val)
      |                                                        ^~~
chris@DESKTOP-Q6SS99B:/mnt/c/Users/User/Documents/Ks/PC_Software/Test$

STANDALONE BRANCH

chris@DESKTOP-Q6SS99B:~$ cd /mnt/c/Users/User/Documents/Ks/PC_Software/Test
chris@DESKTOP-Q6SS99B:/mnt/c/Users/User/Documents/Ks/PC_Software/Test$ g++ -Wall -Wextra -pedantic -Os -march=native -std=c++11 -I/mnt/c/boost/boost_multiprecision_standalone/include -I/mnt/c/boost/modular_boost/boost/libs/math/include -I/mnt/c/boost/modular_boost/boost/libs/config/include ./test.cpp -o test.exe
In file included from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/generic_interconvert.hpp:9,
                 from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/number.hpp:12,
                 from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_dec_float.hpp:29,
                 from ./test.cpp:61:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/default_ops.hpp: In functionvoid boost::multiprecision::default_ops::eval_karatsuba_sqrt(Backend&, const Backend&, Backend&, Backend&, size_t)’:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/default_ops.hpp:1691:25: warning: ISO C++ does not support__int128fora’ [-Wpedantic]
 1691 |       unsigned __int128 a{}, b{}, c{};
      |                         ^
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/default_ops.hpp:1691:30: warning: ISO C++ does not support__int128forb’ [-Wpedantic]
 1691 |       unsigned __int128 a{}, b{}, c{};
      |                              ^
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/default_ops.hpp:1691:35: warning: ISO C++ does not support__int128forc’ [-Wpedantic]
 1691 |       unsigned __int128 a{}, b{}, c{};
      |                                   ^
In file included from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/number.hpp:16,
                 from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_dec_float.hpp:29,
                 from ./test.cpp:61:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/hash.hpp: At global scope:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/hash.hpp:23:49: warning: ISO C++ does not support__int128forval’ [-Wpedantic]
   23 | std::size_t hash_value(const unsigned __int128& val);
      |                                                 ^~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/hash.hpp:25:47: warning: ISO C++ does not support__int128forval’ [-Wpedantic]
   25 | inline std::size_t hash_value(const __int128& val)
      |                                               ^~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/hash.hpp: In functionstd::size_t boost::multiprecision::detail::hash_value(const __int128&)’:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/hash.hpp:27:43: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   27 |    return hash_value(static_cast<unsigned __int128>(val));
      |                                           ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/hash.hpp: At global scope:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/hash.hpp:44:56: warning: ISO C++ does not support__int128forval’ [-Wpedantic]
   44 | inline std::size_t hash_value(const unsigned __int128& val)
      |                                                        ^~~
chris@DESKTOP-Q6SS99B:/mnt/c/Users/User/Documents/Ks/PC_Software/Test$

@ckormanyos
Copy link
Member

ckormanyos commented Dec 12, 2021

Thnks @mborland.

Sorry Matt, I missed a few __int128 warnings on -pedantic. My last report was for cpp_dec_float.

I have found the following shown below for cpp_bin_float. Note that this should cover now dec-float, bin-float and cpp_int as well. (I'll run gmp_float and mpfr_floatthrough ASAP.)

STANDALONE BRANCH with cpp_bin_float

chris@DESKTOP-Q6SS99B:~$ cd /mnt/c/Users/User/Documents/Ks/PC_Software/Test
chris@DESKTOP-Q6SS99B:/mnt/c/Users/User/Documents/Ks/PC_Software/Test$ g++ -Wall -Wextra -pedantic -Os -march=native -std=c++11 -I/mnt/c/boost/boost_multiprecision_standalone/include -I/mnt/c/boost/modular_boost/boost/libs/math/include -I/mnt/c/boost/modular_boost/boost/libs/config/include ./test.cpp -o test.exe
chris@DESKTOP-Q6SS99B:/mnt/c/Users/User/Documents/Ks/PC_Software/Test$ g++ -Wall -Wextra -pedantic -Os -march=native -std=c++11 -I/mnt/c/boost/boost_multiprecision_standalone/include -I/mnt/c/boost/modular_boost/boost/libs/math/include -I/mnt/c/boost/modular_boost/boost/libs/config/include ./test.cpp -o test.exe
./test.cpp:10:1: warning: multi-line comment [-Wcomment]
   10 | //TGT_INCLUDES  = -IC:/boost/modular_boost/boost/libs/multiprecision/include        \
      | ^
In file included from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/endian.hpp:9,
                 from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int.hpp:14,
                 from /mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_bin_float.hpp:11,
                 from ./test.cpp:60:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp: In function ‘constexpr T boost::multiprecision::backends::detail::type_max()’:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:82:25: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   82 | #    define INT128_MAX (__int128) (((unsigned __int128) 1 << ((__SIZEOF_INT128__ * __CHAR_BIT__) - 1)) - 1)
      |                         ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp:29:65: note: in expansion of macroINT128_MAX29 |    std::is_same<T, boost::multiprecision::int128_type>::value ? INT128_MAX :
      |                                                                 ^~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:82:47: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   82 | #    define INT128_MAX (__int128) (((unsigned __int128) 1 << ((__SIZEOF_INT128__ * __CHAR_BIT__) - 1)) - 1)
      |                                               ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp:29:65: note: in expansion of macroINT128_MAX29 |    std::is_same<T, boost::multiprecision::int128_type>::value ? INT128_MAX :
      |                                                                 ^~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:88:41: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   88 | #    define UINT128_MAX ((2 * (unsigned __int128) INT128_MAX) + 1)
      |                                         ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp:30:66: note: in expansion of macroUINT128_MAX30 |    std::is_same<T, boost::multiprecision::uint128_type>::value ? UINT128_MAX :
      |                                                                  ^~~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:82:25: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   82 | #    define INT128_MAX (__int128) (((unsigned __int128) 1 << ((__SIZEOF_INT128__ * __CHAR_BIT__) - 1)) - 1)
      |                         ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:88:51: note: in expansion of macroINT128_MAX88 | #    define UINT128_MAX ((2 * (unsigned __int128) INT128_MAX) + 1)
      |                                                   ^~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp:30:66: note: in expansion of macroUINT128_MAX30 |    std::is_same<T, boost::multiprecision::uint128_type>::value ? UINT128_MAX :
      |                                                                  ^~~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:82:47: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   82 | #    define INT128_MAX (__int128) (((unsigned __int128) 1 << ((__SIZEOF_INT128__ * __CHAR_BIT__) - 1)) - 1)
      |                                               ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:88:51: note: in expansion of macroINT128_MAX88 | #    define UINT128_MAX ((2 * (unsigned __int128) INT128_MAX) + 1)
      |                                                   ^~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp:30:66: note: in expansion of macroUINT128_MAX30 |    std::is_same<T, boost::multiprecision::uint128_type>::value ? UINT128_MAX :
      |                                                                  ^~~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp: In function ‘constexpr T boost::multiprecision::backends::detail::type_min()’:
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:82:25: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   82 | #    define INT128_MAX (__int128) (((unsigned __int128) 1 << ((__SIZEOF_INT128__ * __CHAR_BIT__) - 1)) - 1)
      |                         ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:85:26: note: in expansion of macroINT128_MAX85 | #    define INT128_MIN (-INT128_MAX - 1)
      |                          ^~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp:40:65: note: in expansion of macroINT128_MIN40 |    std::is_same<T, boost::multiprecision::int128_type>::value ? INT128_MIN :
      |                                                                 ^~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:82:47: warning: ISO C++ does not support__int128fortype name’ [-Wpedantic]
   82 | #    define INT128_MAX (__int128) (((unsigned __int128) 1 << ((__SIZEOF_INT128__ * __CHAR_BIT__) - 1)) - 1)
      |                                               ^~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/detail/standalone_config.hpp:85:26: note: in expansion of macroINT128_MAX85 | #    define INT128_MIN (-INT128_MAX - 1)
      |                          ^~~~~~~~~~
/mnt/c/boost/boost_multiprecision_standalone/include/boost/multiprecision/cpp_int/checked.hpp:40:65: note: in expansion of macroINT128_MIN40 |    std::is_same<T, boost::multiprecision::int128_type>::value ? INT128_MIN :
      |                                                                 ^~~~~~~~~~

@ckormanyos
Copy link
Member

ckormanyos commented Dec 13, 2021

Hi folks, this is moving along well.

There are several issues I'd like to highlight and get feedback on.

  1. Are we testing standalone? I remember some comment(s) from @jzmaddock but did not understand the exact depth or meaning thereof. So @mborland do you think we have explicit standalone tests in the branch and/or on develop?
  2. When embedding cpp_dec_float versus cpp_bin_float, I find that bin-float adds about 200 additional kilobytes of program code almost entirely needed for string-streaming (via std::stringstream). Since my to total benchmark size (101 decimal digit cube root) is less than 100 kilobyte program code, adding 200 kilobyte for string-streaming is exorbitant. The heavy weight comes from reading string limbs and converting them to cpp_int. I'd suggest we try to replace std::stringstream-ing with lightweight std::stoul() or similar as we did in cpp_dec_float.
  3. Now that standalone can basically, ... stand alone, it would be really helpful to ramp up the warning level. i have good experiences with adding GCC's -Wconversion and -Wsign-conversion. This results in about 25 warnings in dec-float and fewer than 50 warnings in bin-float. I would like to pursue resolution of these warnings since both i as well as a couple folks I know use such warnings and perpetually need to disable them for Boost. Are we agreed should I repair them? Or should I let them go?
  4. I will finish searching for -pedantic ISO-C++ non-conformace resulting from __int128. I will report thest presently.

Thoughts... Suggestions... ?

Cc: @mborland and @jzmaddock and @NAThompson

@mborland
Copy link
Member Author

@ckormanyos I am working on re-writing test_arithmetic to contextually disable any boost usage. That will resolve the standalone testing problem I think.

@ckormanyos
Copy link
Member

working on re-writing test_arithmetic to contextually disable any boost usage. That will resolve the standalone testing problem I think.

Great! Thanks @mborland. As soon as this is/will be running, I think we have a really good standpoint on standalone.

Optionally... I will take a look at remaining __int128 warnings, and check out any other elevated conversion warnings. i will also investigate the situation with string-streaming.

Again, this stuff is mostly optional and not needed for standalone (with the exception of the __int128 warnings on -pedantic that we did identify as a goal). So I will get back to you if any changes are required on that detail.

The higher conversin warnings and string-streaming investigations are not required but I will try to find a chance to look into them anyway.

@ckormanyos
Copy link
Member

Hi @mborland I did a quick check of bin-float, dec-float, gmp-float and mpfr-float.

Many thanks. I find in this quick check that the all formerly remaining __int128 warnings on -pedantic are now gone.

@ckormanyos
Copy link
Member

ckormanyos commented Dec 18, 2021

Hi @mborland we might want to merge from develop again. We observe that @jzmaddock is excellently handling issue-after-issue.

When I merged develop over to my standalone_chris branch, I found a merge conflict or two in misc.hpp. In addition to all the changes from mborland:standalone, I did, however, also have a small number of changes handling conversion warnings in cpp_dec_float. So that might have complicated my particular merge.

It might be easier to merge to mborland:standalone_fixes directly from develop first --- or optionally from my branch remotely. Either way, the few changes I had/have for -Wconversion and -Wsign-conversion I would also be happy to reproduce any time if needed.

@mborland
Copy link
Member Author

@jzmaddock Good call. Ran the new gmp_float to long double conversion through valgrind to validate the change.

==101192== Memcheck, a memory error detector
==101192== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==101192== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==101192== Command: ./log_adptr --leak-check=full
==101192== 
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and h
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and a
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and a
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and c
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and c
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and s
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and s
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and t
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and i
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and i
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and j
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and l
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and l
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and m
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and x
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and x
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and y
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and f
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and f
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and d
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and d
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and e
Testing mixed arithmetic with type: N5boost14multiprecision6numberINS0_8backends14logged_adaptorINS2_9gmp_floatILj0EEEEELNS0_26expression_template_optionE1EEE and e
No errors detected.
==101192== 
==101192== HEAP SUMMARY:
==101192==     in use at exit: 0 bytes in 0 blocks
==101192==   total heap usage: 13,335 allocs, 13,335 frees, 607,350 bytes allocated
==101192== 
==101192== All heap blocks were freed -- no leaks are possible
==101192== 
==101192== For lists of detected and suppressed errors, rerun with: -s
==101192== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Will fix the gmp_int conversions.

@mborland mborland marked this pull request as ready for review December 21, 2021 09:36
@mborland
Copy link
Member Author

@ckormanyos Do you see any large or outstanding issues that we should address here? CI is back to green with all these changes.

@ckormanyos
Copy link
Member

ckormanyos commented Dec 21, 2021

This is great @mborland !

Do you see any large or outstanding issues that we should address here? CI is back to green with all these changes.

Functionally, all is OK.

One thing you might want, ... or freely choose to disregard...

I just did commit (via ci skip in my standalone_chris branch) a bunch of changes to handle conversion warnings in cpp_bin_float. Some of the casts I selected were quite difficult to choose due to all the massively templated code. I also added missing default cases to switches. If you would like to take a look at the casts and added default cases missing from switches, we could consider integrating my changes.

It's your choice if you'd like to look at and potentially accept these corrections to conversoin warnings or not. If not, I am good to go at this point with the excellent state we have reached.

If we decide to accept my corrections to conversoin warnings in Multiprecision, there is an associated little list of changes needed in Math that I can also present.

Cc: @jzmaddock

@ckormanyos
Copy link
Member

ckormanyos commented Dec 21, 2021

CI is back to green with all these changes.

my corrections to conversoin warnings

Ummm... Alternatively, we could merge to develop immediately. If I would want to to work on elevated warning levels on certain compilers, I should be happy to do that in future branches.

This would be more consistent with the topic of this PR which is going standalone.

@mborland and @jzmaddock thoughts...?

Cc: @NAThompson

ss >> d;

return d;
return std::strtod(str(std::numeric_limits<double>::digits10 + (2 + 1), std::ios_base::scientific).c_str(), nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pain in the butt, but can you check that this doesn't re-awaken this issue: #167. The problem is that the global locale can sometimes interpret strings in mighty weird ways :(

@jzmaddock
Copy link
Collaborator

Ummm... Alternatively, we could merge to develop immediately. If I would want to to work on elevated warning levels on certain compilers, I should be happy to do that in future branches.

+1, otherwise the diffs get too big!

@ckormanyos ckormanyos merged commit b034196 into boostorg:develop Dec 21, 2021
@ckormanyos
Copy link
Member

merge to develop immediately

+1, otherwise the diffs get too big!

Done. Merged to develop. Review comments to be handled in a future PR.

@jzmaddock
Copy link
Collaborator

Review comments to be handled in a future PR.

OK, but some of those look like proper breakages to me.

@ckormanyos
Copy link
Member

Review comments to be handled in a future PR.

OK, but some of those look like proper breakages to me.

Uggghhh. Sorry that was not the intent and I had not realized that. I guess my intensity in the drive toward standalone got the better of me. I'd need help on some of those details.

@mborland and @jzmaddock should we move forward with next PR or should I revert the changes?

Sorry for any disruption I caused.

@jzmaddock
Copy link
Collaborator

@mborland and @jzmaddock should we move forward with next PR or should I revert the changes?

I think I would vote for reverting.

Sorry for any disruption I caused.

No worries!

@mborland
Copy link
Member Author

@ckormanyos I go on holiday starting tonight so I concur with reverting the merge.

ckormanyos added a commit that referenced this pull request Dec 22, 2021
This reverts commit b034196, reversing
changes made to 75b17b9.
@ckormanyos
Copy link
Member

vote for reverting

concur with reverting the merge

OK. Agreed. With one episode of back and forth, I have reverted to 75b17b9

This includes John's fix of #407 but reverts my recent commit/push from Matt's standalone_fixes branch. Due to the one episode of back-and-forth, I inadvertently fired off three CI runs.

I was a bit shaky on the revert syntax, but i'm pretty sure I hit the right commit now. Could you please check @mborland and @jzmaddock ?

Thank you.

@jzmaddock
Copy link
Collaborator

@ckormanyos
Copy link
Member

I think you got it

OK, Cool --- back on track.

Thanks John. Thanks Matt. Let's pick this up maybe after the holidays where we left off...

@mborland mborland deleted the standalone_fixes branch January 2, 2022 12:24
mborland added a commit to mborland/multiprecision that referenced this pull request Jan 2, 2022
@mborland mborland mentioned this pull request Jan 2, 2022
mborland added a commit to mborland/multiprecision that referenced this pull request Jan 6, 2022
@ckormanyos ckormanyos mentioned this pull request Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants