Return copies of static const members

This prevents segfaults from happening when users accidentaly try to mutate references to static const members Signed-off-by: Addisu Z. Taddese <[email protected]>
gazebosim · Jan 14, 2022 · e0b037c · e0b037c
1 parent cb86347
commit e0b037c
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 27 deletions.
diff --git a/src/python_pybind11/src/Vector2.hh b/src/python_pybind11/src/Vector2.hh
@@ -120,9 +120,12 @@ void defineMathVector2(py::module &m, const std::string &typestr)
     .def("y", py::overload_cast<>(&Class::Y), "Get the y value.")
     .def("x", py::overload_cast<const T&>(&Class::X), "Set the x value.")
     .def("y", py::overload_cast<const T&>(&Class::Y), "Set the y value.")
-    .def_readonly_static("ZERO", &Class::Zero, "math::Vector2(0, 0)")
-    .def_readonly_static("ONE", &Class::One, "math::Vector2(1, 1)")
-    .def_readonly_static("NAN", &Class::NaN, "math::Vector3(NaN, NaN)")
+    .def_readonly_static("ZERO", &Class::Zero, py::return_value_policy::copy,
+        "math::Vector2(0, 0)")
+    .def_readonly_static("ONE", &Class::One, py::return_value_policy::copy,
+        "math::Vector2(1, 1)")
+    .def_readonly_static("NAN", &Class::NaN, py::return_value_policy::copy,
+        "math::Vector2(NaN, NaN)")
     .def("__copy__", [](const Class &self) {
       return Class(self);
     })

diff --git a/src/python_pybind11/src/Vector3.hh b/src/python_pybind11/src/Vector3.hh
@@ -141,12 +141,18 @@ void defineMathVector3(py::module &m, const std::string &typestr)
     .def("x", py::overload_cast<const T&>(&Class::X), "Set the x value.")
     .def("y", py::overload_cast<const T&>(&Class::Y), "Set the y value.")
     .def("z", py::overload_cast<const T&>(&Class::Z), "Set the z value.")
-    .def_readonly_static("ZERO", &Class::Zero, "math::Vector3(0, 0, 0)")
-    .def_readonly_static("ONE", &Class::One, "math::Vector3(1, 1, 1)")
-    .def_readonly_static("UNIT_X", &Class::UnitX, "math::Vector3(1, 0, 0)")
-    .def_readonly_static("UNIT_Y", &Class::UnitY, "math::Vector3(0, 1, 0)")
-    .def_readonly_static("UNIT_Z", &Class::UnitZ, "math::Vector3(0, 0, 1)")
-    .def_readonly_static("NAN", &Class::NaN, "math::Vector3(NaN, NaN, NaN)")
+    .def_readonly_static("ZERO", &Class::Zero, py::return_value_policy::copy,
+        "math::Vector3(0, 0, 0)")
+    .def_readonly_static("ONE", &Class::One, py::return_value_policy::copy,
+        "math::Vector3(1, 1, 1)")
+    .def_readonly_static("UNIT_X", &Class::UnitX, py::return_value_policy::copy,
+        "math::Vector3(1, 0, 0)")
+    .def_readonly_static("UNIT_Y", &Class::UnitY, py::return_value_policy::copy,
+        "math::Vector3(0, 1, 0)")
+    .def_readonly_static("UNIT_Z", &Class::UnitZ, py::return_value_policy::copy,
+        "math::Vector3(0, 0, 1)")
+    .def_readonly_static("NAN", &Class::NaN, py::return_value_policy::copy,
+        "math::Vector3(NaN, NaN, NaN)")
     .def("__copy__", [](const Class &self) {
       return Class(self);
     })

diff --git a/src/python_pybind11/src/Vector4.hh b/src/python_pybind11/src/Vector4.hh
@@ -127,11 +127,12 @@ void defineMathVector4(py::module &m, const std::string &typestr)
     .def("y", py::overload_cast<const T&>(&Class::Y), "Set the y value.")
     .def("z", py::overload_cast<const T&>(&Class::Z), "Set the z value.")
     .def("w", py::overload_cast<const T&>(&Class::W), "Set the w value.")
