Skip to content

Commit

Permalink
[fix] re #336: remove eager assertion from NodeRef::duplicate()
Browse files Browse the repository at this point in the history
  • Loading branch information
biojppm committed Nov 30, 2022
1 parent b619f78 commit 94d7bbd
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 51 deletions.
4 changes: 2 additions & 2 deletions changelog/0.5.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
Parser parser;
assert(parser.options().locations() == false);
```
- Deprecate `Tree::arena_pos()`: use `Tree::arena_size()` instead ([PR#290](https://github.com/biojppm/rapidyaml/pull/290)).
- Deprecate pointless `has_siblings()`: use `Tree::has_other_siblings()` instead ([PR#330](https://github.com/biojppm/rapidyaml/pull/330).
### Performance improvements
Expand Down Expand Up @@ -144,7 +146,6 @@
```
- Fix [#280](https://github.com/biojppm/rapidyaml/issues/280) ([PR#281](https://github.com/biojppm/rapidyaml/pull/281)): deserialization of `std::vector<bool>` failed because its `operator[]` returns a `reference` instead of `value_type`.
- Fix [#288](https://github.com/biojppm/rapidyaml/issues/288) ([PR#290](https://github.com/biojppm/rapidyaml/pull/290)): segfault on successive calls to `Tree::_grow_arena()`, caused by using the arena position instead of its length as starting point for the new arena capacity.
- Deprecate `Tree::arena_pos()`: use `Tree::arena_size()` instead ([PR#290](https://github.com/biojppm/rapidyaml/pull/290)).
- Fix [#324](https://github.com/biojppm/rapidyaml/issues/324) ([PR#328](https://github.com/biojppm/rapidyaml/pull/328)): eager assertion prevented moving nodes to the first position in a parent.
- Fix `Tree::_clear_val()`: was clearing key instead ([PR#335](https://github.com/biojppm/rapidyaml/pull/335)).
- YAML test suite events emitter: fix emission of inheriting nodes. The events for `{<<: *anchor, foo: bar}` are now correctly emitted as:
Expand All @@ -157,7 +158,6 @@
- Fix [#246](https://github.com/biojppm/rapidyaml/issues/246): add missing `#define` for the include guard of the amalgamated header.
- Fix [#326](https://github.com/biojppm/rapidyaml/issues/326): honor runtime settings for calling debugbreak, add option to disable any calls to debugbreak.
- Fix [cmake#8](https://github.com/biojppm/cmake/issues/8): `SOVERSION` missing from shared libraries.
- Deprecate pointless `has_siblings()` ([PR#330](https://github.com/biojppm/rapidyaml/pull/330).


## Thanks
Expand Down
34 changes: 27 additions & 7 deletions src/c4/yml/node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1112,18 +1112,20 @@ class RYML_EXPORT NodeRef : public detail::RoNodeMethods<NodeRef, ConstNodeRef>

public:

/** change the node's position within its parent. To move to the
* first position in the parent, simply pass an empty or
* default-constructed reference like this: `n.move({})`, which is equivalent to */
/** change the node's position within its parent, placing it after
* @p after. To move to the first position in the parent, simply
* pass an empty or default-constructed reference like this:
* `n.move({})`. */
inline void move(ConstNodeRef const& after)
{
_C4RV();
m_tree->move(m_id, after.m_id);
}

/** move the node to a different parent, which may belong to a different
* tree. When this is the case, then this node's tree pointer is reset to
* the tree of the parent node. */
/** move the node to a different @p parent (which may belong to a
* different tree), placing it after @p after. When the
* destination parent is in a new tree, then this node's tree
* pointer is reset to the tree of the parent node. */
inline void move(NodeRef const& parent, ConstNodeRef const& after)
{
_C4RV();
Expand All @@ -1138,10 +1140,28 @@ class RYML_EXPORT NodeRef : public detail::RoNodeMethods<NodeRef, ConstNodeRef>
}
}

/** duplicate the current node somewhere within its parent, and
* place it after the node @p after. To place into the first
* position of the parent, simply pass an empty or
* default-constructed reference like this: `n.move({})`. */
inline NodeRef duplicate(ConstNodeRef const& after) const
{
_C4RV();
RYML_ASSERT(m_tree == after.m_tree || after.m_id == NONE);
size_t dup = m_tree->duplicate(m_id, m_tree->parent(m_id), after.m_id);
NodeRef r(m_tree, dup);
return r;
}

/** duplicate the current node somewhere into a different @p parent
* (possibly from a different tree), and place it after the node
* @p after. To place into the first position of the parent,
* simply pass an empty or default-constructed reference like
* this: `n.move({})`. */
inline NodeRef duplicate(NodeRef const& parent, ConstNodeRef const& after) const
{
_C4RV();
RYML_ASSERT(parent.m_tree == after.m_tree);
RYML_ASSERT(parent.m_tree == after.m_tree || after.m_id == NONE);
if(parent.m_tree == m_tree)
{
size_t dup = m_tree->duplicate(m_id, parent.m_id, after.m_id);
Expand Down
29 changes: 19 additions & 10 deletions test/test_case.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,31 +87,40 @@ void print_path(ConstNodeRef const& p);


template<class CheckFn>
void test_check_emit_check(csubstr yaml, CheckFn check_fn)
void test_check_emit_check(Tree const& t, CheckFn check_fn)
{
Tree t = parse_in_arena(yaml);
#ifdef RYML_DBG
print_tree(t);
#endif
{
SCOPED_TRACE("original yaml");
test_invariants(t);
check_fn(t);
}
auto emit_and_parse = [&](const char* identifier){
auto emit_and_parse = [&check_fn](Tree const& tp, const char* identifier){
SCOPED_TRACE(identifier);
std::string emitted = emitrs_yaml<std::string>(t);
std::string emitted = emitrs_yaml<std::string>(tp);
#ifdef RYML_DBG
printf("~~~%s~~~\n%.*s", identifier, (int)emitted.size(), emitted.data());
#endif
t = parse_in_arena(to_csubstr(emitted));
Tree cp = parse_in_arena(to_csubstr(emitted));
#ifdef RYML_DBG
print_tree(t);
print_tree(cp);
#endif
check_fn(t);
test_invariants(cp);
check_fn(cp);
return cp;
};
emit_and_parse("emitted 1");
emit_and_parse("emitted 2");
emit_and_parse("emitted 3");
Tree cp = emit_and_parse(t, "emitted 1");
cp = emit_and_parse(cp, "emitted 2");
cp = emit_and_parse(cp, "emitted 3");
}

template<class CheckFn>
void test_check_emit_check(csubstr yaml, CheckFn check_fn)
{
Tree t = parse_in_arena(yaml);
test_check_emit_check(t, check_fn);
}


Expand Down
133 changes: 101 additions & 32 deletions test/test_noderef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,40 +504,109 @@ TEST(NodeRef, move_to_other_tree_to_first_position)
test_invariants(t1);
}

TEST(NodeRef, duplicate)
TEST(NodeRef, duplicate_to_same_tree)
{
Tree t;
NodeRef r = t;

std::vector<std::vector<int>> vec2({{100, 200}, {300, 400}, {500, 600}, {700, 800, 900}});
std::map<std::string, int> map2({{"bar", 200}, {"baz", 300}, {"foo", 100}});

r |= SEQ;
r.append_child() << vec2;
r.append_child() << map2;
r.append_child() << "elm2";
r.append_child() << "elm3";
Tree t = parse_in_arena("[{a0: [b0, c0], a1: [b1, c1], a2: [b2, c2], a3: [b3, c3]}]");
auto checkseq = [](ConstNodeRef const& s){
ASSERT_EQ(s.num_children(), 4u);
ASSERT_EQ(s[0].num_children(), 2u);
ASSERT_EQ(s[1].num_children(), 2u);
ASSERT_EQ(s[2].num_children(), 2u);
ASSERT_EQ(s[3].num_children(), 2u);
EXPECT_EQ(s[0].key(), "a0");
EXPECT_EQ(s[0][0].val(), "b0");
EXPECT_EQ(s[0][1].val(), "c0");
EXPECT_EQ(s[1].key(), "a1");
EXPECT_EQ(s[1][0].val(), "b1");
EXPECT_EQ(s[1][1].val(), "c1");
EXPECT_EQ(s[2].key(), "a2");
EXPECT_EQ(s[2][0].val(), "b2");
EXPECT_EQ(s[2][1].val(), "c2");
EXPECT_EQ(s[3].key(), "a3");
EXPECT_EQ(s[3][0].val(), "b3");
EXPECT_EQ(s[3][1].val(), "c3");
};
{
SCOPED_TRACE("at the beginning");
t[0].duplicate({});
test_check_emit_check(t, [&checkseq](ConstNodeRef r){
checkseq(r[0]);
checkseq(r[1]);
});
}
{
SCOPED_TRACE("at the end");
t[0].duplicate(t.rootref().last_child());
test_check_emit_check(t, [&checkseq](ConstNodeRef r){
checkseq(r[0]);
checkseq(r[1]);
checkseq(r[2]);
});
}
{
SCOPED_TRACE("in the middle");
t[0].duplicate(t.rootref().first_child());
test_check_emit_check(t, [&checkseq](ConstNodeRef r){
checkseq(r[0]);
checkseq(r[1]);
checkseq(r[2]);
});
}
}

EXPECT_EQ(r[0][0].num_children(), 2u);
NodeRef dup = r[1].duplicate(r[0][0], r[0][0][1]);
EXPECT_EQ(r[0][0].num_children(), 3u);
EXPECT_EQ(r[0][0][2].num_children(), map2.size());
EXPECT_NE(dup.get(), r[1].get());
EXPECT_EQ(dup[0].key(), "bar");
EXPECT_EQ(dup[0].val(), "200");
EXPECT_EQ(dup[1].key(), "baz");
EXPECT_EQ(dup[1].val(), "300");
EXPECT_EQ(dup[2].key(), "foo");
EXPECT_EQ(dup[2].val(), "100");
EXPECT_EQ(dup[0].key().str, r[1][0].key().str);
EXPECT_EQ(dup[0].val().str, r[1][0].val().str);
EXPECT_EQ(dup[0].key().len, r[1][0].key().len);
EXPECT_EQ(dup[0].val().len, r[1][0].val().len);
EXPECT_EQ(dup[1].key().str, r[1][1].key().str);
EXPECT_EQ(dup[1].val().str, r[1][1].val().str);
EXPECT_EQ(dup[1].key().len, r[1][1].key().len);
EXPECT_EQ(dup[1].val().len, r[1][1].val().len);
test_invariants(t);
TEST(NodeRef, duplicate_to_different_tree)
{
Tree t = parse_in_arena("[{a0: [b0, c0], a1: [b1, c1], a2: [b2, c2], a3: [b3, c3]}]");
auto checkseq = [](ConstNodeRef const& s){
ASSERT_EQ(s.num_children(), 4u);
ASSERT_EQ(s[0].num_children(), 2u);
ASSERT_EQ(s[1].num_children(), 2u);
ASSERT_EQ(s[2].num_children(), 2u);
ASSERT_EQ(s[3].num_children(), 2u);
EXPECT_EQ(s[0].key(), "a0");
EXPECT_EQ(s[0][0].val(), "b0");
EXPECT_EQ(s[0][1].val(), "c0");
EXPECT_EQ(s[1].key(), "a1");
EXPECT_EQ(s[1][0].val(), "b1");
EXPECT_EQ(s[1][1].val(), "c1");
EXPECT_EQ(s[2].key(), "a2");
EXPECT_EQ(s[2][0].val(), "b2");
EXPECT_EQ(s[2][1].val(), "c2");
EXPECT_EQ(s[3].key(), "a3");
EXPECT_EQ(s[3][0].val(), "b3");
EXPECT_EQ(s[3][1].val(), "c3");
};
auto check_orig = [&checkseq](ConstNodeRef const& r){
ASSERT_TRUE(r.is_seq());
ASSERT_GE(r.num_children(), 1u);
checkseq(r[0]);
};
Tree d = parse_in_arena("[]");
{
SCOPED_TRACE("at the beginning");
t[0].duplicate(d, {});
test_check_emit_check(t, check_orig);
test_check_emit_check(d, check_orig);
}
{
SCOPED_TRACE("at the end");
t[0].duplicate(d, d.rootref().last_child());
test_check_emit_check(t, check_orig);
test_check_emit_check(d, check_orig);
test_check_emit_check(d, [&checkseq](ConstNodeRef r){
checkseq(r[1]);
});
}
{
SCOPED_TRACE("in the middle");
t[0].duplicate(d, d.rootref().first_child());
test_check_emit_check(t, check_orig);
test_check_emit_check(d, check_orig);
test_check_emit_check(d, [&checkseq](ConstNodeRef r){
checkseq(r[1]);
checkseq(r[2]);
});
}
}

TEST(NodeRef, intseq)
Expand Down

0 comments on commit 94d7bbd

Please sign in to comment.