Fixup custom portal iterator logic.

The convenience functions `ArrayPortalToIteratorBegin()` and
`ArrayPortalToIteratorEnd()` wouldn't detect specializations of
`ArrayPortalToIterators<PortalType>` since the specializations aren't
visible when the `Begin`/`End` functions are declared.

Since the CUDA iterators rely on a specialization, the convenience
functions would not compile on CUDA.

Now, instead of specializing `ArrayPortalToIterators` to provide custom
iterators for a particular portal, the portal may advertise custom
iterators by defining `IteratorType`, `GetIteratorBegin()`, and
`GetIteratorEnd()`. `ArrayPortalToIterators` will detect such portals
and automatically switch to using the specialized portals.

This eliminates the need for the specializations to be visible to the
convenience functions and allows them to be usable on CUDA.

docs/changelog/custom_portal_iterators.md

@@ -0,0 +1,28 @@
+# Portals may advertise custom iterators
+
+The `ArrayPortalToIterator` utilities are used to produce STL-style iterators 
+from vtk-m's `ArrayHandle` portals. By default, a facade class is constructed 
+around the portal API, adapting it to an iterator interface. 
+
+However, some portals use iterators internally, or may be able to construct a
+lightweight iterator easily. For these, it is preferable to directly use the
+specialized iterators instead of going through the generic facade. A portal may
+now declare the following optional API to advertise that it has custom 
+iterators:
+
+```
+struct MyPortal
+{
+  using IteratorType = ...; // alias to the portal's specialized iterator type
+  IteratorType GetIteratorBegin(); // Return the begin iterator
+  IteratorType GetIteratorEnd(); // Return the end iterator
+
+  // ...rest of ArrayPortal API...
+};
+```
+
+If these members are present, `ArrayPortalToIterators` will forward the portal's
+specialized iterators instead of constructing a facade. This works when using 
+the `ArrayPortalToIterators` class directly, and also with the
+`ArrayPortalToIteratorBegin` and `ArrayPortalToIteratorEnd` convenience 
+functions.

vtkm/cont/ArrayHandleImplicit.h

@@ -74,12 +74,6 @@ public:
   VTKM_EXEC_CONT
   ValueType Get(vtkm::Id index) const { return this->Functor(index); }
 
-  using IteratorType =
-    vtkm::cont::internal::IteratorFromArrayPortal<ArrayPortalImplicit<FunctorType>>;
-
-  VTKM_CONT
-  IteratorType GetIteratorBegin() const { return IteratorType(*this); }
-
 private:
   FunctorType Functor;
   vtkm::Id NumberOfValues;

vtkm/cont/ArrayHandleZip.h

@@ -39,7 +39,6 @@ public:
   using T = typename ValueType::FirstType;
   using U = typename ValueType::SecondType;
 
-  using IteratorType = ValueType_;
   using PortalTypeFirst = PortalTypeFirst_;
   using PortalTypeSecond = PortalTypeSecond_;
 

vtkm/cont/ArrayPortal.h

@@ -51,6 +51,13 @@ namespace cont
 /// (e.g., ArrayHandleCast may be casting a read-only OR read-write array), the
 /// Set method may be conditionally removed using SFINAE.
 ///
+/// The ArrayPortalToIterators utilities wrap ArrayPortals in STL-style
+/// iterators. If an ArrayPortal implementation wishes to provide a custom
+/// iterator type, it may define an IteratorType type alias along with the
+/// methods `IteratorType GetIteratorBegin()` and
+/// `IteratorType GetIteratorEnd()`. These are not required members, but if
+/// present, will allow additional optimizations for certain portals.
+///
 template <typename T>
 class ArrayPortal
 {

vtkm/cont/ArrayPortalToIterators.h

@@ -13,6 +13,17 @@
 #include <vtkm/cont/ArrayPortal.h>
 #include <vtkm/cont/internal/IteratorFromArrayPortal.h>
 
+namespace vtkmstd
+{
+
+/// Implementation of std::void_t (C++17):
+/// Allows for specialization of class templates based on members of template
+/// parameters.
+template <typename...>
+using void_t = void;
+
+} // end namespace vtkmstd
+
 namespace vtkm
 {
 namespace cont
@@ -31,7 +42,7 @@ namespace cont
 /// ArrayPortalFromIterator has a specialization to return the original
 /// iterators.
 ///
-template <typename PortalType>
+template <typename PortalType, typename CustomIterSFINAE = void>
 class ArrayPortalToIterators
 {
 public:
@@ -40,7 +51,7 @@ public:
   ///
   VTKM_SUPPRESS_EXEC_WARNINGS
   VTKM_EXEC_CONT
-  ArrayPortalToIterators(const PortalType& portal)
+  explicit ArrayPortalToIterators(const PortalType& portal)
     : Portal(portal)
   {
   }
@@ -65,6 +76,34 @@ private:
   PortalType Portal;
 };
 
+// Specialize for custom iterator types:
+template <typename PortalType>
+class ArrayPortalToIterators<PortalType, vtkmstd::void_t<typename PortalType::IteratorType>>
+{
+public:
+  using IteratorType = typename PortalType::IteratorType;
+
+  VTKM_SUPPRESS_EXEC_WARNINGS
+  VTKM_EXEC_CONT
+  explicit ArrayPortalToIterators(const PortalType& portal)
+    : Begin(portal.GetIteratorBegin())
+    , End(portal.GetIteratorEnd())
+  {
+  }
+
+  VTKM_SUPPRESS_EXEC_WARNINGS
+  VTKM_EXEC_CONT
+  IteratorType GetBegin() const { return this->Begin; }
+
+  VTKM_SUPPRESS_EXEC_WARNINGS
+  VTKM_EXEC_CONT
+  IteratorType GetEnd() const { return this->End; }
+
+private:
+  IteratorType Begin;
+  IteratorType End;
+};
+
 /// Convenience function for converting an ArrayPortal to a begin iterator.
 ///
 VTKM_SUPPRESS_EXEC_WARNINGS

vtkm/cont/cuda/testing/CMakeLists.txt

@@ -24,6 +24,7 @@ set(unit_tests
   UnitTestCudaDeviceAdapter.cu
   UnitTestCudaGeometry.cu
   UnitTestCudaImplicitFunction.cu
+  UnitTestCudaIterators.cu
   UnitTestCudaMath.cu
   UnitTestCudaShareUserProvidedManagedMemory.cu
   UnitTestCudaPointLocatorUniformGrid.cu

vtkm/cont/cuda/testing/UnitTestCudaIterators.cu

@@ -0,0 +1,50 @@
+//============================================================================
+//  Copyright (c) Kitware, Inc.
+//  All rights reserved.
+//  See LICENSE.txt for details.
+//
+//  This software is distributed WITHOUT ANY WARRANTY; without even
+//  the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+//  PURPOSE.  See the above copyright notice for more information.
+//============================================================================
+
+#include <vtkm/cont/ArrayHandle.h>
+
+#include <vtkm/cont/ArrayPortalToIterators.h>
+#include <vtkm/cont/cuda/DeviceAdapterCuda.h>
+#include <vtkm/cont/testing/Testing.h>
+
+namespace
+{
+
+// cuda portals created from basic array handles should produce raw device
+// pointers with ArrayPortalToIterator (see ArrayPortalFromThrust).
+void TestIteratorSpecialization()
+{
+  vtkm::cont::ArrayHandle<int> handle;
+
+  auto outputPortal = handle.PrepareForOutput(1, vtkm::cont::DeviceAdapterTagCuda{});
+  auto inputPortal = handle.PrepareForInput(vtkm::cont::DeviceAdapterTagCuda{});
+  auto inPlacePortal = handle.PrepareForInPlace(vtkm::cont::DeviceAdapterTagCuda{});
+
+  auto outputIter = vtkm::cont::ArrayPortalToIteratorBegin(outputPortal);
+  auto inputIter = vtkm::cont::ArrayPortalToIteratorBegin(inputPortal);
+  auto inPlaceIter = vtkm::cont::ArrayPortalToIteratorBegin(inPlacePortal);
+
+  (void)outputIter;
+  (void)inputIter;
+  (void)inPlaceIter;
+
+  VTKM_TEST_ASSERT(std::is_same<decltype(outputIter), int*>::value);
+  VTKM_TEST_ASSERT(std::is_same<decltype(inputIter), int const*>::value);
+  VTKM_TEST_ASSERT(std::is_same<decltype(inPlaceIter), int*>::value);
+}
+
+} // end anon namespace
+
+int UnitTestCudaIterators(int argc, char* argv[])
+{
+  auto& tracker = vtkm::cont::GetRuntimeDeviceTracker();
+  tracker.ForceDevice(vtkm::cont::DeviceAdapterTagCuda{});
+  return vtkm::cont::testing::Testing::Run(TestIteratorSpecialization, argc, argv);
+}

vtkm/cont/internal/ArrayPortalFromIterators.h

@@ -91,6 +91,16 @@ public:
   VTKM_EXEC_CONT
   IteratorT GetIteratorBegin() const { return this->BeginIterator; }
 
+  VTKM_SUPPRESS_EXEC_WARNINGS
+  VTKM_EXEC_CONT
+  IteratorT GetIteratorEnd() const
+  {
+    IteratorType iterator = this->BeginIterator;
+    using difference_type = typename std::iterator_traits<IteratorType>::difference_type;
+    iterator += static_cast<difference_type>(this->NumberOfValues);
+    return iterator;
+  }
+
 private:
   IteratorT BeginIterator;
   vtkm::Id NumberOfValues;
@@ -175,6 +185,16 @@ public:
   VTKM_EXEC_CONT
   IteratorT GetIteratorBegin() const { return this->BeginIterator; }
 
+  VTKM_SUPPRESS_EXEC_WARNINGS
+  VTKM_EXEC_CONT
+  IteratorT GetIteratorEnd() const
+  {
+    using difference_type = typename std::iterator_traits<IteratorType>::difference_type;
+    IteratorType iterator = this->BeginIterator;
+    iterator += static_cast<difference_type>(this->NumberOfValues);
+    return iterator;
+  }
+
 private:
   IteratorT BeginIterator;
   vtkm::Id NumberOfValues;
@@ -193,79 +213,4 @@ private:
 }
 } // namespace vtkm::cont::internal
 
-namespace vtkm
-{
-namespace cont
-{
-
-/// Partial specialization of \c ArrayPortalToIterators for \c
-/// ArrayPortalFromIterators. Returns the original array rather than
-/// the portal wrapped in an \c IteratorFromArrayPortal.
-///
-template <typename IterType>
-class ArrayPortalToIterators<vtkm::cont::internal::ArrayPortalFromIterators<IterType>>
-{
-  using PortalType = vtkm::cont::internal::ArrayPortalFromIterators<IterType>;
-
-public:
-#if !defined(VTKM_MSVC) || (defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL == 0)
-  using IteratorType = IterType;
-
-  VTKM_SUPPRESS_EXEC_WARNINGS
-  VTKM_CONT
-  ArrayPortalToIterators(const PortalType& portal)
-    : Iterator(portal.GetIteratorBegin())
-    , NumberOfValues(portal.GetNumberOfValues())
-  {
-  }
-
-#else // VTKM_MSVC
-  // The MSVC compiler issues warnings when using raw pointer math when in
-  // debug mode. To keep the compiler happy (and add some safety checks),
-  // wrap the iterator in checked_array_iterator.
-  using IteratorType = stdext::checked_array_iterator<IterType>;
-
-  VTKM_SUPPRESS_EXEC_WARNINGS
-  VTKM_CONT
-  ArrayPortalToIterators(const PortalType& portal)
-    : Iterator(portal.GetIteratorBegin(), static_cast<size_t>(portal.GetNumberOfValues()))
-    , NumberOfValues(portal.GetNumberOfValues())
-  {
-  }
-
-#endif // VTKM_MSVC
-
-  VTKM_SUPPRESS_EXEC_WARNINGS
-  VTKM_EXEC_CONT
-  IteratorType GetBegin() const { return this->Iterator; }
-
-  VTKM_SUPPRESS_EXEC_WARNINGS
-  VTKM_EXEC_CONT
-  IteratorType GetEnd() const
-  {
-    IteratorType iterator = this->Iterator;
-    using difference_type = typename std::iterator_traits<IteratorType>::difference_type;
-
-#if !defined(VTKM_MSVC) || (defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL == 0)
-    std::advance(iterator, static_cast<difference_type>(this->NumberOfValues));
-#else
-    //Visual Studio checked iterators throw exceptions when you try to advance
-    //nullptr iterators even if the advancement length is zero. So instead
-    //don't do the advancement at all
-    if (this->NumberOfValues > 0)
-    {
-      std::advance(iterator, static_cast<difference_type>(this->NumberOfValues));
-    }
-#endif
-
-    return iterator;
-  }
-
-private:
-  IteratorType Iterator;
-  vtkm::Id NumberOfValues;
-};
-}
-} // namespace vtkm::cont
-
 #endif //vtk_m_cont_internal_ArrayPortalFromIterators_h

vtkm/cont/internal/testing/UnitTestArrayPortalFromIterators.cxx

@@ -74,10 +74,9 @@ struct TemplatedTests
       array, array + ARRAY_SIZE);
 
     std::cout << "  Check that ArrayPortalToIterators is not doing indirection." << std::endl;
