fix inconsistent choice of init list constructor

[grisumbras]

On some compilers value{x} invokes initializer_list constructor, on others it is equivalent to value(x). This is very problematic, but this isn't something we can fix. On the other hand, we can make init list construction to be equivalent to value(x), if the size of init list is one. This commit does exactly that.

Fix #920

[cppalliance-bot]

Benchmark test results. More info at https://benchmark.cppalliance.org/jsonbenchmarks-pullrequests/921/pullrequest.html

[codecov]

Codecov Report

Merging #921 (25e740d) into develop (2acdb29) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #921   +/-   ##
========================================
  Coverage    92.95%   92.96%           
========================================
  Files           85       85           
  Lines         8052     8056    +4     
========================================
+ Hits          7485     7489    +4     
  Misses         567      567           

Files Changed	Coverage Δ
include/boost/json/value.hpp	98.92% <ø> (ø)
include/boost/json/impl/value.ipp	100.00% <100.00%> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2acdb29...25e740d. Read the comment docs.

[vinniefalco]

Hey good idea !

[cppalliance-bot]

Benchmark test results. More info at https://benchmark.cppalliance.org/jsonbenchmarks-pullrequests/921/pullrequest.html

[nkolotov]

¿What if caller actually wants to produce an array with a single item? Some people may already be using code that supplies initializer_list explicitly expecting array to be produced.

void Pack(::std::initializer_list<::boost::json::value_ref> items)
{
    ::boost::json::value json{items};
    ::boost::json::array & array{json.as_array()};
    // do something with array...
}

And proposed change will break this code when only a single item is supplied.

Also i'm not sure about ::new(this) value (.... ¿Is an attempt to reuse storage of the object which lifetime hasn't started yet a well-defined behavior? ¿Will access to newly created object through reference to the old one be valid?

[grisumbras]

The proposed change is the only way I could come up with that would make the behaviour consistent. The problem of behaviour being inconsistent between compilers have been haunting us for years.

That being said, creating an array from one element init list is very easy:

std::initializer_list<value_ref> items = {x};
value jv = array(items);

or

value jv = array{x};

[grisumbras]

Also i'm not sure about ::new(this) value (.... ¿Is an attempt to reuse storage of the object which lifetime hasn't started yet a well-defined behavior? ¿Will access to newly created object through reference to the old one be valid?

How could there be a reference to an object that does not exist yet?

[nkolotov]

The proposed change is the only way I could come up with that would make the behaviour consistent.

However you make behavior of constructor taking initializer_list inconsistent. Potentially breaking existing code.

How could there be a reference to an object that does not exist yet?

In case if ::new(this), this is a pointer to the object being currently constructed. And if object here turns out to be not transparently replaceable then this pointer as well as any other names referring to the current object will become invalid. Also even if object is transparently replaceable, ::new(this) is dangerous in other ways since ::boost::json::value is not trivially destructible type.

[grisumbras]

However you make behavior of constructor taking initializer_list inconsistent. Potentially breaking existing code.

No, it will not make it inconsistent. It will be consistent with documented behaviour (which reminds me that I should have amended documentation). And yes, it will break existing code. So, I'm leaning towards temporary providing a macro to disable the new behaviour.

In case if ::new(this), this is a pointer to the object being currently constructed. And if object here turns out to be not transparently replaceable then this pointer as well as any other names referring to the current object will become invalid. Also even if object is transparently replaceable, ::new(this) is dangerous in other ways since ::boost::json::value is not trivially destructible type.

Ok, just to be sure I will replace new(this) with a different implementation.

[cppalliance-bot]

Benchmark test results. More info at https://benchmark.cppalliance.org/jsonbenchmarks-pullrequests/921/pullrequest.html

[cppalliance-bot]

Benchmark test results. More info at https://benchmark.cppalliance.org/jsonbenchmarks-pullrequests/921/pullrequest.html

[cppalliance-bot]

Benchmark test results. More info at https://benchmark.cppalliance.org/jsonbenchmarks-pullrequests/921/pullrequest.html

[cppalliance-bot]

Benchmark test results. More info at https://benchmark.cppalliance.org/jsonbenchmarks-pullrequests/921/pullrequest.html