Merge topic 'write-to-array-handle-cast'

6592e528 Fix IsWritableArrayHandle for portals that exist but cannot be written
0e15a111 Enable writing to ArrayHandleCast
6d37ce94

 Remove invalid PortalType

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

docs/changelog/write-to-array-handle-cast.md

+# Enable writing to ArrayHandleCast
+
+Previously, `ArrayHandleCast` was considered a read-only array handle.
+However, it is trivial to reverse the cast (now that `ArrayHandleTransform`
+supports an inverse transform). So now you can write to a cast array
+(assuming the underlying array is writable).
+
+One trivial consequence of this change is that you can no longer make a
+cast that cannot be reversed. For example, it was possible to cast a simple
+scalar to a `Vec` even though it is not possible to convert a `Vec` to a
+scalar value. This was of dubious correctness (it is more of a construction
+than a cast) and is easy to recreate with `ArrayHandleTransform`.

vtkm/cont/ArrayHandle.h

@@ -82,19 +82,21 @@ struct IsInValidArrayHandle
 /// Both of these have a typedef named value with the respective boolean value.
 ///
 template <typename ArrayHandle>
-struct IsWriteableArrayHandle
+struct IsWritableArrayHandle
 {
 private:
-  using ValueType = typename ArrayHandle::PortalControl::ValueType;
+  template <typename U,
+            typename S = decltype(std::declval<U>().Set(vtkm::Id{},
+                                                        std::declval<typename U::ValueType>()))>
+  static std::true_type hasSet(int);
+  template <typename U>
+  static std::false_type hasSet(...);
 
-  //All ArrayHandles that use ImplicitStorage as the final writable location
-  //will have a value type of void*, which is what we are trying to detect
-  using RawValueType = typename std::remove_pointer<ValueType>::type;
-  using IsVoidType = std::is_void<RawValueType>;
+  using PortalType = typename ArrayHandle::PortalControl;
 
 public:
-  using type = std::integral_constant<bool, !IsVoidType::value>;
-  static constexpr bool value = !IsVoidType::value;
+  using type = decltype(hasSet<PortalType>(0));
+  static constexpr bool value = type::value;
 };
 
 /// Checks to see if the given object is an array handle. This check is

vtkm/cont/ArrayHandleCast.h

@@ -45,14 +45,16 @@ struct VTKM_ALWAYS_EXPORT Cast
 template <typename T, typename ArrayHandleType>
 class ArrayHandleCast
   : public vtkm::cont::ArrayHandleTransform<ArrayHandleType,
-                                            internal::Cast<typename ArrayHandleType::ValueType, T>>
+                                            internal::Cast<typename ArrayHandleType::ValueType, T>,
+                                            internal::Cast<T, typename ArrayHandleType::ValueType>>
 {
 public:
   VTKM_ARRAY_HANDLE_SUBCLASS(
     ArrayHandleCast,
     (ArrayHandleCast<T, ArrayHandleType>),
     (vtkm::cont::ArrayHandleTransform<ArrayHandleType,
-                                      internal::Cast<typename ArrayHandleType::ValueType, T>>));
+                                      internal::Cast<typename ArrayHandleType::ValueType, T>,
+                                      internal::Cast<T, typename ArrayHandleType::ValueType>>));
 
   ArrayHandleCast(const ArrayHandleType& handle)
     : Superclass(handle)
@@ -172,7 +174,9 @@ struct SerializableTypeString<vtkm::cont::internal::Cast<T1, T2>>
 template <typename T, typename AH>
 struct SerializableTypeString<vtkm::cont::ArrayHandleCast<T, AH>>
   : SerializableTypeString<
-      vtkm::cont::ArrayHandleTransform<AH, vtkm::cont::internal::Cast<typename AH::ValueType, T>>>
+      vtkm::cont::ArrayHandleTransform<AH,
+                                       vtkm::cont::internal::Cast<typename AH::ValueType, T>,
+                                       vtkm::cont::internal::Cast<T, typename AH::ValueType>>>
 {
 };
 }
@@ -192,7 +196,9 @@ struct Serialization<vtkm::cont::internal::Cast<T1, T2>>
 template <typename T, typename AH>
 struct Serialization<vtkm::cont::ArrayHandleCast<T, AH>>
   : Serialization<
-      vtkm::cont::ArrayHandleTransform<AH, vtkm::cont::internal::Cast<typename AH::ValueType, T>>>
+      vtkm::cont::ArrayHandleTransform<AH,
+                                       vtkm::cont::internal::Cast<typename AH::ValueType, T>,
+                                       vtkm::cont::internal::Cast<T, typename AH::ValueType>>>
 {
 };
 

vtkm/cont/ArrayHandleTransform.h

@@ -16,6 +16,8 @@
 #include <vtkm/cont/ExecutionAndControlObjectBase.h>
 #include <vtkm/cont/RuntimeDeviceTracker.h>
 
+#include <vtkm/internal/ArrayPortalHelpers.h>
+
 #include <vtkm/cont/serial/internal/DeviceAdapterRuntimeDetectorSerial.h>
 
 namespace vtkm
@@ -148,7 +150,8 @@ public:
   VTKM_EXEC_CONT
   void Set(vtkm::Id index, const ValueType& value) const
   {
-    return this->Portal.Set(index, this->InverseFunctor(value));
+    using call_supported_t = typename vtkm::internal::PortalSupportsSets<PortalType>::type;
+    this->Set(call_supported_t(), index, value);
   }
 
   VTKM_SUPPRESS_EXEC_WARNINGS
@@ -157,6 +160,14 @@ public:
 
 private:
   InverseFunctorType InverseFunctor;
+
+  VTKM_SUPPRESS_EXEC_WARNINGS
+  VTKM_EXEC_CONT
+  inline void Set(std::true_type, vtkm::Id index, const ValueType& value) const
+  {
+    this->Portal.Set(index, this->InverseFunctor(value));
+  }
+  VTKM_EXEC_CONT inline void Set(std::false_type, vtkm::Id, const ValueType&) const {}
 };
 }
 }
@@ -274,19 +285,16 @@ class Storage<typename StorageTagTransform<ArrayHandleType, FunctorType>::ValueT
 public:
   using ValueType = typename StorageTagTransform<ArrayHandleType, FunctorType>::ValueType;
 
-  // This is meant to be invalid. Because Transform arrays are read only, you
-  // should only be able to use the const version.
-  struct PortalType
-  {
-    using ValueType = void*;
-    using IteratorType = void*;
-  };
-
   using PortalConstType =
     vtkm::exec::internal::ArrayPortalTransform<ValueType,
                                                typename ArrayHandleType::PortalConstControl,
                                                typename FunctorManager::FunctorType>;
 
+  // Note that this array is read only, so you really should only be getting the const
+  // version of the portal. If you actually try to write to this portal, you will
+  // get an error.
+  using PortalType = PortalConstType;
+
   VTKM_CONT
   Storage()
     : Valid(false)

vtkm/cont/StorageExtrude.h

@@ -12,6 +12,8 @@
 
 #include <vtkm/internal/IndicesExtrude.h>
 
+#include <vtkm/cont/ErrorBadType.h>
+
 #include <vtkm/cont/serial/DeviceAdapterSerial.h>
 #include <vtkm/cont/tbb/DeviceAdapterTBB.h>
 
@@ -97,17 +99,14 @@ class VTKM_ALWAYS_EXPORT Storage<T, internal::StorageTagExtrudePlane>
 public:
   using ValueType = T;
 
-  // This is meant to be invalid. Because point arrays are read only, you
-  // should only be able to use the const version.
-  struct PortalType
-  {
-    using ValueType = void*;
-    using IteratorType = void*;
-  };
-
   using PortalConstType =
     vtkm::exec::ArrayPortalExtrudePlane<typename HandleType::PortalConstControl>;
 
+  // Note that this array is read only, so you really should only be getting the const
+  // version of the portal. If you actually try to write to this portal, you will
+  // get an error.
+  using PortalType = PortalConstType;
+
   Storage()
     : Array()
     , NumberOfPlanes(0)
@@ -120,7 +119,11 @@ public:
   {
   }
 
-  PortalType GetPortal() { return PortalType{}; }
+  PortalType GetPortal()
+  {
+    throw vtkm::cont::ErrorBadType(
+      "Extrude ArrayHandles are read only. Cannot get writable portal.");
+  }
 
   PortalConstType GetPortalConst() const
   {
@@ -382,16 +385,13 @@ class Storage<T, internal::StorageTagExtrude>
 public:
   using ValueType = T;
 
-  // This is meant to be invalid. Because point arrays are read only, you
-  // should only be able to use the const version.
-  struct PortalType
-  {
-    using ValueType = void*;
-    using IteratorType = void*;
-  };
-
   using PortalConstType = exec::ArrayPortalExtrude<TPortalType>;
 
+  // Note that this array is read only, so you really should only be getting the const
+  // version of the portal. If you actually try to write to this portal, you will
+  // get an error.
+  using PortalType = PortalConstType;
+
   Storage()
     : Array()
     , NumberOfPlanes(0)
@@ -415,7 +415,11 @@ public:
     VTKM_ASSERT(this->Array.GetNumberOfValues() >= 0);
   }
 
