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

Perplexing template deduction failure serialising a 3rd party type using base class #3267

Closed
willat343 opened this issue Jan 13, 2022 · 14 comments · Fixed by #3427
Closed

Perplexing template deduction failure serialising a 3rd party type using base class #3267

willat343 opened this issue Jan 13, 2022 · 14 comments · Fixed by #3427
Assignees
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@willat343
Copy link

willat343 commented Jan 13, 2022

I know (and have checked with a basic code example) that if a serialisation is defined for a base class/struct, then derived classes can be serialised too without additional code (the derived classes are equivalent to the base in the json). For example the following code works as intended:

#include <iostream>
struct A { int i; };
struct B : public A { void do_something(){} };

void to_json(nlohmann::json& j, const A& a) {
    j["a"] = a.i;
}

void from_json(const nlohmann::json& j, A& a) {
    j.at("a").get_to(a.i);
}

int main() {
     B b;
     b.i = 6;
     nlohmann::json j = b;
     std::cerr << j.dump() << "\n";
     B b2 = j.get<B>();
     if (b2.i == b.i) {
         std::cerr << "correctly deserialised!\n";
     }
     return 0;
}

I am facing an equivalent issue with a 3rd party type, in particular Eigen matrices/vectors, which seems to have nothing to do with the contents of the serialisation functions themselves, but the deduction that these to_json/from_json functions need to be used in the template deduction. The following code (which i do not want to use for reasons I will discuss in a moment) is tested and works as intended:

// header file
namespace Eigen {
    template<typename Scalar, int Rows, int Cols>
    void to_json(nlohmann::json& j, const Matrix<Scalar, Rows, Cols>& matrix) {
        for (int row = 0; row < matrix.rows(); ++row) {
            nlohmann::json column = nlohmann::json::array();
            for (int col = 0; col < matrix.cols(); ++col) {
                column.push_back(matrix(row, col));
            }
            j.push_back(column);
        }
    }

    template<typename Scalar, int Rows, int Cols>
    void from_json(const nlohmann::json& j, Matrix<Scalar, Rows, Cols>& matrix) {        
        for (std::size_t row = 0; row < j.size(); ++row) {
            const auto& jrow = j.at(row);
            for (std::size_t col = 0; col < jrow.size(); ++col) {
                const auto& value = jrow.at(col);
                value.get_to(matrix(row, col));
            }
        }
    }
}

// source file
int main() {
    Eigen::Matrix4f m;
    m <<  1.0,   2.0,   3.0,   4.0,
             11.0, 12.0, 13.0, 14.0,
             21.0, 22.0, 23.0, 24.0,
             31.0, 32.0, 33.0, 34.0;
    nlohmann::json j = m;
    std::cerr << j.dump() << std::endl;
    auto m2 = j.get<Eigen::Matrix4f>();
    std::cerr << m2 << "\n";
    return 0;
}

(note that Matrix4f is a typedef for Matrix<float, 4, 4>)