-    .def_readonly_static("ZERO", &Class::Zero, "math::Vector4(0, 0, 0, 0)")
-    .def_readonly_static("ONE", &Class::One, "math::Vector4(1, 1, 1, 1)")
-    .def_readonly_static("NAN",
-                         &Class::NaN,
-                         "math::Vector4(NaN, NaN, NaN, NaN)")
+    .def_readonly_static("ZERO", &Class::Zero, py::return_value_policy::copy,
+        "math::Vector4(0, 0, 0, 0)")
+    .def_readonly_static("ONE", &Class::One, py::return_value_policy::copy,
+        "math::Vector4(1, 1, 1, 1)")
+    .def_readonly_static("NAN", &Class::NaN, py::return_value_policy::copy,
+        "math::Vector4(NaN, NaN, NaN, NaN)")
     .def("__copy__", [](const Class &self) {
       return Class(self);
     })

diff --git a/src/ruby/Vector2.i b/src/ruby/Vector2.i
@@ -33,10 +33,22 @@ namespace ignition
     template<typename T>
     class Vector2
     {
-      public: static const Vector2 Zero;
-      public: static const Vector2 One;
-      public: static const Vector2 NaN;
-
+      // Use %extend to override getters for static member variables so that
+      // copies of the variables are returned instead of references to the variables.
+      public: %extend {
+        static Vector2 Zero()
+        {
+          return ignition::math::Vector2<T>::Zero;
+        }
+        static Vector2 One()
+        {
+          return ignition::math::Vector2<T>::One;
+        }
+        static Vector2 NaN()
+        {
+          return ignition::math::Vector2<T>::NaN;
+        }
+      }
       public: Vector2();
       public: Vector2(const T &_x, const T &_y);
       public: Vector2(const Vector2<T> &_v);

diff --git a/src/ruby/Vector3.i b/src/ruby/Vector3.i
@@ -33,12 +33,34 @@ namespace ignition
     template<typename T>
     class Vector3
     {
-      public: static const Vector3 Zero;
-      public: static const Vector3 One;
-      public: static const Vector3 UnitX;
-      public: static const Vector3 UnitY;
-      public: static const Vector3 UnitZ;
-      public: static const Vector3 NaN;
+      // Use %extend to override getters for static member variables so that
+      // copies of the variables are returned instead of references to the variables.
+      public: %extend {
+        static Vector3 Zero()
+        {
+          return ignition::math::Vector3<T>::Zero;
+        }
+        static Vector3 One()
+        {
+          return ignition::math::Vector3<T>::One;
+        }
+        static Vector3 UnitX()
+        {
+          return ignition::math::Vector3<T>::UnitX;
+        }
+        static Vector3 UnitY()
+        {
+          return ignition::math::Vector3<T>::UnitY;
+        }
+        static Vector3 UnitZ()
+        {
+          return ignition::math::Vector3<T>::UnitZ;
+        }
+        static Vector3 NaN()
+        {
+          return ignition::math::Vector3<T>::NaN;
+        }
+      }
       public: Vector3();
       public: Vector3(const T &_x, const T &_y, const T &_z);
       public: Vector3(const Vector3<T> &_v);

diff --git a/src/ruby/Vector4.i b/src/ruby/Vector4.i
@@ -33,9 +33,22 @@ namespace ignition
     template<typename T>
     class Vector4
     {
-      public: static const Vector4 Zero;
-      public: static const Vector4 One;
-      public: static const Vector4 NaN;
+      // Use %extend to override getters for static member variables so that
+      // copies of the variables are returned instead of references to the variables.
+      public: %extend {
+        static Vector4 Zero()
+        {
+          return ignition::math::Vector4<T>::Zero;
+        }
+        static Vector4 One()
+        {
+          return ignition::math::Vector4<T>::One;
+        }
+        static Vector4 NaN()
+        {
+          return ignition::math::Vector4<T>::NaN;
+        }
+      }
       public: Vector4();
       public: Vector4(const T &_x, const T &_y, const T &_z, const T &_w);
       public: Vector4(const Vector4<T> &_v);