-// If you get a compile error here about mismatched types, it might be
-// that ArrayPortalToIterators is not properly overloaded to return the
-// original iterator.
-#if !defined(VTKM_MSVC) || (defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL == 0)
+    // If you get a compile error here about mismatched types, it might be
+    // that ArrayPortalToIterators is not properly overloaded to return the
+    // original iterator.
     VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorBegin(portal) == array,
                      "Begin iterator wrong.");
     VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorEnd(portal) == array + ARRAY_SIZE,
@@ -86,24 +85,6 @@ struct TemplatedTests
                      "Begin const iterator wrong.");
     VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorEnd(const_portal) == array + ARRAY_SIZE,
                      "End const iterator wrong.");
-#else  //VTKM_MSVC
-    // The MSVC compiler issues warnings when using raw pointer math when in
-    // debug mode. To keep the compiler happy (and add some safety checks),
-    // wrap the iterator in checked_array_iterator.
-    VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorBegin(portal) ==
-                       stdext::checked_array_iterator<ValueType*>(array, ARRAY_SIZE),
-                     "Begin iterator wrong.");
-    VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorEnd(portal) ==
-                       stdext::checked_array_iterator<ValueType*>(array, ARRAY_SIZE) + ARRAY_SIZE,
-                     "End iterator wrong.");
-    VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorBegin(const_portal) ==
-                       stdext::checked_array_iterator<const ValueType*>(array, ARRAY_SIZE),
-                     "Begin const iterator wrong.");
-    VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorEnd(const_portal) ==
-                       stdext::checked_array_iterator<const ValueType*>(array, ARRAY_SIZE) +
-                         ARRAY_SIZE,
-                     "End const iterator wrong.");
-#endif // VTKM_MSVC
 
     VTKM_TEST_ASSERT(portal.GetNumberOfValues() == ARRAY_SIZE, "Portal array size wrong.");
     VTKM_TEST_ASSERT(const_portal.GetNumberOfValues() == ARRAY_SIZE,

vtkm/cont/testing/UnitTestArrayPortalToIterators.cxx

@@ -163,9 +163,68 @@ struct TestFunctor
   }
 };
 