As described in their documentation (https://eigen.tuxfamily.org/dox/TopicFunctionTakingEigenTypes.html), it is more general and powerful to templatise according to an appropriate base class which will permit serialisation of a wider group of eigen structures. Noting that Matrix4f inherits from MatrixBase<Matrix4f>, the following serialisation functions should work identically:

namespace Eigen {
template<typename Derived>
    void to_json(nlohmann::json& j, const MatrixBase<Derived>& matrix) {
        for (int row = 0; row < matrix.rows(); ++row) {
            nlohmann::json column = nlohmann::json::array();
            for (int col = 0; col < matrix.cols(); ++col) {
                column.push_back(matrix(row, col));
            }
            j.push_back(column);
        }
    }

    template<typename Derived>
    void from_json(const nlohmann::json& j, MatrixBase<Derived>& matrix) {        
        for (std::size_t row = 0; row < j.size(); ++row) {
            const auto& jrow = j.at(row);
            for (std::size_t col = 0; col < jrow.size(); ++col) {
                const auto& value = jrow.at(col);
                value.get_to(matrix(row, col));
            }
        }
    }
}

This does not compile, with the first part of the error log (to me) showing that it is not deducing that it should use these serialisation functions, but instead trying to use an array type converter (void nlohmann::detail::to_json(BasicJsonType&, const CompatibleArrayType&) [with BasicJsonType = nlohmann::basic_json<>; CompatibleArrayType = Eigen::Matrix<float, 4, 4>...):

In file included from /home/williamtalbot/temp/eigenjsontest/eigenjsontest.cpp:1:0:
/usr/local/include/nlohmann/json.hpp: In instantiation of ‘static void nlohmann::detail::external_constructor<(nlohmann::detail::value_t)2>::construct(BasicJsonType&, const CompatibleArrayType&) [with BasicJsonType = nlohmann::basic_json<>; CompatibleArrayType = Eigen::Matrix<float, 4, 4>; typename std::enable_if<(! std::is_same<CompatibleArrayType, typename BasicJsonType::array_t>::value), int>::type <anonymous> = 0]’:
/usr/local/include/nlohmann/json.hpp:4276:52:   required from ‘void nlohmann::detail::to_json(BasicJsonType&, const CompatibleArrayType&) [with BasicJsonType = nlohmann::basic_json<>; CompatibleArrayType = Eigen::Matrix<float, 4, 4>; typename std::enable_if<((((nlohmann::detail::is_compatible_array_type<BasicJsonType, CompatibleArrayType>::value && (! nlohmann::detail::is_compatible_object_type<BasicJsonType, CompatibleObjectType>::value)) && (! nlohmann::detail::is_compatible_string_type<BasicJsonType, ConstructibleStringType>::value)) && (! std::is_same<typename BasicJsonType::binary_t, CompatibleArrayType>::value)) && (! nlohmann::detail::is_basic_json<T>::value)), int>::type <anonymous> = 0]’
/usr/local/include/nlohmann/json.hpp:4353:23:   required from ‘decltype ((nlohmann::detail::to_json(j, forward<T>(val)), void())) nlohmann::detail::to_json_fn::operator()(BasicJsonType&, T&&) const [with BasicJsonType = nlohmann::basic_json<>; T = Eigen::Matrix<float, 4, 4>&; decltype ((nlohmann::detail::to_json(j, forward<T>(val)), void())) = void]’
/usr/local/include/nlohmann/json.hpp:4403:28:   required from ‘static decltype ((nlohmann::{anonymous}::to_json(j, forward<ValueType>(val)), void())) nlohmann::adl_serializer<T, SFINAE>::to_json(BasicJsonType&, ValueType&&) [with BasicJsonType = nlohmann::basic_json<>; ValueType = Eigen::Matrix<float, 4, 4>&; <template-parameter-1-1> = Eigen::Matrix<float, 4, 4>; <template-parameter-1-2> = void; decltype ((nlohmann::{anonymous}::to_json(j, forward<ValueType>(val)), void())) = void]’
/usr/local/include/nlohmann/json.hpp:17931:35:   required from ‘nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>::basic_json(CompatibleType&&) [with CompatibleType = Eigen::Matrix<float, 4, 4>&; U = Eigen::Matrix<float, 4, 4>; typename std::enable_if<((! nlohmann::detail::is_basic_json<U>::value) && nlohmann::detail::is_compatible_type<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, U>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; BinaryType = std::vector<unsigned char>]’
/home/williamtalbot/temp/eigenjsontest/eigenjsontest.cpp:42:24:   required from here
/usr/local/include/nlohmann/json.hpp:4143:25: error: invalid use of void expression
         j.m_value.array = j.template create<typename BasicJsonType::array_t>(begin(arr), end(arr));
/usr/local/include/nlohmann/json.hpp: In instantiation of ‘void nlohmann::detail::from_json_array_impl(const BasicJsonType&, ConstructibleArrayType&, nlohmann::detail::priority_tag<0>) [with BasicJsonType = nlohmann::basic_json<>; ConstructibleArrayType = Eigen::Matrix<float, 4, 4>]’:
/usr/local/include/nlohmann/json.hpp:3648:25:   required from ‘decltype (((nlohmann::detail::from_json_array_impl(j, arr, nlohmann::detail::priority_tag<3>{}), j.get<typename ConstructibleArrayType::value_type>()), void())) nlohmann::detail::from_json(const BasicJsonType&, ConstructibleArrayType&) [with BasicJsonType = nlohmann::basic_json<>; ConstructibleArrayType = Eigen::Matrix<float, 4, 4>; typename std::enable_if<((((nlohmann::detail::is_constructible_array_type<BasicJsonType, ConstructibleArrayType>::value && (! nlohmann::detail::is_constructible_object_type<BasicJsonType, ConstructibleObjectType>::value)) && (! nlohmann::detail::is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value)) && (! std::is_same<ConstructibleArrayType, typename BasicJsonType::binary_t>::value)) && (! nlohmann::detail::is_basic_json<T>::value)), int>::type <anonymous> = 0; decltype (((nlohmann::detail::from_json_array_impl(j, arr, nlohmann::detail::priority_tag<3>{}), j.get<typename ConstructibleArrayType::value_type>()), void())) = void; typename ConstructibleArrayType::value_type = float]’
/usr/local/include/nlohmann/json.hpp:3791:25:   required from ‘decltype ((nlohmann::detail::from_json(j, val), void())) nlohmann::detail::from_json_fn::operator()(const BasicJsonType&, T&) const [with BasicJsonType = nlohmann::basic_json<>; T = Eigen::Matrix<float, 4, 4>; decltype ((nlohmann::detail::from_json(j, val), void())) = void]’
/usr/local/include/nlohmann/json.hpp:4386:30:   required from ‘static decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) nlohmann::adl_serializer<T, SFINAE>::from_json(BasicJsonType&&, ValueType&) [with BasicJsonType = const nlohmann::basic_json<>&; ValueType = Eigen::Matrix<float, 4, 4>; <template-parameter-1-1> = Eigen::Matrix<float, 4, 4>; <template-parameter-1-2> = void; decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) = void]’
/usr/local/include/nlohmann/json.hpp:19421:45:   required from ‘ValueType nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>::get() const [with ValueTypeCV = Eigen::Matrix<float, 4, 4>; ValueType = Eigen::Matrix<float, 4, 4>; typename std::enable_if<(((! nlohmann::detail::is_basic_json<U>::value) && nlohmann::detail::has_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, ValueType>::value) && (! nlohmann::detail::has_non_default_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, ValueType>::value)), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; BinaryType = std::vector<unsigned char>]’
/home/williamtalbot/temp/eigenjsontest/eigenjsontest.cpp:43:38:   required from here
/usr/local/include/nlohmann/json.hpp:3619:42: error: invalid use of void expression
         j.begin(), j.end(), std::inserter(ret, end(ret)),

If I static cast to the base class, to_json works (although this is obviously not a solution):

    nlohmann::json j = static_cast<Eigen::MatrixBase<Eigen::Matrix4f>&>(m);
    std::cerr << j.dump() << std::endl;

yeilds [[1.0,2.0,3.0,4.0],[11.0,12.0,13.0,14.0],[21.0,22.0,23.0,24.0],[31.0,32.0,33.0,34.0]] as it should.

I am stumped. Has this template deduction issue arisen before? Any ideas what I might be doing wrong?

Setup:

  • JSON was installed from the develop branch at commit 6471a63 from mid-2021, which is part of version 3.9.1
@NeroBurner
Copy link

could you provide information about your setup as specified in the bug-issue-template

https://github.com/nlohmann/json/blob/develop/.github/ISSUE_TEMPLATE/Bug_report.md

@willat343
Copy link
Author

I have updated my pose with my setup at the end

@NeroBurner
Copy link

what compiler are you using? Have you tried with the current version of develop?

@willat343
Copy link
Author

willat343 commented Feb 6, 2022

I've just checked out an installed the latest release, 3.10.5, and made a basic main.cpp and CMakeLists.txt to demonstrate the issue. My compiler is g++-7.5. I tested 2 methods, one with to_json/from_json functions in the Eigen namespace (call this method 1), and one with adl_serializer specialisation (call this method 2), as per the docs on working with 3rd party types.

With method 1, I am able to encode to json but not decode from json (this is an improvement compared to version 3.9.1 I was on before). With method 2, I am not able to encode or decode. As before, if I static cast to the base class type, the conversion works. Each of these permutations is shown in the following code.

code.zip

@gregmarr
Copy link
Contributor

gregmarr commented Feb 7, 2022

@willat343 I haven't dug into this in detail, but it's likely because the type is designed to be used as an array, and thus has an interface that matches "can serialize this using the built-in array serialization". Have you tried to see if removing these to_json/from_json methods altogether makes it work properly?

@willat343
Copy link
Author

@gregmarr that's a plausible hypothesis, however removing the to_json/from_json functions does not work.

Edit from future me: this is quite a long comment, because I think i've worked out the issue. I wrote the following during my investigation.

Encoding fails with:

~/src/personal/json_eigen_bug/main.cpp: In function ‘int main()’:
~/src/personal/json_eigen_bug/main.cpp:65:24: error: conversion from ‘Eigen::Matrix2d {aka Eigen::Matrix<double, 2, 2>}’ to non-scalar type ‘nlohmann::json {aka nlohmann::basic_json<>}’ requested
     nlohmann::json j = m;

Decoding fails with:

In file included from ~/src/personal/json_eigen_bug/main.cpp:2:0:
/usr/local/include/nlohmann/json.hpp: In instantiation of ‘void nlohmann::detail::from_json(const BasicJsonType&, ConstructibleStringType&) [with BasicJsonType = nlohmann::basic_json<>; ConstructibleStringType = Eigen::Matrix<double, 2, 2>; typename std::enable_if<(nlohmann::detail::is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value && (! std::is_same<typename BasicJsonType::string_t, ConstructibleStringType>::value)), int>::type <anonymous> = 0]’:
/usr/local/include/nlohmann/json.hpp:4273:25:   required from ‘decltype (nlohmann::detail::from_json(j, forward<T>(val))) nlohmann::detail::from_json_fn::operator()(const BasicJsonType&, T&&) const [with BasicJsonType = nlohmann::basic_json<>; T = Eigen::Matrix<double, 2, 2>&; decltype (nlohmann::detail::from_json(j, forward<T>(val))) = void]’
/usr/local/include/nlohmann/json.hpp:4934:30:   required from ‘static decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) nlohmann::adl_serializer<T, SFINAE>::from_json(BasicJsonType&&, TargetType&) [with BasicJsonType = const nlohmann::basic_json<>&; TargetType = Eigen::Matrix<double, 2, 2>; ValueType = Eigen::Matrix<double, 2, 2>; <template-parameter-1-2> = void; decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) = void]’
/usr/local/include/nlohmann/json.hpp:18955:45:   required from ‘ValueType& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>::get_to(ValueType&) const [with ValueType = Eigen::Matrix<double, 2, 2>; typename std::enable_if<((! nlohmann::detail::is_basic_json<BasicJsonType>::value) && nlohmann::detail::has_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, ValueType>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; BinaryType = std::vector<unsigned char>]’
~/src/personal/json_eigen_bug/main.cpp:79:16:   required from here
/usr/local/include/nlohmann/json.hpp:3914:7: error: no match for ‘operator=’ (operand types are ‘Eigen::Matrix<double, 2, 2>’ and ‘const string_t {aka const std::__cxx11::basic_string<char>}’)
     s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
In file included from /usr/local/include/eigen3/Eigen/Core:295:0,
                 from ~/src/personal/json_eigen_bug/main.cpp:1:
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:206:33: note: candidate: Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>& Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::operator=(const Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>&) [with _Scalar = double; int _Rows = 2; int _Cols = 2; int _Options = 0; int _MaxRows = 2; int _MaxCols = 2]
     EIGEN_STRONG_INLINE Matrix& operator=(const Matrix& other)
                                 ^~~~~~~~
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:206:33: note:   no known conversion for argument 1 from ‘const string_t {aka const std::__cxx11::basic_string<char>}’ to ‘const Eigen::Matrix<double, 2, 2>&’
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:223:33: note: candidate: template<class OtherDerived> Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>& Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::operator=(const Eigen::DenseBase<OtherDerived>&) [with OtherDerived = OtherDerived; _Scalar = double; int _Rows = 2; int _Cols = 2; int _Options = 0; int _MaxRows = 2; int _MaxCols = 2]
     EIGEN_STRONG_INLINE Matrix& operator=(const DenseBase<OtherDerived>& other)
                                 ^~~~~~~~
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:223:33: note:   template argument deduction/substitution failed:
In file included from ~/src/personal/json_eigen_bug/main.cpp:2:0:
/usr/local/include/nlohmann/json.hpp:3914:7: note:   ‘const string_t {aka const std::__cxx11::basic_string<char>}’ is not derived from ‘const Eigen::DenseBase<Derived>’
     s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
In file included from /usr/local/include/eigen3/Eigen/Core:295:0,
                 from ~/src/personal/json_eigen_bug/main.cpp:1:
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:236:33: note: candidate: template<class OtherDerived> Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>& Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::operator=(const Eigen::EigenBase<OtherDerived>&) [with OtherDerived = OtherDerived; _Scalar = double; int _Rows = 2; int _Cols = 2; int _Options = 0; int _MaxRows = 2; int _MaxCols = 2]
     EIGEN_STRONG_INLINE Matrix& operator=(const EigenBase<OtherDerived> &other)
                                 ^~~~~~~~
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:236:33: note:   template argument deduction/substitution failed:
In file included from ~/src/personal/json_eigen_bug/main.cpp:2:0:
/usr/local/include/nlohmann/json.hpp:3914:7: note:   ‘const string_t {aka const std::__cxx11::basic_string<char>}’ is not derived from ‘const Eigen::EigenBase<Derived>’
     s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
In file included from /usr/local/include/eigen3/Eigen/Core:295:0,
                 from ~/src/personal/json_eigen_bug/main.cpp:1:
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:243:33: note: candidate: template<class OtherDerived> Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>& Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::operator=(const Eigen::ReturnByValue<OtherDerived>&) [with OtherDerived = OtherDerived; _Scalar = double; int _Rows = 2; int _Cols = 2; int _Options = 0; int _MaxRows = 2; int _MaxCols = 2]
     EIGEN_STRONG_INLINE Matrix& operator=(const ReturnByValue<OtherDerived>& func)
                                 ^~~~~~~~
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:243:33: note:   template argument deduction/substitution failed:
In file included from ~/src/personal/json_eigen_bug/main.cpp:2:0:
/usr/local/include/nlohmann/json.hpp:3914:7: note:   ‘const string_t {aka const std::__cxx11::basic_string<char>}’ is not derived from ‘const Eigen::ReturnByValue<Derived>’
     s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
In file included from /usr/local/include/eigen3/Eigen/Core:295:0,
                 from ~/src/personal/json_eigen_bug/main.cpp:1:
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:279:13: note: candidate: Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>& Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::operator=(Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>&&) [with _Scalar = double; int _Rows = 2; int _Cols = 2; int _Options = 0; int _MaxRows = 2; int _MaxCols = 2]
     Matrix& operator=(Matrix&& other) EIGEN_NOEXCEPT_IF(std::is_nothrow_move_assignable<Scalar>::value)
             ^~~~~~~~
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:279:13: note:   no known conversion for argument 1 from ‘const string_t {aka const std::__cxx11::basic_string<char>}’ to ‘Eigen::Matrix<double, 2, 2>&&’
/usr/local/include/eigen3/Eigen/src/Core/Matrix.h:438:13: note: candidate: template<class OtherDerived> Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>& Eigen::Matrix<_Scalar, _Rows, _Cols, _Options, _MaxRows, _MaxCols>::operator=(const Eigen::RotationBase<OtherDerived, ColsAtCompileTime>&) [with OtherDerived = OtherDerived; _Scalar = double; int _Rows = 2; int _Cols = 2; int _Options = 0; int _MaxRows = 2; int _MaxCols = 2]
     Matrix& operator=(const RotationBase<OtherDerived,ColsAtCompileTime>& r);

Trying to work through this, it seems to be using the overload:

/usr/local/include/nlohmann/json.hpp:18955:45:   required from ‘ValueType& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>::get_to(ValueType&) const [with ValueType = Eigen::Matrix<double, 2, 2>; typename std::enable_if<((! nlohmann::detail::is_basic_json<BasicJsonType>::value) && nlohmann::detail::has_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, ValueType>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; BinaryType = std::vector<unsigned char>]’

This is still a bit hard to read, so here is the function:

    /// @brief get a value (explicit)
    /// @sa https://json.nlohmann.me/api/basic_json/get_to/
    template < typename ValueType,
               detail::enable_if_t <
                   !detail::is_basic_json<ValueType>::value&&
                   detail::has_from_json<basic_json_t, ValueType>::value,
                   int > = 0 >
    ValueType & get_to(ValueType& v) const noexcept(noexcept(
                JSONSerializer<ValueType>::from_json(std::declval<const basic_json_t&>(), v)))
    {
        JSONSerializer<ValueType>::from_json(*this, v);
        return v;
    }

As someone who doesn't know detail::enable_if_t, I'm not an expert, but my interpretation is that this function template is enabled if a) the ValueType (Matrix2d) is not a basic json type, and b) there is a from_json function of the ValueType (Matrix2d) to a basic json type (basic_json_t), which looks like it is any one of ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; BinaryType = std::vector<unsigned char>.

