replace boost::variant by std::variant

fix clang compilation errors
moderate clang-tidy cleanup for the files with compilation errors

examples/mask.cc

@@ -22,7 +22,6 @@
 
 #include <cstdio>
 #include <string>
-#include <algorithm>
 
 #include "lsst/geom.h"
 #include "lsst/utils/Utils.h"

include/lsst/afw/fits.h

@@ -312,7 +312,7 @@ class Fits {
     std::string getFileName() const;
 
     /// Return the current HDU (0-indexed; 0 is the Primary HDU).
-    int getHdu();
+    int getHdu() const;
 
     /**
      *  Set the current HDU.
@@ -669,8 +669,8 @@ class Fits {
  * - names in `second`, omitting "COMMENT" and "HISTORY" if valid versions appear in `first`
  */
 std::shared_ptr<daf::base::PropertyList> combineMetadata(
-        std::shared_ptr<const daf::base::PropertyList> first,
-        std::shared_ptr<const daf::base::PropertyList> second);
+        const std::shared_ptr<const daf::base::PropertyList>& first,
+        const std::shared_ptr<const daf::base::PropertyList>& second);
 
 /** Read FITS header
  *

include/lsst/afw/image/Image.h

@@ -125,7 +125,7 @@ class Image : public ImageBase<PixelT> {
      * this may not be what you want.  See also assign(rhs) to copy pixels between Image%s
      */
     Image(const Image& rhs, const bool deep = false);
-    Image(Image&& rhs);
+    Image(Image&& rhs) noexcept;
 
     /**
      *  Construct an Image by reading a regular FITS file.
@@ -142,7 +142,7 @@ class Image : public ImageBase<PixelT> {
      *                              when on-disk values may overflow or truncate.
      */
     explicit Image(std::string const& fileName, int hdu = fits::DEFAULT_HDU,
-                   std::shared_ptr<lsst::daf::base::PropertySet> metadata =
+                   const std::shared_ptr<lsst::daf::base::PropertySet>& metadata =
                            std::shared_ptr<lsst::daf::base::PropertySet>(),
                    lsst::geom::Box2I const& bbox = lsst::geom::Box2I(), ImageOrigin origin = PARENT,
                    bool allowUnsafe = false);
