DM-8106: SpherePoint does not have move constructors/assignment

include/lsst/afw/geom.h

@@ -39,10 +39,11 @@
 #include "lsst/afw/geom/Box.h"
 #include "lsst/afw/geom/Span.h"
 #include "lsst/afw/geom/SpanSet.h"
+#include "lsst/afw/geom/SpherePoint.h"
 #include "lsst/afw/geom/XYTransform.h"
 #include "lsst/afw/geom/Functor.h"
 #include "lsst/afw/geom/SeparableXYTransform.h"
 #include "lsst/afw/geom/polygon/Polygon.h"
 #include "lsst/afw/geom/TransformMap.h"
 
-#endif // LSST_GEOM_H
+#endif  // LSST_GEOM_H

include/lsst/afw/geom/SpherePoint.h

@@ -94,9 +94,17 @@ class SpherePoint
      *
      * @param other The point to copy.
      *
-     * @exceptsafe Provides strong exception guarantee.
+     * @exceptsafe Shall not throw exceptions.
+     */
+    SpherePoint(SpherePoint const& other) noexcept;
+
+    /**
+     * @copybrief SpherePoint(SpherePoint const&)
+     *
+     * As SpherePoint(SpherePoint const&), except that `other` may be left
+     * in an unspecified but valid state.
      */
-    SpherePoint(SpherePoint const& other) = default;
+    SpherePoint(SpherePoint&& other) noexcept;
 
     /**
      * Overwrite this object with the value of another SpherePoint.
@@ -110,7 +118,15 @@ class SpherePoint
      *
      * @exceptsafe Shall not throw exceptions.
      */
-    SpherePoint& operator=(SpherePoint const& other) = default;
+    SpherePoint& operator=(SpherePoint const& other) noexcept;
+
+    /**
+     * @copybrief operator=(SpherePoint const&)
+     *
+     * As operator=(SpherePoint const&), except that `other` may be left
+     * in an unspecified but valid state.
+     */
+    SpherePoint& operator=(SpherePoint&& other) noexcept;
 
     /*
      * Accessors
@@ -218,7 +234,7 @@ class SpherePoint
     /**
      * `false` if two points represent the same position.
      *
-     * This operator shall always return the logical negation of operator==;
+     * This operator shall always return the logical negation of operator==();
      * see its documentation for a detailed specification.
      */
     bool operator!=(SpherePoint const& other) const noexcept;

src/geom/SpherePoint.cc

@@ -75,7 +75,7 @@ Angle haversine(Angle const& deltaLon, Angle const& deltaLat, double cosLat1, do
     double const sinDHalf = sqrt(havD);
     return (2.0 * asin(sinDHalf)) * radians;
 }
-}
+}  // end namespace
 
 SpherePoint::SpherePoint(Angle const& longitude, Angle const& latitude)
         : _longitude(wrap(longitude).asRadians()), _latitude(latitude.asRadians()) {
@@ -107,6 +107,14 @@ SpherePoint::SpherePoint(Point3D const& vector) {
     _latitude = asin(z);
 }
 
+SpherePoint::SpherePoint(SpherePoint const& other) noexcept = default;
+
+SpherePoint::SpherePoint(SpherePoint&& other) noexcept = default;
+
+SpherePoint& SpherePoint::operator=(SpherePoint const& other) noexcept = default;
+
+SpherePoint& SpherePoint::operator=(SpherePoint&& other) noexcept = default;
+
 Point3D SpherePoint::getVector() const noexcept {
     return Point3D(cos(_longitude) * cos(_latitude), sin(_longitude) * cos(_latitude), sin(_latitude));
 }

tests/testSpherePoint.cc

@@ -40,14 +40,14 @@ namespace afw {
 namespace geom {
 
 /**
- * Tests whether the result of SpherePoint::SpherePoint(SpherePoint const &)
- * makes an identical but independent copy.
+ * Tests whether the result of SpherePoint::SpherePoint(SpherePoint const&)
+ * is an identical but independent copy.
  */
 BOOST_AUTO_TEST_CASE(SpherePointCopyResult, *boost::unit_test::tolerance(1e-14)) {
     SpherePoint original(Point3D(0.34, -1.2, 0.97));
     SpherePoint copy(original);
 
-    // Want exact equality, not floating-point equality
+    // Want exact equality, not floating-point equality, for results of copy-construction
     BOOST_TEST(original == copy);
 
     // Don't compare Angles in case there's aliasing of some sort
@@ -60,6 +60,62 @@ BOOST_AUTO_TEST_CASE(SpherePointCopyResult, *boost::unit_test::tolerance(1e-14))
     BOOST_TEST(copy.getLatitude().asDegrees() == copyLat);
 }
 
+/**
+ * Tests whether the result of SpherePoint::SpherePoint(SpherePoint&&)
+ * is an identical copy.
+ */
+BOOST_AUTO_TEST_CASE(SpherePointMoveResult, *boost::unit_test::tolerance(1e-14)) {
+    SpherePoint original(Point3D(0.34, -1.2, 0.97));
+    // Don't compare Angles in case there's aliasing of some sort
+    double const oldLon = original.getLongitude().asDegrees();
+    double const oldLat = original.getLatitude().asDegrees();
+
+    SpherePoint copy(std::move(original));
+
+    BOOST_TEST(copy.getLongitude().asDegrees() == oldLon);
+    BOOST_TEST(copy.getLatitude().asDegrees() == oldLat);
+}
+
+/**
+ * Tests whether SpherePoint::operator=(SpherePoint const&) makes an identical
+ * but independent copy.
+ */
+BOOST_AUTO_TEST_CASE(assignCopyResult, *boost::unit_test::tolerance(1e-14)) {
+    SpherePoint original(Point3D(0.34, -1.2, 0.97));
+    // Don't compare Angles in case there's aliasing of some sort
+    double const oldLon = original.getLongitude().asDegrees();
+    double const oldLat = original.getLatitude().asDegrees();
+
+    SpherePoint copy(45.0 * degrees, -23.5 * degrees);
+    // Want exact equality, not floating-point equality, for results of assignment
+    BOOST_REQUIRE(original != copy);
+    copy = original;
+    BOOST_TEST(original == copy);
+
+    original = SpherePoint(-42 * degrees, 45 * degrees);
+    BOOST_TEST(original != copy);
+    BOOST_TEST(copy.getLongitude().asDegrees() == oldLon);
+    BOOST_TEST(copy.getLatitude().asDegrees() == oldLat);
+}
+
+/**
+ * Tests whether SpherePoint::operator=(SpherePoint const&) makes an identical
+ * copy.
+ */
+BOOST_AUTO_TEST_CASE(assignMoveResult, *boost::unit_test::tolerance(1e-14)) {
+    SpherePoint original(Point3D(0.34, -1.2, 0.97));
+    // Don't compare Angles in case there's aliasing of some sort
+    double const oldLon = original.getLongitude().asDegrees();
+    double const oldLat = original.getLatitude().asDegrees();
+
+    SpherePoint copy(45.0 * degrees, -23.5 * degrees);
+    BOOST_REQUIRE(original != copy);
+    copy = std::move(original);
+
+    BOOST_TEST(copy.getLongitude().asDegrees() == oldLon);
+    BOOST_TEST(copy.getLatitude().asDegrees() == oldLat);
+}
+
 /**
  * Tests whether SpherePoint::operator[](size_t) handles invalid indices
  * correctly.