+// Defines minimal API needed for ArrayPortalToIterators to detect and
+// use custom iterators:
+struct SpecializedIteratorAPITestPortal
+{
+  using IteratorType = int;
+  IteratorType GetIteratorBegin() const { return 32; }
+  IteratorType GetIteratorEnd() const { return 13; }
+};
+
+void TestCustomIterator()
+{
+  std::cout << "  Testing custom iterator detection." << std::endl;
+
+  // Dummy portal type for this test:
+  using PortalType = SpecializedIteratorAPITestPortal;
+  using ItersType = vtkm::cont::ArrayPortalToIterators<PortalType>;
+
+  PortalType portal;
+  ItersType iters{ portal };
+
+  VTKM_TEST_ASSERT(
+    std::is_same<typename ItersType::IteratorType, typename PortalType::IteratorType>::value);
+  VTKM_TEST_ASSERT(
+    std::is_same<decltype(iters.GetBegin()), typename PortalType::IteratorType>::value);
+  VTKM_TEST_ASSERT(
+    std::is_same<decltype(iters.GetEnd()), typename PortalType::IteratorType>::value);
+  VTKM_TEST_ASSERT(iters.GetBegin() == 32);
+  VTKM_TEST_ASSERT(iters.GetEnd() == 13);
+
+  // Convenience API, too:
+  VTKM_TEST_ASSERT(std::is_same<decltype(vtkm::cont::ArrayPortalToIteratorBegin(portal)),
+                                typename PortalType::IteratorType>::value);
+  VTKM_TEST_ASSERT(std::is_same<decltype(vtkm::cont::ArrayPortalToIteratorEnd(portal)),
+                                typename PortalType::IteratorType>::value);
+  VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorBegin(portal) == 32);
+  VTKM_TEST_ASSERT(vtkm::cont::ArrayPortalToIteratorEnd(portal) == 13);
+}
+
+void TestBasicStorageSpecialization()
+{
+  // Control iterators from basic storage arrays should just be pointers:
+  vtkm::cont::ArrayHandle<int> handle;
+  handle.Allocate(1);
+
+  auto portal = handle.GetPortalControl();
+  auto portalConst = handle.GetPortalConstControl();
+
+  auto iter = vtkm::cont::ArrayPortalToIteratorBegin(portal);
+  auto iterConst = vtkm::cont::ArrayPortalToIteratorBegin(portalConst);
+
+  (void)iter;
+  (void)iterConst;
+
+  VTKM_TEST_ASSERT(std::is_same<decltype(iter), int*>::value);
+  VTKM_TEST_ASSERT(std::is_same<decltype(iterConst), int const*>::value);
+}
+
 void TestArrayPortalToIterators()
 {
   vtkm::testing::Testing::TryTypes(TestFunctor());
+  TestCustomIterator();
+  TestBasicStorageSpecialization();
 }
 
 } // Anonymous namespace

