vector<json> copy constructor really weird

[ajneu]

Hi there!

Try guess the output of the following:

#include <iostream>
#include <cassert>
#include <vector>

#include <json.hpp>

// for convenience
using json = nlohmann::json;

template <typename T>
class WeirdClass {
public:
  WeirdClass(const std::vector<T> &v) : vec{v}
  {
    std::cout << "v.size()   == " << v.size()   << std::endl;
    std::cout << "vec.size() == " << vec.size() << std::endl;

    std::cout << std::endl;

    for (decltype(v.size()) idx = 0; idx != v.size(); ++idx) {
      std::cout << "v[" << idx << "]   == " << v[idx] << std::endl;
    }

    std::cout << std::endl;

    for (decltype(vec.size()) idx = 0; idx != vec.size(); ++idx) {
      std::cout << "vec[" << idx << "] == " << vec[idx] << std::endl;
    }
    std::cout << std::endl;

    assert(v.size() == vec.size());
  }
private:
  const std::vector<T> vec;
 };


int main()
{
  json x, y;
  x["value"] = 0;
  y["value"] = 1;
  WeirdClass<json> weird{{x, y}};

  return 0;
}

It aborts!!!!! and gives me this

v.size()   == 2
vec.size() == 1

v[0]   == {"value":0}
v[1]   == {"value":1}

vec[0] == [{"value":0},{"value":1}]

