From ac2258b8e10290a3993645997b36cc578fcba107 Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Wed, 8 Feb 2023 23:07:57 +0000 Subject: [PATCH 1/5] going back to SDF_ASSERT instead of FATAL_ERROR Signed-off-by: Marco A. Gutierrez --- include/sdf/Element.hh | 5 - src/Converter.cc | 169 +++++++------------------------ src/Converter.hh | 4 +- src/Converter_TEST.cc | 43 ++------ src/Element.cc | 67 +++--------- test/integration/error_output.cc | 26 +++-- 6 files changed, 72 insertions(+), 242 deletions(-) diff --git a/include/sdf/Element.hh b/include/sdf/Element.hh index 1afc35799..f1e9541ad 100644 --- a/include/sdf/Element.hh +++ b/include/sdf/Element.hh @@ -559,11 +559,6 @@ namespace sdf /// \param[in] _child Pointer to the child to remove. public: void RemoveChild(ElementPtr _child); - /// \brief Remove a child element. - /// \param[in] _child Pointer to the child to remove. - /// \param[out] _errors Vector of errors. - public: void RemoveChild(ElementPtr _child, sdf::Errors &_errors); - /// \brief Remove all child elements. public: void ClearElements(); diff --git a/src/Converter.cc b/src/Converter.cc index 92dd54ed1..8947d3333 100644 --- a/src/Converter.cc +++ b/src/Converter.cc @@ -62,14 +62,8 @@ void UpdatePose(tinyxml2::XMLElement *_elem, { std::string poseRelTo = pose->Attribute("relative_to"); - if (poseRelTo.compare(0, _modelName.size(), _modelName) != 0) - { - std::stringstream ss; - ss << "Error: Pose attribute 'relative_to' does not start with " - << _modelName; - _errors.push_back({ErrorCode::FATAL_ERROR, ss.str()}); - return; - } + SDF_ASSERT(poseRelTo.compare(0, _modelName.size(), _modelName) == 0, + "Error: Pose attribute 'relative_to' does not start with " + _modelName); poseRelTo = poseRelTo.substr(_childNameIdx); pose->SetAttribute("relative_to", poseRelTo.c_str()); @@ -90,11 +84,8 @@ bool Converter::Convert(sdf::Errors &_errors, const ParserConfig &_config, bool _quiet) { - if (_doc == nullptr) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF XML doc is NULL"}); - return false; - } + + SDF_ASSERT(_doc != nullptr, "SDF XML doc is NULL"); tinyxml2::XMLElement *elem = _doc->FirstChildElement("sdf"); @@ -192,16 +183,8 @@ void Converter::Convert(sdf::Errors &_errors, tinyxml2::XMLDocument *_convertDoc, const ParserConfig &_config) { - if (_doc == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF XML doc is NULL"}); - return; - } - if (_convertDoc == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "Convert XML doc is NULL"}); - return; - } + SDF_ASSERT(_doc != NULL, "SDF XML doc is NULL"); + SDF_ASSERT(_convertDoc != NULL, "Convert XML doc is NULL"); ConvertImpl(_doc->FirstChildElement(), _convertDoc->FirstChildElement(), _config, _errors); @@ -246,16 +229,8 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem, const ParserConfig &_config, sdf::Errors &_errors) { - if (_elem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF element is NULL"}); - return; - } - if (_elem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "Convert element is NULL"}); - return; - } + SDF_ASSERT(_elem != NULL, "SDF element is NULL"); + SDF_ASSERT(_convert != NULL, "Convert element is NULL"); CheckDeprecation(_elem, _convert, _config, _errors); @@ -289,7 +264,7 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem, } else if (name == "copy") { - Move(_elem, childElem, true, _errors); + Move(_elem, childElem, true); } else if (name == "map") { @@ -297,7 +272,7 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem, } else if (name == "move") { - Move(_elem, childElem, false, _errors); + Move(_elem, childElem, false); } else if (name == "add") { @@ -329,11 +304,7 @@ void Converter::ConvertImpl(tinyxml2::XMLElement *_elem, ///////////////////////////////////////////////// void Converter::Unflatten(tinyxml2::XMLElement *_elem, sdf::Errors &_errors) { - if (_elem == nullptr) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF element is nullptr"}); - return; - } + SDF_ASSERT(_elem != nullptr, "SDF element is nullptr"); tinyxml2::XMLDocument *doc = _elem->GetDocument(); @@ -449,15 +420,9 @@ bool Converter::FindNewModelElements(tinyxml2::XMLElement *_elem, if (elem->Attribute("attached_to")) { attachedTo = elem->Attribute("attached_to"); - - if (attachedTo.compare(0, newModelNameSize, newModelName) != 0) - { - std::stringstream ss; - ss << "Error: Frame attribute 'attached_to' does not start with " - << newModelName; - _errors.push_back({ErrorCode::FATAL_ERROR, ss.str()}); - return false; - } + SDF_ASSERT(attachedTo.compare(0, newModelNameSize, newModelName) == 0, + "Error: Frame attribute 'attached_to' does not start with " + + newModelName); // strip new model prefix from attached_to attachedTo = attachedTo.substr(_childNameIdx); @@ -496,14 +461,8 @@ bool Converter::FindNewModelElements(tinyxml2::XMLElement *_elem, { eText = e->GetText(); - if (eText.compare(0, newModelNameSize, newModelName) != 0) - { - std::stringstream ss; - ss << "Error: Joint's value does not start with " - << newModelName; - _errors.push_back({ErrorCode::FATAL_ERROR, ss.str()}); - return false; - } + SDF_ASSERT(eText.compare(0, newModelNameSize, newModelName) == 0, + "Error: Joint's value does not start with " + newModelName); e->SetText(eText.substr(_childNameIdx).c_str()); } @@ -514,14 +473,8 @@ bool Converter::FindNewModelElements(tinyxml2::XMLElement *_elem, { eText = e->GetText(); - if (eText.compare(0, newModelNameSize, newModelName) != 0) - { - std::stringstream ss; - ss << "Error: Joint's value does not start with " - << newModelName; - _errors.push_back({ErrorCode::FATAL_ERROR, ss.str()}); - return false; - } + SDF_ASSERT(eText.compare(0, newModelNameSize, newModelName) == 0, + "Error: Joint's value does not start with " + newModelName); e->SetText(eText.substr(_childNameIdx).c_str()); } @@ -539,14 +492,10 @@ bool Converter::FindNewModelElements(tinyxml2::XMLElement *_elem, std::string expressIn = axisElem->FirstChildElement("xyz")->Attribute("expressed_in"); - if (expressIn.compare(0, newModelNameSize, newModelName) != 0) - { - std::stringstream ss; - ss << "Error: 's attribute 'expressed_in' does not start " - << "with " << newModelName; - _errors.push_back({ErrorCode::FATAL_ERROR, ss.str()}); - return false; - } + SDF_ASSERT( + expressIn.compare(0, newModelNameSize, newModelName) == 0, + "Error: 's attribute 'expressed_in' does not start with " + + newModelName); expressIn = expressIn.substr(_childNameIdx); @@ -608,14 +557,9 @@ bool Converter::FindNewModelElements(tinyxml2::XMLElement *_elem, { eText = e->GetText(); - if (eText.compare(0, newModelNameSize, newModelName) != 0) - { - std::stringstream ss; - ss << "Error: Gripper's value does not start with " - << newModelName; - _errors.push_back({ErrorCode::FATAL_ERROR, ss.str()}); - return false; - } + SDF_ASSERT(eText.compare(0, newModelNameSize, newModelName) == 0, + "Error: Gripper's value does not start with " + + newModelName); e->SetText(eText.substr(_childNameIdx).c_str()); } @@ -635,16 +579,8 @@ void Converter::Rename(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_renameElem, sdf::Errors &_errors) { - if (_elem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF element is NULL"}); - return; - } - if (_renameElem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "Rename element is NULL"}); - return; - } + SDF_ASSERT(_elem != NULL, "SDF element is NULL"); + SDF_ASSERT(_renameElem != NULL, "Rename element is NULL"); auto *fromConvertElem = _renameElem->FirstChildElement("from"); auto *toConvertElem = _renameElem->FirstChildElement("to"); @@ -698,16 +634,8 @@ void Converter::Add(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_addElem, sdf::Errors &_errors) { - if (_elem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF element is NULL"}); - return; - } - if (_addElem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "Add element is NULL"}); - return; - } + SDF_ASSERT(_elem != NULL, "SDF element is NULL"); + SDF_ASSERT(_addElem != NULL, "Add element is NULL"); const char *attributeName = _addElem->Attribute("attribute"); const char *elementName = _addElem->Attribute("element"); @@ -752,16 +680,8 @@ void Converter::Remove(sdf::Errors &_errors, tinyxml2::XMLElement *_removeElem, bool _removeOnlyEmpty) { - if (_elem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF element is NULL"}); - return; - } - if (_removeElem == NULL) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "remove element is NULL"}); - return; - } + SDF_ASSERT(_elem != NULL, "SDF element is NULL"); + SDF_ASSERT(_removeElem != NULL, "remove element is NULL"); const char *attributeName = _removeElem->Attribute("attribute"); const char *elementName = _removeElem->Attribute("element"); @@ -803,16 +723,8 @@ void Converter::Map(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_mapElem, sdf::Errors &_errors) { - if (_elem == nullptr) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF element is nullptr"}); - return; - } - if (_mapElem == nullptr) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "Map element is nullptr"}); - return; - } + SDF_ASSERT(_elem != nullptr, "SDF element is nullptr"); + SDF_ASSERT(_mapElem != nullptr, "Map element is nullptr"); tinyxml2::XMLElement *fromConvertElem = _mapElem->FirstChildElement("from"); tinyxml2::XMLElement *toConvertElem = _mapElem->FirstChildElement("to"); @@ -1011,19 +923,10 @@ void Converter::Map(tinyxml2::XMLElement *_elem, ///////////////////////////////////////////////// void Converter::Move(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_moveElem, - const bool _copy, - sdf::Errors &_errors) + const bool _copy) { - if (_elem == nullptr) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "SDF element is NULL"}); - return; - } - if (_moveElem == nullptr) - { - _errors.push_back({ErrorCode::FATAL_ERROR, "Move element is NULL"}); - return; - } + SDF_ASSERT(_elem != NULL, "SDF element is NULL"); + SDF_ASSERT(_moveElem != NULL, "Move element is NULL"); tinyxml2::XMLElement *fromConvertElem = _moveElem->FirstChildElement("from"); tinyxml2::XMLElement *toConvertElem = _moveElem->FirstChildElement("to"); diff --git a/src/Converter.hh b/src/Converter.hh index cfba5413f..e992b124c 100644 --- a/src/Converter.hh +++ b/src/Converter.hh @@ -108,11 +108,9 @@ namespace sdf /// \param[in] _moveElem A 'convert' element that describes the move /// operation. /// \param[in] _copy True to copy the element - /// \param[out] _errors Vector of errors. private: static void Move(tinyxml2::XMLElement *_elem, tinyxml2::XMLElement *_moveElem, - const bool _copy, - sdf::Errors &_errors); + const bool _copy); /// \brief Add an element or attribute to an element. /// \param[in] _elem The element to receive the value. diff --git a/src/Converter_TEST.cc b/src/Converter_TEST.cc index 679e31d2e..81c30413d 100644 --- a/src/Converter_TEST.cc +++ b/src/Converter_TEST.cc @@ -2852,44 +2852,19 @@ TEST(Converter, NullDoc) tinyxml2::XMLDocument xmlDoc; tinyxml2::XMLDocument convertXmlDoc; - std::stringstream buffer; - sdf::testing::RedirectConsoleStream redir( - sdf::Console::Instance()->GetMsgStream(), &buffer); - - #ifdef _WIN32 - sdf::Console::Instance()->SetQuiet(false); - sdf::testing::ScopeExit revertSetQuiet( - [] - { - sdf::Console::Instance()->SetQuiet(true); - }); - #endif - sdf::ParserConfig parserConfig; parserConfig.SetWarningsPolicy(sdf::EnforcementPolicy::ERR); sdf::Errors errors; - sdf::Converter::Convert(errors, nullptr, &convertXmlDoc, parserConfig); - ASSERT_EQ(errors.size(), 1u); - EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::FATAL_ERROR); - EXPECT_NE(std::string::npos, - errors[0].Message().find("SDF XML doc is NULL")); - errors.clear(); - - sdf::Converter::Convert(errors, &xmlDoc, nullptr, parserConfig); - ASSERT_EQ(errors.size(), 1u); - EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::FATAL_ERROR); - EXPECT_NE(std::string::npos, - errors[0].Message().find("Convert XML doc is NULL")); - errors.clear(); - - sdf::Converter::Convert(errors, nullptr, "1.4", parserConfig); - EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::FATAL_ERROR); - EXPECT_NE(std::string::npos, - errors[0].Message().find("SDF XML doc is NULL")); - - // Check nothing has been printed - EXPECT_TRUE(buffer.str().empty()) << buffer.str(); + ASSERT_THROW(sdf::Converter::Convert(errors, nullptr, &convertXmlDoc, parserConfig), + sdf::AssertionInternalError); + ASSERT_TRUE(errors.empty()); + ASSERT_THROW(sdf::Converter::Convert(errors, &xmlDoc, nullptr, parserConfig), + sdf::AssertionInternalError); + ASSERT_TRUE(errors.empty()); + ASSERT_THROW(sdf::Converter::Convert(errors, nullptr, "1.4", parserConfig), + sdf::AssertionInternalError); + ASSERT_TRUE(errors.empty()); } //////////////////////////////////////////////////// diff --git a/src/Element.cc b/src/Element.cc index 5dfc243c6..2b5bca9b8 100644 --- a/src/Element.cc +++ b/src/Element.cc @@ -182,11 +182,8 @@ void Element::AddValue(const std::string &_type, std::make_shared(this->dataPtr->name, _type, _defaultValue, _required, _minValue, _maxValue, _errors, _description); - if (!this->dataPtr->value->SetParentElement(shared_from_this(), _errors)) - { - _errors.push_back({ErrorCode::FATAL_ERROR, - "Cannot set parent Element of value to itself."}); - } + SDF_ASSERT(this->dataPtr->value->SetParentElement(shared_from_this()), + "Cannot set parent Element of value to itself."); } ///////////////////////////////////////////////// @@ -199,12 +196,8 @@ ParamPtr Element::CreateParam(const std::string &_key, { ParamPtr param = std::make_shared( _key, _type, _defaultValue, _required, _errors, _description); - - if(!param->SetParentElement(shared_from_this(), _errors)) - { - _errors.push_back({ErrorCode::FATAL_ERROR, - "Cannot set parent Element of created Param to itself."}); - } + SDF_ASSERT(param->SetParentElement(shared_from_this()), + "Cannot set parent Element of created Param to itself."); return param; } @@ -263,13 +256,9 @@ ElementPtr Element::Clone(sdf::Errors &_errors) const aiter != this->dataPtr->attributes.end(); ++aiter) { auto clonedAttribute = (*aiter)->Clone(); - if (!clonedAttribute->SetParentElement(clone, _errors)) - { - _errors.push_back({ErrorCode::FATAL_ERROR, - "Cannot set parent Element of cloned attribute Param to cloned " - "Element."}); - return nullptr; - } + SDF_ASSERT(clonedAttribute->SetParentElement(clone), + "Cannot set parent Element of cloned attribute Param to cloned " + "Element."); clone->dataPtr->attributes.push_back(clonedAttribute); } @@ -290,14 +279,8 @@ ElementPtr Element::Clone(sdf::Errors &_errors) const if (this->dataPtr->value) { clone->dataPtr->value = this->dataPtr->value->Clone(); - - if (!clone->dataPtr->value->SetParentElement(clone, _errors)) - { - _errors.push_back({ErrorCode::FATAL_ERROR, - "Cannot set parent Element of cloned value Param to cloned " - "Element."}); - return nullptr; - } + SDF_ASSERT(clone->dataPtr->value->SetParentElement(clone), + "Cannot set parent Element of cloned value Param to cloned Element."); } if (this->dataPtr->includeElement) @@ -341,13 +324,8 @@ void Element::Copy(const ElementPtr _elem, sdf::Errors &_errors) } ParamPtr param = this->GetAttribute((*iter)->GetKey()); (*param) = (**iter); - - if (!param->SetParentElement(shared_from_this(), _errors)) - { - _errors.push_back({ErrorCode::FATAL_ERROR, - "Cannot set parent Element of copied attribute Param to itself."}); - return; - } + SDF_ASSERT(param->SetParentElement(shared_from_this()), + "Cannot set parent Element of copied attribute Param to itself."); } if (_elem->GetValue()) @@ -360,12 +338,8 @@ void Element::Copy(const ElementPtr _elem, sdf::Errors &_errors) { *(this->dataPtr->value) = *(_elem->GetValue()); } - if (!this->dataPtr->value->SetParentElement(shared_from_this(), _errors)) - { - _errors.push_back({ErrorCode::FATAL_ERROR, - "Cannot set parent Element of copied attribute Param to itself."}); - return; - } + SDF_ASSERT(this->dataPtr->value->SetParentElement(shared_from_this()), + "Cannot set parent Element of copied value Param to itself."); } this->dataPtr->elementDescriptions.clear(); @@ -1303,20 +1277,7 @@ void Element::RemoveFromParent() ///////////////////////////////////////////////// void Element::RemoveChild(ElementPtr _child) { - sdf::Errors errors; - RemoveChild(_child, errors); - throwOrPrintErrors(errors); -} - -///////////////////////////////////////////////// -void Element::RemoveChild(ElementPtr _child, sdf::Errors &_errors) -{ - if (!_child) - { - _errors.push_back({ErrorCode::FATAL_ERROR, - "Cannot remove a nullptr child pointer"}); - return; - } + SDF_ASSERT(_child, "Cannot remove a nullptr child pointer"); ElementPtr_V::iterator iter; iter = std::find(this->dataPtr->elements.begin(), diff --git a/test/integration/error_output.cc b/test/integration/error_output.cc index c4607fc1d..c8e676762 100644 --- a/test/integration/error_output.cc +++ b/test/integration/error_output.cc @@ -22,6 +22,7 @@ #include "sdf/Element.hh" #include "sdf/Error.hh" +#include "sdf/Exception.hh" #include "sdf/Model.hh" #include "sdf/Param.hh" #include "sdf/Sensor.hh" @@ -205,9 +206,14 @@ TEST(ErrorOutput, ElementErrorOutput) EXPECT_NE(std::string::npos, errors[1].Message().find( "Invalid parameter")); + // Check nothing has been printed + EXPECT_TRUE(buffer.str().empty()) << buffer.str(); + errors.clear(); - elem->AddValue("type", "value", true, "a", "b", errors); - ASSERT_EQ(errors.size(), 9u); + ASSERT_THROW(elem->AddValue("type", "value", true, "a", "b", errors), + sdf::AssertionInternalError); + ASSERT_EQ(errors.size(), 6u); + EXPECT_NE(std::string::npos, errors[0].Message().find( "Unknown parameter type[type]")); EXPECT_NE(std::string::npos, errors[1].Message().find( @@ -220,27 +226,19 @@ TEST(ErrorOutput, ElementErrorOutput) "Unknown parameter type[type]")); EXPECT_NE(std::string::npos, errors[5].Message().find( "Invalid [max] parameter in SDFormat description of [testElement]")); - EXPECT_NE(std::string::npos, errors[6].Message().find( - "Unknown parameter type[type]")); - EXPECT_NE(std::string::npos, errors[7].Message().find( - "Failed to set value '0' to key [testElement] for new parent element" - " of name 'testElement', reverting to previous value '0'.")); - EXPECT_NE(std::string::npos, errors[8].Message().find( - "Cannot set parent Element of value to itself.")); errors.clear(); + buffer.str(std::string()); elem->GetElement("nonExistentElement", errors); ASSERT_EQ(errors.size(), 1u); EXPECT_NE(std::string::npos, errors[0].Message().find( "Missing element description for [nonExistentElement]")); - errors.clear(); - elem->RemoveChild(sdf::ElementPtr(), errors); - ASSERT_EQ(errors.size(), 1u); - EXPECT_NE(std::string::npos, errors[0].Message().find( - "Cannot remove a nullptr child pointer")); // Check nothing has been printed EXPECT_TRUE(buffer.str().empty()) << buffer.str(); + + ASSERT_THROW(elem->RemoveChild(sdf::ElementPtr()), + sdf::AssertionInternalError); } //////////////////////////////////////// From 917386a53caa0e02b8c3aace8572ce6b49ff221b Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Wed, 8 Feb 2023 23:22:50 +0000 Subject: [PATCH 2/5] fix line > 80 chars. Signed-off-by: Marco A. Gutierrez --- src/Converter_TEST.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Converter_TEST.cc b/src/Converter_TEST.cc index 81c30413d..443774017 100644 --- a/src/Converter_TEST.cc +++ b/src/Converter_TEST.cc @@ -2856,8 +2856,8 @@ TEST(Converter, NullDoc) parserConfig.SetWarningsPolicy(sdf::EnforcementPolicy::ERR); sdf::Errors errors; - ASSERT_THROW(sdf::Converter::Convert(errors, nullptr, &convertXmlDoc, parserConfig), - sdf::AssertionInternalError); + ASSERT_THROW(sdf::Converter::Convert(errors, nullptr, &convertXmlDoc, + parserConfig), sdf::AssertionInternalError); ASSERT_TRUE(errors.empty()); ASSERT_THROW(sdf::Converter::Convert(errors, &xmlDoc, nullptr, parserConfig), sdf::AssertionInternalError); From eedac58931f4dc0fe5135a0598f8bf6585d25cfb Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Thu, 9 Feb 2023 21:36:46 +0000 Subject: [PATCH 3/5] adding back RemoveChild with sdf:Errors Signed-off-by: Marco A. Gutierrez --- include/sdf/Element.hh | 5 +++++ src/Element.cc | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/include/sdf/Element.hh b/include/sdf/Element.hh index f1e9541ad..1afc35799 100644 --- a/include/sdf/Element.hh +++ b/include/sdf/Element.hh @@ -559,6 +559,11 @@ namespace sdf /// \param[in] _child Pointer to the child to remove. public: void RemoveChild(ElementPtr _child); + /// \brief Remove a child element. + /// \param[in] _child Pointer to the child to remove. + /// \param[out] _errors Vector of errors. + public: void RemoveChild(ElementPtr _child, sdf::Errors &_errors); + /// \brief Remove all child elements. public: void ClearElements(); diff --git a/src/Element.cc b/src/Element.cc index 2b5bca9b8..e5b64e8dd 100644 --- a/src/Element.cc +++ b/src/Element.cc @@ -1276,6 +1276,14 @@ void Element::RemoveFromParent() ///////////////////////////////////////////////// void Element::RemoveChild(ElementPtr _child) +{ + sdf::Errors errors; + RemoveChild(_child, errors); + throwOrPrintErrors(errors); +} + +///////////////////////////////////////////////// +void Element::RemoveChild(ElementPtr _child, sdf::Errors &_errors) { SDF_ASSERT(_child, "Cannot remove a nullptr child pointer"); From d9474b39d27604fc70f48187423d561d830f340c Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Thu, 9 Feb 2023 21:44:50 +0000 Subject: [PATCH 4/5] test to make sure both RemoveChild behave the same Signed-off-by: Marco A. Gutierrez --- test/integration/error_output.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/error_output.cc b/test/integration/error_output.cc index c8e676762..06d6bc293 100644 --- a/test/integration/error_output.cc +++ b/test/integration/error_output.cc @@ -237,8 +237,11 @@ TEST(ErrorOutput, ElementErrorOutput) // Check nothing has been printed EXPECT_TRUE(buffer.str().empty()) << buffer.str(); + // Check both RemoveChild methods behave in the same way ASSERT_THROW(elem->RemoveChild(sdf::ElementPtr()), sdf::AssertionInternalError); + ASSERT_THROW(elem->RemoveChild(sdf::ElementPtr(), errors), + sdf::AssertionInternalError); } //////////////////////////////////////// From 6b9c929ad76b4b3a03638731857911f15b97f1ef Mon Sep 17 00:00:00 2001 From: "Marco A. Gutierrez" Date: Thu, 9 Feb 2023 22:36:26 +0000 Subject: [PATCH 5/5] Remove warning for unused sdf::Errors Co-authored-by: Addisu Z. Taddese i Signed-off-by: Marco A. Gutierrez --- src/Element.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Element.cc b/src/Element.cc index e5b64e8dd..01f6a0b84 100644 --- a/src/Element.cc +++ b/src/Element.cc @@ -1283,7 +1283,7 @@ void Element::RemoveChild(ElementPtr _child) } ///////////////////////////////////////////////// -void Element::RemoveChild(ElementPtr _child, sdf::Errors &_errors) +void Element::RemoveChild(ElementPtr _child, sdf::Errors &) { SDF_ASSERT(_child, "Cannot remove a nullptr child pointer");