From 4bf3243cd51faa241430b2eae9b74e3d784ca1ea Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 10 Feb 2020 00:00:21 +0100 Subject: [PATCH] Fixes #31: add assertions covering wrong tree construction --- CHANGELOG.md | 2 ++ ROADMAP.md | 25 +++++++++++++++++ src/c4/yml/tree.cpp | 2 ++ src/c4/yml/tree.hpp | 61 +++++++++++++++++++++++++++++++++++------- test/github_issues.cpp | 57 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 ROADMAP.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..4dc68c6f --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,2 @@ +# Changelog + diff --git a/ROADMAP.md b/ROADMAP.md new file mode 100644 index 00000000..cdcb0244 --- /dev/null +++ b/ROADMAP.md @@ -0,0 +1,25 @@ +# Roadmap + +Roughly in order of priority: + + * Cleanup: + * turn calls to `C4_ASSERT()` into calls to `RYML_ASSERT()` + * use `c4::MemoryResource` in place of `c4::yml::MemoryResource`, and + remove `c4::yml::MemoryResource` + * Review `parse()` API: add suffixes `_in_situ` and `_in_arena` to clarify + intent. Ie: + * rename `parse(substr)` to `parse_in_situ(substr)` + * rename `parse(csubstr)` to `parse_in_arena(csubstr)` + * Add emit formatting controls: + * add single-line flow formatter + * add multi-line flow formatters + * indenting + * non indenting + * keep current block formatter + * add customizable linebreak limits (number of columns) to every formatter + * add per node format flags + * (lesser priority) add auto formatter using reasonable heuristics to + switch between other existing formatters + * Increase YAML test suite coverage + * use `csubstr` instead `csubstr const&` in return and parameter types, but + quantify effect in runtimes. diff --git a/src/c4/yml/tree.cpp b/src/c4/yml/tree.cpp index 9c7acb0c..61c64234 100644 --- a/src/c4/yml/tree.cpp +++ b/src/c4/yml/tree.cpp @@ -907,6 +907,7 @@ void Tree::merge_with(Tree const *src, size_t src_node, size_t dst_node) { remove_children(dst_node); } + _clear_type(dst_node); if(src->has_key(src_node)) { to_seq(dst_node, src->key(src_node)); @@ -932,6 +933,7 @@ void Tree::merge_with(Tree const *src, size_t src_node, size_t dst_node) { remove_children(dst_node); } + _clear_type(dst_node); if(src->has_key(src_node)) { to_map(dst_node, src->key(src_node)); diff --git a/src/c4/yml/tree.hpp b/src/c4/yml/tree.hpp index e9ea7910..f563bea1 100644 --- a/src/c4/yml/tree.hpp +++ b/src/c4/yml/tree.hpp @@ -24,11 +24,11 @@ class NodeRef; class Tree; -/** the integer necessary to cover all the bits marking node types */ +/** the integral type necessary to cover all the bits marking node types */ using type_bits = uint64_t; -/** the regular bytes for */ +/** a bit mask for marking node types */ typedef enum : type_bits { // a convenience define, undefined below @@ -792,14 +792,50 @@ private: public: - inline void _add_flags(size_t node, NodeType_e f) { _p(node)->m_type = (f | _p(node)->m_type); } - inline void _add_flags(size_t node, type_bits f) { _p(node)->m_type = (f | _p(node)->m_type); } + #ifndef RYML_USE_ASSERT + #define _check_next_flags(node, f) + #else + inline void _check_next_flags(size_t node, type_bits f) + { + auto n = _p(node); + type_bits o = n->m_type; // old + if(f & MAP) + { + RYML_ASSERT_MSG((f & SEQ) == 0, "cannot mark simultaneously as map and seq"); + RYML_ASSERT_MSG((f & VAL) == 0, "cannot mark simultaneously as map and val"); + RYML_ASSERT_MSG((o & SEQ) == 0, "cannot turn a seq into a map; clear first"); + RYML_ASSERT_MSG((o & VAL) == 0, "cannot turn a val into a map; clear first"); + } + else if(f & SEQ) + { + RYML_ASSERT_MSG((f & MAP) == 0, "cannot mark simultaneously as seq and map"); + RYML_ASSERT_MSG((f & VAL) == 0, "cannot mark simultaneously as seq and val"); + RYML_ASSERT_MSG((o & MAP) == 0, "cannot turn a map into a seq; clear first"); + RYML_ASSERT_MSG((o & VAL) == 0, "cannot turn a val into a seq; clear first"); + } + if(f & KEY) + { + RYML_ASSERT(!is_root(node)); + auto pid = parent(node); + RYML_ASSERT(is_map(pid)); + } + if(f & VAL) + { + RYML_ASSERT(!is_root(node)); + auto pid = parent(node); + RYML_ASSERT(is_map(pid) || is_seq(pid)); + } + } + #endif - inline void _rem_flags(size_t node, NodeType_e f) { _p(node)->m_type = ((~f) & _p(node)->m_type); } - inline void _rem_flags(size_t node, type_bits f) { _p(node)->m_type = ((~f) & _p(node)->m_type); } + inline void _set_flags(size_t node, NodeType_e f) { _check_next_flags(node, f); _p(node)->m_type = f; } + inline void _set_flags(size_t node, type_bits f) { _check_next_flags(node, f); _p(node)->m_type = f; } - inline void _set_flags(size_t node, NodeType_e f) { _p(node)->m_type = f; } - inline void _set_flags(size_t node, type_bits f) { _p(node)->m_type = f; } + inline void _add_flags(size_t node, NodeType_e f) { NodeData *d = _p(node); type_bits fb = f | d->m_type; _check_next_flags(node, fb); d->m_type = (NodeType_e) fb; } + inline void _add_flags(size_t node, type_bits f) { NodeData *d = _p(node); f |= d->m_type; _check_next_flags(node, f); d->m_type = f; } + + inline void _rem_flags(size_t node, NodeType_e f) { NodeData *d = _p(node); type_bits fb = d->m_type & ~f; _check_next_flags(node, fb); d->m_type = (NodeType_e) fb; } + inline void _rem_flags(size_t node, type_bits f) { NodeData *d = _p(node); f = d->m_type & ~f; _check_next_flags(node, f); d->m_type = f; } void _set_key(size_t node, csubstr const& key, type_bits more_flags=0) { @@ -814,8 +850,8 @@ public: void _set_val(size_t node, csubstr const& val, type_bits more_flags=0) { - C4_ASSERT(num_children(node) == 0); - C4_ASSERT( ! is_container(node)); + RYML_ASSERT(num_children(node) == 0); + RYML_ASSERT( ! is_container(node)); _p(node)->m_val.scalar = val; _add_flags(node, VAL|more_flags); } @@ -923,6 +959,11 @@ public: dst.m_val = src.m_val; } + inline void _clear_type(size_t node) + { + _p(node)->m_type = NOTYPE; + } + inline void _clear(size_t node) { auto *C4_RESTRICT n = _p(node); diff --git a/test/github_issues.cpp b/test/github_issues.cpp index e5a370a8..04968788 100644 --- a/test/github_issues.cpp +++ b/test/github_issues.cpp @@ -3,6 +3,63 @@ namespace c4 { namespace yml { + +TEST(github, 31) +{ + Tree tree; + NodeRef r = tree.rootref(); + r |= MAP; + + auto meas = r["meas"]; + meas |= MAP; + + auto plist = meas["createParameterList"]; + plist |= SEQ; + + { + auto lumi = plist.append_child(); + lumi << "Lumi"; + EXPECT_TRUE(lumi.is_val()); + } + + { + auto lumi = plist.append_child(); + lumi |= MAP; + lumi["value"] << 1; + lumi["relErr"] << 0.1; + EXPECT_TRUE(lumi.is_map()); + } + + { + ExpectError::do_check([&](){ + auto lumi = plist.append_child(); + lumi << "Lumi"; + lumi |= MAP; + }); + } + + { + ExpectError::do_check([&](){ + auto lumi = plist.append_child(); + lumi << "Lumi"; + lumi |= SEQ; + }); + } + + { + ExpectError::do_check([&](){ + auto lumi = plist.append_child(); + lumi |= MAP; + lumi << "Lumi"; + }); + } +} + + +//----------------------------------------------------------------------------- +//----------------------------------------------------------------------------- +//----------------------------------------------------------------------------- + #define GITHUB_ISSUES_CASES \ "github3-problem1",\ "github3-problem2-ex1",\