From 3c1afe53dca2221dfc0c205d93a7b59765b48dee Mon Sep 17 00:00:00 2001
From: Kenneth Moreland <morelandkd@ornl.gov>
Date: Wed, 19 Apr 2023 11:14:05 -0600
Subject: [PATCH] Fix new instances of ArrayHandleRuntimeVec in
 UnknownArrayHandle

`UnknownArrayHandle` is supposed to treat `ArrayHandleRuntimeVec` the
same as `ArrayHandleBasic`. However, the `NewInstance` methods were
failing because they need custom handling of the vec size.
---
 .../unknown-runtime-vec-new-instance.md       |  8 ++++
 vtkm/cont/UnknownArrayHandle.cxx              | 37 +++++++++++++++++++
 .../testing/UnitTestUnknownArrayHandle.cxx    | 22 +++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 docs/changelog/unknown-runtime-vec-new-instance.md

diff --git a/docs/changelog/unknown-runtime-vec-new-instance.md b/docs/changelog/unknown-runtime-vec-new-instance.md
new file mode 100644
index 000000000..5a13b1567
--- /dev/null
+++ b/docs/changelog/unknown-runtime-vec-new-instance.md
@@ -0,0 +1,8 @@
+# Fix new instances of ArrayHandleRuntimeVec in UnknownArrayHandle
+
+`UnknownArrayHandle` is supposed to treat `ArrayHandleRuntimeVec` the same
+as `ArrayHandleBasic`. However, the `NewInstance` methods were failing
+because they need custom handling of the vec size. Special cases in the
+`UnknownArrayHandle::NewInstance*()` methods have been added to fix this
+problem.
+
diff --git a/vtkm/cont/UnknownArrayHandle.cxx b/vtkm/cont/UnknownArrayHandle.cxx
index 1c5f1f4ab..21037bc99 100644
--- a/vtkm/cont/UnknownArrayHandle.cxx
+++ b/vtkm/cont/UnknownArrayHandle.cxx
@@ -177,6 +177,16 @@ VTKM_CONT bool UnknownArrayHandle::IsValid() const
 
 VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstance() const
 {
+  if (this->IsStorageType<vtkm::cont::StorageTagRuntimeVec>())
+  {
+    // Special case for `ArrayHandleRuntimeVec`, which (1) can be used in place of
+    // a basic array in `UnknownArrayHandle` and (2) needs a special construction to
+    // capture the correct number of components. Also note that we are allowing this
+    // special case to be implemented in `NewInstanceBasic` because it has a better
+    // fallback (throw an exception rather than create a potentially incompatible
+    // with the wrong number of components).
+    return this->NewInstanceBasic();
+  }
   UnknownArrayHandle newArray;
   if (this->Container)
   {
@@ -188,6 +198,25 @@ VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstance() const
 VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstanceBasic() const
 {
   UnknownArrayHandle newArray;
+  if (this->IsStorageType<vtkm::cont::StorageTagRuntimeVec>())
+  {
+    // Special case for `ArrayHandleRuntimeVec`, which (1) can be used in place of
+    // a basic array in `UnknownArrayHandle` and (2) needs a special construction to
+    // capture the correct number of components.
+    auto runtimeVecArrayCreator = [&](auto exampleComponent) {
+      using ComponentType = decltype(exampleComponent);
+      if (this->IsBaseComponentType<ComponentType>())
+      {
+        newArray =
+          vtkm::cont::make_ArrayHandleRuntimeVec<ComponentType>(this->GetNumberOfComponentsFlat());
+      }
+    };
+    vtkm::ListForEach(runtimeVecArrayCreator, vtkm::TypeListBaseC{});
+    if (newArray.IsValid())
+    {
+      return newArray;
+    }
+  }
   if (this->Container)
   {
     newArray.Container = this->Container->NewInstanceBasic();
@@ -197,6 +226,14 @@ VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstanceBasic() const
 
 VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstanceFloatBasic() const
 {
+  if (this->IsStorageType<vtkm::cont::StorageTagRuntimeVec>())
+  {
+    // Special case for `ArrayHandleRuntimeVec`, which (1) can be used in place of
+    // a basic array in `UnknownArrayHandle` and (2) needs a special construction to
+    // capture the correct number of components.
+    return vtkm::cont::make_ArrayHandleRuntimeVec<vtkm::FloatDefault>(
+      this->GetNumberOfComponentsFlat());
+  }
   UnknownArrayHandle newArray;
   if (this->Container)
   {
diff --git a/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx b/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx
index 036487fd5..b8ae60a62 100644
--- a/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx
+++ b/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx
@@ -11,6 +11,7 @@
 #include <vtkm/cont/UncertainArrayHandle.h>
 #include <vtkm/cont/UnknownArrayHandle.h>
 
+#include <vtkm/cont/ArrayCopy.h>
 #include <vtkm/cont/ArrayHandleCast.h>
 #include <vtkm/cont/ArrayHandleConstant.h>
 #include <vtkm/cont/ArrayHandleCounting.h>
@@ -574,6 +575,27 @@ void TryConvertRuntimeVec()
   VTKM_TEST_ASSERT(unknownWithRuntimeVec.CanConvert<BasicArrayType>());
   BasicArrayType outputArray = unknownWithRuntimeVec.AsArrayHandle<BasicArrayType>();
   VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, outputArray));
+
+  std::cout << "    Copy ArrayHandleRuntimeVec to a new instance" << std::endl;
+  vtkm::cont::UnknownArrayHandle unknownCopy = unknownWithRuntimeVec.NewInstance();
+  VTKM_TEST_ASSERT(unknownWithRuntimeVec.GetNumberOfComponentsFlat() ==
+                   unknownCopy.GetNumberOfComponentsFlat());
+  vtkm::cont::ArrayCopy(unknownWithRuntimeVec, unknownCopy);
+  VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, unknownCopy));
+
+  std::cout << "    Copy ArrayHandleRuntimeVec as basic array" << std::endl;
+  unknownCopy = unknownWithRuntimeVec.NewInstanceBasic();
+  VTKM_TEST_ASSERT(unknownWithRuntimeVec.GetNumberOfComponentsFlat() ==
+                   unknownCopy.GetNumberOfComponentsFlat());
+  vtkm::cont::ArrayCopy(unknownWithRuntimeVec, unknownCopy);
+  VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, unknownCopy));
+
+  std::cout << "    Copy ArrayHandleRuntimeVec to float array" << std::endl;
+  unknownCopy = unknownWithRuntimeVec.NewInstanceFloatBasic();
+  VTKM_TEST_ASSERT(unknownWithRuntimeVec.GetNumberOfComponentsFlat() ==
+                   unknownCopy.GetNumberOfComponentsFlat());
+  vtkm::cont::ArrayCopy(unknownWithRuntimeVec, unknownCopy);
+  VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, unknownCopy));
 }
 
 void TryConvertRuntimeVec()