From 84980e859486fd60153a3d04c6239eaff842b838 Mon Sep 17 00:00:00 2001 From: Matt Kuruc Date: Wed, 1 Feb 2023 16:46:47 -0800 Subject: [PATCH 1/2] Replace `boost::equality_comparable` with explicit implementation of `operator!=` in `pxr/usd/sdf` --- pxr/usd/sdf/allowed.h | 8 +++- pxr/usd/sdf/childrenProxy.h | 8 +++- pxr/usd/sdf/namespaceEdit.cpp | 12 ++++++ pxr/usd/sdf/namespaceEdit.h | 10 ++--- .../sdf/testenv/testSdfBatchNamespaceEdit.py | 41 +++++++++++++++++++ pxr/usd/sdf/testenv/testSdfTypes.py | 12 +++++- pxr/usd/sdf/types.cpp | 5 +++ pxr/usd/sdf/types.h | 6 ++- 8 files changed, 89 insertions(+), 13 deletions(-) diff --git a/pxr/usd/sdf/allowed.h b/pxr/usd/sdf/allowed.h index 46e8b52aac..e37f616924 100644 --- a/pxr/usd/sdf/allowed.h +++ b/pxr/usd/sdf/allowed.h @@ -32,7 +32,6 @@ #include #include -#include #include PXR_NAMESPACE_OPEN_SCOPE @@ -44,7 +43,7 @@ PXR_NAMESPACE_OPEN_SCOPE /// A \c SdfAllowed either evaluates to \c true in a boolean context /// or evaluates to \c false and has a string annotation. /// -class SdfAllowed : private boost::equality_comparable { +class SdfAllowed { private: typedef boost::optional _State; @@ -113,6 +112,11 @@ class SdfAllowed : private boost::equality_comparable { return _state == other._state; } + bool operator!=(const SdfAllowed& other) const + { + return !(*this == other); + } + private: _State _state; }; diff --git a/pxr/usd/sdf/childrenProxy.h b/pxr/usd/sdf/childrenProxy.h index 85835a1157..0f59315a12 100644 --- a/pxr/usd/sdf/childrenProxy.h +++ b/pxr/usd/sdf/childrenProxy.h @@ -35,7 +35,6 @@ #include #include -#include #include #include #include @@ -43,7 +42,7 @@ PXR_NAMESPACE_OPEN_SCOPE template -class SdfChildrenProxy : boost::equality_comparable > { +class SdfChildrenProxy { public: typedef _View View; typedef typename View::Adapter Adapter; @@ -353,6 +352,11 @@ class SdfChildrenProxy : boost::equality_comparable > { return _view == other._view; } + bool operator!=(const This& other) const + { + return !(*this == other); + } + /// Explicit bool conversion operator. The proxy object converts to /// \c true if it is valid, \c false otherwise. explicit operator bool() const diff --git a/pxr/usd/sdf/namespaceEdit.cpp b/pxr/usd/sdf/namespaceEdit.cpp index 5d0b1ed0f0..8e4e7c8f14 100644 --- a/pxr/usd/sdf/namespaceEdit.cpp +++ b/pxr/usd/sdf/namespaceEdit.cpp @@ -665,6 +665,12 @@ SdfNamespaceEdit::operator==(const SdfNamespaceEdit& rhs) const index == rhs.index; } +bool +SdfNamespaceEdit::operator!=(const SdfNamespaceEdit& rhs) const +{ + return !(*this == rhs); +} + std::ostream& operator<<(std::ostream& s, const SdfNamespaceEdit& x) { @@ -715,6 +721,12 @@ SdfNamespaceEditDetail::operator==(const SdfNamespaceEditDetail& rhs) const reason == rhs.reason; } +bool +SdfNamespaceEditDetail::operator!=(const SdfNamespaceEditDetail& other) const +{ + return !(*this == other); +} + std::ostream& operator<<(std::ostream& s, const SdfNamespaceEditDetail& x) { diff --git a/pxr/usd/sdf/namespaceEdit.h b/pxr/usd/sdf/namespaceEdit.h index d4e6fab177..70dfc53001 100644 --- a/pxr/usd/sdf/namespaceEdit.h +++ b/pxr/usd/sdf/namespaceEdit.h @@ -30,8 +30,6 @@ #include "pxr/usd/sdf/api.h" #include "pxr/usd/sdf/path.h" -#include - #include #include #include @@ -44,8 +42,7 @@ PXR_NAMESPACE_OPEN_SCOPE /// A single namespace edit. It supports renaming, reparenting, reparenting /// with a rename, reordering, and removal. /// -struct SdfNamespaceEdit : - boost::equality_comparable { +struct SdfNamespaceEdit { public: typedef SdfNamespaceEdit This; typedef SdfPath Path; @@ -114,6 +111,7 @@ struct SdfNamespaceEdit : } SDF_API bool operator==(const This& rhs) const; + SDF_API bool operator!=(const This& rhs) const; public: Path currentPath; ///< Path of the object when this edit starts. @@ -131,8 +129,7 @@ SDF_API std::ostream& operator<<(std::ostream&, const SdfNamespaceEditVector&); /// /// Detailed information about a namespace edit. /// -struct SdfNamespaceEditDetail : - boost::equality_comparable { +struct SdfNamespaceEditDetail { public: /// Validity of an edit. enum Result { @@ -146,6 +143,7 @@ struct SdfNamespaceEditDetail : const std::string& reason); SDF_API bool operator==(const SdfNamespaceEditDetail& rhs) const; + SDF_API bool operator!=(const SdfNamespaceEditDetail& other) const; public: Result result; ///< Validity. diff --git a/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py b/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py index 8b56fbc424..fa12ebd232 100644 --- a/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py +++ b/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py @@ -31,6 +31,47 @@ #verbose = True class TestSdfBatchNamespaceEdit(unittest.TestCase): + def test_EqualityOperators(self): + self.assertTrue(Sdf.NamespaceEdit('/A', '/B') == Sdf.NamespaceEdit('/A', '/B')) + self.assertFalse(Sdf.NamespaceEdit('/A', '/B') != Sdf.NamespaceEdit('/A', '/B')) + self.assertFalse(Sdf.NamespaceEdit('/B', '/A') == Sdf.NamespaceEdit('/A', '/B')) + self.assertTrue(Sdf.NamespaceEdit('/B', '/A') != Sdf.NamespaceEdit('/A', '/B')) + + # Validate the equality operators by varying a single field from a prototype edit + # at a time + prototype = Sdf.NamespaceEditDetail( + Sdf.NamespaceEditDetail.Okay, + Sdf.NamespaceEdit('/A', '/B'), + "reason" + ) + self.assertTrue(prototype == prototype) + self.assertFalse(prototype != prototype) + self.assertTrue(prototype != Sdf.NamespaceEditDetail( + Sdf.NamespaceEditDetail.Unbatched, + prototype.edit, + prototype.reason)) + self.assertFalse(prototype == Sdf.NamespaceEditDetail( + Sdf.NamespaceEditDetail.Unbatched, + prototype.edit, + prototype.reason)) + self.assertTrue(prototype != Sdf.NamespaceEditDetail( + prototype.result, + Sdf.NamespaceEdit('/C', '/D'), + prototype.reason)) + self.assertFalse(prototype == Sdf.NamespaceEditDetail( + prototype.result, + Sdf.NamespaceEdit('/C', '/D'), + prototype.reason)) + self.assertTrue(prototype != Sdf.NamespaceEditDetail( + prototype.result, + prototype.edit, + "a different reason")) + self.assertFalse(prototype == Sdf.NamespaceEditDetail( + prototype.result, + prototype.edit, + "a different reason")) + + def test_Basic(self): print('Test constructors') diff --git a/pxr/usd/sdf/testenv/testSdfTypes.py b/pxr/usd/sdf/testenv/testSdfTypes.py index ec4bc9affe..63b76e5bd9 100644 --- a/pxr/usd/sdf/testenv/testSdfTypes.py +++ b/pxr/usd/sdf/testenv/testSdfTypes.py @@ -380,6 +380,16 @@ def _TestValueTypeName(attrPath, expectedValueTypeName): def test_Hash(self): self.assertEqual(hash(Sdf.ValueTypeNames.Point3d), hash(Sdf.ValueTypeNames.Point3d)) - + + def test_UnregisteredValueEquality(self): + self.assertTrue(Sdf.UnregisteredValue(str(5)) == + Sdf.UnregisteredValue(str(5))) + self.assertFalse(Sdf.UnregisteredValue(str(5)) != + Sdf.UnregisteredValue(str(5))) + self.assertTrue(Sdf.UnregisteredValue(str(5)) != + Sdf.UnregisteredValue(str(6))) + self.assertFalse(Sdf.UnregisteredValue(str(5)) == + Sdf.UnregisteredValue(str(6))) + if __name__ == "__main__": unittest.main() diff --git a/pxr/usd/sdf/types.cpp b/pxr/usd/sdf/types.cpp index c3f4718369..32f943b331 100644 --- a/pxr/usd/sdf/types.cpp +++ b/pxr/usd/sdf/types.cpp @@ -756,6 +756,11 @@ bool SdfUnregisteredValue::operator==(const SdfUnregisteredValue &other) const return _value == other._value; } +bool SdfUnregisteredValue::operator!=(const SdfUnregisteredValue &other) const +{ + return !(*this == other); +} + std::ostream &operator << (std::ostream &out, const SdfUnregisteredValue &value) { return out << value.GetValue(); diff --git a/pxr/usd/sdf/types.h b/pxr/usd/sdf/types.h index 26aa03a1cb..0168aa0901 100644 --- a/pxr/usd/sdf/types.h +++ b/pxr/usd/sdf/types.h @@ -484,8 +484,7 @@ std::ostream &VtStreamOut(const SdfVariantSelectionMap &, std::ostream &); /// well as limited inspection and editing capabilities (e.g., moving /// this data to a different spec or field) even when the data type /// of the value isn't known. -class SdfUnregisteredValue : - public boost::equality_comparable +class SdfUnregisteredValue { public: /// Wraps an empty VtValue @@ -513,6 +512,9 @@ class SdfUnregisteredValue : /// Returns true if the wrapped VtValues are equal SDF_API bool operator==(const SdfUnregisteredValue &other) const; + /// Returns true if the wrapped VtValues are not equal + SDF_API bool operator!=(const SdfUnregisteredValue &other) const; + private: VtValue _value; }; From b43f82859c6708392afdb6f6c57e139f21039690 Mon Sep 17 00:00:00 2001 From: Matt Kuruc Date: Mon, 19 Jun 2023 19:27:55 -0700 Subject: [PATCH 2/2] Address review notes --- pxr/usd/sdf/namespaceEdit.cpp | 4 +-- pxr/usd/sdf/namespaceEdit.h | 2 +- .../sdf/testenv/testSdfBatchNamespaceEdit.py | 26 +++++-------------- pxr/usd/sdf/testenv/testSdfTypes.py | 10 +++---- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/pxr/usd/sdf/namespaceEdit.cpp b/pxr/usd/sdf/namespaceEdit.cpp index 8e4e7c8f14..2a4f54ef0d 100644 --- a/pxr/usd/sdf/namespaceEdit.cpp +++ b/pxr/usd/sdf/namespaceEdit.cpp @@ -722,9 +722,9 @@ SdfNamespaceEditDetail::operator==(const SdfNamespaceEditDetail& rhs) const } bool -SdfNamespaceEditDetail::operator!=(const SdfNamespaceEditDetail& other) const +SdfNamespaceEditDetail::operator!=(const SdfNamespaceEditDetail& rhs) const { - return !(*this == other); + return !(*this == rhs); } std::ostream& diff --git a/pxr/usd/sdf/namespaceEdit.h b/pxr/usd/sdf/namespaceEdit.h index 70dfc53001..04ed4910c0 100644 --- a/pxr/usd/sdf/namespaceEdit.h +++ b/pxr/usd/sdf/namespaceEdit.h @@ -143,7 +143,7 @@ struct SdfNamespaceEditDetail { const std::string& reason); SDF_API bool operator==(const SdfNamespaceEditDetail& rhs) const; - SDF_API bool operator!=(const SdfNamespaceEditDetail& other) const; + SDF_API bool operator!=(const SdfNamespaceEditDetail& rhs) const; public: Result result; ///< Validity. diff --git a/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py b/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py index fa12ebd232..26fe068754 100644 --- a/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py +++ b/pxr/usd/sdf/testenv/testSdfBatchNamespaceEdit.py @@ -32,10 +32,8 @@ class TestSdfBatchNamespaceEdit(unittest.TestCase): def test_EqualityOperators(self): - self.assertTrue(Sdf.NamespaceEdit('/A', '/B') == Sdf.NamespaceEdit('/A', '/B')) - self.assertFalse(Sdf.NamespaceEdit('/A', '/B') != Sdf.NamespaceEdit('/A', '/B')) - self.assertFalse(Sdf.NamespaceEdit('/B', '/A') == Sdf.NamespaceEdit('/A', '/B')) - self.assertTrue(Sdf.NamespaceEdit('/B', '/A') != Sdf.NamespaceEdit('/A', '/B')) + self.assertEqual(Sdf.NamespaceEdit('/A', '/B'), Sdf.NamespaceEdit('/A', '/B')) + self.assertNotEqual(Sdf.NamespaceEdit('/B', '/A'), Sdf.NamespaceEdit('/A', '/B')) # Validate the equality operators by varying a single field from a prototype edit # at a time @@ -44,29 +42,19 @@ def test_EqualityOperators(self): Sdf.NamespaceEdit('/A', '/B'), "reason" ) - self.assertTrue(prototype == prototype) - self.assertFalse(prototype != prototype) - self.assertTrue(prototype != Sdf.NamespaceEditDetail( - Sdf.NamespaceEditDetail.Unbatched, + self.assertEqual(prototype, Sdf.NamespaceEditDetail( + prototype.result, prototype.edit, prototype.reason)) - self.assertFalse(prototype == Sdf.NamespaceEditDetail( + self.assertNotEqual(prototype, Sdf.NamespaceEditDetail( Sdf.NamespaceEditDetail.Unbatched, prototype.edit, prototype.reason)) - self.assertTrue(prototype != Sdf.NamespaceEditDetail( - prototype.result, - Sdf.NamespaceEdit('/C', '/D'), - prototype.reason)) - self.assertFalse(prototype == Sdf.NamespaceEditDetail( + self.assertNotEqual(prototype, Sdf.NamespaceEditDetail( prototype.result, Sdf.NamespaceEdit('/C', '/D'), prototype.reason)) - self.assertTrue(prototype != Sdf.NamespaceEditDetail( - prototype.result, - prototype.edit, - "a different reason")) - self.assertFalse(prototype == Sdf.NamespaceEditDetail( + self.assertNotEqual(prototype, Sdf.NamespaceEditDetail( prototype.result, prototype.edit, "a different reason")) diff --git a/pxr/usd/sdf/testenv/testSdfTypes.py b/pxr/usd/sdf/testenv/testSdfTypes.py index 63b76e5bd9..6ef801861b 100644 --- a/pxr/usd/sdf/testenv/testSdfTypes.py +++ b/pxr/usd/sdf/testenv/testSdfTypes.py @@ -382,14 +382,10 @@ def test_Hash(self): self.assertEqual(hash(Sdf.ValueTypeNames.Point3d), hash(Sdf.ValueTypeNames.Point3d)) def test_UnregisteredValueEquality(self): - self.assertTrue(Sdf.UnregisteredValue(str(5)) == - Sdf.UnregisteredValue(str(5))) - self.assertFalse(Sdf.UnregisteredValue(str(5)) != + self.assertEqual(Sdf.UnregisteredValue(str(5)), Sdf.UnregisteredValue(str(5))) - self.assertTrue(Sdf.UnregisteredValue(str(5)) != - Sdf.UnregisteredValue(str(6))) - self.assertFalse(Sdf.UnregisteredValue(str(5)) == - Sdf.UnregisteredValue(str(6))) + self.assertNotEqual(Sdf.UnregisteredValue(str(5)), + Sdf.UnregisteredValue(str(6))) if __name__ == "__main__": unittest.main()