Merge topic 'scoped-enum-cell-classification'

5d90102f1 Remove deprecated use of vtkm::CellClassification
ccef4d2db Fix scoping of vtkm::CellClassification

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !2737

docs/changelog/scoped-enums.md

@@ -0,0 +1,12 @@
+# Favor scoped `enum`s
+
+Several `enum` declarations were changed from a standard `enum` to a
+"scoped" `enum` (i.e. `enum struct`). The advantage of a scoped enum is
+that they provide better type safety because they won't be converted
+willy-nilly to other types. They also prevent the names they define from
+being accessible on the inner scope.
+
+There are some cases where you do want the `enum` to convert to other types
+(but still want the scope of the symbols to be contained in the `enum`
+type). In this case, we worked around the problem by placing an unscoped
+`enum` inside of a standard `struct`.

vtkm/CellClassification.h

@@ -10,22 +10,70 @@
 #ifndef vtk_m_CellClassification_h
 #define vtk_m_CellClassification_h
 
+#include <vtkm/Deprecated.h>
 #include <vtkm/Types.h>
 
 namespace vtkm
 {
 
-enum CellClassification : vtkm::UInt8
+/// \brief Bit flags used in ghost arrays to identify what type a cell is.
+///
+/// `CellClassification` contains several bit flags that determine whether a cell is
+/// normal or if it should be treated as duplicated or removed in some way. The flags
+/// can be (and should be) treated as `vtkm::UInt8` and or-ed together.
+///
+class CellClassification
 {
-  NORMAL = 0,       //Valid cell
-  GHOST = 1 << 0,   //Ghost cell
-  INVALID = 1 << 1, //Cell is invalid
-  UNUSED0 = 1 << 2,
-  BLANKED = 1 << 3, //Blanked cell in AMR
-  UNUSED3 = 1 << 4,
-  UNUSED4 = 1 << 5,
-  UNUSED5 = 1 << 6,
+  // Implementation note: An enum would be a natural representation for these flags.
+  // However, a non-scoped enum leaks the names into the broader namespace and a
+  // scoped enum is too difficult to convert to the `vtkm::UInt8` we really want to
+  // treat it as. Thus, use constexpr to define the `vtkm::UInt8`s.
+  vtkm::UInt8 Flags;
+
+public:
+  // Use an unscoped enum here, where it will be properly scoped in the class.
+  // Using unscoped enums in this way is sort of obsolete, but prior to C++17
+  // a `static constexpr` may require a definition in a .cxx file, and that is
+  // not really possible for this particular class (which could be used on a GPU).
+  enum : vtkm::UInt8
+  {
+    Normal = 0,       //Valid cell
+    Ghost = 1 << 0,   //Ghost cell
+    Invalid = 1 << 1, //Cell is invalid
+    Unused0 = 1 << 2,
+    Blanked = 1 << 3, //Blanked cell in AMR
+    Unused3 = 1 << 4,
+    Unused4 = 1 << 5,
+    Unused5 = 1 << 6,
+
+    NORMAL VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Normal") = Normal,
+    GHOST VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Ghost") = Ghost,
+    INVALID VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Invalid") = Invalid,
+    UNUSED0 VTKM_DEPRECATED(1.8) = Unused0,
+    BLANKED VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Blanked") = Blanked,
+    UNUSED3 VTKM_DEPRECATED(1.8) = Unused3,
+    UNUSED4 VTKM_DEPRECATED(1.8) = Unused4,
+    UNUSED5 VTKM_DEPRECATED(1.8) = Unused5,
+  };
+
+  VTKM_EXEC constexpr CellClassification(vtkm::UInt8 flags = vtkm::UInt8{ Normal })
+    : Flags(flags)
+  {
+  }
+
+  VTKM_EXEC constexpr operator vtkm::UInt8() const { return this->Flags; }
 };
-}
+
+// Deprecated scoping.
+VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Normal.")
+constexpr vtkm::CellClassification NORMAL = vtkm::CellClassification::Normal;
+VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Ghost.")
+constexpr vtkm::CellClassification GHOST = vtkm::CellClassification::Ghost;
+VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Invalid.")
+constexpr vtkm::CellClassification INVALID = vtkm::CellClassification::Invalid;
+VTKM_DEPRECATED(1.8, "Use vtkm::CellClassification::Blanked.")
+constexpr vtkm::CellClassification BLANKED = vtkm::CellClassification::Blanked;
+
+} // namespace vtkm
 
 #endif // vtk_m_CellClassification_h

vtkm/filter/AmrArrays.hxx

@@ -54,7 +54,7 @@ struct GenerateGhostTypeWorklet : vtkm::worklet::WorkletVisitCellsWithPoints
         (Dim == 3 && boundsIntersection.Volume() > 0.5 * boundsCell.Volume()))
     {
       //      std::cout<<boundsCell<<" is (partly) contained in "<<BoundsChild<<" "<<boundsIntersection<<" "<<boundsIntersection.Area()<<std::endl;
-      ghostArray = ghostArray + vtkm::CellClassification::BLANKED;
+      ghostArray = ghostArray + vtkm::CellClassification::Blanked;
     }
   }
 

vtkm/filter/entity_extraction/testing/UnitTestGhostCellRemove.cxx

