boost::safe_numerics::safe<...> is convertible to std::string?

[usefulcat]

Is it intentional that the following code does not produce an error when compiled?

#include <boost/safe_numerics/safe_integer.hpp>
#include <type_traits>
#include <string>

static_assert(std::is_convertible_v<boost::safe_numerics::safe<int>, std::string>);

It seems wrong, but maybe there is a reason for it?

I'm trying to track down a (to me) relatively obscure problem elsewhere, and I think this may be at least related to, if not the cause of the problems I'm seeing. I checked the two most recent versions of clang and gcc and got the same behavior in all cases.

[robertramey]

what file is this code in? I don't find it anywhere in the codebase

[usefulcat]

The code I presented isn't in the codebase; it's an example of code that uses boost.safe_numerics. My question is whether it is intentional and/or necessary for boost::safe_numerics::safe types to be convertible to std::string. It seems wrong that std::is_convertible<boost::safe_numerics::safe<int>, std::string>::value should be true. Why should a safe<int> automatically be convertible to std::string?

[robertramey]

Hmmm - one would have to check the rest of the code. If somewhere there is a function which converts and integer or safe_integer into a string, the static_assert value would be true. Look for code which does such a conversion. Assuming one fines such code, I would consider it bad practice.

[usefulcat]

I'm afraid you misunderstand. The following code, which is a complete example, does not compile:

#include <boost/safe_numerics/safe_integer.hpp>
#include <type_traits>
#include <string>

static_assert(std::is_convertible_v<boost::safe_numerics::safe<int>, std::string> == false);

int main() {
    return 0;
}

[usefulcat]

After further investigation, I believe this method from safe_base is the culprit:

    /////////////////////////////////////////////////////////////////
    // casting operators for intrinsic integers
    // convert to any type which is not safe.  safe types need to be
    // excluded to prevent ambiguous function selection which
    // would otherwise occur.  validity of safe types is checked in
    // the constructor of safe types
    template<
        class R,
        typename std::enable_if<
            ! boost::safe_numerics::is_safe<R>::value,
            int
        >::type = 0
    >
    constexpr /*explicit*/ operator R () const;

If I understand this correctly, this method allows the compiler to attempt to convert an instance of safe_base to std::string, or for that matter to any type that isn't itself some flavor of safe_base.

If I change the enable_if as follows (and do the same for the method body in safe_base_operations.hpp), std::is_convertible no longer reports that safe_base is convertible to std::string:

    template<
        class R,
        typename std::enable_if<
            ! boost::safe_numerics::is_safe<R>::value
            && std::is_integral<R>::value,
            int
        >::type = 0
    >
    constexpr /*explicit*/ operator R () const;

This change seems reasonable, since I thought that boost::safe_numerics was only intended to be used with integral types, but I freely admit that I'm not remotely an expert in this particular area.

[robertramey]

Looks like you've nailed it! Good work! I have to think about the remedy though. Actually I have to think about why that template is in there at all! And the comment isn't particularly helpful. I'd be inclined to comment out that code and re-run the whole test/example suite in order to figure out the true motivation for putting this in there. Thanks for your efforts.

safe numerics is currently implemented only for integral types. I'd like to do a fine tuning/improvement so that it support floating point types as well as other arithmetic types.

[correaa]

I think the current behavior consistent, in terms of substitutability of int with safe<int> It is not an ideal situation, but that is a defect of std::string.

#include <string>

#include <boost/safe_numerics/safe_integer.hpp>

void f(std::string);

int main() {
    static_assert(std::is_convertible_v<int, std::string>);

    int a = 0;
    // f(a);  // error, good
    f(std::string{a});  // accepted, narrowing conversion warning

    static_assert(std::is_convertible_v<boost::safe_numerics::safe<int>, std::string>);

    boost::safe_numerics::safe<int> as = a;
    // f(as);  // error, good, although deep in the library
    f(std::string{as});  // accepted, no warning
}

The only QoI issue is that 1) the error (of f(as) happens deep in the library). 2) The warning -Wnarrowing doesn't happen at all f(std::string{as}) even with warns active.

1. can be solved by adding more SFINAE conditions here

    template<
        class R,
        typename std::enable_if<
            ! boost::safe_numerics::is_safe<R>::value
            && std::is_integral<R>::value,
            int
        >::type = 0
    >
    constexpr /*explicit*/ operator R () const;

I particular adding `enable_if< ... , std::is_convertible<Stored, R>, ...>, or something like it. In this case, the error will move from deep inside the library to the interface.

[usefulcat]

I don't think this problem has anything to do with std::string. I just used that as what I thought would be an obvious example of this feature not working as intended. Here's a (hopefully) more obvious example:

class Foo {};
static_assert(std::is_convertible_v<boost::safe_numerics::safe<int>, Foo> == false);

IMO that should compile (is_convertible_v should be false) but it doesn't.

The problem I ran into has to do with writing custom formatters for fmtlib, and in the case of a safe_numerics::safe type, fmtlib is unable to figure out which overload (or specialization--I forget) to use because, as currently implemented, safe_numerics::safe::operator R() says that the compiler is allowed to attempt to convert it to any non-safe_numerics type. Which is, to say the very least, extremely surprising.

Ultimately I ended up significantly reducing my usage of safe_numerics since that was the only solution I could find that didn't require me to maintain my own fork of the library.