Merge topic 'refactor-194-dataset-addfield-replace-dup'

3b55797d4 replaces Dataset fields vector to a map

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2058

vtkm/cont/DataSet.cxx

@@ -44,13 +44,17 @@ void DataSet::CopyStructure(const vtkm::cont::DataSet& source)
 const vtkm::cont::Field& DataSet::GetField(vtkm::Id index) const
 {
   VTKM_ASSERT((index >= 0) && (index < this->GetNumberOfFields()));
-  return this->Fields[static_cast<std::size_t>(index)];
+  auto it = this->Fields.cbegin();
+  std::advance(it, index);
+  return it->second;
 }
 
 vtkm::cont::Field& DataSet::GetField(vtkm::Id index)
 {
   VTKM_ASSERT((index >= 0) && (index < this->GetNumberOfFields()));
-  return this->Fields[static_cast<std::size_t>(index)];
+  auto it = this->Fields.begin();
+  std::advance(it, index);
+  return it->second;
 }
 
 vtkm::Id DataSet::GetFieldIndex(const std::string& name, vtkm::cont::Field::Association assoc) const
@@ -150,19 +154,22 @@ vtkm::Id DataSet::FindFieldIndex(const std::string& name,
                                  vtkm::cont::Field::Association association,
                                  bool& found) const
 {
-  for (std::size_t index = 0; index < this->Fields.size(); ++index)
+  const auto it = this->Fields.find({ name, association });
+  if (it != this->Fields.end())
   {
-    if ((association == vtkm::cont::Field::Association::ANY ||
-         association == this->Fields[index].GetAssociation()) &&
-        this->Fields[index].GetName() == name)
-    {
-      found = true;
-      return static_cast<vtkm::Id>(index);
-    }
+    found = true;
+    return static_cast<vtkm::Id>(std::distance(this->Fields.begin(), it));
   }
+
   found = false;
   return -1;
 }
 
+VTKM_CONT void DataSet::AddField(const Field& field)
+{
+  // map::insert will not replace the duplicated element
+  this->Fields[{ field.GetName(), field.GetAssociation() }] = field;
+}
+
 } // namespace cont
 } // namespace vtkm

vtkm/cont/DataSet.h

@@ -39,7 +39,7 @@ public:
   /// to have the same number of points.
   VTKM_CONT vtkm::Id GetNumberOfPoints() const;
 
-  VTKM_CONT void AddField(const Field& field) { this->Fields.push_back(field); }
+  VTKM_CONT void AddField(const Field& field);
 
   VTKM_CONT
   const vtkm::cont::Field& GetField(vtkm::Id index) const;
@@ -73,14 +73,14 @@ public:
   }
 
 
-  /// Returns the first field that matches the provided name and association
+  /// Returns the field that matches the provided name and association
   /// Will return -1 if no match is found
   VTKM_CONT
   vtkm::Id GetFieldIndex(
     const std::string& name,
     vtkm::cont::Field::Association assoc = vtkm::cont::Field::Association::ANY) const;
 
-  /// Returns the first field that matches the provided name and association
+  /// Returns the field that matches the provided name and association
   /// Will throw an exception if no match is found
   //@{
   VTKM_CONT
@@ -203,8 +203,23 @@ public:
   void PrintSummary(std::ostream& out) const;
 
 private:
+  struct FieldCompare
+  {
+    using Key = std::pair<std::string, vtkm::cont::Field::Association>;
+
+    template <typename T>
+    bool operator()(const T& a, const T& b) const
+    {
+      if (a.first == b.first)
+        return a.second < b.second && a.second != vtkm::cont::Field::Association::ANY &&
+          b.second != vtkm::cont::Field::Association::ANY;
+
+      return a.first < b.first;
+    }
+  };
+
   std::vector<vtkm::cont::CoordinateSystem> CoordSystems;
-  std::vector<vtkm::cont::Field> Fields;
+  std::map<FieldCompare::Key, vtkm::cont::Field, FieldCompare> Fields;
   vtkm::cont::DynamicCellSet CellSet;
 
   VTKM_CONT

vtkm/cont/testing/UnitTestMoveConstructors.cxx

@@ -40,6 +40,26 @@ void is_noexcept_movable()
   VTKM_TEST_ASSERT(valid, msg);
 }
 
+// DataSet uses a map which is not nothrow constructible/assignable in the
+// following implementations
+template<>
+void is_noexcept_movable<vtkm::cont::DataSet>()
+{
+  using T = vtkm::cont::DataSet;
+
+  constexpr bool valid =
+#if ((defined(__GNUC__) && (__GNUC__ <= 5)) || defined(_WIN32))
+    std::is_move_constructible<T>::value &&
+    std::is_move_assignable<T>::value;
+#else
+    std::is_nothrow_move_constructible<T>::value &&
+    std::is_nothrow_move_assignable<T>::value;
+#endif
+
+  std::string msg = typeid(T).name() + std::string(" should be noexcept moveable");
+  VTKM_TEST_ASSERT(valid, msg);
+}
+
 template<typename T>
 void is_triv_noexcept_movable()
 {