-  PortalType GetPortal() { return PortalType{}; }
+  PortalType GetPortal()
+  {
+    throw vtkm::cont::ErrorBadType(
+      "Extrude ArrayHandles are read only. Cannot get writable portal.");
+  }
 
   PortalConstType GetPortalConst() const
   {

vtkm/cont/StorageImplicit.h

@@ -53,13 +53,9 @@ public:
   using ValueType = typename ArrayPortalType::ValueType;
   using PortalConstType = ArrayPortalType;
 
-  // This is meant to be invalid. Because implicit arrays are read only, you
-  // should only be able to use the const version.
-  struct PortalType
-  {
-    using ValueType = void*;
-    using IteratorType = void*;
-  };
+  // Note that this portal is likely to be read-only, so you will probably get an error
+  // if you try to write to it.
+  using PortalType = ArrayPortalType;
 
   VTKM_CONT
   Storage(const PortalConstType& portal = PortalConstType())

vtkm/cont/StorageVirtual.hxx

@@ -206,9 +206,9 @@ void StorageVirtualImpl<T, S>::ControlPortalForOutput(
   vtkm::cont::internal::TransferInfoArray& payload)
 {
   using HT = vtkm::cont::ArrayHandle<T, S>;
-  constexpr auto isWriteable = typename vtkm::cont::internal::IsWriteableArrayHandle<HT>::type{};
+  constexpr auto isWritable = typename vtkm::cont::internal::IsWritableArrayHandle<HT>::type{};
 
-  detail::make_writableHostPortal(isWriteable, payload, this->Handle);
+  detail::make_writableHostPortal(isWritable, payload, this->Handle);
 }
 
 template <typename T, typename S>

vtkm/cont/internal/ConnectivityExplicitInternals.h

@@ -60,8 +60,8 @@ void buildIndexOffsets(const ArrayHandleIndices& numIndices,
                        ArrayHandleOffsets offsets,
                        vtkm::cont::DeviceAdapterId deviceId)
 {
-  using IsWriteable = vtkm::cont::internal::IsWriteableArrayHandle<ArrayHandleOffsets>;
-  buildIndexOffsets(numIndices, offsets, deviceId, typename IsWriteable::type());
+  using IsWritable = vtkm::cont::internal::IsWritableArrayHandle<ArrayHandleOffsets>;
+  buildIndexOffsets(numIndices, offsets, deviceId, typename IsWritable::type());
 }
 
 template <typename ShapeStorageTag = VTKM_DEFAULT_STORAGE_TAG,

vtkm/cont/testing/TestingFancyArrayHandles.h

@@ -681,6 +681,37 @@ private:
     }
   };
 
+  struct TestCastAsOutput
+  {
+    template <typename CastFromType>
+    VTKM_CONT void operator()(CastFromType vtkmNotUsed(type)) const
+    {
+      using InputArrayType = vtkm::cont::ArrayHandleIndex;
+      using ResultArrayType = vtkm::cont::ArrayHandle<CastFromType>;
+
+      InputArrayType input(ARRAY_SIZE);
+
+      ResultArrayType result;
+      vtkm::cont::ArrayHandleCast<vtkm::Id, ResultArrayType> castArray =
+        vtkm::cont::make_ArrayHandleCast<CastFromType>(result);
+
+      vtkm::worklet::DispatcherMapField<PassThrough> dispatcher;
+      dispatcher.Invoke(input, castArray);
+
+      vtkm::cont::printSummary_ArrayHandle(castArray, std::cout);
+      std::cout << std::endl;
+
+      // verify results
+      vtkm::Id length = ARRAY_SIZE;
+      for (vtkm::Id i = 0; i < length; ++i)
+      {
+        VTKM_TEST_ASSERT(input.GetPortalConstControl().Get(i) ==
+                           static_cast<vtkm::Id>(result.GetPortalConstControl().Get(i)),
+                         "Casting ArrayHandle Failed");
+      }
+    }
+  };
+
   template <vtkm::IdComponent NUM_COMPONENTS>
   struct TestGroupVecAsInput
   {
@@ -1284,6 +1315,11 @@ private:
       vtkm::testing::Testing::TryTypes(
         TestingFancyArrayHandles<DeviceAdapterTag>::TestCastAsInput(), CastTypesToTest());
 
+      std::cout << "-------------------------------------------" << std::endl;
+      std::cout << "Testing ArrayHandleCast as Output" << std::endl;
+      vtkm::testing::Testing::TryTypes(
+        TestingFancyArrayHandles<DeviceAdapterTag>::TestCastAsOutput(), CastTypesToTest());
+
       std::cout << "-------------------------------------------" << std::endl;
       std::cout << "Testing ArrayHandleGroupVec<3> as Input" << std::endl;
       vtkm::testing::Testing::TryTypes(

vtkm/cont/testing/UnitTestSerializationArrayHandle.cxx

@@ -109,6 +109,15 @@ struct TestArrayHandleCast
     RunTest(array);
     RunTest(MakeTestVariantArrayHandle(array));
   }
+
+  template <typename T, vtkm::IdComponent N>
+  void operator()(vtkm::Vec<T, N>) const
+  {
+    auto array = vtkm::cont::make_ArrayHandleCast<vtkm::Vec<T, N>>(
+      RandomArrayHandle<vtkm::Vec<vtkm::Int8, N>>::Make(ArraySize));
+    RunTest(array);
+    RunTest(MakeTestVariantArrayHandle(array));
+  }
 };
 
 struct TestArrayHandleCompositeVector

vtkm/cont/testing/UnitTestStorageImplicit.cxx

@@ -56,7 +56,6 @@ struct TemplatedTests
 
   using ValueType = typename StorageType::ValueType;
   using PortalType = typename StorageType::PortalType;
-  using IteratorType = typename PortalType::IteratorType;
 
   void BasicAllocation()
   {