Build fails after changes in modernjson

[volkm]

The update of the modernjson library (see moves-rwth/storm#424) leads to build failures:

error: constexpr variable cannot have non-literal type 'const std::string'

Adding the pybind11_json header file seems to resolve the problem for ValueType double, but not for RationalNumber.

A more general question could be whether we need Python bindings for the storm::json object or whether it would suffice to directly export json as string within C++.

[tquatmann]

Do we get the same error message with pybind11_json included?

std::string becomes a literal type in C++20. There is a realistic chance that this issue will solve itself once we switch.

[volkm]

Using pybind11_json, the issue is fixed for double, but I still get the same error for RationalNumber.

[sjunges]

I believe that it is nice to not have an std::string for a json object and then the need to parse it. In particular, our json objects contain rational numbers and if we cast them to a string and then parse them, they will likely be a float.

So, I guess there is value in having these bindings. Now my main problem is that I do not understand the error message. Looking up solutions for this error message brought me to https://en.cppreference.com/w/cpp/string/basic_string_view, but I dont know what to do with that.

[volkm]

I also saw that we are using the json object quite often in stormpy and not just in a few isolated places (as I initially thought). So I agree that we want to keep this functionality.

I am also not sure how to address this issue. I tried compiling Storm/stormpy with C++20 but this does not make a difference (or I need to do more than set(CMAKE_CXX_STANDARD 20)).
Using pybind11_json could be a promising route, but would introduce more dependencies and we need to adapt it to also handle RationalNumber.

[tquatmann]

I might have found the issue:
On my machine I get (in c++20 mode) the following info:

/Users/tim/stormpy/stormpy/build/temp.macosx-14-arm64-cpython-311/_deps/pybind11-src/include/pybind11/cast.h:1402:39: note: non-constexpr function 'concat<std::string, pybind11::detail::descr<3, nlohmann::basic_json<>>>' cannot be used in a constant expression
    static constexpr auto arg_names = concat(type_descr(make_caster<Args>::name)...);
                                      ^
/Users/tim/storm/resources/3rdparty/modernjson/include/nlohmann/detail/string_concat.hpp:137:22: note: declared here
inline OutStringType concat(Args && ... args)

note that concat is called somewhere within pybind but found somewhere within nlohmann. I think this is just using the wrong concat function.

[volkm]

Ah cool, that seems to be the root cause. Apparently the wrong concat function is used: the one from modernjson and not the one from pybind11. If I add the namespace detail::concat in include/pybind11/cast.h it works for me.

[tquatmann]

Interestingly, the "wrong" concat also seems to be in namespace detail, see here. I wonder what's going on here.

[sjunges]

@volkm Does upgrading it to the latest pybind version fix things?

[volkm]

No, I tried pybind 2.11.1 from July, and it does not make a difference, same with the master branch which also has the same issue.