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

In basic_json::basic_json(const CompatibleArrayType& val), the requirement of CompatibleArrayType is not strict enough. #174

Closed
jinCN opened this issue Jan 8, 2016 · 4 comments

Comments

@jinCN
Copy link

jinCN commented Jan 8, 2016

This is a question Ralated to #167 :
In basic_json::basic_json(const CompatibleArrayType& val), the requirement of CompatibleArrayType is not strict enough.

template <class CompatibleArrayType, typename
              std::enable_if<
                  not std::is_same<CompatibleArrayType, typename basic_json_t::iterator>::value and
                  not std::is_same<CompatibleArrayType, typename basic_json_t::const_iterator>::value and
                  not std::is_same<CompatibleArrayType, typename basic_json_t::reverse_iterator>::value and
                  not std::is_same<CompatibleArrayType, typename basic_json_t::const_reverse_iterator>::value and
                  not std::is_same<CompatibleArrayType, typename array_t::iterator>::value and
                  not std::is_same<CompatibleArrayType, typename array_t::const_iterator>::value and
                  std::is_constructible<basic_json, typename CompatibleArrayType::value_type>::value, int>::type
              = 0>
    basic_json(const CompatibleArrayType& val)
        : m_type(value_t::array)
    {
        using std::begin;
        using std::end;
        m_value.array = create<array_t>(begin(val), end(val));
    }

Template constructor of basic_json_t above can receive any type that has a type member value_type which can be used as the only parameter in one of basic_json_t's constructors and regard that type as a CompatibleArrayType.
I think the scope of the target types is too much wide, though it excludes some of the types already meet the requirement but are not CompatibleArrayType, still not fine with 3rd party types.

@nlohmann
Copy link
Owner

The code was inspired by some similar constructor from dropbox/json11. In fact, I only added constraints to make the code compile in my setting.

@mysyljr, do you have an example where the code actually breaks?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Jan 10, 2016
@jinCN
Copy link
Author

jinCN commented Jan 11, 2016

Simply define a type whose value_type is basic_json< xxx > will break the functionality, in which case a wrong conversion will be defined and unless you define a operator<< (or anything basic_json has) for that type, otherwise calling a operator<< will instantialize the conversion constructor ending with a compiling error.

@nlohmann
Copy link
Owner

I do not really see the need to change the library's code nor how a change would look like...

@nlohmann
Copy link
Owner

@mysyljr Sorry, I really need an actual example to proceed here.

@nlohmann nlohmann closed this as completed Feb 2, 2016
@nlohmann nlohmann removed the state: please discuss please discuss the issue or vote for your favorite option label Nov 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants