Suppress Clang-Tidy warning

[nlohmann]

modernize-use-designated-initializers does not work with C++11

[coveralls]

coverage: 100.0%. remained the same
when pulling 4b7721c on clang-tidy
into 0457de2 on develop.

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @nlohmann
Please read and follow the Contribution Guidelines.

[gregmarr]

Can we just do assert_invariant(false); in the body (assert it in this after moving instead of asserting in other) to avoid this mess?

[gregmarr]

Another possibility for avoiding all these issues is to do std::swap() on m_data and other.m_data in the body.

[nlohmann]

I am not at all happy with this. I also thought about swapping...

[fsandhei]

Sorry for asking a question for something that is probably obvious, but which constructor of json_base_class_t is this call valid for? I suppose it is one of the basic_json constructors being used here?

FWIW, it should be completely fine to check the invariants after the initializer list has been evaluated. Is there a possibility to consider rewriting the data move constructor so these assignments don't need to be done at this level?

[nlohmann]

I am currently blocked here:

template < typename BasicJsonType, typename T, std::size_t... Idx >
std::array<T, sizeof...(Idx)> from_json_inplace_array_impl(BasicJsonType&& j,
        identity_tag<std::array<T, sizeof...(Idx)>> /*unused*/, index_sequence<Idx...> /*unused*/)
{
    return { { std::forward<BasicJsonType>(j).at(Idx).template get<T>()... } };
}

gives

/__w/json/json/include/nlohmann/detail/conversions/from_json.hpp:280:44: error: 'j' used after it was forwarded [bugprone-use-after-move,hicpp-invalid-access-moved,-warnings-as-errors]
  280 |     return { { std::forward<BasicJsonType>(j).at(Idx).template get<T>()... } };
      |                                            ^
/__w/json/json/include/nlohmann/detail/conversions/from_json.hpp:280:47: note: forward occurred here
  280 |     return { { std::forward<BasicJsonType>(j).at(Idx).template get<T>()... } };
      |                                               ^
/__w/json/json/include/nlohmann/detail/conversions/from_json.hpp:280:44: note: the use happens in a later loop iteration than the forward
  280 |     return { { std::forward<BasicJsonType>(j).at(Idx).template get<T>()... } };
      |                                            ^

Any ideas?

[gregmarr]

Every other from_json function except for auto from_json(BasicJsonType&& j, TupleRelated&& t) takes const BasicJsonType &. If this was the same, that eliminates the need for the std::forward.

Alternatively, if there is a defined need for moving from the json object into an std::array, then have separate const BasicJsonType & and BasicJsonType && functions and put the && on the array element type: template get<T&&>() in the latter.

[nlohmann]

I made some progress and now have an interesting issue:

template<class KeyType, detail::enable_if_t<
             detail::is_usable_as_basic_json_key_type<basic_json_t, KeyType>::value, int> = 0>
reference at(KeyType && key)
{
    auto it = m_data.m_value.object->find(std::forward<KeyType>(key));
    if (it == m_data.m_value.object->end())
    {
        JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(std::forward<KeyType>(key)), "' not found"), this)); // <-- warning here: 'key' used after it was forwarded
    }
    return set_parent(it->second);
}

To fix this, I could either make a copy of key which defeats the purpose of taking KeyType && in the first place - or to dumb down the error message and not mention key. What do you think?

[nlohmann]

We already have two versions for const references and rvalues. Unfortunately, it seems to depend on the compiler and C++ standard which version is used, so I am currently struggling to adjust the test cases (with and without key in the exception).

[github-actions]

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @nlohmann
Please read and follow the Contribution Guidelines.

[nlohmann]

I opened #4489 to get the CI green. Most issues from this PR are not addressed, but at least we have a clean slate. It would be great if someone could have a look.

[github-actions]

This pull request has been marked as stale because it has had no activity for 30 days. While we won’t close it automatically, we encourage you to update or comment if it is still relevant. Keeping pull requests active and up-to-date helps us review and merge changes more efficiently. Thank you for your contributions!