The compiler then fails to compile line 18955, which is in fact JSONSerializer<ValueType>::from_json(*this, v);, which leads us a couple functions later to:

template <
    typename BasicJsonType, typename ConstructibleStringType,
    enable_if_t <
        is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value&&
        !std::is_same<typename BasicJsonType::string_t,
                      ConstructibleStringType>::value,
        int > = 0 >
void from_json(const BasicJsonType& j, ConstructibleStringType& s)
{
    if (JSON_HEDLEY_UNLIKELY(!j.is_string()))
    {
        JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name()), j));
    }

    s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}

with with BasicJsonType = nlohmann::basic_json<>; ConstructibleStringType = Eigen::Matrix<double, 2, 2>. Repeating the same logic, it appears this template function should be enabled if ConstructibleStringType = Eigen::Matrix<double, 2, 2> a) passes the is_contructible_string_type type trait test, and b) is not the same type as a BasicJsonType::string_t. The second condition (I strongly assume) is true, so the next question in my mind is: "why is is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value true?

The type trait is defined as:

template<typename BasicJsonType, typename ConstructibleStringType>
struct is_constructible_string_type
{
    static constexpr auto value =
        is_constructible<ConstructibleStringType,
        typename BasicJsonType::string_t>::value;
};

which leads us to is_constructible<ConstructibleStringType, typename BasicJsonType::string_t> which has the following 5 definitions:

template <typename T, typename... Args>
struct is_constructible : std::is_constructible<T, Args...> {};

template <typename T1, typename T2>
struct is_constructible<std::pair<T1, T2>> : is_default_constructible<std::pair<T1, T2>> {};

template <typename T1, typename T2>
struct is_constructible<const std::pair<T1, T2>> : is_default_constructible<const std::pair<T1, T2>> {};

template <typename... Ts>
struct is_constructible<std::tuple<Ts...>> : is_default_constructible<std::tuple<Ts...>> {};

template <typename... Ts>
struct is_constructible<const std::tuple<Ts...>> : is_default_constructible<const std::tuple<Ts...>> {};

I believe it is using the 1st one since the other 4 are templated with pairs/tuples, and hence my hypothesis is that std::is_constructible<Eigen::Matrix<double, 2, 2>, BasicJsonType::string_t>::value must be returning true. Now it appears from both json.hpp and my error logs, that BasicJsonType::string_t is std::string or std::__cxx11::basic_string<char> (which i believe is the same?).

SO! Taking this hypothesis and writing the following basic program, we find that indeed Eigen::Matrix<double, 2, 2> is passing the constructibility test:

int main() {
    std::cerr << (std::is_constructible<Eigen::Matrix<double, 2, 2>, nlohmann::json::string_t>::value ? "passed!" : "failed!") << std::endl;
    std::cerr << (std::is_constructible<Eigen::Matrix<double, 2, 2>, std::string>::value ? "passed!" : "failed!") << std::endl;
    std::cerr << (std::is_constructible<Eigen::Matrix<double, 2, 2>, std::__cxx11::basic_string<char>>::value ? "passed!" : "failed!") << std::endl;
    return 0;
}

