is_nothrow_copy_constructible fails for json::const_iterator on MSVC2015 x86 Debug build

[nickaein]

• What is the issue you have?

The following check in unit-concepts fails on MSVC 2015 x86 Debug mode (log):

CHECK(std::is_nothrow_copy_constructible<json::const_iterator>::value)

• Please describe the steps to reproduce the issue. Can you provide a small but working code example?

By borrowing build flags, I have reproduced it here: https://godbolt.org/z/MOkndK

• What is the expected behavior?

This unit test should pass.

• And what is the actual behavior instead?

The unit-test fails merely because of x86 debug build on VS2015.

• Which compiler and operating system are you using?

This only fails on MSVC 2015 (msvc 19.0), x86 target in debug mode.

Speculation

This bug slipped under the radar since our Debug mode for x86 was incorrectly configured and was actually building in Release mode #1543. This issue is a blocker for PR #1570 which aims to bring back the Debug build for x86 MSVC 2015.

By playing with build flags in Godbolt, it seems that the presence of /MDd flag causes this error. This could be similar to #1536 where the C++ compiler on VS2015 added some definitions and symbols to binary in debug mode. This error only happens on VS2015 (msvc 19.0) Debug build (where /MDd is present) and doesn't happen in Release build and/or later compiler versions.

[nlohmann]

Any idea how to proceed here?

[nickaein]

I don't have much experience in this matter but this could be similar to #1536, i.e. some symbol(s) is being added to const_iterator in debug mode which causes it to lose is_nothrow_copy_constructible criteria. Therefore, the solution might be something like #1536 (comment) but I haven't figured how to track it down.

[nickaein]

I did some tweaking and it seems the issue can be fixed by adding the following copy constructor to iter_impl class template:

template<typename BasicJsonType>
class iter_impl
{
...
    // Adding this fixes the is_nothrow_copy_constructible error
    iter_impl(const iter_impl<const BasicJsonType>& other) noexcept
        : m_object(other.m_object), m_it(other.m_it) {}

    /*!
    @note The conventional copy constructor and copy assignment are implicitly
          defined. Combined with the following converting constructor and
          assignment, they support: (1) copy from iterator to iterator, (2)
          copy from const iterator to const iterator, and (3) conversion from
          iterator to const iterator. However conversion from const iterator
          to iterator is not defined.
    */

Note the prior comment on the implicit copy constructor.
Here is the fix on Godbolt: https://godbolt.org/z/VM5zxb

There might be a bug in VS 2015 which doesn't generate the noexcept copy constructor in debug mode (due to addition of some symbols, like #1536 (comment) ?). The fix might be explicitly adding the copy constructor (at least for const iter_impl).

I'm by no means expert in template programming and C++ type system and I'm not sure I have done it correctly. And I haven't tested it thoroughly on the other compilers. Any feedback and thoughts on this are very welcomed.

[nickaein]

I've pushed the changes to see how this initial solution plays out.

Meanwhile, about the notes where says

The conventional copy constructor and copy assignment are implicitly defined.

Is there any drawbacks of defining them explicitly?

[nlohmann]

@nickaein Is there a PR for the fix you mentioned?

[nickaein]

@nlohmann: Yes, This is the PR #1570 that includes this fix. This PR was originally aimed to fix another issue (#1543) with MSVC builds but got blocked because of this issue.

It seems to be fixed now by 4c23af1 , but I am not sure if this change is semantically correct.