vtkm/exec/cuda/internal/ArrayPortalFromThrust.h

@@ -356,73 +356,4 @@ private:
 }
 } // namespace vtkm::exec::cuda::internal
 
-namespace vtkm
-{
-namespace cont
-{
-
-/// Partial specialization of \c ArrayPortalToIterators for \c
-/// ArrayPortalFromThrust. Returns the original array rather than
-/// the portal wrapped in an \c IteratorFromArrayPortal.
-///
-template <typename T>
-class ArrayPortalToIterators<vtkm::exec::cuda::internal::ArrayPortalFromThrust<T>>
-{
-  using PortalType = vtkm::exec::cuda::internal::ArrayPortalFromThrust<T>;
-
-public:
-  using IteratorType = typename PortalType::IteratorType;
-
-  VTKM_CONT
-  ArrayPortalToIterators(const PortalType& portal)
-    : BIterator(portal.GetIteratorBegin())
-    , EIterator(portal.GetIteratorEnd())
-  {
-  }
-
-  VTKM_CONT
-  IteratorType GetBegin() const { return this->BIterator; }
-
-  VTKM_CONT
-  IteratorType GetEnd() const { return this->EIterator; }
-
-private:
-  IteratorType BIterator;
-  IteratorType EIterator;
-  vtkm::Id NumberOfValues;
-};
-
-/// Partial specialization of \c ArrayPortalToIterators for \c
-/// ConstArrayPortalFromThrust. Returns the original array rather than
-/// the portal wrapped in an \c IteratorFromArrayPortal.
-///
-template <typename T>
-class ArrayPortalToIterators<vtkm::exec::cuda::internal::ConstArrayPortalFromThrust<T>>
-{
-  using PortalType = vtkm::exec::cuda::internal::ConstArrayPortalFromThrust<T>;
-
-public:
-  using IteratorType = typename PortalType::IteratorType;
-
-  VTKM_CONT
-  ArrayPortalToIterators(const PortalType& portal)
-    : BIterator(portal.GetIteratorBegin())
-    , EIterator(portal.GetIteratorEnd())
-  {
-  }
-
-  VTKM_CONT
-  IteratorType GetBegin() const { return this->BIterator; }
-
-  VTKM_CONT
-  IteratorType GetEnd() const { return this->EIterator; }
-
-private:
-  IteratorType BIterator;
-  IteratorType EIterator;
-  vtkm::Id NumberOfValues;
-};
-}
-} // namespace vtkm::cont
-
 #endif //vtk_m_exec_cuda_internal_ArrayPortalFromThrust_h