From abc841d184e8aeceb4aa7ed0060f82eb7b9e6465 Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 28 Apr 2021 15:00:18 +0200 Subject: [PATCH 01/14] get this all compiling; currently with a pair notation; should be a simple return type instead --- include/yaml-cpp/exceptions.h | 8 +++ include/yaml-cpp/node/convert.h | 106 ++++++++++++++++------------ include/yaml-cpp/node/detail/impl.h | 28 +++++--- include/yaml-cpp/node/impl.h | 31 +++++--- src/convert.cpp | 14 ++-- 5 files changed, 118 insertions(+), 69 deletions(-) diff --git a/include/yaml-cpp/exceptions.h b/include/yaml-cpp/exceptions.h index c3f44747e..f08f17a26 100644 --- a/include/yaml-cpp/exceptions.h +++ b/include/yaml-cpp/exceptions.h @@ -298,6 +298,14 @@ class YAML_CPP_API BadFile : public Exception { BadFile(const BadFile&) = default; ~BadFile() YAML_CPP_NOEXCEPT override; }; + + +namespace conversion{ + class DecodeException : public std::runtime_error { + using runtime_error::runtime_error; + }; +} + } // namespace YAML #endif // EXCEPTIONS_H_62B23520_7C8E_11DE_8A39_0800200C9A66 diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 596898da6..12089ea38 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -15,6 +15,7 @@ #include #include #include +#include #include "yaml-cpp/binary.h" #include "yaml-cpp/node/impl.h" @@ -24,6 +25,19 @@ #include "yaml-cpp/null.h" + +namespace YAML{ +namespace conversion{ +namespace exception{ +class DecodeException : public std::runtime_error { + using runtime_error::runtime_error; +}; +} +} +} + + + namespace YAML { class Binary; struct _Null; @@ -63,12 +77,12 @@ template <> struct convert { static Node encode(const std::string& rhs) { return Node(rhs); } - static bool decode(const Node& node, std::string& rhs) { + static std::pair decode(const Node& node) { if (!node.IsScalar()) - return false; - rhs = node.Scalar(); - return true; + throw conversion::DecodeException(""); + return std::make_pair(true, node.Scalar()); } + }; // C-strings can only be encoded @@ -91,8 +105,10 @@ template <> struct convert<_Null> { static Node encode(const _Null& /* rhs */) { return Node(); } - static bool decode(const Node& node, _Null& /* rhs */) { - return node.IsNull(); + static std::pair decode(const Node& node) { + if (!node.IsNull()) + throw conversion::DecodeException(""); + return std::make_pair(node.IsNull(), _Null()); } }; @@ -156,37 +172,39 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { return Node(stream.str()); \ } \ \ - static bool decode(const Node& node, type& rhs) { \ + static std::pair decode(const Node& node) { \ if (node.Type() != NodeType::Scalar) { \ - return false; \ + throw conversion::DecodeException(""); \ } \ const std::string& input = node.Scalar(); \ std::stringstream stream(input); \ stream.unsetf(std::ios::dec); \ if ((stream.peek() == '-') && std::is_unsigned::value) { \ - return false; \ + throw conversion::DecodeException(""); \ } \ + type rhs; \ if (conversion::ConvertStreamTo(stream, rhs)) { \ - return true; \ + throw conversion::DecodeException(""); \ } \ if (std::numeric_limits::has_infinity) { \ if (conversion::IsInfinity(input)) { \ - rhs = std::numeric_limits::infinity(); \ - return true; \ + return std::make_pair(true, \ + std::numeric_limits::infinity()); \ } else if (conversion::IsNegativeInfinity(input)) { \ - rhs = negative_op std::numeric_limits::infinity(); \ - return true; \ + return std::make_pair(true, \ + negative_op std::numeric_limits::infinity()); \ } \ } \ \ if (std::numeric_limits::has_quiet_NaN) { \ if (conversion::IsNaN(input)) { \ rhs = std::numeric_limits::quiet_NaN(); \ - return true; \ + return std::make_pair(true, \ + std::numeric_limits::quiet_NaN()); \ } \ } \ \ - return false; \ + throw conversion::DecodeException(""); \ } \ } @@ -222,7 +240,7 @@ template <> struct convert { static Node encode(bool rhs) { return rhs ? Node("true") : Node("false"); } - YAML_CPP_API static bool decode(const Node& node, bool& rhs); + YAML_CPP_API static std::pair decode(const Node& node); }; // std::map @@ -235,11 +253,11 @@ struct convert> { return node; } - static bool decode(const Node& node, std::map& rhs) { + static std::pair > decode(const Node& node) { if (!node.IsMap()) - return false; + throw conversion::DecodeException(""); - rhs.clear(); + std::map rhs; for (const auto& element : node) #if defined(__GNUC__) && __GNUC__ < 4 // workaround for GCC 3: @@ -247,7 +265,7 @@ struct convert> { #else rhs[element.first.as()] = element.second.as(); #endif - return true; + return std::make_pair(true, rhs); } }; @@ -261,11 +279,11 @@ struct convert> { return node; } - static bool decode(const Node& node, std::vector& rhs) { + static std::pair > decode(const Node& node) { if (!node.IsSequence()) - return false; + throw conversion::DecodeException(""); - rhs.clear(); + std::vector rhs; for (const auto& element : node) #if defined(__GNUC__) && __GNUC__ < 4 // workaround for GCC 3: @@ -273,7 +291,7 @@ struct convert> { #else rhs.push_back(element.as()); #endif - return true; + return std::make_pair(true, rhs); } }; @@ -287,11 +305,11 @@ struct convert> { return node; } - static bool decode(const Node& node, std::list& rhs) { + static std::pair > decode(const Node& node) { if (!node.IsSequence()) - return false; + throw conversion::DecodeException(""); - rhs.clear(); + std::list rhs; for (const auto& element : node) #if defined(__GNUC__) && __GNUC__ < 4 // workaround for GCC 3: @@ -299,7 +317,7 @@ struct convert> { #else rhs.push_back(element.as()); #endif - return true; + return std::make_pair(true, rhs); } }; @@ -314,11 +332,11 @@ struct convert> { return node; } - static bool decode(const Node& node, std::array& rhs) { - if (!isNodeValid(node)) { - return false; - } + static std::pair > decode(const Node& node) { + if (!isNodeValid(node)) + throw conversion::DecodeException(""); + std::array rhs; for (auto i = 0u; i < node.size(); ++i) { #if defined(__GNUC__) && __GNUC__ < 4 // workaround for GCC 3: @@ -327,7 +345,7 @@ struct convert> { rhs[i] = node[i].as(); #endif } - return true; + return std::make_pair(true, rhs); } private: @@ -346,12 +364,11 @@ struct convert> { return node; } - static bool decode(const Node& node, std::pair& rhs) { - if (!node.IsSequence()) - return false; - if (node.size() != 2) - return false; + static std::pair > decode(const Node& node) { + if (!node.IsSequence() or node.size() != 2) + throw conversion::DecodeException(""); + std::pair rhs; #if defined(__GNUC__) && __GNUC__ < 4 // workaround for GCC 3: rhs.first = node[0].template as(); @@ -364,7 +381,7 @@ struct convert> { #else rhs.second = node[1].as(); #endif - return true; + return std::make_pair(true, rhs); } }; @@ -375,16 +392,17 @@ struct convert { return Node(EncodeBase64(rhs.data(), rhs.size())); } - static bool decode(const Node& node, Binary& rhs) { + static std::pair decode(const Node& node) { if (!node.IsScalar()) - return false; + throw conversion::DecodeException(""); std::vector data = DecodeBase64(node.Scalar()); if (data.empty() && !node.Scalar().empty()) - return false; + throw conversion::DecodeException(""); + Binary rhs; rhs.swap(data); - return true; + return std::make_pair(true, rhs); } }; } diff --git a/include/yaml-cpp/node/detail/impl.h b/include/yaml-cpp/node/detail/impl.h index b38038dfd..fe95b5df9 100644 --- a/include/yaml-cpp/node/detail/impl.h +++ b/include/yaml-cpp/node/detail/impl.h @@ -98,19 +98,31 @@ struct remove_idx inline bool node::equals(const T& rhs, shared_memory_holder pMemory) { - T lhs; - if (convert::decode(Node(*this, pMemory), lhs)) { - return lhs == rhs; + try { + const auto rslt = convert::decode(Node(*this, pMemory)); + if (rslt.first) { + return rslt.second == rhs; + } + return false; + } catch (const conversion::DecodeException& e) { + return false; + } catch(...) { + std::rethrow_exception(std::current_exception()); } - return false; } inline bool node::equals(const char* rhs, shared_memory_holder pMemory) { - std::string lhs; - if (convert::decode(Node(*this, std::move(pMemory)), lhs)) { - return lhs == rhs; + try { + const auto rslt = + convert::decode(Node(*this, std::move(pMemory))); + if (rslt.first) + return rslt.second == rhs; + return false; + } catch (const conversion::DecodeException& e) { + return false; + } catch(...) { + std::rethrow_exception(std::current_exception()); } - return false; } // indexing diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index 97dc282d9..50e193972 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -15,6 +15,7 @@ #include #include + namespace YAML { inline Node::Node() : m_isValid(true), m_invalidKey{}, m_pMemory(nullptr), m_pNode(nullptr) {} @@ -97,10 +98,15 @@ struct as_if { if (!node.m_pNode) return fallback; - T t; - if (convert::decode(node, t)) - return t; - return fallback; + try { + const auto rslt = convert::decode(node); + if (rslt.first) + return rslt.second; + } catch (conversion::DecodeException& e) { + return fallback; + } catch (...) { + std::rethrow_exception(std::current_exception()); + } } }; @@ -127,11 +133,18 @@ struct as_if { if (!node.m_pNode) throw TypedBadConversion(node.Mark()); - T t; - if (convert::decode(node, t)) - return t; - throw TypedBadConversion(node.Mark()); - } + try { + auto result = convert::decode(node); + if (result.first) + return result.second; + else + throw TypedBadConversion(node.Mark()); + } catch(const conversion::DecodeException& e) { + throw TypedBadConversion(node.Mark()); + } catch (...) { + std::rethrow_exception(std::current_exception()); + } + }; }; template <> diff --git a/src/convert.cpp b/src/convert.cpp index 9658b3603..5f0b83219 100644 --- a/src/convert.cpp +++ b/src/convert.cpp @@ -38,9 +38,9 @@ bool IsFlexibleCase(const std::string& str) { } // namespace namespace YAML { -bool convert::decode(const Node& node, bool& rhs) { +std::pair convert::decode(const Node& node) { if (!node.IsScalar()) - return false; + throw conversion::DecodeException(""); // we can't use iostream bool extraction operators as they don't // recognize all possible values in the table below (taken from @@ -55,20 +55,18 @@ bool convert::decode(const Node& node, bool& rhs) { }; if (!IsFlexibleCase(node.Scalar())) - return false; + throw conversion::DecodeException(""); for (const auto& name : names) { if (name.truename == tolower(node.Scalar())) { - rhs = true; - return true; + return std::make_pair(true, true); } if (name.falsename == tolower(node.Scalar())) { - rhs = false; - return true; + return std::make_pair(true, false); } } - return false; + throw conversion::DecodeException(""); } } // namespace YAML From 7a984e275a8f9a4707d368d937e40e2eed2a82ef Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 28 Apr 2021 15:21:37 +0200 Subject: [PATCH 02/14] return to single arg returns --- include/yaml-cpp/node/convert.h | 49 ++++++++++++++--------------- include/yaml-cpp/node/detail/impl.h | 9 ++---- include/yaml-cpp/node/impl.h | 39 ++++++++++++++++++----- src/convert.cpp | 6 ++-- 4 files changed, 59 insertions(+), 44 deletions(-) diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 12089ea38..099faaff5 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -66,7 +66,8 @@ template <> struct convert { static Node encode(const Node& rhs) { return rhs; } - static bool decode(const Node& node, Node& rhs) { + static bool decode(const Node& node, Node& rhs) { //FIXME, this is dangerous + throw std::runtime_error("this should not have been encountered"); rhs.reset(node); return true; } @@ -77,10 +78,10 @@ template <> struct convert { static Node encode(const std::string& rhs) { return Node(rhs); } - static std::pair decode(const Node& node) { + static std::string decode(const Node& node) { if (!node.IsScalar()) throw conversion::DecodeException(""); - return std::make_pair(true, node.Scalar()); + return node.Scalar(); } }; @@ -105,10 +106,10 @@ template <> struct convert<_Null> { static Node encode(const _Null& /* rhs */) { return Node(); } - static std::pair decode(const Node& node) { + static _Null decode(const Node& node) { if (!node.IsNull()) throw conversion::DecodeException(""); - return std::make_pair(node.IsNull(), _Null()); + return _Null(); } }; @@ -172,7 +173,7 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { return Node(stream.str()); \ } \ \ - static std::pair decode(const Node& node) { \ + static type decode(const Node& node) { \ if (node.Type() != NodeType::Scalar) { \ throw conversion::DecodeException(""); \ } \ @@ -188,19 +189,15 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { } \ if (std::numeric_limits::has_infinity) { \ if (conversion::IsInfinity(input)) { \ - return std::make_pair(true, \ - std::numeric_limits::infinity()); \ + return std::numeric_limits::infinity(); \ } else if (conversion::IsNegativeInfinity(input)) { \ - return std::make_pair(true, \ - negative_op std::numeric_limits::infinity()); \ + return negative_op std::numeric_limits::infinity(); \ } \ } \ \ if (std::numeric_limits::has_quiet_NaN) { \ if (conversion::IsNaN(input)) { \ - rhs = std::numeric_limits::quiet_NaN(); \ - return std::make_pair(true, \ - std::numeric_limits::quiet_NaN()); \ + return std::numeric_limits::quiet_NaN(); \ } \ } \ \ @@ -240,7 +237,7 @@ template <> struct convert { static Node encode(bool rhs) { return rhs ? Node("true") : Node("false"); } - YAML_CPP_API static std::pair decode(const Node& node); + YAML_CPP_API static bool decode(const Node& node); }; // std::map @@ -253,7 +250,7 @@ struct convert> { return node; } - static std::pair > decode(const Node& node) { + static std::map decode(const Node& node) { if (!node.IsMap()) throw conversion::DecodeException(""); @@ -265,7 +262,7 @@ struct convert> { #else rhs[element.first.as()] = element.second.as(); #endif - return std::make_pair(true, rhs); + return rhs; } }; @@ -279,7 +276,7 @@ struct convert> { return node; } - static std::pair > decode(const Node& node) { + static std::vector decode(const Node& node) { if (!node.IsSequence()) throw conversion::DecodeException(""); @@ -291,7 +288,7 @@ struct convert> { #else rhs.push_back(element.as()); #endif - return std::make_pair(true, rhs); + return rhs; } }; @@ -305,7 +302,7 @@ struct convert> { return node; } - static std::pair > decode(const Node& node) { + static std::list decode(const Node& node) { if (!node.IsSequence()) throw conversion::DecodeException(""); @@ -317,7 +314,7 @@ struct convert> { #else rhs.push_back(element.as()); #endif - return std::make_pair(true, rhs); + return rhs; } }; @@ -332,7 +329,7 @@ struct convert> { return node; } - static std::pair > decode(const Node& node) { + static std::array decode(const Node& node) { if (!isNodeValid(node)) throw conversion::DecodeException(""); @@ -345,7 +342,7 @@ struct convert> { rhs[i] = node[i].as(); #endif } - return std::make_pair(true, rhs); + return rhs; } private: @@ -364,7 +361,7 @@ struct convert> { return node; } - static std::pair > decode(const Node& node) { + static std::pair decode(const Node& node) { if (!node.IsSequence() or node.size() != 2) throw conversion::DecodeException(""); @@ -381,7 +378,7 @@ struct convert> { #else rhs.second = node[1].as(); #endif - return std::make_pair(true, rhs); + return rhs; } }; @@ -392,7 +389,7 @@ struct convert { return Node(EncodeBase64(rhs.data(), rhs.size())); } - static std::pair decode(const Node& node) { + static Binary decode(const Node& node) { if (!node.IsScalar()) throw conversion::DecodeException(""); @@ -402,7 +399,7 @@ struct convert { Binary rhs; rhs.swap(data); - return std::make_pair(true, rhs); + return rhs; } }; } diff --git a/include/yaml-cpp/node/detail/impl.h b/include/yaml-cpp/node/detail/impl.h index fe95b5df9..19dc42652 100644 --- a/include/yaml-cpp/node/detail/impl.h +++ b/include/yaml-cpp/node/detail/impl.h @@ -100,10 +100,7 @@ template inline bool node::equals(const T& rhs, shared_memory_holder pMemory) { try { const auto rslt = convert::decode(Node(*this, pMemory)); - if (rslt.first) { - return rslt.second == rhs; - } - return false; + return rslt == rhs; } catch (const conversion::DecodeException& e) { return false; } catch(...) { @@ -115,9 +112,7 @@ inline bool node::equals(const char* rhs, shared_memory_holder pMemory) { try { const auto rslt = convert::decode(Node(*this, std::move(pMemory))); - if (rslt.first) - return rslt.second == rhs; - return false; + return rslt == rhs; } catch (const conversion::DecodeException& e) { return false; } catch(...) { diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index 50e193972..3f200a001 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -99,9 +99,7 @@ struct as_if { return fallback; try { - const auto rslt = convert::decode(node); - if (rslt.first) - return rslt.second; + return convert::decode(node); } catch (conversion::DecodeException& e) { return fallback; } catch (...) { @@ -110,6 +108,35 @@ struct as_if { } }; +//specialize for Node +template +struct as_if { + explicit as_if(const Node& node_) : node(node_) {} + const Node& node; + + Node operator()(const S& fallback) const { + if (!node.m_pNode) + return fallback; + + try { + node.reset(node); + return node; + } catch (conversion::DecodeException& e) { + return fallback; + } catch (...) { + std::rethrow_exception(std::current_exception()); + } + } +}; + + + + + + + + + template struct as_if { explicit as_if(const Node& node_) : node(node_) {} @@ -134,11 +161,7 @@ struct as_if { throw TypedBadConversion(node.Mark()); try { - auto result = convert::decode(node); - if (result.first) - return result.second; - else - throw TypedBadConversion(node.Mark()); + return convert::decode(node); } catch(const conversion::DecodeException& e) { throw TypedBadConversion(node.Mark()); } catch (...) { diff --git a/src/convert.cpp b/src/convert.cpp index 5f0b83219..c299d5fd8 100644 --- a/src/convert.cpp +++ b/src/convert.cpp @@ -38,7 +38,7 @@ bool IsFlexibleCase(const std::string& str) { } // namespace namespace YAML { -std::pair convert::decode(const Node& node) { +bool convert::decode(const Node& node) { if (!node.IsScalar()) throw conversion::DecodeException(""); @@ -59,11 +59,11 @@ std::pair convert::decode(const Node& node) { for (const auto& name : names) { if (name.truename == tolower(node.Scalar())) { - return std::make_pair(true, true); + return true; } if (name.falsename == tolower(node.Scalar())) { - return std::make_pair(true, false); + return false; } } From 8783c5ef9689053d50b33bfdc8d7cae2a4b9c86a Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 28 Apr 2021 15:27:41 +0200 Subject: [PATCH 03/14] fixes --- include/yaml-cpp/node/impl.h | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index 3f200a001..ae49dd15b 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -119,7 +119,8 @@ struct as_if { return fallback; try { - node.reset(node); + Node n; + n.reset(node); return node; } catch (conversion::DecodeException& e) { return fallback; @@ -129,14 +130,6 @@ struct as_if { } }; - - - - - - - - template struct as_if { explicit as_if(const Node& node_) : node(node_) {} From e40fa447d84f33e34ea94609a3ce5a10c843898b Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 28 Apr 2021 16:24:01 +0200 Subject: [PATCH 04/14] fixes --- include/yaml-cpp/node/impl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index ae49dd15b..c89171cef 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -100,7 +100,7 @@ struct as_if { try { return convert::decode(node); - } catch (conversion::DecodeException& e) { + } catch (const conversion::DecodeException& e) { return fallback; } catch (...) { std::rethrow_exception(std::current_exception()); @@ -122,7 +122,7 @@ struct as_if { Node n; n.reset(node); return node; - } catch (conversion::DecodeException& e) { + } catch (const conversion::DecodeException& e) { return fallback; } catch (...) { std::rethrow_exception(std::current_exception()); From 4d77eee9f149877968a2c37502bf5722098ff03a Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 28 Apr 2021 16:38:57 +0200 Subject: [PATCH 05/14] fixes of introduced bug --- include/yaml-cpp/node/convert.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 099faaff5..70927dd13 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -66,10 +66,11 @@ template <> struct convert { static Node encode(const Node& rhs) { return rhs; } - static bool decode(const Node& node, Node& rhs) { //FIXME, this is dangerous + static Node decode(const Node& node) { //FIXME, this is dangerous throw std::runtime_error("this should not have been encountered"); + Node rhs; rhs.reset(node); - return true; + return rhs; } }; @@ -185,7 +186,7 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { } \ type rhs; \ if (conversion::ConvertStreamTo(stream, rhs)) { \ - throw conversion::DecodeException(""); \ + return rhs; \ } \ if (std::numeric_limits::has_infinity) { \ if (conversion::IsInfinity(input)) { \ From 92559b8ec80d7c5f9fe014145672129be03831f9 Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 28 Apr 2021 16:41:07 +0200 Subject: [PATCH 06/14] push push tutorial --- docs/Tutorial.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/Tutorial.md b/docs/Tutorial.md index a7b0e21d0..7494b1ae3 100644 --- a/docs/Tutorial.md +++ b/docs/Tutorial.md @@ -178,11 +178,12 @@ struct convert { return node; } - static bool decode(const Node& node, Vec3& rhs) { + static Vec3 decode(const Node& node) { if(!node.IsSequence() || node.size() != 3) { - return false; + return YAML::conversion::DecodeException(""); } + Vec3 rhs; rhs.x = node[0].as(); rhs.y = node[1].as(); rhs.z = node[2].as(); From 5f5b183275effbf888dfa1e7ee9f4156c0fd4738 Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 28 Apr 2021 17:08:34 +0200 Subject: [PATCH 07/14] simplify life a little bit --- docs/Tutorial.md | 2 +- include/yaml-cpp/exceptions.h | 3 ++- include/yaml-cpp/node/convert.h | 32 +++++++++++++++++++------------- src/convert.cpp | 6 +++--- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/docs/Tutorial.md b/docs/Tutorial.md index 7494b1ae3..ba44d6780 100644 --- a/docs/Tutorial.md +++ b/docs/Tutorial.md @@ -180,7 +180,7 @@ struct convert { static Vec3 decode(const Node& node) { if(!node.IsSequence() || node.size() != 3) { - return YAML::conversion::DecodeException(""); + throw YAML::conversion::DecodeException(""); } Vec3 rhs; diff --git a/include/yaml-cpp/exceptions.h b/include/yaml-cpp/exceptions.h index f08f17a26..0d62c3a07 100644 --- a/include/yaml-cpp/exceptions.h +++ b/include/yaml-cpp/exceptions.h @@ -302,7 +302,8 @@ class YAML_CPP_API BadFile : public Exception { namespace conversion{ class DecodeException : public std::runtime_error { - using runtime_error::runtime_error; + public: + DecodeException(const std::string& s="") : std::runtime_error(s) {}; }; } diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 70927dd13..9d22c744d 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -41,10 +41,16 @@ class DecodeException : public std::runtime_error { namespace YAML { class Binary; struct _Null; + template -struct convert; +struct convert { + using this_type = T; +}; } // namespace YAML +#define BAD_DECODE_EXCEPTION throw YAML::conversion::DecodeException(); + + namespace YAML { namespace conversion { inline bool IsInfinity(const std::string& input) { @@ -81,7 +87,7 @@ struct convert { static std::string decode(const Node& node) { if (!node.IsScalar()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION return node.Scalar(); } @@ -109,7 +115,7 @@ struct convert<_Null> { static _Null decode(const Node& node) { if (!node.IsNull()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION return _Null(); } }; @@ -176,13 +182,13 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { \ static type decode(const Node& node) { \ if (node.Type() != NodeType::Scalar) { \ - throw conversion::DecodeException(""); \ + BAD_DECODE_EXCEPTION; \ } \ const std::string& input = node.Scalar(); \ std::stringstream stream(input); \ stream.unsetf(std::ios::dec); \ if ((stream.peek() == '-') && std::is_unsigned::value) { \ - throw conversion::DecodeException(""); \ + BAD_DECODE_EXCEPTION \ } \ type rhs; \ if (conversion::ConvertStreamTo(stream, rhs)) { \ @@ -202,7 +208,7 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { } \ } \ \ - throw conversion::DecodeException(""); \ + BAD_DECODE_EXCEPTION \ } \ } @@ -253,7 +259,7 @@ struct convert> { static std::map decode(const Node& node) { if (!node.IsMap()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION std::map rhs; for (const auto& element : node) @@ -279,7 +285,7 @@ struct convert> { static std::vector decode(const Node& node) { if (!node.IsSequence()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION std::vector rhs; for (const auto& element : node) @@ -305,7 +311,7 @@ struct convert> { static std::list decode(const Node& node) { if (!node.IsSequence()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION std::list rhs; for (const auto& element : node) @@ -332,7 +338,7 @@ struct convert> { static std::array decode(const Node& node) { if (!isNodeValid(node)) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION std::array rhs; for (auto i = 0u; i < node.size(); ++i) { @@ -364,7 +370,7 @@ struct convert> { static std::pair decode(const Node& node) { if (!node.IsSequence() or node.size() != 2) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION std::pair rhs; #if defined(__GNUC__) && __GNUC__ < 4 @@ -392,11 +398,11 @@ struct convert { static Binary decode(const Node& node) { if (!node.IsScalar()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION std::vector data = DecodeBase64(node.Scalar()); if (data.empty() && !node.Scalar().empty()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION Binary rhs; rhs.swap(data); diff --git a/src/convert.cpp b/src/convert.cpp index c299d5fd8..7dda81916 100644 --- a/src/convert.cpp +++ b/src/convert.cpp @@ -40,7 +40,7 @@ bool IsFlexibleCase(const std::string& str) { namespace YAML { bool convert::decode(const Node& node) { if (!node.IsScalar()) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION // we can't use iostream bool extraction operators as they don't // recognize all possible values in the table below (taken from @@ -55,7 +55,7 @@ bool convert::decode(const Node& node) { }; if (!IsFlexibleCase(node.Scalar())) - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION for (const auto& name : names) { if (name.truename == tolower(node.Scalar())) { @@ -67,6 +67,6 @@ bool convert::decode(const Node& node) { } } - throw conversion::DecodeException(""); + BAD_DECODE_EXCEPTION } } // namespace YAML From 6bd373fb8af53bf4516ed556a7c8ad4274356954 Mon Sep 17 00:00:00 2001 From: marcel Date: Fri, 30 Apr 2021 09:12:31 +0200 Subject: [PATCH 08/14] a single copy of this exception is enough --- include/yaml-cpp/node/convert.h | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 9d22c744d..5a07b6e07 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -25,27 +25,12 @@ #include "yaml-cpp/null.h" - -namespace YAML{ -namespace conversion{ -namespace exception{ -class DecodeException : public std::runtime_error { - using runtime_error::runtime_error; -}; -} -} -} - - - namespace YAML { class Binary; struct _Null; template -struct convert { - using this_type = T; -}; +struct convert; } // namespace YAML #define BAD_DECODE_EXCEPTION throw YAML::conversion::DecodeException(); From 737117332d962d47fef697f46313b9a001ab7775 Mon Sep 17 00:00:00 2001 From: marcel Date: Mon, 30 Aug 2021 14:15:48 +0200 Subject: [PATCH 09/14] add a Node::ContainsKey function for quick checking of subordinated --- include/yaml-cpp/node/impl.h | 10 ++++++++++ include/yaml-cpp/node/node.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index c89171cef..261dc883e 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -350,6 +350,16 @@ std::string key_to_string(const Key& key) { } // indexing +template +inline bool Node::ContainsKey(const Key& key) const { + EnsureNodeExists(); + if (! IsMap()) + return false; + detail::node* value = + static_cast(*m_pNode).get(key, m_pMemory); + return (bool)value; +} + template inline const Node Node::operator[](const Key& key) const { EnsureNodeExists(); diff --git a/include/yaml-cpp/node/node.h b/include/yaml-cpp/node/node.h index c9e9a0a4b..634f09d6a 100644 --- a/include/yaml-cpp/node/node.h +++ b/include/yaml-cpp/node/node.h @@ -99,6 +99,8 @@ class YAML_CPP_API Node { // indexing template + bool ContainsKey(const Key& key) const; + template const Node operator[](const Key& key) const; template Node operator[](const Key& key); From 8120c0cdd000a094b919dbfb1bd6b90e541fee63 Mon Sep 17 00:00:00 2001 From: marcel Date: Fri, 28 Jan 2022 13:58:47 +0100 Subject: [PATCH 10/14] fix this test by making a convert description --- include/yaml-cpp/node/convert.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 4182bd936..78f28e3ae 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -269,11 +269,11 @@ struct convert> { return node; } - static bool decode(const Node& node, std::unordered_map& rhs) { + static std::unordered_map decode(const Node& node) { if (!node.IsMap()) - return false; + BAD_DECODE_EXCEPTION - rhs.clear(); + std::unordered_map rhs; for (const auto& element : node) #if defined(__GNUC__) && __GNUC__ < 4 // workaround for GCC 3: @@ -281,7 +281,7 @@ struct convert> { #else rhs[element.first.as()] = element.second.as(); #endif - return true; + return rhs; } }; From 0bb04280510a7a14d72d806d54c4e0eaa83bcbd6 Mon Sep 17 00:00:00 2001 From: marcel Date: Mon, 28 Feb 2022 10:58:43 +0100 Subject: [PATCH 11/14] slip of hand: or >>> || --- include/yaml-cpp/node/convert.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 78f28e3ae..a1bb42a1e 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -381,7 +381,7 @@ struct convert> { } static std::pair decode(const Node& node) { - if (!node.IsSequence() or node.size() != 2) + if (!node.IsSequence() || node.size() != 2) BAD_DECODE_EXCEPTION std::pair rhs; From 433689e510e147f856a46e3b3e50001213589c2b Mon Sep 17 00:00:00 2001 From: marcel Date: Thu, 3 Mar 2022 11:03:33 +0100 Subject: [PATCH 12/14] a working formulation integrating a new API for non-default constructable custom types, while seamlessly supporting the old API; see Tutorial.mp for details --- docs/Tutorial.md | 49 +++++++++++- include/yaml-cpp/exceptions.h | 9 +++ include/yaml-cpp/node/detail/impl.h | 69 +++++++++++++++- include/yaml-cpp/node/impl.h | 120 +++++++++++++++++++++++++--- include/yaml-cpp/node/node.h | 3 + test/node/node_test.cpp | 81 +++++++++++++++++++ 6 files changed, 316 insertions(+), 15 deletions(-) diff --git a/docs/Tutorial.md b/docs/Tutorial.md index a7b0e21d0..5654a51d0 100644 --- a/docs/Tutorial.md +++ b/docs/Tutorial.md @@ -197,5 +197,50 @@ Then you could use `Vec3` wherever you could use any other type: ```cpp YAML::Node node = YAML::Load("start: [1, 3, 0]"); Vec3 v = node["start"].as(); -node["end"] = Vec3(2, -1, 0); -``` \ No newline at end of file +node["end"] = Vec3{2, -1, 0}; +``` + +Observe that in the above example the custom type is, decalred as +a struct, explicit default constructable and all its members are +exposed. For non default constructable types like + +```cpp +class NonDefCtorVec3 : public Vec3 { + using Vec3::x; + using Vec3::y; + using Vec3::z; + public: + NonDefCtorVec3(double x, double y, double z) + : Vec3() { this->x=x; this->y=y; this->z=z; + }; +}; +``` +a new API is available, that freshens up the signature of the 'convert::decode' +method and introduces the abortion of the deserialization process by throwing +an `DecodeException`. + +```cpp +namespace YAML { +template <> +struct convert { + static Node encode(const NonDefCtorVec3& rhs) { + return convert::encode(rhs); + } + + static NonDefCtorVec3 decode(const Node& node) { + if (!node.IsSequence() || node.size() != 3) { + throw YAML::conversion::DecodeException(); + } + return {node[0].as(), node[1].as(), node[2].as()}; + } +}; +} +``` + +The behavior is exactly the same + +```cpp +YAML::Node node = YAML::Load("start: [1, 3, 0]"); +NonDefCtorVec3 v = node["start"].as(); +node["end"] = NonDefCtorVec3(2, -1, 0); +``` diff --git a/include/yaml-cpp/exceptions.h b/include/yaml-cpp/exceptions.h index f6b2602ae..47884188a 100644 --- a/include/yaml-cpp/exceptions.h +++ b/include/yaml-cpp/exceptions.h @@ -298,6 +298,15 @@ class YAML_CPP_API BadFile : public Exception { BadFile(const BadFile&) = default; ~BadFile() YAML_CPP_NOEXCEPT override; }; + + +namespace conversion{ + class DecodeException : public std::runtime_error { + public: + DecodeException(const std::string& s="") : std::runtime_error(s) {}; + }; +} + } // namespace YAML #endif // EXCEPTIONS_H_62B23520_7C8E_11DE_8A39_0800200C9A66 diff --git a/include/yaml-cpp/node/detail/impl.h b/include/yaml-cpp/node/detail/impl.h index b38038dfd..ab5d8c3bd 100644 --- a/include/yaml-cpp/node/detail/impl.h +++ b/include/yaml-cpp/node/detail/impl.h @@ -96,14 +96,72 @@ struct remove_idx +struct static_switch; + +template<> //new api +struct static_switch { + template + static T call(const Node& node) { + return convert::decode(node); + } +}; + +template<> //old api +struct static_switch { + template + static T call(const Node& node) { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +}; +#endif + +//detect the method of the new api +template +std::false_type has_decode_new_api(long); + +template +auto has_decode_new_api(int) + -> decltype( T::decode(std::declval()), std::true_type{}); + + + template inline bool node::equals(const T& rhs, shared_memory_holder pMemory) { - T lhs; - if (convert::decode(Node(*this, pMemory), lhs)) { - return lhs == rhs; + + try { +#ifdef PRE_CPP17_SHIM + return static_switch>( + 0))::value>::template call(Node(*this, pMemory)) == rhs; +#else + if constexpr (decltype(has_decode_new_api>(0))::value >) + return convert::decode(Node(*this, pMemory)) == rhs; + else { + T lhs; + if (convert::decode(Node(*this, pMemory), lhs)) + return lhs == rhs; + throw conversion::DecodeException(); + } +#endif + } catch(const conversion::DecodeException& e) { + //throw; //prefer to throw over returning just the inability to deserialize + return false; //not doing this breaks upstream functionality + } catch (...) { + throw; } - return false; } +#undef PRE_CPP17_SHIM + inline bool node::equals(const char* rhs, shared_memory_holder pMemory) { std::string lhs; @@ -113,6 +171,9 @@ inline bool node::equals(const char* rhs, shared_memory_holder pMemory) { return false; } + + + // indexing template inline node* node_data::get(const Key& key, diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index 312281f18..46917da9b 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -14,6 +14,7 @@ #include "yaml-cpp/node/node.h" #include #include +#include namespace YAML { inline Node::Node() @@ -87,6 +88,44 @@ inline NodeType::value Node::Type() const { // access +//detect the method of the new api +template +std::false_type has_decode_new_api(long); + +template +auto has_decode_new_api(int) + -> decltype( T::decode(std::declval()), std::true_type{}); + +//shim to emulate constexpr-if, which is a feature of c++17 +#if __cplusplus < 201703L || (defined(_MSVC_LANG) && _MSVC_LANG < 201703L) +#define PRE_CPP17_SHIM +#endif + +#ifdef PRE_CPP17_SHIM +template +struct static_switch; + +template<> //new api +struct static_switch { + template + static T call(const Node& node) { + return convert::decode(node); + } +}; + +template<> //old api +struct static_switch { + template + static T call(const Node& node) { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +}; +#endif + + // template helpers template struct as_if { @@ -97,10 +136,47 @@ struct as_if { if (!node.m_pNode) return fallback; - T t; - if (convert::decode(node, t)) - return t; - return fallback; + try { +#ifdef PRE_CPP17_SHIM + return static_switch>( + 0))::value>::template call(node); +#else + if constexpr (decltype(has_decodex>(0))::value >) + return convert::decodex(node); + else { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +#endif + } catch (const conversion::DecodeException& e) { + return fallback; + } catch (...) { + throw; + } + }; +}; + +//specialize for Node +template +struct as_if { + explicit as_if(const Node& node_) : node(node_) {} + const Node& node; + + Node operator()(const S& fallback) const { + if (!node.m_pNode) + return fallback; + + try { + Node n; + n.reset(node); + return node; + } catch (const conversion::DecodeException& e) { + return fallback; + } catch (...) { + throw; + } } }; @@ -127,12 +203,28 @@ struct as_if { if (!node.m_pNode) throw TypedBadConversion(node.Mark()); - T t; - if (convert::decode(node, t)) - return t; - throw TypedBadConversion(node.Mark()); - } + try { +#ifdef PRE_CPP17_SHIM + return static_switch>( + 0))::value>::template call(node); +#else + if constexpr (decltype(has_decodex>(0))::value >) + return convert::decodex(node); + else { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +#endif + } catch(const conversion::DecodeException& e) { + throw TypedBadConversion(node.Mark()); + } catch (...) { + throw; + } + }; }; +#undef PRE_CPP17_SHIM template <> struct as_if { @@ -321,6 +413,16 @@ std::string key_to_string(const Key& key) { } // indexing +template +inline bool Node::ContainsKey(const Key& key) const { + EnsureNodeExists(); + if (! IsMap()) + return false; + detail::node* value = + static_cast(*m_pNode).get(key, m_pMemory); + return (bool)value; +} + template inline const Node Node::operator[](const Key& key) const { EnsureNodeExists(); diff --git a/include/yaml-cpp/node/node.h b/include/yaml-cpp/node/node.h index c9e9a0a4b..602036fc0 100644 --- a/include/yaml-cpp/node/node.h +++ b/include/yaml-cpp/node/node.h @@ -99,6 +99,8 @@ class YAML_CPP_API Node { // indexing template + bool ContainsKey(const Key& key) const; + template const Node operator[](const Key& key) const; template Node operator[](const Key& key); @@ -141,6 +143,7 @@ YAML_CPP_API bool operator==(const Node& lhs, const Node& rhs); YAML_CPP_API Node Clone(const Node& node); +//forward declare this customization point for all types template struct convert; } diff --git a/test/node/node_test.cpp b/test/node/node_test.cpp index d4367c502..6dd4ba773 100644 --- a/test/node/node_test.cpp +++ b/test/node/node_test.cpp @@ -42,6 +42,30 @@ template using CustomList = std::list>; template > using CustomMap = std::map>>; template , class P=std::equal_to> using CustomUnorderedMap = std::unordered_map>>; +struct Vec3 { + double x, y, z; + bool operator==(const Vec3& rhs) const { + return x == rhs.x && y == rhs.y && z == rhs.z; + } +}; + +class NonDefCtorVec3 : public Vec3 { + using Vec3::x; + using Vec3::y; + using Vec3::z; + public: + NonDefCtorVec3(double x, double y, double z) + : Vec3() { + this->x=x; + this->y=y; + this->z=z; + }; + bool operator==(const NonDefCtorVec3& rhs) const { + return x == rhs.x && y == rhs.y && z == rhs.z; + } +}; + + } // anonymous namespace using ::testing::AnyOf; @@ -56,6 +80,45 @@ using ::testing::Eq; } namespace YAML { + +//define custom convert structs +template<> +struct convert { + static Node encode(const Vec3& rhs) { + Node node; + node.push_back(rhs.x); + node.push_back(rhs.y); + node.push_back(rhs.z); + return node; + } + + static bool decode(const Node& node, Vec3& rhs) { + if(!node.IsSequence() || node.size() != 3) { + return false; + } + + rhs.x = node[0].as(); + rhs.y = node[1].as(); + rhs.z = node[2].as(); + return true; + } +}; + +template <> +struct convert { + static Node encode(const NonDefCtorVec3& rhs) { + return convert::encode(rhs); + } + + static NonDefCtorVec3 decode(const Node& node) { + if (!node.IsSequence() || node.size() != 3) { + throw YAML::conversion::DecodeException(); + } + return {node[0].as(), node[1].as(), node[2].as()}; + } +}; + + namespace { TEST(NodeTest, SimpleScalar) { Node node = Node("Hello, World!"); @@ -674,6 +737,24 @@ TEST(NodeTest, AccessNonexistentKeyOnConstNode) { ASSERT_FALSE(other["5"]); } +TEST(NodeTest, CustomClassDecoding) { + YAML::Node node; + node.push_back(1.0); + node.push_back(2.0); + node.push_back(3.0); + ASSERT_TRUE(node.IsSequence()); + EXPECT_EQ(node.as(), (Vec3{1.0, 2.0, 3.0})); +} + +TEST(NodeTest, CustomNonDefaultConstructibleClassDecoding) { + YAML::Node node; + node.push_back(1.0); + node.push_back(2.0); + node.push_back(3.0); + ASSERT_TRUE(node.IsSequence()); + EXPECT_EQ(node.as(), (NonDefCtorVec3{1.0, 2.0, 3.0})); +} + class NodeEmitterTest : public ::testing::Test { protected: void ExpectOutput(const std::string& output, const Node& node) { From b46d569f8cc4e634c469970ec2c2037071633215 Mon Sep 17 00:00:00 2001 From: marcel Date: Thu, 3 Mar 2022 11:03:33 +0100 Subject: [PATCH 13/14] a working formulation integrating a new API for non-default constructable custom types, while seamlessly supporting the old API; see Tutorial.mp for details --- docs/Tutorial.md | 49 ++++++++++++++++- include/yaml-cpp/node/detail/impl.h | 82 ++++++++++++++++++++++++----- include/yaml-cpp/node/impl.h | 79 ++++++++++++++++++++++++--- include/yaml-cpp/node/node.h | 1 + test/node/node_test.cpp | 81 ++++++++++++++++++++++++++++ 5 files changed, 268 insertions(+), 24 deletions(-) diff --git a/docs/Tutorial.md b/docs/Tutorial.md index ba44d6780..5db31e9f8 100644 --- a/docs/Tutorial.md +++ b/docs/Tutorial.md @@ -198,5 +198,50 @@ Then you could use `Vec3` wherever you could use any other type: ```cpp YAML::Node node = YAML::Load("start: [1, 3, 0]"); Vec3 v = node["start"].as(); -node["end"] = Vec3(2, -1, 0); -``` \ No newline at end of file +node["end"] = Vec3{2, -1, 0}; +``` + +Observe that in the above example the custom type is, decalred as +a struct, explicit default constructable and all its members are +exposed. For non default constructable types like + +```cpp +class NonDefCtorVec3 : public Vec3 { + using Vec3::x; + using Vec3::y; + using Vec3::z; + public: + NonDefCtorVec3(double x, double y, double z) + : Vec3() { this->x=x; this->y=y; this->z=z; + }; +}; +``` +a new API is available, that freshens up the signature of the 'convert::decode' +method and introduces the abortion of the deserialization process by throwing +an `DecodeException`. + +```cpp +namespace YAML { +template <> +struct convert { + static Node encode(const NonDefCtorVec3& rhs) { + return convert::encode(rhs); + } + + static NonDefCtorVec3 decode(const Node& node) { + if (!node.IsSequence() || node.size() != 3) { + throw YAML::conversion::DecodeException(); + } + return {node[0].as(), node[1].as(), node[2].as()}; + } +}; +} +``` + +The behavior is exactly the same + +```cpp +YAML::Node node = YAML::Load("start: [1, 3, 0]"); +NonDefCtorVec3 v = node["start"].as(); +node["end"] = NonDefCtorVec3(2, -1, 0); +``` diff --git a/include/yaml-cpp/node/detail/impl.h b/include/yaml-cpp/node/detail/impl.h index 19dc42652..ab5d8c3bd 100644 --- a/include/yaml-cpp/node/detail/impl.h +++ b/include/yaml-cpp/node/detail/impl.h @@ -96,30 +96,84 @@ struct remove_idx +struct static_switch; + +template<> //new api +struct static_switch { + template + static T call(const Node& node) { + return convert::decode(node); + } +}; + +template<> //old api +struct static_switch { + template + static T call(const Node& node) { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +}; +#endif + +//detect the method of the new api +template +std::false_type has_decode_new_api(long); + +template +auto has_decode_new_api(int) + -> decltype( T::decode(std::declval()), std::true_type{}); + + + template inline bool node::equals(const T& rhs, shared_memory_holder pMemory) { + try { - const auto rslt = convert::decode(Node(*this, pMemory)); - return rslt == rhs; - } catch (const conversion::DecodeException& e) { - return false; - } catch(...) { - std::rethrow_exception(std::current_exception()); +#ifdef PRE_CPP17_SHIM + return static_switch>( + 0))::value>::template call(Node(*this, pMemory)) == rhs; +#else + if constexpr (decltype(has_decode_new_api>(0))::value >) + return convert::decode(Node(*this, pMemory)) == rhs; + else { + T lhs; + if (convert::decode(Node(*this, pMemory), lhs)) + return lhs == rhs; + throw conversion::DecodeException(); + } +#endif + } catch(const conversion::DecodeException& e) { + //throw; //prefer to throw over returning just the inability to deserialize + return false; //not doing this breaks upstream functionality + } catch (...) { + throw; } } +#undef PRE_CPP17_SHIM + inline bool node::equals(const char* rhs, shared_memory_holder pMemory) { - try { - const auto rslt = - convert::decode(Node(*this, std::move(pMemory))); - return rslt == rhs; - } catch (const conversion::DecodeException& e) { - return false; - } catch(...) { - std::rethrow_exception(std::current_exception()); + std::string lhs; + if (convert::decode(Node(*this, std::move(pMemory)), lhs)) { + return lhs == rhs; } + return false; } + + + // indexing template inline node* node_data::get(const Key& key, diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index 587f4c5dd..46917da9b 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -14,7 +14,7 @@ #include "yaml-cpp/node/node.h" #include #include - +#include namespace YAML { inline Node::Node() @@ -88,6 +88,44 @@ inline NodeType::value Node::Type() const { // access +//detect the method of the new api +template +std::false_type has_decode_new_api(long); + +template +auto has_decode_new_api(int) + -> decltype( T::decode(std::declval()), std::true_type{}); + +//shim to emulate constexpr-if, which is a feature of c++17 +#if __cplusplus < 201703L || (defined(_MSVC_LANG) && _MSVC_LANG < 201703L) +#define PRE_CPP17_SHIM +#endif + +#ifdef PRE_CPP17_SHIM +template +struct static_switch; + +template<> //new api +struct static_switch { + template + static T call(const Node& node) { + return convert::decode(node); + } +}; + +template<> //old api +struct static_switch { + template + static T call(const Node& node) { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +}; +#endif + + // template helpers template struct as_if { @@ -99,13 +137,25 @@ struct as_if { return fallback; try { - return convert::decode(node); +#ifdef PRE_CPP17_SHIM + return static_switch>( + 0))::value>::template call(node); +#else + if constexpr (decltype(has_decodex>(0))::value >) + return convert::decodex(node); + else { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +#endif } catch (const conversion::DecodeException& e) { return fallback; } catch (...) { - std::rethrow_exception(std::current_exception()); + throw; } - } + }; }; //specialize for Node @@ -125,7 +175,7 @@ struct as_if { } catch (const conversion::DecodeException& e) { return fallback; } catch (...) { - std::rethrow_exception(std::current_exception()); + throw; } } }; @@ -154,14 +204,27 @@ struct as_if { throw TypedBadConversion(node.Mark()); try { - return convert::decode(node); +#ifdef PRE_CPP17_SHIM + return static_switch>( + 0))::value>::template call(node); +#else + if constexpr (decltype(has_decodex>(0))::value >) + return convert::decodex(node); + else { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } +#endif } catch(const conversion::DecodeException& e) { - throw TypedBadConversion(node.Mark()); + throw TypedBadConversion(node.Mark()); } catch (...) { - std::rethrow_exception(std::current_exception()); + throw; } }; }; +#undef PRE_CPP17_SHIM template <> struct as_if { diff --git a/include/yaml-cpp/node/node.h b/include/yaml-cpp/node/node.h index 634f09d6a..602036fc0 100644 --- a/include/yaml-cpp/node/node.h +++ b/include/yaml-cpp/node/node.h @@ -143,6 +143,7 @@ YAML_CPP_API bool operator==(const Node& lhs, const Node& rhs); YAML_CPP_API Node Clone(const Node& node); +//forward declare this customization point for all types template struct convert; } diff --git a/test/node/node_test.cpp b/test/node/node_test.cpp index d4367c502..6dd4ba773 100644 --- a/test/node/node_test.cpp +++ b/test/node/node_test.cpp @@ -42,6 +42,30 @@ template using CustomList = std::list>; template > using CustomMap = std::map>>; template , class P=std::equal_to> using CustomUnorderedMap = std::unordered_map>>; +struct Vec3 { + double x, y, z; + bool operator==(const Vec3& rhs) const { + return x == rhs.x && y == rhs.y && z == rhs.z; + } +}; + +class NonDefCtorVec3 : public Vec3 { + using Vec3::x; + using Vec3::y; + using Vec3::z; + public: + NonDefCtorVec3(double x, double y, double z) + : Vec3() { + this->x=x; + this->y=y; + this->z=z; + }; + bool operator==(const NonDefCtorVec3& rhs) const { + return x == rhs.x && y == rhs.y && z == rhs.z; + } +}; + + } // anonymous namespace using ::testing::AnyOf; @@ -56,6 +80,45 @@ using ::testing::Eq; } namespace YAML { + +//define custom convert structs +template<> +struct convert { + static Node encode(const Vec3& rhs) { + Node node; + node.push_back(rhs.x); + node.push_back(rhs.y); + node.push_back(rhs.z); + return node; + } + + static bool decode(const Node& node, Vec3& rhs) { + if(!node.IsSequence() || node.size() != 3) { + return false; + } + + rhs.x = node[0].as(); + rhs.y = node[1].as(); + rhs.z = node[2].as(); + return true; + } +}; + +template <> +struct convert { + static Node encode(const NonDefCtorVec3& rhs) { + return convert::encode(rhs); + } + + static NonDefCtorVec3 decode(const Node& node) { + if (!node.IsSequence() || node.size() != 3) { + throw YAML::conversion::DecodeException(); + } + return {node[0].as(), node[1].as(), node[2].as()}; + } +}; + + namespace { TEST(NodeTest, SimpleScalar) { Node node = Node("Hello, World!"); @@ -674,6 +737,24 @@ TEST(NodeTest, AccessNonexistentKeyOnConstNode) { ASSERT_FALSE(other["5"]); } +TEST(NodeTest, CustomClassDecoding) { + YAML::Node node; + node.push_back(1.0); + node.push_back(2.0); + node.push_back(3.0); + ASSERT_TRUE(node.IsSequence()); + EXPECT_EQ(node.as(), (Vec3{1.0, 2.0, 3.0})); +} + +TEST(NodeTest, CustomNonDefaultConstructibleClassDecoding) { + YAML::Node node; + node.push_back(1.0); + node.push_back(2.0); + node.push_back(3.0); + ASSERT_TRUE(node.IsSequence()); + EXPECT_EQ(node.as(), (NonDefCtorVec3{1.0, 2.0, 3.0})); +} + class NodeEmitterTest : public ::testing::Test { protected: void ExpectOutput(const std::string& output, const Node& node) { From 6f79903b89e1d2cbc68794b304a3f8ba8db9953b Mon Sep 17 00:00:00 2001 From: marcel Date: Thu, 3 Mar 2022 12:16:50 +0100 Subject: [PATCH 14/14] slightly better structure; remove id-constexpr all together in favour of clearer code structure --- include/yaml-cpp/node/api_switch.h | 53 +++++++++++++++++++++ include/yaml-cpp/node/convert.h | 47 +++++++++--------- include/yaml-cpp/node/detail/impl.h | 74 +++++------------------------ include/yaml-cpp/node/impl.h | 70 ++------------------------- src/convert.cpp | 6 +-- 5 files changed, 96 insertions(+), 154 deletions(-) create mode 100644 include/yaml-cpp/node/api_switch.h diff --git a/include/yaml-cpp/node/api_switch.h b/include/yaml-cpp/node/api_switch.h new file mode 100644 index 000000000..716ab9819 --- /dev/null +++ b/include/yaml-cpp/node/api_switch.h @@ -0,0 +1,53 @@ +// +// Created by marcel on 3/3/22. +// + +#ifndef YAML_CPP_API_SWITCH_H +#define YAML_CPP_API_SWITCH_H + +#if defined(_MSC_VER) || \ + (defined(__GNUC__) && (__GNUC__ == 3 && __GNUC_MINOR__ >= 4) || \ + (__GNUC__ >= 4)) // GCC supports "pragma once" correctly since 3.4 +#pragma once +#endif + +#include "yaml-cpp/node/node.h" +#include "yaml-cpp/exceptions.h" +#include + +namespace YAML{ +namespace detail { + + //detect the method of the new api + template + std::false_type has_decode_new_api(long); + + template + auto has_decode_new_api(int) + -> decltype( T::decode(std::declval()), std::true_type{}); + + template + struct static_api_switch; + + template<> //new api call-path + struct static_api_switch { + template + static T decode(const Node& node) { + return convert::decode(node); + } + }; + + template<> //old api call-path + struct static_api_switch { + template + static T decode(const Node& node) { + T t; + if (convert::decode(node, t)) + return t; + throw conversion::DecodeException(); + } + }; +} +} + +#endif // YAML_CPP_API_SWITCH_H diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h index 22a222634..ed77cf5b9 100644 --- a/include/yaml-cpp/node/convert.h +++ b/include/yaml-cpp/node/convert.h @@ -34,9 +34,6 @@ template struct convert; } // namespace YAML -#define BAD_DECODE_EXCEPTION throw YAML::conversion::DecodeException(); - - namespace YAML { namespace conversion { inline bool IsInfinity(const std::string& input) { @@ -58,12 +55,17 @@ template <> struct convert { static Node encode(const Node& rhs) { return rhs; } - static Node decode(const Node& node) { //FIXME, this is dangerous - throw std::runtime_error("this should not have been encountered"); - Node rhs; + static bool decode(const Node& node, Node& rhs) { rhs.reset(node); - return rhs; + return true; } + +// static Node decode(const Node& node) { +// throw std::runtime_error("this should not have been encountered"); +// Node rhs; +// rhs.reset(node); +// return rhs; +// } }; // std::string @@ -73,10 +75,9 @@ struct convert { static std::string decode(const Node& node) { if (!node.IsScalar()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); return node.Scalar(); } - }; // C-strings can only be encoded @@ -101,7 +102,7 @@ struct convert<_Null> { static _Null decode(const Node& node) { if (!node.IsNull()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); return _Null(); } }; @@ -166,19 +167,19 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { return Node(stream.str()); \ } \ \ - static type decode(const Node& node) { \ + static type decode(const Node& node) { \ if (node.Type() != NodeType::Scalar) { \ - BAD_DECODE_EXCEPTION; \ + throw YAML::conversion::DecodeException();; \ } \ const std::string& input = node.Scalar(); \ std::stringstream stream(input); \ stream.unsetf(std::ios::dec); \ if ((stream.peek() == '-') && std::is_unsigned::value) { \ - BAD_DECODE_EXCEPTION \ + throw YAML::conversion::DecodeException(); \ } \ type rhs; \ if (conversion::ConvertStreamTo(stream, rhs)) { \ - return rhs; \ + return rhs; \ } \ if (std::numeric_limits::has_infinity) { \ if (conversion::IsInfinity(input)) { \ @@ -194,7 +195,7 @@ ConvertStreamTo(std::stringstream& stream, T& rhs) { } \ } \ \ - BAD_DECODE_EXCEPTION \ + throw YAML::conversion::DecodeException(); \ } \ } @@ -245,7 +246,7 @@ struct convert> { static std::map decode(const Node& node) { if (!node.IsMap()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); std::map rhs; for (const auto& element : node) @@ -271,7 +272,7 @@ struct convert> { static std::unordered_map decode(const Node& node) { if (!node.IsMap()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); std::unordered_map rhs; for (const auto& element : node) @@ -297,7 +298,7 @@ struct convert> { static std::vector decode(const Node& node) { if (!node.IsSequence()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); std::vector rhs; for (const auto& element : node) @@ -323,7 +324,7 @@ struct convert> { static std::list decode(const Node& node) { if (!node.IsSequence()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); std::list rhs; for (const auto& element : node) @@ -350,7 +351,7 @@ struct convert> { static std::array decode(const Node& node) { if (!isNodeValid(node)) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); std::array rhs; for (auto i = 0u; i < node.size(); ++i) { @@ -382,7 +383,7 @@ struct convert> { static std::pair decode(const Node& node) { if (!node.IsSequence() || node.size() != 2) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); std::pair rhs; #if defined(__GNUC__) && __GNUC__ < 4 @@ -410,11 +411,11 @@ struct convert { static Binary decode(const Node& node) { if (!node.IsScalar()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); std::vector data = DecodeBase64(node.Scalar()); if (data.empty() && !node.Scalar().empty()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); Binary rhs; rhs.swap(data); diff --git a/include/yaml-cpp/node/detail/impl.h b/include/yaml-cpp/node/detail/impl.h index ab5d8c3bd..f574383d9 100644 --- a/include/yaml-cpp/node/detail/impl.h +++ b/include/yaml-cpp/node/detail/impl.h @@ -9,6 +9,7 @@ #include "yaml-cpp/node/detail/node.h" #include "yaml-cpp/node/detail/node_data.h" +#include "yaml-cpp/node/api_switch.h" #include #include @@ -96,63 +97,11 @@ struct remove_idx -struct static_switch; - -template<> //new api -struct static_switch { - template - static T call(const Node& node) { - return convert::decode(node); - } -}; - -template<> //old api -struct static_switch { - template - static T call(const Node& node) { - T t; - if (convert::decode(node, t)) - return t; - throw conversion::DecodeException(); - } -}; -#endif - -//detect the method of the new api -template -std::false_type has_decode_new_api(long); - -template -auto has_decode_new_api(int) - -> decltype( T::decode(std::declval()), std::true_type{}); - - - template inline bool node::equals(const T& rhs, shared_memory_holder pMemory) { - try { -#ifdef PRE_CPP17_SHIM - return static_switch>( - 0))::value>::template call(Node(*this, pMemory)) == rhs; -#else - if constexpr (decltype(has_decode_new_api>(0))::value >) - return convert::decode(Node(*this, pMemory)) == rhs; - else { - T lhs; - if (convert::decode(Node(*this, pMemory), lhs)) - return lhs == rhs; - throw conversion::DecodeException(); - } -#endif + return static_api_switch>( + 0))::value>::template decode(Node(*this, pMemory)) == rhs; } catch(const conversion::DecodeException& e) { //throw; //prefer to throw over returning just the inability to deserialize return false; //not doing this breaks upstream functionality @@ -160,20 +109,19 @@ inline bool node::equals(const T& rhs, shared_memory_holder pMemory) { throw; } } -#undef PRE_CPP17_SHIM - inline bool node::equals(const char* rhs, shared_memory_holder pMemory) { - std::string lhs; - if (convert::decode(Node(*this, std::move(pMemory)), lhs)) { - return lhs == rhs; + try { + return static_api_switch>( + 0))::value>::template decode(Node(*this, std::move(pMemory))) == rhs; + } catch(const conversion::DecodeException& e) { + //throw; //prefer to throw over returning just the inability to deserialize + return false; //not doing this breaks upstream functionality + } catch (...) { + throw; } - return false; } - - - // indexing template inline node* node_data::get(const Key& key, diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index 46917da9b..c565d6d0a 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -12,6 +12,7 @@ #include "yaml-cpp/node/detail/node.h" #include "yaml-cpp/node/iterator.h" #include "yaml-cpp/node/node.h" +#include "yaml-cpp/node/api_switch.h" #include #include #include @@ -88,44 +89,6 @@ inline NodeType::value Node::Type() const { // access -//detect the method of the new api -template -std::false_type has_decode_new_api(long); - -template -auto has_decode_new_api(int) - -> decltype( T::decode(std::declval()), std::true_type{}); - -//shim to emulate constexpr-if, which is a feature of c++17 -#if __cplusplus < 201703L || (defined(_MSVC_LANG) && _MSVC_LANG < 201703L) -#define PRE_CPP17_SHIM -#endif - -#ifdef PRE_CPP17_SHIM -template -struct static_switch; - -template<> //new api -struct static_switch { - template - static T call(const Node& node) { - return convert::decode(node); - } -}; - -template<> //old api -struct static_switch { - template - static T call(const Node& node) { - T t; - if (convert::decode(node, t)) - return t; - throw conversion::DecodeException(); - } -}; -#endif - - // template helpers template struct as_if { @@ -137,19 +100,8 @@ struct as_if { return fallback; try { -#ifdef PRE_CPP17_SHIM - return static_switch>( - 0))::value>::template call(node); -#else - if constexpr (decltype(has_decodex>(0))::value >) - return convert::decodex(node); - else { - T t; - if (convert::decode(node, t)) - return t; - throw conversion::DecodeException(); - } -#endif + return detail::static_api_switch>( + 0))::value>::template decode(node); } catch (const conversion::DecodeException& e) { return fallback; } catch (...) { @@ -204,19 +156,8 @@ struct as_if { throw TypedBadConversion(node.Mark()); try { -#ifdef PRE_CPP17_SHIM - return static_switch>( - 0))::value>::template call(node); -#else - if constexpr (decltype(has_decodex>(0))::value >) - return convert::decodex(node); - else { - T t; - if (convert::decode(node, t)) - return t; - throw conversion::DecodeException(); - } -#endif + return detail::static_api_switch>( + 0))::value>::template decode(node); } catch(const conversion::DecodeException& e) { throw TypedBadConversion(node.Mark()); } catch (...) { @@ -224,7 +165,6 @@ struct as_if { } }; }; -#undef PRE_CPP17_SHIM template <> struct as_if { diff --git a/src/convert.cpp b/src/convert.cpp index 7dda81916..957aedaf2 100644 --- a/src/convert.cpp +++ b/src/convert.cpp @@ -40,7 +40,7 @@ bool IsFlexibleCase(const std::string& str) { namespace YAML { bool convert::decode(const Node& node) { if (!node.IsScalar()) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); // we can't use iostream bool extraction operators as they don't // recognize all possible values in the table below (taken from @@ -55,7 +55,7 @@ bool convert::decode(const Node& node) { }; if (!IsFlexibleCase(node.Scalar())) - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); for (const auto& name : names) { if (name.truename == tolower(node.Scalar())) { @@ -67,6 +67,6 @@ bool convert::decode(const Node& node) { } } - BAD_DECODE_EXCEPTION + throw YAML::conversion::DecodeException(); } } // namespace YAML