@@ -162,7 +162,7 @@ class Image : public ImageBase<PixelT> {
      *                              when on-disk values may overflow or truncate.
      */
     explicit Image(fits::MemFileManager& manager, int hdu = fits::DEFAULT_HDU,
-                   std::shared_ptr<lsst::daf::base::PropertySet> metadata =
+                   const std::shared_ptr<lsst::daf::base::PropertySet>& metadata =
                            std::shared_ptr<lsst::daf::base::PropertySet>(),
                    lsst::geom::Box2I const& bbox = lsst::geom::Box2I(), ImageOrigin origin = PARENT,
                    bool allowUnsafe = false);
@@ -179,7 +179,7 @@ class Image : public ImageBase<PixelT> {
      *                              when on-disk values may overflow or truncate.
      */
     explicit Image(fits::Fits& fitsfile,
-                   std::shared_ptr<lsst::daf::base::PropertySet> metadata =
+                   const std::shared_ptr<lsst::daf::base::PropertySet>& metadata =
                            std::shared_ptr<lsst::daf::base::PropertySet>(),
                    lsst::geom::Box2I const& bbox = lsst::geom::Box2I(), ImageOrigin origin = PARENT,
                    bool allowUnsafe = false);
@@ -197,7 +197,7 @@ class Image : public ImageBase<PixelT> {
     // Assignment operators are not inherited
     //
     /// Set the %image's pixels to rhs
-    Image& operator=(const PixelT rhs);
+    Image& operator=(const PixelT rhs) override;
     /**
      * Assignment operator.
      *
@@ -208,7 +208,7 @@ class Image : public ImageBase<PixelT> {
      * private
      */
     Image& operator=(const Image& rhs);
-    Image& operator=(Image&& rhs);
+    Image& operator=(Image&& rhs) noexcept;
 
     /**
      * Return a subimage corresponding to the given box.

include/lsst/afw/image/ImageBase.h

@@ -190,7 +190,7 @@ class ImageBase {
      * this may not be what you want.  See also assign(rhs) to copy pixels between Image%s
      */
     ImageBase(const ImageBase& src, const bool deep = false);
-    ImageBase(ImageBase&& src);
+    ImageBase(ImageBase&& src) noexcept;
     /**
      * Copy constructor to make a copy of part of an %image.
      *
@@ -237,7 +237,7 @@ class ImageBase {
      *  will be deep-copied in Python, but the constructor will fail to compile in pure C++.
      */
     explicit ImageBase(Array const& array, bool deep = false,
-                       lsst::geom::Point2I const& xy0 = lsst::geom::Point2I());
+                       lsst::geom::Point2I  xy0 = lsst::geom::Point2I());
 
     virtual ~ImageBase() = default;
     /** Shallow assignment operator.
@@ -249,8 +249,9 @@ class ImageBase {
      * declare this function private
      */
     ImageBase& operator=(const ImageBase& rhs);
-    ImageBase& operator=(ImageBase&& rhs);
-    /// Set the %image's pixels to rhs
+    ImageBase& operator=(ImageBase&& rhs) noexcept;
+
+    virtual /// Set the %image's pixels to rhs
     ImageBase& operator=(const PixelT rhs);
 
     /**
@@ -267,15 +268,18 @@ class ImageBase {
     void assign(ImageBase const& rhs, lsst::geom::Box2I const& bbox = lsst::geom::Box2I(),
                 ImageOrigin origin = PARENT);
 
-    // TODO: should deprecate these, but the replacement should take two ints
+    virtual // TODO: should deprecate these, but the replacement should take two ints
     // rather than a Point2I, to keep C++ loops simple
     /// Return a reference to the pixel `(x, y)` in LOCAL coordinates
     PixelReference operator()(int x, int y);
-    /// Return a reference to the pixel `(x, y)` in LOCAL coordinates with bounds checking
+
+    virtual /// Return a reference to the pixel `(x, y)` in LOCAL coordinates with bounds checking
     PixelReference operator()(int x, int y, CheckIndices const&);
-    /// Return a const reference to the pixel `(x, y)` in LOCAL coordinates
+
+    virtual /// Return a const reference to the pixel `(x, y)` in LOCAL coordinates
     PixelConstReference operator()(int x, int y) const;
-    /// Return a const reference to the pixel `(x, y)` in LOCAL coordinates with bounds checking
+
+    virtual /// Return a const reference to the pixel `(x, y)` in LOCAL coordinates with bounds checking
     PixelConstReference operator()(int x, int y, CheckIndices const&) const;
 
     /// Return a reference to a single pixel (with no bounds check).

include/lsst/afw/image/Mask.h

@@ -80,13 +80,6 @@ class Mask : public ImageBase<MaskPixelT> {
 
     typedef detail::Mask_tag image_category;
 
-    /// A templated class to return this classes' type (present in Image/Mask/MaskedImage)
-    template <typename MaskPT = MaskPixelT>
-    struct ImageTypeFactory {
-        /// Return the desired type
-        typedef Mask<MaskPT> type;
-    };
-
     // Constructors
     /**
      * Construct a Mask initialized to 0x0
@@ -161,7 +154,7 @@ class Mask : public ImageBase<MaskPixelT> {
      *  on-disk version.
      */
     explicit Mask(std::string const& fileName, int hdu = fits::DEFAULT_HDU,
-                  std::shared_ptr<lsst::daf::base::PropertySet> metadata =
+                  const std::shared_ptr<lsst::daf::base::PropertySet>& metadata =
                           std::shared_ptr<lsst::daf::base::PropertySet>(),
                   lsst::geom::Box2I const& bbox = lsst::geom::Box2I(), ImageOrigin origin = PARENT,
                   bool conformMasks = false, bool allowUnsafe = false);
@@ -187,7 +180,7 @@ class Mask : public ImageBase<MaskPixelT> {
      *  on-disk version.
      */
     explicit Mask(fits::MemFileManager& manager, int hdu = fits::DEFAULT_HDU,
-                  std::shared_ptr<lsst::daf::base::PropertySet> metadata =
+                  const std::shared_ptr<lsst::daf::base::PropertySet>& metadata =
                           std::shared_ptr<lsst::daf::base::PropertySet>(),
                   lsst::geom::Box2I const& bbox = lsst::geom::Box2I(), ImageOrigin origin = PARENT,
                   bool conformMasks = false, bool allowUnsafe = false);
@@ -210,7 +203,7 @@ class Mask : public ImageBase<MaskPixelT> {
      *  on-disk version.
      */
     explicit Mask(fits::Fits& fitsfile,
-                  std::shared_ptr<lsst::daf::base::PropertySet> metadata =
+                  const std::shared_ptr<lsst::daf::base::PropertySet>& metadata =
                           std::shared_ptr<lsst::daf::base::PropertySet>(),
                   lsst::geom::Box2I const& bbox = lsst::geom::Box2I(), ImageOrigin origin = PARENT,
                   bool conformMasks = false, bool allowUnsafe = false);
@@ -226,8 +219,8 @@ class Mask : public ImageBase<MaskPixelT> {
      * @param src mask to copy
      * @param deep deep copy? (construct a view with shared pixels if false)
      */
-    Mask(const Mask& src, const bool deep = false);
-    Mask(Mask&& src);
+    Mask(const Mask& src, bool deep = false);
+    Mask(Mask&& src) noexcept;
     ~Mask() override;
     /**
      * Construct a Mask from a subregion of another Mask
@@ -237,28 +230,28 @@ class Mask : public ImageBase<MaskPixelT> {
      * @param origin coordinate system of the bbox
      * @param deep deep copy? (construct a view with shared pixels if false)
      */
-    Mask(const Mask& src, const lsst::geom::Box2I& bbox, ImageOrigin const origin = PARENT,
-         const bool deep = false);
+    Mask(const Mask& src, const lsst::geom::Box2I& bbox, ImageOrigin origin = PARENT,
+         bool deep = false);
 
     explicit Mask(ndarray::Array<MaskPixelT, 2, 1> const& array, bool deep = false,
                   lsst::geom::Point2I const& xy0 = lsst::geom::Point2I());
 
     void swap(Mask& rhs);
     // Operators
 
-    Mask& operator=(MaskPixelT const rhs);
+    Mask& operator=(MaskPixelT rhs) override;
     Mask& operator=(const Mask& rhs);
-    Mask& operator=(Mask&& rhs);
+    Mask& operator=(Mask&& rhs) noexcept;
 
     /// OR a Mask into a Mask
     Mask& operator|=(Mask const& rhs);
     /// OR a bitmask into a Mask
-    Mask& operator|=(MaskPixelT const rhs);
+    Mask& operator|=(MaskPixelT rhs);
 
     /// AND a Mask into a Mask
     Mask& operator&=(Mask const& rhs);
     /// AND a bitmask into a Mask
-    Mask& operator&=(MaskPixelT const rhs);
+    Mask& operator&=(MaskPixelT rhs);
 
     /**
      * Return a subimage corresponding to the given box.
@@ -292,22 +285,22 @@ class Mask : public ImageBase<MaskPixelT> {
     /// XOR a Mask into a Mask
     Mask& operator^=(Mask const& rhs);
     /// XOR a bitmask into a Mask
-    Mask& operator^=(MaskPixelT const rhs);
+    Mask& operator^=(MaskPixelT rhs);
 
     /**
      * get a reference to the specified pixel
      *
      * @param x x index
      * @param y y index
      */
-    typename ImageBase<MaskPixelT>::PixelReference operator()(int x, int y);
+    typename ImageBase<MaskPixelT>::PixelReference operator()(int x, int y) override;
     /**
      * get the specified pixel (const version)
      *
      * @param x x index
      * @param y y index
      */
-    typename ImageBase<MaskPixelT>::PixelConstReference operator()(int x, int y) const;
+    typename ImageBase<MaskPixelT>::PixelConstReference operator()(int x, int y) const override;
     /**
      * is the specified mask plane set in the specified pixel?
      *
@@ -323,7 +316,7 @@ class Mask : public ImageBase<MaskPixelT> {
      * @param y y index
      * @param check Check array bounds?
      */
-    typename ImageBase<MaskPixelT>::PixelReference operator()(int x, int y, CheckIndices const& check);
+    typename ImageBase<MaskPixelT>::PixelReference operator()(int x, int y, CheckIndices const& check) override;
     /**
      * get the specified pixel with array checking (const version)
      *
@@ -332,7 +325,7 @@ class Mask : public ImageBase<MaskPixelT> {
      * @param check Check array bounds?
      */
     typename ImageBase<MaskPixelT>::PixelConstReference operator()(int x, int y,
-                                                                   CheckIndices const& check) const;
+                                                                   CheckIndices const& check) const override;
     /**
      * is the specified mask plane set in the specified pixel, checking array bounds?
      *
@@ -447,14 +440,14 @@ class Mask : public ImageBase<MaskPixelT> {
     /**
      * Set the bit specified by "planeId" for pixels (x0, y) ... (x1, y)
      */
-    void setMaskPlaneValues(const int plane, const int x0, const int x1, const int y);
+    void setMaskPlaneValues(int plane, int x0, int x1, int y);
     /**
      * Given a PropertySet that contains the MaskPlane assignments, setup the MaskPlanes.
      *
      * @param metadata metadata from a Mask
      * @returns a dictionary of mask plane name: plane ID
      */
-    static MaskPlaneDict parseMaskPlaneMetadata(std::shared_ptr<lsst::daf::base::PropertySet const> metadata);
+    static MaskPlaneDict parseMaskPlaneMetadata(const std::shared_ptr<lsst::daf::base::PropertySet const>& metadata);
     //
     // Operations on the mask plane dictionary
     //
@@ -471,7 +464,7 @@ class Mask : public ImageBase<MaskPixelT> {
      *
      * @throws lsst::pex::exceptions::InvalidParameterError if plane is invalid
      */
-    void removeAndClearMaskPlane(const std::string& name, bool const removeFromDefault = false);
+    void removeAndClearMaskPlane(const std::string& name, bool removeFromDefault = false);
 
     /**
      * Return the mask plane number corresponding to a plane name

include/lsst/afw/table/FieldBase.h

@@ -91,9 +91,8 @@ struct FieldBase {
  *
  *  The Array tag is used for both fixed-length (same size in every record, accessible via ColumnView)
  *  and variable-length arrays; variable-length arrays are initialized with a size of 0.  Ideally,
- *  we'd use complete different tag classes for those two very different types, but boost::variant and
- *  boost::mpl put a limit of 20 on the number of field types, and we're running out.  In a future
- *  reimplementation of afw::table, we should fix this.
+ *  we'd use complete different tag classes for those two very different types, but this was limited in
+ *  the original (pre-C++11) implementation and now it would be an API change.
  */
 template <typename U>
 struct FieldBase<Array<U> > {

include/lsst/afw/table/Schema.h

@@ -3,6 +3,7 @@
 #define AFW_TABLE_Schema_h_INCLUDED
 
 #include <memory>
+#include <algorithm>
 #include <vector>
 
 #include "ndarray.h"
@@ -199,6 +200,7 @@ class Schema final {
     template <typename T>
     void replaceField(Key<T> const& key, Field<T> const& field);
 
+    //@{
     /**
      *  Apply a functor to each SchemaItem in the Schema.
      *
@@ -209,10 +211,18 @@ class Schema final {
      *  Fields will be processed in the order they were added to the schema.
      */
     template <typename F>
-    void forEach(F&& func) const {
-        Impl::VisitorWrapper<F> visitor(std::forward<F>(func));
-        std::for_each(_impl->getItems().begin(), _impl->getItems().end(), visitor);
+    void forEach(F & func) const {
+        for (auto const & item : _impl->getItems()) {
+            std::visit(func, item);
+        }
+    }
+    template <typename F>
+    void forEach(F const & func) const {
+        for (auto const & item : _impl->getItems()) {
+            std::visit(func, item);
+        }
     }
+    //@}
 
     //@{
     /**

include/lsst/afw/table/SchemaMapper.h

@@ -132,6 +132,7 @@ class SchemaMapper final {
     template <typename T>
     Key<T> getMapping(Key<T> const& inputKey) const;
 
+    //@{
     /**
      *  Call the given functor for each key pair in the mapper.
      *
@@ -146,10 +147,18 @@ class SchemaMapper final {
      *  The order of iteration is the same as the order in which mappings were added.
      */
     template <typename F>
-    void forEach(F&& func) const {
-        Impl::VisitorWrapper<F> visitor(std::forward<F>(func));
-        std::for_each(_impl->_map.begin(), _impl->_map.end(), visitor);
+    void forEach(F& func) const {
+        for (auto const & item : _impl->_map) {
+            std::visit([&func](auto const & pair) { func(pair.first, pair.second); }, item);
+        }
+    }
+    template <typename F>
+    void forEach(F const& func) const {
+        for (auto const & item : _impl->_map) {
+            std::visit([&func](auto const & pair) { func(pair.first, pair.second); }, item);
+        }
     }
+    //@}
 
     /// Construct an empty mapper; useless unless you assign a fully-constructed one to it.
     explicit SchemaMapper();