Merge pull request #1385 from boutproject/field-move-ctors

Field* constructor tidy

include/bout/array.hxx

@@ -77,7 +77,7 @@ public:
    * a.empty(); // True
    * 
    */
-  Array() : ptr(nullptr) {}
+  Array() noexcept : ptr(nullptr) {}
 
   /*!
    * Create an array of given length
@@ -89,14 +89,14 @@ public:
   /*!
    * Destructor. Releases the underlying ArrayData
    */
-  ~Array() {
+  ~Array() noexcept {
     release(ptr);
   }
 
   /*!
    * Copy constructor
    */
-  Array(const Array &other) {
+  Array(const Array &other) noexcept {
     ptr = other.ptr; 
   }
 
@@ -106,15 +106,15 @@ public:
    *
    * Uses copy-and-swap idiom
    */
-  Array& operator=(Array other) {
+  Array& operator=(Array other) noexcept {
     swap(*this, other);
     return *this;
   }
 
   /*!
    * Move constructor
    */
-  Array(Array&& other) {
+  Array(Array&& other) noexcept {
     swap(*this, other);
   }
 
@@ -127,7 +127,7 @@ public:
    * but can be set to false by passing "false" as input.
    * Once set to false it can't be changed back to true.
    */
-  static bool useStore( bool keep_using = true ) {
+  static bool useStore( bool keep_using = true ) noexcept {
     static bool value = true;
     if (keep_using) {
       return value; 
@@ -141,7 +141,7 @@ public:
    * Release data. After this the Array is empty and any data access
    * will be invalid
    */
-  void clear() {
+  void clear() noexcept {
     release(ptr);
   }
 
@@ -161,17 +161,20 @@ public:
   /*!
    * Returns true if the Array is empty
    */
-  bool empty() const {
+  bool empty() const noexcept {
     return ptr == nullptr;
   }
 
   /*!
    * Return size of the array. Zero if the array is empty.
    */
-  int size() const {
+  int size() const noexcept {
     if(!ptr)
       return 0;
 #ifdef BOUT_ARRAY_WITH_VALARRAY
+    // Note: std::valarray::size is technically not noexcept, so
+    // Array::size shouldn't be either if we're using valarrays -- in
+    // practice, it is so this shouldn't matter
     return ptr->size();
 #else
     return ptr->len;
@@ -182,7 +185,7 @@ public:
    * Returns true if the data is unique to this Array.
    * 
    */
-  bool unique() const {
+  bool unique() const noexcept {
     return ptr.use_count() == 1;
   }
 
@@ -216,20 +219,20 @@ public:
   typedef T* iterator;
   typedef const T* const_iterator;
 #ifndef BOUT_ARRAY_WITH_VALARRAY
-  iterator begin() { 
+  iterator begin() noexcept {
     return (ptr) ? ptr->data : nullptr;
   }
 
-  iterator end() {
+  iterator end() noexcept {
     return (ptr) ? ptr->data + ptr->len : nullptr;
   }
 
   // Const iterators  
-  const_iterator begin() const {
+  const_iterator begin() const noexcept {
     return (ptr) ? ptr->data : nullptr;
   }
 
-  const_iterator end() const {
+  const_iterator end() const noexcept {
     return (ptr) ? ptr->data + ptr->len : nullptr;
   }
 #else
@@ -279,7 +282,7 @@ public:
    * Exchange contents with another Array of the same type.
    * Sizes of the arrays may differ.
    */
-  friend void swap(Array<T> &first, Array<T> &second) {
+  friend void swap(Array<T> &first, Array<T> &second) noexcept {
     using std::swap;
     swap(first.ptr, second.ptr);
   }
@@ -391,40 +394,48 @@ private:
       p = st.back();
       st.pop_back();
     } else {
+      // Ensure that when we release the data block later we'll have
+      // enough space to put it in the store so that `release` can be
+      // noexcept
+      st.reserve(1);
       p = std::make_shared<dataBlock>(len);
     }
 
     return p;
   }
-  
+
   /*!
-   * Release an ArrayData object, reducing its reference count by one. 
+   * Release an ArrayData object, reducing its reference count by one.
    * If no more references, then put back into the store.
    * It's important to pass a reference to the pointer, otherwise we get
    * a copy of the shared_ptr, which therefore increases the use count
    * and doesn't allow us to free the pass pointer directly
+   *
+   * Note that this is noexcept only because we've ensure that both a)
+   * store()[<size>] already exists, and b) it has space for at least
+   * one data block. Of course, store() could throw -- in which case
+   * we're doomed anyway, so the only thing we can do is abort
    */
-  void release(dataPtrType &d) {
+  void release(dataPtrType& d) noexcept {
     if (!d)
       return;
-    
+
     // Reduce reference count, and if zero return to store
-    if(d.use_count()==1) {
+    if (d.use_count() == 1) {
       if (useStore()) {
-	// Put back into store
+        // Put back into store
 #ifdef BOUT_ARRAY_WITH_VALARRAY
-	store()[d->size()].push_back(std::move(d));
-#else	  
-	store()[d->len   ].push_back(std::move(d));
+        store()[d->size()].push_back(std::move(d));
+#else
+        store()[d->len].push_back(std::move(d));
 #endif
-	//Could return here but seems to slow things down a lot
+        // Could return here but seems to slow things down a lot
       }
     }
 
-    //Finish by setting pointer to nullptr if not putting on store
-    d=nullptr;
+    // Finish by setting pointer to nullptr if not putting on store
+    d = nullptr;
   }
-
 };
 
 /*!

include/field.hxx

@@ -53,7 +53,7 @@ extern Mesh * mesh; ///< Global mesh
  */
 class Field {
  public:
-  Field();
+  Field() = default;
   Field(Mesh * localmesh);
   virtual ~Field() { }
 
@@ -82,9 +82,10 @@ class Field {
     return true;
   }
 
-  // Status of the 4 boundaries
-  bool bndry_xin, bndry_xout, bndry_yup, bndry_ydown;
+  /// Status of the 4 boundaries
+  bool bndry_xin{true}, bndry_xout{true}, bndry_yup{true}, bndry_ydown{true};
 #endif
+
   virtual Mesh * getMesh() const{
     if (fieldmesh){
       return fieldmesh;
@@ -115,9 +116,9 @@ class Field {
    */
   virtual int getNz() const;
 
- protected:
-  Mesh * fieldmesh;
-  mutable Coordinates * fieldCoordinates = nullptr;
+protected:
+  Mesh* fieldmesh{nullptr};
+  mutable Coordinates* fieldCoordinates{nullptr};
 };
 
 /// Unary + operator. This doesn't do anything

include/field2d.hxx

@@ -74,7 +74,7 @@ class Field2D : public Field, public FieldData {
   /*!
    * Move constructor
    */
-  Field2D(Field2D&& f) = default;
+  Field2D(Field2D&& f) noexcept { swap(*this, f); };
 
   /*!
    * Constructor. This creates a Field2D using the global Mesh pointer (mesh)
@@ -235,16 +235,35 @@ class Field2D : public Field, public FieldData {
   void applyBoundary(const std::string &region, const std::string &condition);
   void applyTDerivBoundary() override;
   void setBoundaryTo(const Field2D &f2d); ///< Copy the boundary region
-
- private:
-  int nx, ny;      ///< Array sizes (from fieldmesh). These are valid only if fieldmesh is not null
-
+
+  friend void swap(Field2D& first, Field2D& second) noexcept {
+    using std::swap;
+    swap(first.data, second.data);
+    swap(first.fieldmesh, second.fieldmesh);
+    swap(first.fieldCoordinates, second.fieldCoordinates);
+    swap(first.nx, second.nx);
+    swap(first.ny, second.ny);
+    swap(first.location, second.location);
+    swap(first.deriv, second.deriv);
+    swap(first.bndry_op, second.bndry_op);
+    swap(first.boundaryIsCopy, second.boundaryIsCopy);
+    swap(first.boundaryIsSet, second.boundaryIsSet);
+    swap(first.bndry_op_par, second.bndry_op_par);
+    swap(first.bndry_generator, second.bndry_generator);
+  }
+
+private:
+  /// Array sizes (from fieldmesh). These are valid only if fieldmesh is not null
+  int nx{-1}, ny{-1};
+
   /// Internal data array. Handles allocation/freeing of memory
   Array<BoutReal> data;
 
-  CELL_LOC location = CELL_CENTRE; ///< Location of the variable in the cell
+  /// Location of the variable in the cell
+  CELL_LOC location{CELL_CENTRE};
 
-  Field2D *deriv; ///< Time-derivative, can be NULL
+  /// Time-derivative, can be nullptr
+  Field2D *deriv{nullptr};
 };
 
 // Non-member overloaded operators

include/field3d.hxx

@@ -171,7 +171,9 @@ class Field3D : public Field, public FieldData {
    * Copy constructor
    */
   Field3D(const Field3D& f);
-
+
+  /// Move constructor
+  Field3D(Field3D&& f) noexcept { swap(*this, f); }
   /// Constructor from 2D field
   Field3D(const Field2D& f);
   /// Constructor from value
@@ -443,22 +445,45 @@ class Field3D : public Field, public FieldData {
   void applyParallelBoundary(const char* condition) { applyParallelBoundary(std::string(condition)); }
   void applyParallelBoundary(const std::string &region, const std::string &condition);
   void applyParallelBoundary(const std::string &region, const std::string &condition, Field3D *f);
+
+  friend void swap(Field3D& first, Field3D& second) noexcept {
+    using std::swap;
+    swap(first.data, second.data);
+    swap(first.fieldmesh, second.fieldmesh);
+    swap(first.fieldCoordinates, second.fieldCoordinates);
+    swap(first.background, second.background);
+    swap(first.nx, second.nx);
+    swap(first.ny, second.ny);
+    swap(first.nz, second.nz);
+    swap(first.location, second.location);
+    swap(first.deriv, second.deriv);
+    swap(first.yup_field, second.yup_field);
+    swap(first.ydown_field, second.ydown_field);
+    swap(first.bndry_op, second.bndry_op);
+    swap(first.boundaryIsCopy, second.boundaryIsCopy);
+    swap(first.boundaryIsSet, second.boundaryIsSet);
+    swap(first.bndry_op_par, second.bndry_op_par);
+    swap(first.bndry_generator, second.bndry_generator);
+  }
 
 private:
   /// Boundary - add a 2D field
-  const Field2D *background;
+  const Field2D *background{nullptr};
+
+  /// Array sizes (from fieldmesh). These are valid only if fieldmesh is not null
+  int nx{-1}, ny{-1}, nz{-1};
 
-  int nx, ny, nz;  ///< Array sizes (from fieldmesh). These are valid only if fieldmesh is not null
-
   /// Internal data array. Handles allocation/freeing of memory
   Array<BoutReal> data;
 
-  CELL_LOC location = CELL_CENTRE; ///< Location of the variable in the cell
+  /// Location of the variable in the cell
+  CELL_LOC location{CELL_CENTRE};
 
-  Field3D *deriv; ///< Time derivative (may be NULL)
+  /// Time derivative (may be nullptr)
+  Field3D *deriv{nullptr};
 
   /// Pointers to fields containing values along Y
-  Field3D *yup_field, *ydown_field;
+  Field3D *yup_field{nullptr}, *ydown_field{nullptr};
 };
 
 // Non-member overloaded operators

include/field_data.hxx

@@ -57,7 +57,7 @@ class FieldVisitor;
 */
 class FieldData {
 public:
-  FieldData();
+  FieldData() = default;
   virtual ~FieldData();
 
   // Visitor pattern support
@@ -88,8 +88,9 @@ public:
 
 protected:
   std::vector<BoundaryOp *> bndry_op; ///< Boundary conditions
-  bool boundaryIsCopy;           ///< True if bndry_op is a copy
-  bool boundaryIsSet;            ///< Set to true when setBoundary called
+  bool boundaryIsCopy{false};         ///< True if bndry_op is a copy
+  bool boundaryIsSet{false};          ///< Set to true when setBoundary called
+
   // Parallel boundaries
   std::vector<BoundaryOpPar *> bndry_op_par; ///< Boundary conditions
 

include/fieldperp.hxx

@@ -242,11 +242,12 @@ class FieldPerp : public Field {
    */
   int getNz() const override {return nz;};
 
- private:
-  int yindex = -1; ///< The Y index at which this FieldPerp is defined
+private:
+  /// The Y index at which this FieldPerp is defined
+  int yindex{-1};
 
   /// The size of the data array
-  int nx, nz;
+  int nx{-1}, nz{-1};
 
   /// The underlying data array
   Array<BoutReal> data;