Add roundtrip engine tests, fix minor problems

Author: biojppm

changelog/current.md

@@ -1,15 +1,36 @@
-### Changes
+### New features
 
-- [PR#547](https://github.com/biojppm/rapidyaml/pull/547) - Fix parsing of implicit first documents with empty sequences, caused by a problem in `Tree::set_root_as_stream()`:
-  ```yaml
-  []  # this container was lost during parsing
-  ---
-  more data here
-  ```
-- [PR#561](https://github.com/biojppm/rapidyaml/pull/561) (fixes [#559](https://github.com/biojppm/rapidyaml/issues/559)) - Byte Order Mark: account for BOM when determining block indentation
-- [PR#563](https://github.com/biojppm/rapidyaml/pull/563) (fixes [#562](https://github.com/biojppm/rapidyaml/issues/562)) - Fix bug in `NodeRef::cend()`
-- [PR#565](https://github.com/biojppm/rapidyaml/pull/565) (fixes [#564](https://github.com/biojppm/rapidyaml/issues/564)) - `Tree` arena: allow relocation of zero-length strings when placed at the end (relax assertions triggered in `Tree::_relocated()`)
-- [PR#557](https://github.com/biojppm/rapidyaml/pull/557) - `Tree` is now non-empty by default, and `Tree::root_id()` will no longer modify the tree when it is empty. To create an empty tree now, it is necessary to use the capacity constructor with a capacity of zero:
+- [PR#550](https://github.com/biojppm/rapidyaml/pull/550) - Implement flow multiline style (`FLOW_ML`):
+  - The parser now detects this style automatically for flow seqs or maps when the terminating bracket sits on a line different from the opening bracket.
+  - Added `ParserOptions::detect_flow_ml()` to enable/disable this behavior
+  - Added `EmitOptions::indent_flow_ml()` to control indentation of FLOW_ML containers
+  - The emit implementation logic was refactored, and is now significantly cleaner
+  - Emitted YAML will now have anchors emitted before tags, as is customary ([see example](https://play.yaml.io/main/parser?input=LSAhdGFnICZhbmNob3IgfAogIG5vdGUgaG93IHRoZSBhbmNob3IgY29tZXMKICBmaXJzdCBpbiB0aGUgZXZlbnRz)). Previously this 
+  - Added `ParserOptions` defaulted argument to temp-parser overloads of `parse_{yaml,json}_in_{place,arena}()`
+
+
+### API changes
+
+- **BREAKING** [PR#503](https://github.com/biojppm/rapidyaml/pull/503) (fixes [#399](https://github.com/biojppm/rapidyaml/issues/399)): change error callbacks.
+  - Errors in ryml now have one of these types:
+    - parse error: when parsing YAML/JSON. See: `pfn_error_parse`,  `ErrorDataParse`, `ExceptionParse`, `err_parse_format()`, `sample_error_parse`.
+    - visit error: when visiting a tree (reading or writing). See: `pfn_error_visit`,  `ErrorDataVisit`, `ExceptionVisit`, `err_visit_format()`, `sample_error_visit`.
+    - basic error: other, non specific errors. See: `pfn_error_basic`,  `ErrorDataBasic`, `ExceptionBasic`, `err_basic_format()`, `sample_error_basic`.
+    - parse and visit errors/exceptions can be treated/caught as basic errors/exceptions.
+  - Add message formatting functions to simplify user-provided implementation of error callbacks:
+    - `err_parse_format()`: format/print a full error message for a parse error
+    - `err_visit_format()`: format/print a full error message for a visit error
+    - `err_basic_format()`: format/print a full error message for a basic error
+    - `location_format()`: format/print a location
+    - `location_format_with_context()`: useful to create a rich error message showing the YAML region causing the error, maybe even for a visit error if the source is kept and locations are enabled.
+    - `format_exc()`: format an exception (when exceptions are enabled)
+  - See the new header `c4/yml/error.hpp` (and `c4/yml/error.def.hpp` for definitions of the functions in `c4/yml/error.hpp`)
+  - See the relevant sample functions in the quickstart sample: `sample_error_basic`, `sample_error_parse` and `sample_error_visit`.
+  - There are breaking user-facing changes in the `Callbacks` structure:
+    - Removed member `m_error `
+    - Added members `m_error_basic`, `m_error_parse`, `m_error_visit`
+    - Added methods `.set_error_basic()`, `.set_error_parse()` and `.set_error_visit()`.
+- **BREAKING** [PR#557](https://github.com/biojppm/rapidyaml/pull/557) - `Tree` is now non-empty by default, and `Tree::root_id()` will no longer modify the tree when it is empty. To create an empty tree, it is now necessary to use the capacity constructor with a capacity of zero:
   ```c++
   // breaking change: default-constructed tree is now non-empty
   Tree tree;
@@ -24,39 +45,25 @@
   ```
   This changeset also enables the python library to call `root_id()` on a default-constructed tree (fixes [#556](https://github.com/biojppm/rapidyaml/issues/556)).
 - [PR#560](https://github.com/biojppm/rapidyaml/pull/560) (see also [#554](https://github.com/biojppm/rapidyaml/issues/554)): python improvements:
-  - expose `Tree::to_arena()` in python. This allows safer and easier programatic creation of trees in python by ensuring scalars have the same lifetime as the tree:
+  - expose `Tree::to_arena()` in python. This allows safer and easier programatic creation of trees in python by ensuring scalars are placed into the tree and so have the same lifetime as the tree:
   ```python
   t = ryml.Tree()
-  s = t.to_arena(temp_string()) # Save a temporary string to the tree's arena.
+  s = t.to_arena(temp_string()) # Copy/serialize a temporary string to the tree's arena.
                                 # Works also with integers and floats.
   t.to_val(t.root_id(), s)      # Now we can safely use the scalar in the tree:
                                 # there is no longer any risk of it being deallocated
   ```
   - improve behavior of `Tree` methods accepting scalars: all standard buffer types are now accepted (ie, `str`, `bytes`, `bytearray` and `memoryview`).
-- [PR#550](https://github.com/biojppm/rapidyaml/pull/550) - Implement FLOW_ML style (flow multiline).
-  - The parser now sets this style automatically for flow seqs or maps when the terminating bracket sits on a line different from the opening bracket.
-  - Added `ParserOptions::detect_flow_ml()` to control this behavior
-  - Added `EmitOptions::indent_flow_ml()` to control indentation of FLOW_ML containers
-  - The emit implementation logic was refactored, and is now significantly cleaner
-  - Emitted YAML will now have anchors emitted before tags, as is customary ([see example](https://play.yaml.io/main/parser?input=LSAhdGFnICZhbmNob3IgfAogIG5vdGUgaG93IHRoZSBhbmNob3IgY29tZXMKICBmaXJzdCBpbiB0aGUgZXZlbnRz)).
-  - Added `ParserOptions` defaulted argument to temp-parser overloads of `parse_{yaml,json}_in_{place,arena}()`
- [PR#503](https://github.com/biojppm/rapidyaml/pull/503) (fixes [#399](https://github.com/biojppm/rapidyaml/issues/399)): change error callbacks.
-  - Errors in ryml now have one of these types:
-    - parse error: when parsing YAML/JSON. See: `pfn_error_parse`,  `ErrorDataParse`, `ExceptionParse`, `err_parse_format()`, `sample_error_parse`.
-    - visit error: when visiting a tree (reading or writing). See: `pfn_error_visit`,  `ErrorDataVisit`, `ExceptionVisit`, `err_visit_format()`, `sample_error_visit`.
-    - basic error: other, non specific errors. See: `pfn_error_basic`,  `ErrorDataBasic`, `ExceptionBasic`, `err_basic_format()`, `sample_error_basic`.
-    - parse and visit errors/exceptions can be treated/caught as basic errors/exceptions
-  - Add message formatting functions to simplify implementation of error callbacks:
-    - `err_parse_format()`: format/print a full error message for a parse error
-    - `err_visit_format()`: format/print a full error message for a visit error
-    - `err_basic_format()`: format/print a full error message for a basic error
-    - `location_format()`: format/print a location
-    - `location_format_with_context()`: useful to create a rich error message showing the YAML region causing the error, maybe even for a visit error if the source is kept and locations are enabled.
-    - `format_exc()`: when exceptions are enabled
-  - See the new header `c4/yml/error.hpp` (and `c4/yml/error.def.hpp` for definitions of the functions in `c4/yml/error.hpp`)
-  - See the relevant sample functions in the quickstart sample: `sample_error_basic`, `sample_error_parse` and `sample_error_visit`.
-  - There are breaking user-facing changes in the `Callbacks` structure:
-    - Removed member `m_error `
-    - Added members `m_error_basic`, `m_error_parse`, `m_error_visit`
-    - Added methods `.set_error_basic()`, `.set_error_parse()` and `.set_error_visit()`.
+- [PR#565](https://github.com/biojppm/rapidyaml/pull/565) (fixes [#564](https://github.com/biojppm/rapidyaml/issues/564)) - `Tree` arena: allow relocation of zero-length strings when placed at the end (relax assertions triggered in `Tree::_relocated()`)
+- [PR#563](https://github.com/biojppm/rapidyaml/pull/563) (fixes [#562](https://github.com/biojppm/rapidyaml/issues/562)) - Fix bug in `NodeRef::cend()`
+
 
+### Fixes in YAML parsing
+
+- [PR#561](https://github.com/biojppm/rapidyaml/pull/561) (fixes [#559](https://github.com/biojppm/rapidyaml/issues/559)) - Byte Order Mark: account for BOM length when determining block indentation
+- [PR#547](https://github.com/biojppm/rapidyaml/pull/547) - Fix parsing of implicit first documents with empty sequences, caused by a problem in `Tree::set_root_as_stream()`:
+  ```yaml
+  []  # this container was lost during parsing
+  ---
+  more data here
+  ```

samples/quickstart.cpp

@@ -4307,7 +4307,7 @@ void sample_style()
 {
     // we will be using this helper throughout this function
     auto tostr = [](ryml::ConstNodeRef n) { return ryml::emitrs_yaml<std::string>(n); };
-    // let's parse this:
+    // let's parse this yaml:
     ryml::csubstr yaml = R"(block map:
   block key: block val
 block seq:
@@ -4345,11 +4345,15 @@ flow seq, multiline: [
     CHECK(tree["flow seq, singleline"].is_flow());
     CHECK(tree["flow map, multiline"].is_flow());
     CHECK(tree["flow seq, multiline"].is_flow());
-    // since the tree nodes are marked with style during the parse,
-    // emission will preserve the original style (minus whitespace):
+    //
+    // since the tree nodes are marked with their original parsed
+    // style, emitting the parsed tree will preserve the original
+    // style (minus whitespace):
+    //
     CHECK(tostr(tree) == yaml); // same as before!
     //
     // you can set/modify the style programatically!
+    //
     // here are more examples.
     //
     {
@@ -4450,9 +4454,9 @@ flow seq, multiline: [flow val,flow val]
     tree.rootref().clear_style(/*recurse*/true);
     // when emitting nodes which have no style set, ryml will default
     // to block format for containers, and call
-    // ryml::scalar_style_choose() to pick the style for each
-    // scalar. Note that it picks single-quoted for the scalars
-    // containing commas:
+    // ryml::scalar_style_choose() to pick the style for each scalar
+    // (at the cost of a scan over each scalar). Note that ryml picks
+    // single-quoted for scalars containing commas:
     CHECK(tostr(tree) ==
           R"(block map:
   block key: block val
@@ -4472,7 +4476,8 @@ block seq:
   - flow val
 )");
     // you can set the style based on type conditions:
-    // set a single key to single-quoted
+    //
+    // eg, set a single key to single-quoted
     tree["block map"].set_style_conditionally(/*type_mask*/ryml::KEY,
                                               /*remflags*/ryml::KEY_STYLE,
                                               /*addflags*/ryml::KEY_SQUO,
@@ -4662,6 +4667,21 @@ void sample_style_flow_ml_filter()
     ]
   }
 }
+)";
+    ryml::csubstr yaml_not_indented = R"({
+map: {
+seq: [
+0,
+1,
+2,
+3,
+[
+40,
+41
+]
+]
+}
+}
 )";
     // note that the parser defaults to detect multiline flow
     // (FLOW_ML) containers:
@@ -4687,22 +4707,7 @@ void sample_style_flow_ml_filter()
         const ryml::EmitOptions noindent = ryml::EmitOptions{}.indent_flow_ml(false);
         const ryml::Tree tree = ryml::parse_in_arena(yaml);
         CHECK(tree["map"].is_flow_ml()); // etc
-        CHECK(ryml::emitrs_yaml<std::string>(tree, noindent) ==
-              R"({
-map: {
-seq: [
-0,
-1,
-2,
-3,
-[
-40,
-41
-]
-]
-}
-}
-)");
+        CHECK(ryml::emitrs_yaml<std::string>(tree, noindent) == yaml_not_indented);
     }
 }
 

src/c4/yml/detail/dbgprint.hpp

@@ -57,8 +57,6 @@
 #endif
 
 
-#define RYML_LOGBUF_SIZE_MAX RYML_ERRMSG_SIZE
-
 #include <c4/dump.hpp>
 
 C4_SUPPRESS_WARNING_GCC_WITH_PUSH("-Wattributes")

src/c4/yml/emit.def.hpp

@@ -41,6 +41,7 @@ substr Emitter<Writer>::emit_as(EmitType_e type, Tree const& tree, id_type id, b
     return this->Writer::_get(error_on_excess);
 }
 
+/** @cond dev */
 
 //-----------------------------------------------------------------------------
 
@@ -1476,6 +1477,8 @@ void Emitter<Writer>::_write_scalar_json_dquo(csubstr s)
     _write('"');
 }
 
+/** @endcond */
+
 } // namespace yml
 } // namespace c4
 

src/c4/yml/emit.hpp

@@ -59,6 +59,7 @@ struct EmitOptions
 {
 public:
 
+    /** @cond dev */
     typedef enum : uint32_t {
         EMIT_NONROOT_KEY = 1u << 0u,
         EMIT_NONROOT_DASH = 1u << 1u,
@@ -69,6 +70,7 @@ struct EmitOptions
         _JSON_ERR_MASK = JSON_ERR_ON_TAG|JSON_ERR_ON_ANCHOR,
         DEFAULT_FLAGS = EMIT_NONROOT_KEY|INDENT_FLOW_ML,
     } EmitOptionFlags_e;
+    /** @endcond */
 
 public:
 
@@ -368,19 +370,27 @@ class Emitter : public Writer
 
 private:
 
+    // g++-4.8 has problems with the operand types here...
+    #if defined(__GNUC__) && (__GNUC__ < 5) && (!defined(__clang__))
+    #pragma GCC diagnostic push
+    #pragma GCC diagnostic ignored "-Wparentheses"
+    #endif
     enum : type_bits {
         _styles_block_key = KEY_LITERAL|KEY_FOLDED,
         _styles_block_val = VAL_LITERAL|VAL_FOLDED,
-        _styles_block     = _styles_block_key|_styles_block_val,
-        _styles_flow_key  = KEY_STYLE & ~_styles_block_key,
-        _styles_flow_val  = VAL_STYLE & ~_styles_block_val,
-        _styles_flow      = _styles_flow_key|_styles_flow_val,
+        _styles_block     = ((type_bits)_styles_block_key) | ((type_bits)_styles_block_val),
+        _styles_flow_key  = KEY_STYLE & (~((type_bits)_styles_block_key)),
+        _styles_flow_val  = VAL_STYLE & (~((type_bits)_styles_block_val)),
+        _styles_flow      = ((type_bits)_styles_flow_key) | ((type_bits)_styles_flow_val),
         _styles_squo      = KEY_SQUO|VAL_SQUO,
         _styles_dquo      = KEY_DQUO|VAL_DQUO,
         _styles_plain     = KEY_PLAIN|VAL_PLAIN,
         _styles_literal   = KEY_LITERAL|VAL_LITERAL,
         _styles_folded    = KEY_FOLDED|VAL_FOLDED,
     };
+    #if defined(__GNUC__) && (__GNUC__ < 5) && (!defined(__clang__))
+    #pragma GCC diagnostic pop
+    #endif
 
     /** @endcond */
 };

src/c4/yml/event_handler_tree.hpp

@@ -349,7 +349,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
         _c4dbgpf("node[{}]: added sibling={} prev={}", m_parent->node_id, m_curr->node_id, m_tree->prev_sibling(m_curr->node_id));
     }
 
-    /** set the previous val as the first key of a new map, with flow style.
+    /** reset the previous val as the first key of a new map, with flow style.
      *
      * See the documentation for @ref doc_event_handlers, which has
      * important notes about this event.
@@ -363,7 +363,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
         _RYML_ASSERT_VISIT_(m_stack.m_callbacks, !m_tree->is_container(m_curr->node_id), m_tree, m_curr->node_id);
         _RYML_ASSERT_VISIT_(m_stack.m_callbacks, !m_tree->has_key(m_curr->node_id), m_tree, m_curr->node_id);
         const NodeData tmp = _val2key_(*m_curr->tr_data);
-        _disable_(_VALMASK|VAL_STYLE);
+        _disable_(_VALMASK|VAL_STYLE|VALNIL);
         m_curr->tr_data->m_val = {};
         begin_map_val_flow();
         m_curr->tr_data->m_type = tmp.m_type;
@@ -739,6 +739,8 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
         r.m_type.type = ((d.m_type.type & (_VALMASK|VAL_STYLE)) >> 1u);
         r.m_type.type = (r.m_type.type & ~(_VALMASK|VAL_STYLE));
         r.m_type.type = (r.m_type.type | KEY);
+        if(d.m_type.type & VALNIL)
+            r.m_type.type = (r.m_type.type | KEYNIL);
         return r;
     }
 

src/c4/yml/node_type.hpp

@@ -270,7 +270,6 @@ RYML_EXPORT inline C4_NO_INLINE bool scalar_is_null(csubstr s) noexcept
 
 /** @} */
 
-
 /** @} */
 
 } // namespace yml

src/c4/yml/parse_engine.def.hpp

@@ -803,6 +803,7 @@ bool ParseEngine<EventHandler>::_is_valid_start_scalar_plain_flow(csubstr s)
             case '}':
             case ']':
             case '\r':
+            _RYML_WITH_TAB_TOKENS(case '\t':)
                 if(s.str[0] == ':')
                 {
                     _c4dbgpf("not a scalar: found non-scalar token '{}{}'", s.str[0], s.str[1]);
@@ -952,7 +953,7 @@ bool ParseEngine<EventHandler>::_scan_scalar_plain_seq_flow(ScannedScalar *C4_RE
                 _c4dbgp("found suspicious '#'");
                 _RYML_ASSERT_BASIC_(m_evt_handler->m_stack.m_callbacks, offs > 0);
                 char prev = s.str[offs - 1];
-                if(prev == ' ' _RYML_WITH_TAB_TOKENS((|| prev == '\t')))
+                if(prev == ' ' _RYML_WITH_TAB_TOKENS(|| prev == '\t'))
                 {
                     _c4dbgpf("found terminating character at {}: '{}'", offs, c);
                     goto ended_scalar;
@@ -2205,7 +2206,7 @@ void ParseEngine<EventHandler>::_scan_block(ScannedBlock *C4_RESTRICT sb, size_t
             if(fns != npos) // non-empty line
             {
                 _RYML_WITH_TAB_TOKENS(
-                    if(C4_UNLIKELY(lc.stripped.begins_with('\t')))
+                    if(C4_UNLIKELY(lc.full.begins_with('\t')))
                         _c4err("parse error");
                 )
                 _c4dbgpf("blck: line not empty. indref={} indprov={} indentation={}", indref, provisional_indentation, lc.indentation);

src/c4/yml/parser_state.hpp

@@ -56,7 +56,7 @@ csubstr _parser_flags_to_str(substr buf, ParserFlag_t flags);
 struct LineContents
 {
     substr  rem;         ///< current line remainder, without newline characters
-    substr  full;        ///< full line, including newline characters \n and \r
+    substr  full;        ///< full line, including newline characters `\n` and `\r`
     size_t  num_cols;    ///< number of columns in the line, excluding newline
                          ///< characters (ie the initial size of rem)
     size_t  indentation; ///< number of spaces on the beginning of the line.

src/c4/yml/tree.hpp

@@ -865,7 +865,7 @@ class RYML_EXPORT Tree
      * you can overload c4::to_chars(substr, T const&)
      *
      * @note To customize how the type gets serialized to the arena,
-     * you can overload @ref c4::yml::serialize_scalar(substr, T const&)
+     * you can overload @ref serialize_scalar()
      *
      * @note Growing the arena may cause relocation of the entire
      * existing arena, and thus change the contents of individual