boostorg / boostorg/multiprecision

boost::mp::number with cpp_dec_float backend and some strange initialization moments and a few strange implementation decisions

Open
#594 22 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
265
Forks
128
Avg merge
4h 48m
Merged PRs (30d)
2

Description

hello guys,

firstly: construction/assignments
let me refer to the [documentation](https://www.boost.org/doc/libs/1_84_0/libs/multiprecision/doc/html/boost_multiprecision/ref/number.html) for `boost::mp::number` class:
```cpp
- number();
- number(see-below);
- number& operator=(see-below);
- number& assign(see-below);

Type number is default constructible, and both copy constructible and assignable from:
- Itself.
- An expression template which is the result of one of the arithmetic operators.
- Any [fundamental (built-in)](https://en.cppreference.com/w/cpp/language/types) arithmetic type, as long as the result would not be lossy (for example float to integer conversion).
- Any type that the Backend is implicitly constructible or assignable from.
- An rvalue reference to another number. Move-semantics are used for construction if the backend also supports rvalue reference construction. In the case of assignment, move semantics are always supported when the argument is an rvalue reference irrespective of the backend.
- Any type in the same family, as long as no loss of precision is involved. For example from int128_t to int256_t, or cpp_dec_float_50 to cpp_dec_float_100.

Type number is explicitly constructible from:
- Any type mentioned above.
- A std::string or any type which is convertible to const char*.
- Any arithmetic type (including those that would result in lossy conversions).
- Any type in the same family, including those that result in loss of precision.
- Any type that the Backend is explicitly constructible from.
- Any pair of types for which a generic interconversion exists: that is from integer to integer, integer to rational, integer to float, rational to rational, rational to float, or float to float.
```
now please look at the [code](https://godbolt.org/z/x3x8E454s):
```cpp
#include

#include
#include

#include

using double_type = boost::multiprecision::number<
boost::multiprecision::cpp_dec_float<8>
,boost::multiprecision::et_off
>;

static const char PI_STR[] = "3.14";

int main() {
// ctor
{
// 0
double_type v = PI_STR; // ERROR
}
{
// 1
const auto *s = PI_STR;
double_type v = s; // ERROR
}
{
// 2
boost::string_view s = PI_STR;
double_type v = s; // ERROR
}
{
// 3
std::string s = PI_STR;
double_type v = s; // ERROR
}

// operator=
{
// 4
double_type v;
v = PI_STR; // ERROR
}
{
// 5
const auto *s = PI_STR;
double_type v;
v = s; // ERROR
}
{
// 6
boost::string_view s = PI_STR;
double_type v;
v = s; // ERROR
}
{
// 7
std::string s = PI_STR;
double_type v;
v = s; // ERROR
}

// assign
{
// 8
double_type v;
v.assign(PI_STR);
}
{
// 9
const auto *s = PI_STR;
double_type v;
v.assign(s);
}
{
// 10
boost::string_view s = PI_STR;
double_type v;
v.assign(s); // ERROR
}
{
// 11
std::string s = PI_STR;
double_type v;
v.assign(s);
}
}

```
as far as you can see the only way to initialize the `number` class is to use the `number::assign()` MF.
**so my first question is**: why? is this a documentation mismatch? or is this a consequence of something being broken?

further about some controversial decisions in terms of optimization.
imagine we end up having to use `number::assign(std::string)`.
in this case we will follow the path: [number::assign(const std::string &)](https://github.com/boostorg/multiprecision/blob/develop/include/boost/multiprecision/number.hpp#L390) -> [number::canonical_value(const std::string &)](https://github.com/boostorg/multiprecision/blob/develop/include/boost/multiprecision/number.hpp#L2236) -> [cpp_dec_float::operator=(const char* v)](https://github.com/boostorg/multiprecision/blob/develop/include/boost/multiprecision/cpp_dec_float.hpp#L401) -> [cpp_dec_float::rd_string(const char* s)](https://github.com/boostorg/multiprecision/blob/develop/include/boost/multiprecision/cpp_dec_float.hpp#L2083).

so, the `cpp_dec_float::rd_string(const char* s)` function is exactly the function that converts a string to the `number`.
OK, the function takes `const char *` as its argument.
BUT, it [creates](https://github.com/boostorg/multiprecision/blob/develop/include/boost/multiprecision/cpp_dec_float.hpp#L2090) a `std::string` object which is initialized with the value of the argument!
that is, we had a `std::string` from which we received `const char *` just to create the `std::string` again!
(in cases where the source string is longer than the SSO buffer - we will also perform additional allocation and deallocation! I'm not even talking about six calls to `std::string::substr()`)

I briefly looked at the implementation of this function it seems to me that it is quite possible to change the implementation to use `boost::string_view` instead of `std::string`.

but there is another problem with this: based [on this line](https://github.com/boostorg/multiprecision/blob/develop/include/boost/multiprecision/cpp_dec_float.hpp#L2104) - this library is meant to be used as the standalone library, and I don't know if `boost::string_view` is included in the package of the standalone library... (ideas?)

I think if I can change the implementation of that function to use `boost::string_view` instead of `std::string`, then it would make sense to change the signature of that function so it accepts `boost::string_view` instead of `const char *` along with the necessary changes regarding the call that function.

ideas?

best!

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.