Skip to content

Commit

Permalink
Improve the node API:
Browse files Browse the repository at this point in the history
- improve explanation of states
- deprecate .valid()
- add .readable() state
- deprecate operator==(nullptr)
- deprecate operator==(csubstr)
  • Loading branch information
biojppm committed Apr 5, 2024
1 parent 12236b7 commit 2c9c4b5
Show file tree
Hide file tree
Showing 12 changed files with 235 additions and 336 deletions.
71 changes: 16 additions & 55 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ CHECK(tree.arena() == "says who2030deere"); // the result of serializations to t
// root["john"] = ryml::to_csubstr(ok); // don't, will dangle
wroot["john"] << ryml::to_csubstr(ok); // OK, copy to the tree's arena
}
CHECK(root["john"] == "in_scope"); // OK!
CHECK(root["john"].val() == "in_scope"); // OK!
// serializing floating points:
wroot["float"] << 2.4;
// to force a particular precision or float format:
Expand Down Expand Up @@ -592,8 +592,9 @@ CHECK(root["newmap"].is_map());
CHECK(root["newmap (serialized)"].num_children() == 0);
CHECK(root["newmap (serialized)"].is_map());
//
// When the tree is mutable, operator[] does not mutate the tree
// until the returned node is written to.
// When the tree is mutable, operator[] first searches the tree
// for the does not mutate the tree until the returned node is
// written to.
//
// Until such time, the NodeRef object keeps in itself the required
// information to write to the proper place in the tree. This is
Expand All @@ -615,36 +616,38 @@ CHECK(root["newmap (serialized)"].is_map());
// tree.
//
CHECK(!root.has_child("I am not nothing"));
ryml::NodeRef nothing = wroot["I am nothing"];
CHECK(nothing.valid()); // points at the tree, and a specific place in the tree
ryml::NodeRef nothing;
CHECK(nothing.invalid()); // invalid because it points at nothing
nothing = wroot["I am nothing"];
CHECK(!nothing.invalid()); // points at the tree, and a specific place in the tree
CHECK(nothing.is_seed()); // ... but nothing is there yet.
CHECK(!root.has_child("I am nothing")); // same as above
CHECK(!nothing.readable()); // ... and this node cannot be used to
// read anything from the tree
ryml::NodeRef something = wroot["I am something"];
ryml::ConstNodeRef constsomething = wroot["I am something"];
CHECK(!root.has_child("I am something")); // same as above
CHECK(something.valid());
CHECK(!something.invalid());
CHECK(something.is_seed()); // same as above
CHECK(!something.readable()); // same as above
CHECK(!constsomething.valid()); // NOTE: because a ConstNodeRef cannot be
// used to mutate a tree, it is only valid()
// if it is pointing at an existing node.
CHECK(constsomething.invalid()); // NOTE: because a ConstNodeRef cannot be
// used to mutate a tree, it is only valid()
// if it is pointing at an existing node.
something = "indeed"; // this will commit the seed to the tree, mutating at the proper place
CHECK(root.has_child("I am something"));
CHECK(root["I am something"].val() == "indeed");
CHECK(something.valid()); // it was already valid
CHECK(!something.invalid()); // it was already valid
CHECK(!something.is_seed()); // now the tree has this node, so the
// ref is no longer a seed
CHECK(something.readable()); // and it is now readable
//
// now the constref is also valid (but it needs to be reassigned):
ryml::ConstNodeRef constsomethingnew = wroot["I am something"];
CHECK(constsomethingnew.valid());
CHECK(!constsomethingnew.invalid());
CHECK(constsomethingnew.readable());
// note that the old constref is now stale, because it only keeps
// the state at creation:
CHECK(!constsomething.valid());
CHECK(constsomething.invalid());
CHECK(!constsomething.readable());
//
// -----------------------------------------------------------
Expand All @@ -664,6 +667,7 @@ ryml::NodeRef wbar = wroot["bar"];
if(wbar.readable() && wbar.is_seq()) // .is_seq() requires .readable()
{
CHECK(wbar[0].readable() && wbar[0].val() == "20");
CHECK( ! wbar[100].readable());
CHECK( ! wbar[100].readable() || wbar[100].val() == "100"); // <- no crash because it is not .readable(), so never tries to call .val()
// this would work as well:
CHECK( ! wbar[0].is_seed() && wbar[0].val() == "20");
Expand Down Expand Up @@ -785,49 +789,6 @@ ryml::Location loc = parser.location(tree2["bar"][1]);
CHECK(parser.location_contents(loc).begins_with("30"));
CHECK(loc.line == 3u);
CHECK(loc.col == 4u);
// For further details in location tracking,
// refer to the sample function below.
```
The [quickstart.cpp sample](./samples/quickstart.cpp) (from which the
above overview was taken) has many more detailed examples, and should
be your first port of call to find out any particular point about
ryml's API. It is tested in the CI, and thus has the correct behavior.
There you can find the following subjects being addressed:
```c++
sample_quick_overview(); ///< briefly skim over most of the features
sample_substr(); ///< about ryml's string views (from c4core)
sample_parse_file(); ///< ready-to-go example of parsing a file from disk
sample_parse_in_place(); ///< parse a mutable YAML source buffer
sample_parse_in_arena(); ///< parse a read-only YAML source buffer
sample_parse_reuse_tree(); ///< parse into an existing tree, maybe into a node
sample_parse_reuse_parser(); ///< reuse an existing parser
sample_parse_reuse_tree_and_parser(); ///< how to reuse existing trees and parsers
sample_iterate_trees(); ///< visit individual nodes and iterate through trees
sample_create_trees(); ///< programatically create trees
sample_tree_arena(); ///< interact with the tree's serialization arena
sample_fundamental_types(); ///< serialize/deserialize fundamental types
sample_formatting(); ///< control formatting when serializing/deserializing
sample_base64(); ///< encode/decode base64
sample_user_scalar_types(); ///< serialize/deserialize scalar (leaf/string) types
sample_user_container_types(); ///< serialize/deserialize container (map or seq) types
sample_std_types(); ///< serialize/deserialize STL containers
sample_float_precision(); ///< control precision of serialized floats
sample_emit_to_container(); ///< emit to memory, eg a string or vector-like container
sample_emit_to_stream(); ///< emit to a stream, eg std::ostream
sample_emit_to_file(); ///< emit to a FILE*
sample_emit_nested_node(); ///< pick a nested node as the root when emitting
sample_emit_style(); ///< set the nodes to FLOW/BLOCK format
sample_json(); ///< JSON parsing and emitting
sample_anchors_and_aliases(); ///< deal with YAML anchors and aliases
sample_tags(); ///< deal with YAML type tags
sample_docs(); ///< deal with YAML docs
sample_error_handler(); ///< set a custom error handler
sample_global_allocator(); ///< set a global allocator for ryml
sample_per_tree_allocator(); ///< set per-tree allocators
sample_static_trees(); ///< how to use static trees in ryml
sample_location_tracking(); ///< track node locations in the parsed source tree
```
Expand Down
21 changes: 17 additions & 4 deletions changelog/current.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

Fix major error handling problem reported in [#389](https://github.com/biojppm/rapidyaml/issues/389) ([PR#411](https://github.com/biojppm/rapidyaml/pull/411)):

- The `NodeRef` and `ConstNodeRef` classes had many methods marked `noexcept` that were doing assertions which could throw exceptions, causing an abort instead of a throw whenever the assertion called an exception-throwing error callback.
- The `NodeRef` and `ConstNodeRef` classes are now conditional noexcept using `RYML_NOEXCEPT`, which evaluates either to nothing when assertions are enabled, and to `noexcept` otherwise. The problem was that these classes had many methods explicitly marked `noexcept`, but were doing assertions which could throw exceptions, causing an abort instead of a throw whenever the assertion called an exception-throwing error callback.
- Also, this problem was compounded by exceptions being enabled in every build type -- despite the intention to have them only in debug builds. There was a problem in the preprocessor code to enable assertions which led to assertions being enabled in release builds even when `RYML_USE_ASSERT` was defined to 0. Thanks to @jdrouhard for reporting this.
- Although the code is and was extensively tested, the testing was addressing mostly the happy path. Tests were added to ensure that the error behavior is as intended.
- Together with this changeset, a major revision was carried out of the asserting/checking status of each function in the node classes. In most cases, assertions were added to functions cases that were missing them. So **beware** - user code that was invalid will now assert or error out. Also, assertions and checks are now directed as much as possible to the callbacks of the closest scope, ie if a tree has custom callbacks, errors should go through those callbacks.
- Together with this changeset, a major revision was carried out of the asserting/checking status of each function in the node classes. In most cases, assertions were added to functions cases that were missing them. So **beware** - user code that was invalid will now assert or error out. Also, assertions and checks are now directed as much as possible to the callbacks of the closest scope, ie if a tree has custom callbacks, errors within the tree class should go through those callbacks.
- Also, the intended assertion behavior is now in place: *no assertions in release builds*. **Beware** as well - user code which was relying on this will now silently succeed and return garbage in release builds. See the next points, which may help:
- Added new methods to the `NodeRef`/`ConstNodeRef` classes:
```c++
Expand Down Expand Up @@ -38,9 +38,21 @@ Fix major error handling problem reported in [#389](https://github.com/biojppm/r
/** Likewise, but return a seed node when pos is not found */
NodeRef at(csubstr key);
```
- The state for `NodeRef` was refined, and now there are three mutually exclusive states (and class predicates) for an object of this class:
- `.invalid()` when the object was not initialized to any node
- `.readable()` when the object points at an existing tree+node
- `.is_seed()` when the object points at an hypotethic tree+node
- The previous state `.valid()` was deprecated: its semantics were confusing as it actually could be any of `.readable()` or `.is_seed()`
- Deprecated also the following methods for `NodeRef`/`ConstNodeRef`:
```c++
RYML_DEPRECATED() bool operator== (std::nullptr_t) const;
RYML_DEPRECATED() bool operator!= (std::nullptr_t) const;
RYML_DEPRECATED() bool operator== (csubstr val) const;
RYML_DEPRECATED() bool operator!= (csubstr val) const;
```
- Added macros and respective cmake options to control error handling:
- `RYML_USE_ASSERT` - enable assertions regardless of build type. This is disabled by default.
- `RYML_DEFAULT_CALLBACK_USES_EXCEPTIONS` - defines the same macro, which will make the default error handler provided by ryml throw exceptions instead of calling `std::abort()`. This is disabled by default.
- `RYML_USE_ASSERT` - enable assertions regardless of build type. This is disabled by default. This macro was already defined; the current PR adds the cmake option.
- `RYML_DEFAULT_CALLBACK_USES_EXCEPTIONS` - make the default error handler provided by ryml throw exceptions instead of calling `std::abort()`. This is disabled by default.
- Also, `RYML_DEBUG_BREAK()` is now enabled only if `RYML_DBG` is defined, as reported in [#362](https://github.com/biojppm/rapidyaml/issues/362).


Expand All @@ -62,6 +74,7 @@ Fix major error handling problem reported in [#389](https://github.com/biojppm/r
- Fix [#356](https://github.com/biojppm/rapidyaml/issues/356) - fix overzealous check in `emit_as()`. An id may be larger than the tree's size, eg when nodes were removed. ([PR#357](https://github.com/biojppm/rapidyaml/pull/357)).
- Fix [#417](https://github.com/biojppm/rapidyaml/issues/417)) - add quickstart example explaining how to avoid precision loss while serializing floats ([PR#420](https://github.com/biojppm/rapidyaml/pull/420)).


### Thanks

- @Neko-Box-Coder
Expand Down
36 changes: 20 additions & 16 deletions samples/quickstart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ void sample_quick_overview()
// root["john"] = ryml::to_csubstr(ok); // don't, will dangle
wroot["john"] << ryml::to_csubstr(ok); // OK, copy to the tree's arena
}
CHECK(root["john"] == "in_scope"); // OK!
CHECK(root["john"].val() == "in_scope"); // OK!
// serializing floating points:
wroot["float"] << 2.4;
// to force a particular precision or float format:
Expand Down Expand Up @@ -518,8 +518,9 @@ void sample_quick_overview()
CHECK(root["newmap (serialized)"].num_children() == 0);
CHECK(root["newmap (serialized)"].is_map());
//
// When the tree is mutable, operator[] does not mutate the tree
// until the returned node is written to.
// When the tree is mutable, operator[] first searches the tree
// for the does not mutate the tree until the returned node is
// written to.
//
// Until such time, the NodeRef object keeps in itself the required
// information to write to the proper place in the tree. This is
Expand All @@ -541,36 +542,38 @@ void sample_quick_overview()
// tree.
//
CHECK(!root.has_child("I am not nothing"));
ryml::NodeRef nothing = wroot["I am nothing"];
CHECK(nothing.valid()); // points at the tree, and a specific place in the tree
ryml::NodeRef nothing;
CHECK(nothing.invalid()); // invalid because it points at nothing
nothing = wroot["I am nothing"];
CHECK(!nothing.invalid()); // points at the tree, and a specific place in the tree
CHECK(nothing.is_seed()); // ... but nothing is there yet.
CHECK(!root.has_child("I am nothing")); // same as above
CHECK(!nothing.readable()); // ... and this node cannot be used to
// read anything from the tree
ryml::NodeRef something = wroot["I am something"];
ryml::ConstNodeRef constsomething = wroot["I am something"];
CHECK(!root.has_child("I am something")); // same as above
CHECK(something.valid());
CHECK(!something.invalid());
CHECK(something.is_seed()); // same as above
CHECK(!something.readable()); // same as above
CHECK(!constsomething.valid()); // NOTE: because a ConstNodeRef cannot be
// used to mutate a tree, it is only valid()
// if it is pointing at an existing node.
CHECK(constsomething.invalid()); // NOTE: because a ConstNodeRef cannot be
// used to mutate a tree, it is only valid()
// if it is pointing at an existing node.
something = "indeed"; // this will commit the seed to the tree, mutating at the proper place
CHECK(root.has_child("I am something"));
CHECK(root["I am something"].val() == "indeed");
CHECK(something.valid()); // it was already valid
CHECK(!something.invalid()); // it was already valid
CHECK(!something.is_seed()); // now the tree has this node, so the
// ref is no longer a seed
CHECK(something.readable()); // and it is now readable
//
// now the constref is also valid (but it needs to be reassigned):
ryml::ConstNodeRef constsomethingnew = wroot["I am something"];
CHECK(constsomethingnew.valid());
CHECK(!constsomethingnew.invalid());
CHECK(constsomethingnew.readable());
// note that the old constref is now stale, because it only keeps
// the state at creation:
CHECK(!constsomething.valid());
CHECK(constsomething.invalid());
CHECK(!constsomething.readable());
//
// -----------------------------------------------------------
Expand All @@ -590,6 +593,7 @@ void sample_quick_overview()
if(wbar.readable() && wbar.is_seq()) // .is_seq() requires .readable()
{
CHECK(wbar[0].readable() && wbar[0].val() == "20");
CHECK( ! wbar[100].readable());
CHECK( ! wbar[100].readable() || wbar[100].val() == "100"); // <- no crash because it is not .readable(), so never tries to call .val()
// this would work as well:
CHECK( ! wbar[0].is_seed() && wbar[0].val() == "20");
Expand Down Expand Up @@ -1994,13 +1998,13 @@ cars: GTO
void sample_create_trees()
{
ryml::NodeRef doe;
CHECK(!doe.valid()); // it's pointing at nowhere
CHECK(doe.invalid()); // it's pointing at nowhere

ryml::Tree tree;
ryml::NodeRef root = tree.rootref();
root |= ryml::MAP; // mark root as a map
doe = root["doe"];
CHECK(doe.valid()); // it's now pointing at the tree
CHECK(!doe.invalid()); // it's now pointing at the tree
CHECK(doe.is_seed()); // but the tree has nothing there, so this is only a seed

// set the value of the node
Expand Down Expand Up @@ -3957,8 +3961,8 @@ ship_to: *id001
CHECK( ! tree["val"].is_key_ref()); // notice *valref is now turned to val
CHECK( ! tree["val"].is_val_ref()); // notice *valref is now turned to val

CHECK(tree["ship_to"]["city"] == "East Centerville");
CHECK(tree["ship_to"]["state"] == "KS");
CHECK(tree["ship_to"]["city"].val() == "East Centerville");
CHECK(tree["ship_to"]["state"].val() == "KS");
}


Expand Down
2 changes: 1 addition & 1 deletion src/c4/yml/emit.def.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ substr Emitter<Writer>::emit_as(EmitType_e type, Tree const& t, bool error_on_ex
template<class Writer>
substr Emitter<Writer>::emit_as(EmitType_e type, ConstNodeRef const& n, bool error_on_excess)
{
_RYML_CB_CHECK(n.tree()->callbacks(), n.valid());
_RYML_CB_CHECK(n.tree()->callbacks(), n.readable());
return this->emit_as(type, *n.tree(), n.id(), error_on_excess);
}

Expand Down
8 changes: 4 additions & 4 deletions src/c4/yml/emit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ CharOwningContainer emitrs_json(Tree const& t)
template<class CharOwningContainer>
substr emitrs_yaml(ConstNodeRef const& n, CharOwningContainer * cont)
{
_RYML_CB_CHECK(n.tree()->callbacks(), n.valid());
_RYML_CB_CHECK(n.tree()->callbacks(), n.readable());
return emitrs_yaml(*n.tree(), n.id(), cont);
}
template<class CharOwningContainer>
Expand All @@ -447,7 +447,7 @@ RYML_DEPRECATE_EMITRS substr emitrs(ConstNodeRef const& n, CharOwningContainer *
template<class CharOwningContainer>
substr emitrs_json(ConstNodeRef const& n, CharOwningContainer * cont)
{
_RYML_CB_CHECK(n.tree()->callbacks(), n.valid());
_RYML_CB_CHECK(n.tree()->callbacks(), n.readable());
return emitrs_json(*n.tree(), n.id(), cont);
}

Expand All @@ -457,7 +457,7 @@ substr emitrs_json(ConstNodeRef const& n, CharOwningContainer * cont)
template<class CharOwningContainer>
CharOwningContainer emitrs_yaml(ConstNodeRef const& n)
{
_RYML_CB_CHECK(n.tree()->callbacks(), n.valid());
_RYML_CB_CHECK(n.tree()->callbacks(), n.readable());
CharOwningContainer c;
emitrs_yaml(*n.tree(), n.id(), &c);
return c;
Expand All @@ -473,7 +473,7 @@ RYML_DEPRECATE_EMITRS CharOwningContainer emitrs(ConstNodeRef const& n)
template<class CharOwningContainer>
CharOwningContainer emitrs_json(ConstNodeRef const& n)
{
_RYML_CB_CHECK(n.tree()->callbacks(), n.valid());
_RYML_CB_CHECK(n.tree()->callbacks(), n.readable());
CharOwningContainer c;
emitrs_json(*n.tree(), n.id(), &c);
return c;
Expand Down
Loading

0 comments on commit 2c9c4b5

Please sign in to comment.