@@ -29,8 +29,8 @@ vtkm::cont::ArrayHandle<vtkm::UInt8> StructuredGhostCellArray(vtkm::Id nx,
   if (nz > 0)
     numCells *= nz;
 
-  constexpr vtkm::UInt8 normalCell = vtkm::CellClassification::NORMAL;
-  constexpr vtkm::UInt8 duplicateCell = vtkm::CellClassification::GHOST;
+  constexpr vtkm::UInt8 normalCell = vtkm::CellClassification::Normal;
+  constexpr vtkm::UInt8 duplicateCell = vtkm::CellClassification::Ghost;
 
   vtkm::cont::ArrayHandle<vtkm::UInt8> ghosts;
   ghosts.Allocate(numCells);
@@ -252,7 +252,7 @@ void TestGhostCellRemove()
           if (rt == "all")
             ghostCellRemoval.RemoveAllGhost();
           else if (rt == "byType")
-            ghostCellRemoval.RemoveByType(vtkm::CellClassification::GHOST);
+            ghostCellRemoval.RemoveByType(vtkm::CellClassification::Ghost);
 
           auto output = ghostCellRemoval.Execute(ds);
           vtkm::Id numCells = output.GetNumberOfCells();

vtkm/filter/mesh_info/GhostCellClassify.cxx

@@ -35,7 +35,7 @@ public:
   VTKM_EXEC void operator()(const vtkm::exec::BoundaryState& boundary, vtkm::UInt8& value) const
   {
     const bool notOnBoundary = boundary.IsRadiusInXBoundary(this->NumLayers);
-    value = (notOnBoundary) ? vtkm::CellClassification::NORMAL : vtkm::CellClassification::GHOST;
+    value = (notOnBoundary) ? vtkm::CellClassification::Normal : vtkm::CellClassification::Ghost;
   }
 
 private:
@@ -57,7 +57,7 @@ public:
   {
     const bool notOnBoundary = boundary.IsRadiusInXBoundary(this->NumLayers) &&
       boundary.IsRadiusInYBoundary(this->NumLayers);
-    value = (notOnBoundary) ? vtkm::CellClassification::NORMAL : vtkm::CellClassification::GHOST;
+    value = (notOnBoundary) ? vtkm::CellClassification::Normal : vtkm::CellClassification::Ghost;
   }
 
 private:
@@ -78,7 +78,7 @@ public:
   VTKM_EXEC void operator()(const vtkm::exec::BoundaryState& boundary, vtkm::UInt8& value) const
   {
     const bool notOnBoundary = boundary.IsRadiusInBoundary(this->NumLayers);
-    value = (notOnBoundary) ? vtkm::CellClassification::NORMAL : vtkm::CellClassification::GHOST;
+    value = (notOnBoundary) ? vtkm::CellClassification::Normal : vtkm::CellClassification::Ghost;
   }
 
 private:

vtkm/filter/mesh_info/testing/UnitTestGhostCellClassify.cxx

@@ -116,7 +116,7 @@ void TestStructured()
 
         vtkm::Id numNormalCells = 0;
         auto portal = ghostArray.ReadPortal();
-        constexpr vtkm::UInt8 normalCell = vtkm::CellClassification::NORMAL;
+        constexpr vtkm::UInt8 normalCell = vtkm::CellClassification::Normal;
         for (vtkm::Id i = 0; i < numCells; i++)
           if (portal.Get(i) == normalCell)
             numNormalCells++;

vtkm/filter/testing/UnitTestStreamlineFilter.cxx

@@ -187,9 +187,9 @@ void TestAMRStreamline(bool useSL)
         {
           //Mark the inner cell as ghost.
           if (i == 4 && j == 4 && k == 4)
-            ghosts[idx] = vtkm::CellClassification::GHOST;
+            ghosts[idx] = vtkm::CellClassification::Ghost;
           else
-            ghosts[idx] = vtkm::CellClassification::NORMAL;
+            ghosts[idx] = vtkm::CellClassification::Normal;
           idx++;
         }
     dsOuter.AddCellField("vtkmGhostCells", ghosts);

vtkm/filter/testing/UnitTestStreamlineFilterMPI.cxx

@@ -106,9 +106,9 @@ void TestAMRStreamline(FilterType fType, bool useThreaded)
         {
           //Mark the inner cell as ghost.
           if (i == 4 && j == 4 && k == 4)
-            ghosts[idx] = vtkm::CellClassification::GHOST;
+            ghosts[idx] = vtkm::CellClassification::Ghost;
           else
-            ghosts[idx] = vtkm::CellClassification::NORMAL;
+            ghosts[idx] = vtkm::CellClassification::Normal;
           idx++;
         }
     dsOuter.AddCellField("vtkmGhostCells", ghosts);

vtkm/worklet/particleadvection/GridEvaluators.h

@@ -146,7 +146,7 @@ private:
   VTKM_EXEC bool InGhostCell(const vtkm::Id& cellId) const
   {
     if (this->HaveGhostCells && cellId != -1)
-      return GhostCells.Get(cellId) == vtkm::CellClassification::GHOST;
+      return GhostCells.Get(cellId) == vtkm::CellClassification::Ghost;
 
     return false;
   }