Prints "passed!" three times.

A little more experimentation reveals that you can construct from an empty string object:

Eigen::Matrix2d m(std::string());
std::cerr << m << std::endl;                 // prints 1

but not from a non-empty string:

Eigen::Matrix2d m2(std::string("1"));   // fails to compile

Looking through the constructors of Matrix (https://eigen.tuxfamily.org/dox/classEigen_1_1Matrix.html), I believe the problematic one is Matrix (Index dim) where Eigen::Index is a typedef for long int, because the following code compiles and runs:

Eigen::Index i(std::string());
std::cerr << "i: " << i << std::endl;     // prints 1

I tried a simple test class:

class Test {
public:
    Test(Eigen::Index i): test(i) {};
    Eigen::Index test;
};

std::cerr << Test(std::string()) << std::endl;

But strangely this failed with a note that surprised me given what I'd just tested: note: no known conversion for argument 1 from ‘std::__cxx11::string {aka std::__cxx11::basic_string<char>}’ to ‘Eigen::Index {aka long int}’.

I think i'll put an issue in with the Eigen devs, because I don't believe Eigen::Matrix2d m(std::string()); should be valid. I'll suggest they should make that constructor explicit if indeed it is that constructor, or suggetst they prevent this construction through some other c++ magic.

@gregmarr
Copy link
Contributor

gregmarr commented Feb 9, 2022

@willat343 Have you run this under the debugger so you can step in and see for sure what constructor is called? I looked at the header, and the actual constructors are a bit different than what's in the docs due to a couple template constructors that chain to the base class init function.

Eigen::Matrix2d m(std::string());
std::cerr << m << std::endl;

@willat343
Copy link
Author

@gregmarr I will do that and report back

@willat343
Copy link
Author

I tried a few methods, but was not able to generate a breakpoint on the matrix constructor with gdb, or see through stepping which constructor was being called unfortunately. Maybe someone else more adept with a debugger can give it a go.

Even so, i've openned up an issue with the Eigen devs here.

I do wonder, though, if there is a subtle json bug here. A from_json template function is enabled because it passes the std::is_constructible test, but the compile error was error: no match for ‘operator=’. So perhaps what should really be assessed I think is if the operator= exists, or both. I think std::is_constructible would usually imply the existence of the copy assignment operator, but I might have stumbled across an unusual case where it doesn't? I am a bit out of my depth now through, and have no idea if there is a c++ type trait check for that.

@willat343
Copy link
Author

Ok, the Eigen devs correctly pointed out that Eigen::Matrix2d m(std::string()); is a most-vexing parse, and explain that std::is_constructible<MatrixType, std::string> passes due to their template <typename T> Matrix (const T&) constructor. They are not yet sure if they want to apply SFINAE to disallow this.

In my previous analysis, I was looking at the compilation error when no specialisation was given, hoping to gain insight into the problem. When the specialisation is given (method 1), the compilation error is:

  In file included from ~/src/personal/json_eigen_bug/main.cpp:2:0:
[build] /usr/local/include/nlohmann/json.hpp: In instantiation of ‘void nlohmann::detail::from_json(const BasicJsonType&, ConstructibleStringType&) [with BasicJsonType = nlohmann::basic_json<>; ConstructibleStringType = Eigen::Matrix<double, 2, 2>; typename std::enable_if<(nlohmann::detail::is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value && (! std::is_same<typename BasicJsonType::string_t, ConstructibleStringType>::value)), int>::type <anonymous> = 0]’:
[build] /usr/local/include/nlohmann/json.hpp:4273:25:   required from ‘decltype (nlohmann::detail::from_json(j, forward<T>(val))) nlohmann::detail::from_json_fn::operator()(const BasicJsonType&, T&&) const [with BasicJsonType = nlohmann::basic_json<>; T = Eigen::Matrix<double, 2, 2>&; decltype (nlohmann::detail::from_json(j, forward<T>(val))) = void]’
[build] /usr/local/include/nlohmann/json.hpp:4934:30:   required from ‘static decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) nlohmann::adl_serializer<T, SFINAE>::from_json(BasicJsonType&&, TargetType&) [with BasicJsonType = const nlohmann::basic_json<>&; TargetType = Eigen::Matrix<double, 2, 2>; ValueType = Eigen::Matrix<double, 2, 2>; <template-parameter-1-2> = void; decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) = void]’
[build] /usr/local/include/nlohmann/json.hpp:18955:45:   required from ‘ValueType& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>::get_to(ValueType&) const [with ValueType = Eigen::Matrix<double, 2, 2>; typename std::enable_if<((! nlohmann::detail::is_basic_json<BasicJsonType>::value) && nlohmann::detail::has_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, ValueType>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; BinaryType = std::vector<unsigned char>]’
[build] ~/src/personal/json_eigen_bug/main.cpp:111:16:   required from here
[build] /usr/local/include/nlohmann/json.hpp:3914:7: error: no match for ‘operator=’ (operand types are ‘Eigen::Matrix<double, 2, 2>’ and ‘const string_t {aka const std::__cxx11::basic_string<char>}’)
[build]      s = *j.template get_ptr<const typename BasicJsonType::string_t*>();

Breaking this down:

  • The get_to template function at json.hpp::18961 enabled is the same as before, and seems correct to me.
  • The from_json in the default adl_serializer at json.hpp:4929 is then called (which I think it should be for method 1, but not method 2, so I'll check method 2 later).
  • This then calls the operator() of the from_json_fn struct at json.hpp:4266, which again seems ok.

It is at line 4273, return from_json(j, std::forward<T>(val));, where I think the template function overload written in method 1 should be chosen, but instead it chooses the function at 3907, the one from a ConstructibleStringType:

template <
    typename BasicJsonType, typename ConstructibleStringType,
    enable_if_t <
        is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value&&
        !std::is_same<typename BasicJsonType::string_t,
                      ConstructibleStringType>::value,
        int > = 0 >
void from_json(const BasicJsonType& j, ConstructibleStringType& s)
{
    if (JSON_HEDLEY_UNLIKELY(!j.is_string()))
    {
        JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name()), j));
    }

    s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
}

