Fix compile error when using nlohmann ordered_map with WITH_DEFAULT macros (#5163)

* Fix compile error when using nlohmann ordered_map with WITH_DEFAULT macros

ordered_map inherits its copy and move assignment from the underlying std vector, which requires value_type to be CopyAssignable. value_type is pair<const Key, T> whose assignment is deleted because of the const Key, so any code that assigns ordered_map (for example the ternary in NLOHMANN_JSON_FROM_WITH_DEFAULT) fails to compile (issue #5122). Provide assignment operators on ordered_map that rebuild via clear plus push_back for copy and transfer the underlying buffer for move, neither of which needs pair assignment. Also switch the map-shaped from_json overload from a transform plus inserter idiom to a range-for plus emplace, which avoids the same hazard.

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Update ordered_map.hpp

removed unwanted comments

Signed-off-by: SamareshSingh <97642706+ssam18@users.noreply.github.com>
Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Update json.hpp

Signed-off-by: SamareshSingh <97642706+ssam18@users.noreply.github.com>
Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Address CI issues for ordered_map fix

Declare an explicit defaulted destructor on ordered_map so the rule of five is complete (clang-tidy cppcoreguidelines-special-member-functions and hicpp-special-member-functions). Initialize the ordered_map field in the regression test struct so GCC effective-C++ stops flagging Example_5122 with a missing member initializer.

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Suppress redundant-member-init lint on Example_5122::c

The empty brace-init on c{} is required by GCC -Weffc++ to mark the member as initialized in the synthesized default constructor, but clang-tidy readability-redundant-member-init flags the same line because ordered_map already has a default constructor. The two checks pull in opposite directions, so add a targeted NOLINT to keep both happy.

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Address review: strong exception safety in copy-assign, simplify move-assign noexcept

Copy assignment now constructs a temporary copy before move-assigning the
Container subobject, preserving *this if the copy throws. Move assignment
uses std::is_nothrow_move_assignable<Container> for a cleaner noexcept
specifier, matching the style of the move constructor.

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Restore self-assignment check in copy-assign to satisfy cert-oop54-cpp

clang-tidy's cert-oop54-cpp flagged the previous revision because it could
not recognize the implicit self-safety of the copy-then-move pattern.
Restore the explicit `if (this != &other)` guard — strong exception safety
is preserved since the temporary copy is still constructed before the
move-assign of the Container subobject.

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Address review: add explicit self-assignment and move-assignment tests for ordered_map

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Address review: gate -Wself-assign-overloaded suppression on Clang version

-Wself-assign-overloaded was introduced in Clang 7. Older Clang versions fail the build with "unknown warning group" when the suppression pragma references it unconditionally. Use __has_warning inside an __clang__ branch so the suppression is only emitted on Clang versions that recognize the warning. The inner check stays inside the __clang__ guard because GCC does not provide __has_warning and would tokenize-error on the argument list.

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

* Address CI: drop unused gating macro to silence -Wunused-macros

The previous attempt defined JSON_TEST_5122_SUPPRESS_SELF_ASSIGN_OVERLOADED
as 0 unconditionally and then overrode it to 1 on Clang versions that recognize the warning. On those Clangs the initial define is immediately
undef'd without being read, which trips Clang's -Wunused-macros under -Weverything in the ci_test_clang job. Drop the macro and gate the
DOCTEST_CLANG_SUPPRESS_WARNING_PUSH/POP pragmas directly with __has_warning inside the existing __clang__ branch.

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>

---------

Signed-off-by: Samaresh Kumar Singh <ssam3003@gmail.com>
Signed-off-by: SamareshSingh <97642706+ssam18@users.noreply.github.com>
This commit is contained in:
SamareshSingh
2026-05-14 01:54:24 -05:00
committed by GitHub
parent 93e49decbd
commit 7192763f15
4 changed files with 133 additions and 14 deletions
@@ -388,14 +388,10 @@ inline void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
ConstructibleObjectType ret;
const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
using value_type = typename ConstructibleObjectType::value_type;
std::transform(
inner_object->begin(), inner_object->end(),
std::inserter(ret, ret.begin()),
[](typename BasicJsonType::object_t::value_type const & p)
for (const auto& p : *inner_object)
{
return value_type(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
});
ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
}
obj = std::move(ret);
}
+19
View File
@@ -50,6 +50,25 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
: Container{first, last, alloc} {}
ordered_map(std::initializer_list<value_type> init, const Allocator& alloc = Allocator() )
: Container{init, alloc} {}
ordered_map(const ordered_map&) = default;
ordered_map(ordered_map&&) noexcept(std::is_nothrow_move_constructible<Container>::value) = default;
~ordered_map() = default;
ordered_map& operator=(const ordered_map& other)
{
if (this != &other)
{
ordered_map tmp(other);
Container::operator=(std::move(static_cast<Container&>(tmp)));
}
return *this;
}
ordered_map& operator=(ordered_map&& other) noexcept(std::is_nothrow_move_assignable<Container>::value)
{
Container::operator=(std::move(static_cast<Container&>(other)));
return *this;
}
std::pair<iterator, bool> emplace(const key_type& key, T&& t)
{
+22 -7
View File
@@ -5306,14 +5306,10 @@ inline void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
ConstructibleObjectType ret;
const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
using value_type = typename ConstructibleObjectType::value_type;
std::transform(
inner_object->begin(), inner_object->end(),
std::inserter(ret, ret.begin()),
[](typename BasicJsonType::object_t::value_type const & p)
for (const auto& p : *inner_object)
{
return value_type(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
});
ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
}
obj = std::move(ret);
}
@@ -20032,6 +20028,25 @@ template <class Key, class T, class IgnoredLess = std::less<Key>,
: Container{first, last, alloc} {}
ordered_map(std::initializer_list<value_type> init, const Allocator& alloc = Allocator() )
: Container{init, alloc} {}
ordered_map(const ordered_map&) = default;
ordered_map(ordered_map&&) noexcept(std::is_nothrow_move_constructible<Container>::value) = default;
~ordered_map() = default;
ordered_map& operator=(const ordered_map& other)
{
if (this != &other)
{
ordered_map tmp(other);
Container::operator=(std::move(static_cast<Container&>(tmp)));
}
return *this;
}
ordered_map& operator=(ordered_map&& other) noexcept(std::is_nothrow_move_assignable<Container>::value)
{
Container::operator=(std::move(static_cast<Container&>(other)));
return *this;
}
std::pair<iterator, bool> emplace(const key_type& key, T&& t)
{
+89
View File
@@ -1240,4 +1240,93 @@ TEST_CASE("regression test #5074 - single-element brace init with JSON_BRACE_INI
}
#endif
struct Example_5122
{
float b = 2;
nlohmann::ordered_map<std::string, std::string> c{}; // NOLINT(readability-redundant-member-init): needed for GCC -Weffc++
int a = 1;
NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(Example_5122, b, c, a)
};
TEST_CASE("regression test #5122 - from_json into types holding nlohmann::ordered_map")
{
Example_5122 src;
src.c.emplace("first", "1");
src.c.emplace("second", "2");
ordered_json const j = src;
Example_5122 const dst = j.get<Example_5122>();
CHECK(dst.b == src.b);
CHECK(dst.a == src.a);
REQUIRE(dst.c.size() == src.c.size());
auto src_it = src.c.begin();
auto dst_it = dst.c.begin();
for (; src_it != src.c.end(); ++src_it, ++dst_it)
{
CHECK(dst_it->first == src_it->first);
CHECK(dst_it->second == src_it->second);
}
}
// -Wself-assign-overloaded was introduced in Clang 7. Gate the pragma on
// __has_warning so older Clang versions do not error with "unknown warning
// group". The __has_warning check has to stay inside the __clang__ branch
// because GCC does not provide it and would tokenize-error on the argument.
#if defined(__clang__) && defined(__has_warning)
#if __has_warning("-Wself-assign-overloaded")
DOCTEST_CLANG_SUPPRESS_WARNING_PUSH
DOCTEST_CLANG_SUPPRESS_WARNING("-Wself-assign-overloaded")
#endif
#endif
TEST_CASE("regression test #5122 - nlohmann::ordered_map copy-assignment is self-assignment safe")
{
nlohmann::ordered_map<std::string, std::string> m;
m.emplace("first", "1");
m.emplace("second", "2");
// Insertion order is preserved by ordered_map, so we can check it directly.
m = m;
REQUIRE(m.size() == 2);
auto it = m.begin();
CHECK(it->first == "first");
CHECK(it->second == "1");
++it;
CHECK(it->first == "second");
CHECK(it->second == "2");
}
#if defined(__clang__) && defined(__has_warning)
#if __has_warning("-Wself-assign-overloaded")
DOCTEST_CLANG_SUPPRESS_WARNING_POP
#endif
#endif
TEST_CASE("regression test #5122 - nlohmann::ordered_map move-assignment transfers contents")
{
nlohmann::ordered_map<std::string, std::string> src;
src.emplace("first", "1");
src.emplace("second", "2");
nlohmann::ordered_map<std::string, std::string> dst;
dst.emplace("stale", "x");
dst = std::move(src);
REQUIRE(dst.size() == 2);
auto it = dst.begin();
CHECK(it->first == "first");
CHECK(it->second == "1");
++it;
CHECK(it->first == "second");
CHECK(it->second == "2");
// Re-assigning into the moved-from object must leave it in a usable state.
src = nlohmann::ordered_map<std::string, std::string>{};
src.emplace("after-move", "3");
REQUIRE(src.size() == 1);
CHECK(src.begin()->first == "after-move");
}
DOCTEST_CLANG_SUPPRESS_WARNING_POP