go: /home/me/main.cpp:31: WeirdClass<T>::WeirdClass(const std::vector<_RealType>&) [with T = nlohmann::basic_json<>]: Assertion `v.size() == vec.size()' failed.
Aborted

Changing the constructor to

  WeirdClass(const std::vector<T> &v) : vec{v.cbegin(), v.cend()}
  { //...

gives me the expected output:

v.size()   == 2
vec.size() == 2

v[0]   == {"value":0}
v[1]   == {"value":1}

vec[0] == {"value":0}
vec[1] == {"value":1}

What's going on here??
I thought things like the copy constructor should behave in a standard way. (Or is my compiler buggy?? I'm using: gcc c++ (Debian 4.9.3-3) 4.9.3; and compiling with -std=c++14)

Thanks

[ajneu]

Also: if I put objects of type json into a vector vector, then iterating through and putting the dereference into boost::any

  json x, y;
  x["value"] = 0;
  y["value"] = 1;

  std::vector<json> vec;
  vec.push_back(x);
  vec.push_back(y);

  for (auto it = vec.cbegin(); it != vec.cend(); ++it) {
    boost::any any = *it;  // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    std::cout << boost::any_cast<json>(any) << std::endl;
  }

I get:

main.cpp: In function ‘int main()’:
main.cpp:22:23: error: conversion from ‘const nlohmann::basic_json<>’ to ‘boost::any’ is ambiguous
     boost::any any = *it;
                       ^
main.cpp:22:23: note: candidates are:
In file included from main.cpp:5:0:
json.hpp:2437:5: note: nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberFloatType, AllocatorType>::operator ValueType() const [with ValueType = boost::any; typename std::enable_if<(! std::is_pointer<_Ptr>::value), int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberFloatType = double; AllocatorType = std::allocator]
     operator ValueType() const
     ^
In file included from main.cpp:6:0:
any.hpp:68:9: note: boost::any::any(ValueType&&, typename boost::disable_if<boost::is_same<boost::any&, ValueType> >::type*, typename boost::disable_if<boost::is_const<T> >::type*) [with ValueType = const nlohmann::basic_json<>&; typename boost::disable_if<boost::is_same<boost::any&, ValueType> >::type = void; typename boost::disable_if<boost::is_const<T> >::type = void]
         any(ValueType&& value
         ^
any.hpp:46:9: note: boost::any::any(const ValueType&) [with ValueType = nlohmann::basic_json<>]
         any(const ValueType & value)

.
.

Is it safe if I do

boost::any any = static_cast<json>(*it);

and then extract with

boost::any_cast<json>(any)

???

Oh and I've seen that this...

std::vector<json> vec{json::object({"value", 0})};
json x = json::object({"value", 0});

...does not work.

Do I always have to use...

json x;
x["value"] = 0;
std::vector<json> vec{x};

... or is there another possibility ???

Thanks for any tipps and suggestions.

[nlohmann]

Hi @ajneu,

I could reproduce your reported behavior on the first issue with the following compilers (all OS X):

• GCC 5 (g++-5 (Homebrew gcc5 5.2.0) 5.2.0)
• clang version 3.4.2 (tags/RELEASE_34/dot2-final)
• clang version 3.5.1 (tags/RELEASE_351/final)
• clang version 3.6.2 (tags/RELEASE_362/final)

However, with Xcode Version 7.0 beta 4 (Clang Apple LLVM version 7.0.0), I can run the code and it runs as expected. I also used the memory sanitizer, etc. and found no issues. I currently have no idea why the code behaves the way it does.

[ajneu]

Wow, I'm stunned!
I kindof thought that your json has some magic (I've just started giving it a try), but I'm rather surprised that you seem to be leaning towards compiler-issues...

I suspect it's something with list-initialization...

  WeirdClass<json> weird{{x, y}};

Be sure to try and compare that with this one:

  WeirdClass<json> weird{{{x, y}}};       // 3 pairs of braces!

[ajneu]

Oh hang on... (I wrote too quickly).

This also asserts and aborts:

  json x, y;
  x["value"] = 0;
  y["value"] = 1;
  const std::vector<json> vec{x, y};
  WeirdClass<json> weird(vec);

[ajneu]

Maby we should get some guys from gcc and clang bugtracks to have a look at this? ;)

(I wonder what maximal simplification (transformation) one could do to the class basic_json, to still have this strange behaviour...)

[ajneu]

Hi Niels,

ok tried a few things here...
I modified /usr/include/c++/4.9.3/debug/vector (backuped it first!
      Note: this is the gcc debug/vector, which needs #define _GLIBCXX_DEBUG []),
to cout the info on which vector constuctors get called (and also output the size() of the constructor arguments).
      [] At the very top of my user-c++ file I #define _GLIBCXX_DEBUG
      and always makeclean&& make (reason: cmake does not realize when I change standard lib files)

Here's what I've found:

  WeirdClass(const vector<T> &v) : vec{v}     // note: braced-init-list!

calls the initializer_list constructor
vector(initializer_list<value_type> __l, const allocator_type& __a = allocator_type())
and here __l.size() == 1

On the other hand if I change the class to

  WeirdClass(const vector<T> &v) : vec(v)     // note: normal constructor

then the normal copy-constructor gets called
vector(const vector& __x))
and here __x.size() == 2

What this means is:
the braced-init-list version vec{v} correctly (edit for emphasis: yes, correctly!!!) first checks if there is a list of element type json.
And yes, there is, because this (possibly over-engineered ?) modern json library allows a whole vector of json, to be converted to a single json:

x["value"] = 0;
y["value"] = 1;
const vector<json> v1{x, y};
json hmmmm = v1;              // !!!!!!!!!!!!!!!!!!!

And thus vec{v}, sees an initializer list with element type json, having only a single element. (The vector of json, was converted to a single json element)

Edit: bottom line is -- this is no bug, but correct C++ behaviour!

[nlohmann]

Hi @ajneu,

thanks for the diagnosis! Indeed I allow a vector of json objects to initialize a json object. The respective constructor for this should be

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& value)
    : m_type(value_t::array)
{
    AllocatorType<array_t> alloc;
    m_value.array = alloc.allocate(1);
    using std::begin;
    using std::end;
    alloc.construct(m_value.array, begin(value), end(value));
}

The idea was to support conversions like

// create an array from std::vector
std::vector<int> c_vector {1, 2, 3, 4};
json j_vec(c_vector);

// create an array from std::deque
std::deque<double> c_deque {1.2, 2.3, 3.4, 5.6};
json j_deque(c_deque);

// create an array from std::list
std::list<bool> c_list {true, true, false, true};
json j_list(c_list);

// create an array from std::forward_list
std::forward_list<int64_t> c_flist {12345678909876, 23456789098765, 34567890987654, 45678909876543};
json j_flist(c_flist);

// create an array from std::array
std::array<unsigned long, 4> c_array {{1, 2, 3, 4}};
json j_array(c_array);

// create an array from std::set
std::set<std::string> c_set {"one", "two", "three", "four", "one"};
json j_set(c_set); // only one entry for "one" is used

// create an array from std::unordered_set
std::unordered_set<std::string> c_uset {"one", "two", "three", "four", "one"};
json j_uset(c_uset); // only one entry for "one" is used

// create an array from std::multiset
std::multiset<std::string> c_mset {"one", "two", "one", "four"};
json j_mset(c_mset); // only one entry for "one" is used

// create an array from std::unordered_multiset
std::unordered_multiset<std::string> c_umset {"one", "two", "one", "four"};
json j_umset(c_umset); // both entries for "one" are used

Maybe this is over-engineered. But it could also be useful...

Do you have a proposal how to fix this?

Cheers,
Niels

[ajneu]

Hi Niels,

ok I agree this could be useful.

Ultimately this issue is thus not a bug; and just confirms: users (such as me) have to know and understand libraries before using them.

So I don't think a fix is needed here. It's my job as a user to understand that there is a conversion from "vector of json" to "single json"; and it's our job as C++ coders to understand the details of C++:

C++11-Standard 8.5.4:
"Note: Initializer-list constructors are favored over other constructors in list-initialization (13.3.1.7)"

// Example:
std::vector<int> vec{10}; // will first favor list_initialization and create a vector with one single element (of value 10)
std::vector<std::string> vec{10}; // will try list_initialization which will not work; and then use normal constructor: will generate a vector with 10 value-initialized strings
std::vector<int> vec(10); // will create a vector with 10 elements (that are value-initialized)

But this is sometimes a trade-off with users, who want and like easy interfaces and minimal surprises... In this case it's not a big issue. I should have known that list-initialization if preferred and then realized that the lib allows conversion of containers to single json-array.

Edit: to put it as clearly as possible: there is no bug here, since it is all correct c++ behaviour.

Side-note:
Anyway, I'm evaluating JSON libraries. What is surprising about many, is that I cannot specify what element I expect next from an istream, e.g.

is.next_expect(json::object);
is.next_expect(json::array);

and allow serialization with checking (according to my specification.)

Libs that seem to have this, are maby json voorhees, IOD, ; perhaps others.
Am I correct that this JSON library does not have that?

Also question:
Why have you chosen such a strange way of reading from stdin ?

// deserialize from standard input
json j;
j << std::cin;

Isn't it usually used like this: std::cin >> j ???

Thanks for comments.

[ajneu]

(The bug label can be removed. All is good, except perhaps for the behaviour of Xcode Version 7.0 beta 4, that you describe above)

As mentioned as a side-note above, if you could briefly comment on the possibilities of verifying parsed json (if the fields and keys conform to what one expects); and the rationale behind j << std::cin (instead of std::cin >> j), I'd appreciate it.

[nlohmann]

Hi @ajneu, thanks for checking back. I'll keep you up to date!

[ajneu]

As mentioned as a side-note above, if you could briefly comment on the possibilities of verifying parsed json (if the fields and keys conform to what one expects); and the rationale behind j << std::cin (instead of std::cin >> j), I'd appreciate it.

bd5d96c#diff-04c6e90faac2675aa89e2176d2eec7d8

Nice to see this! ;)