So my question is then: why is it selecting this function over my specialisation? should it not complain about function ambiguity when you have two valid from_json functions? (I imagine there would be other cases where one can construct from string but also specialise their from_json function)

Now testing method 2, compilation of decoding fails with the same error:

[build] In file included from ~/src/personal/json_eigen_bug/main.cpp:2:0:
[build] /usr/local/include/nlohmann/json.hpp: In instantiation of ‘void nlohmann::detail::from_json(const BasicJsonType&, ConstructibleStringType&) [with BasicJsonType = nlohmann::basic_json<>; ConstructibleStringType = Eigen::Matrix<double, 2, 2>; typename std::enable_if<(nlohmann::detail::is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value && (! std::is_same<typename BasicJsonType::string_t, ConstructibleStringType>::value)), int>::type <anonymous> = 0]’:
[build] /usr/local/include/nlohmann/json.hpp:4273:25:   required from ‘decltype (nlohmann::detail::from_json(j, forward<T>(val))) nlohmann::detail::from_json_fn::operator()(const BasicJsonType&, T&&) const [with BasicJsonType = nlohmann::basic_json<>; T = Eigen::Matrix<double, 2, 2>&; decltype (nlohmann::detail::from_json(j, forward<T>(val))) = void]’
[build] /usr/local/include/nlohmann/json.hpp:4934:30:   required from ‘static decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) nlohmann::adl_serializer<T, SFINAE>::from_json(BasicJsonType&&, TargetType&) [with BasicJsonType = const nlohmann::basic_json<>&; TargetType = Eigen::Matrix<double, 2, 2>; ValueType = Eigen::Matrix<double, 2, 2>; <template-parameter-1-2> = void; decltype ((nlohmann::{anonymous}::from_json(forward<BasicJsonType>(j), val), void())) = void]’
[build] /usr/local/include/nlohmann/json.hpp:18955:45:   required from ‘ValueType& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>::get_to(ValueType&) const [with ValueType = Eigen::Matrix<double, 2, 2>; typename std::enable_if<((! nlohmann::detail::is_basic_json<BasicJsonType>::value) && nlohmann::detail::has_from_json<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer, BinaryType>, ValueType>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; BinaryType = std::vector<unsigned char>]’
[build] ~/src/personal/json_eigen_bug/main.cpp:111:16:   required from here
[build] /usr/local/include/nlohmann/json.hpp:3914:7: error: no match for ‘operator=’ (operand types are ‘Eigen::Matrix<double, 2, 2>’ and ‘const string_t {aka const std::__cxx11::basic_string<char>}’)
[build]      s = *j.template get_ptr<const typename BasicJsonType::string_t*>();

This implies to me that the adl_serializer specialisation is not being found at all.
Also encoding fails for method 2 without useful information:

[build] /home/williamtalbot/src/personal/json_eigen_bug/main.cpp: In function ‘int main()’:
[build] /home/williamtalbot/src/personal/json_eigen_bug/main.cpp:98:24: error: conversion from ‘Eigen::Matrix2d {aka Eigen::Matrix<double, 2, 2>}’ to non-scalar type ‘nlohmann::json {aka nlohmann::basic_json<>}’ requested
[build]      nlohmann::json j = m;

@gregmarr
Copy link
Contributor

explain that std::is_constructible<MatrixType, std::string> passes due to their template Matrix (const T&) constructor.

Yeah, that's what I thought.

So my question is then: why is it selecting this function over my specialisation? should it not complain about function ambiguity when you have two valid from_json functions? (I imagine there would be other cases where one can construct from string but also specialise their from_json function)

Your specialization takes a base class reference. This function takes a reference of the exact type of the object. Thus, this function is a better match.

the one from a ConstructibleStringType:

I think this template parameter should be named something like UserDefinedTypeThatIsConstructibleFromString rather than ConstructibleStringType. That might make this function a bit easier to understand. It took me a while to work that bit out.

I do wonder, though, if there is a subtle json bug here. A from_json template function is enabled because it passes the std::is_constructible test, but the compile error was error: no match for ‘operator=’. So perhaps what should really be assessed I think is if the operator= exists, or both.

Yes, that does seem to be accurate. You have a type that is constructible from a string but can not be assigned to from a string, and this is doing assignment, not construction, so it should be looking at assignment.

@nickanthony-dgl
Copy link

I'm running into this same issue with Eigen. Is there any currently any recommended way of working around this?

@willat343
Copy link
Author

@nickanthony-dgl It is not possible currently to use the MatrixBase<Derived> specialisation methods, so the current workaround is to use my original functions (specialise according Matrix<Scalar, Rows, Cols>) in my first comment.

I am not sure how to resolve the apparent subtle bug in the json api discussed in the last few comments (check for assignment not construction). I am hoping a json and/or type-traits expert can offer some guidance here.

I am also not sure if there will be much progress on the Eigen side at https://gitlab.com/libeigen/eigen/-/issues/2438 since being constructible from string is not strictly a bug.

falbrechtskirchinger added a commit to falbrechtskirchinger/json that referenced this issue Apr 6, 2022
Constrain from_json() overload for ConstructibleStringType to not accept
json_ref and require assignability.

Re-enable C++14 tests on Clang <4.0.

Fixes nlohmann#3171, nlohmann#3312, nlohmann#3384.
Maybe fixes nlohmann#3267.
falbrechtskirchinger added a commit to falbrechtskirchinger/json that referenced this issue Apr 6, 2022
Constrain from_json() overload for ConstructibleStringType to not accept
json_ref and require it to be assignable from basic_json::string_t.

Re-enable C++14 tests on Clang <4.0.

Fixes nlohmann#3171.
Fixes nlohmann#3267.
Fixes nlohmann#3312.
Fixes nlohmann#3384.
falbrechtskirchinger added a commit to falbrechtskirchinger/json that referenced this issue Apr 7, 2022
Constrain from_json() overload for StringType to not accept json_ref and
require it to be assignable, instead of constructible, from
basic_json::string_t.

Re-enable C++14 tests on Clang <4.0.

Fixes nlohmann#3171.
Fixes nlohmann#3267.
Fixes nlohmann#3312.
Fixes nlohmann#3384.
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Apr 7, 2022
@nlohmann nlohmann self-assigned this Apr 8, 2022
@nlohmann nlohmann added this to the Release 3.11.0 milestone Apr 8, 2022
nlohmann pushed a commit that referenced this issue Apr 8, 2022
Constrain from_json() overload for StringType to not accept json_ref and
require it to be assignable, instead of constructible, from
basic_json::string_t.

Re-enable C++14 tests on Clang <4.0.

Fixes #3171.
Fixes #3267.
Fixes #3312.
Fixes #3384.
@patrikhuber
Copy link
Contributor

@falbrechtskirchinger I'm on nlohmann/json v3.11.3, and Method 2 (with struct adl_serializer<Eigen::MatrixBase<Derived>>) still produces a compiler error on the latest MSVC 2022. Is that to be expected? (For the record, Method 1 works, but I believe that was partially the case already before the issue/PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
6 participants