From c0929c355de3d4ec07917235ed5cf88ee208b768 Mon Sep 17 00:00:00 2001
From: Andrei Elovikov <andrei.elovikov@intel.com>
Date: Thu, 17 Jul 2025 11:50:40 -0700
Subject: [PATCH] [NFC][SYCL] Simplify `variadic_iterator` usage

`std::weak_ptr<node_impl>` aren't used to create `nodes_range`
anymore (https://github.com/intel/llvm/pull/19438 being the last change
that enabled that, I think) and we also added `SyclTy` template
parameter to `variadic_iterator` so that the dereference operator can be
implemented in a generic way in the base class.
---
 sycl/source/detail/device_impl.hpp     | 22 ++++------------------
 sycl/source/detail/graph/node_impl.hpp | 26 +-------------------------
 sycl/source/detail/helpers.hpp         | 21 ++++++++++++++-------
 3 files changed, 19 insertions(+), 50 deletions(-)

diff --git a/sycl/source/detail/device_impl.hpp b/sycl/source/detail/device_impl.hpp
index 1b86d170267db..faeaba0c8b492 100644
--- a/sycl/source/detail/device_impl.hpp
+++ b/sycl/source/detail/device_impl.hpp
@@ -2282,24 +2282,10 @@ class device_impl : public std::enable_shared_from_this<device_impl> {
 
 }; // class device_impl
 
-struct devices_deref_impl {
-  template <typename T> static device_impl &dereference(T &Elem) {
-    using Ty = std::decay_t<decltype(Elem)>;
-    if constexpr (std::is_same_v<Ty, device>) {
-      return *getSyclObjImpl(Elem);
-    } else if constexpr (std::is_same_v<Ty, device_impl>) {
-      return Elem;
-    } else {
-      return *Elem;
-    }
-  }
-};
-using devices_iterator =
-    variadic_iterator<devices_deref_impl, device,
-                      std::vector<std::shared_ptr<device_impl>>::const_iterator,
-                      std::vector<device>::const_iterator,
-                      std::vector<device_impl *>::const_iterator,
-                      device_impl *>;
+using devices_iterator = variadic_iterator<
+    device, std::vector<std::shared_ptr<device_impl>>::const_iterator,
+    std::vector<device>::const_iterator,
+    std::vector<device_impl *>::const_iterator, device_impl *>;
 
 class devices_range : public iterator_range<devices_iterator> {
 private:
diff --git a/sycl/source/detail/graph/node_impl.hpp b/sycl/source/detail/graph/node_impl.hpp
index 3501830f14cb3..bfcdb18f63a4f 100644
--- a/sycl/source/detail/graph/node_impl.hpp
+++ b/sycl/source/detail/graph/node_impl.hpp
@@ -749,37 +749,13 @@ class node_impl : public std::enable_shared_from_this<node_impl> {
   }
 };
 
-struct nodes_deref_impl {
-  template <typename T> static node_impl &dereference(T &Elem) {
-    using Ty = std::decay_t<decltype(Elem)>;
-    if constexpr (std::is_same_v<Ty, std::weak_ptr<node_impl>>) {
-      // This assumes that weak_ptr doesn't actually manage lifetime and
-      // the object is guaranteed to be alive (which seems to be the
-      // assumption across all graph code).
-      return *Elem.lock();
-    } else if constexpr (std::is_same_v<Ty, node>) {
-      return *getSyclObjImpl(Elem);
-    } else {
-      return *Elem;
-    }
-  }
-};
-
 template <typename... ContainerTy>
 using nodes_iterator_impl =
-    variadic_iterator<nodes_deref_impl, node,
-                      typename ContainerTy::const_iterator...>;
+    variadic_iterator<node, typename ContainerTy::const_iterator...>;
 
 using nodes_iterator = nodes_iterator_impl<
     std::vector<std::shared_ptr<node_impl>>, std::vector<node_impl *>,
-    // Next one is temporary. It looks like `weak_ptr`s aren't
-    // used for the actual lifetime management and the objects are
-    // always guaranteed to be alive. Once the code is cleaned
-    // from `weak_ptr`s this alternative should be removed too.
-    std::vector<std::weak_ptr<node_impl>>,
-    //
     std::set<std::shared_ptr<node_impl>>, std::set<node_impl *>,
-    //
     std::list<node_impl *>, std::vector<node>>;
 
 class nodes_range : public iterator_range<nodes_iterator> {
diff --git a/sycl/source/detail/helpers.hpp b/sycl/source/detail/helpers.hpp
index a1a49361e5755..0629327878297 100644
--- a/sycl/source/detail/helpers.hpp
+++ b/sycl/source/detail/helpers.hpp
@@ -35,8 +35,7 @@ const RTDeviceBinaryImage *
 retrieveKernelBinary(queue_impl &Queue, KernelNameStrRefT KernelName,
                      CGExecKernel *CGKernel = nullptr);
 
-template <typename DereferenceImpl, typename SyclTy, typename... Iterators>
-class variadic_iterator {
+template <typename SyclTy, typename... Iterators> class variadic_iterator {
   using storage_iter = std::variant<Iterators...>;
 
   storage_iter It;
@@ -44,12 +43,11 @@ class variadic_iterator {
 public:
   using iterator_category = std::forward_iterator_tag;
   using difference_type = std::ptrdiff_t;
-  using reference = decltype(DereferenceImpl::dereference(
-      *std::declval<nth_type_t<0, Iterators...>>()));
-  using value_type = std::remove_reference_t<reference>;
+  using value_type = std::remove_reference_t<decltype(*getSyclObjImpl(
+      std::declval<SyclTy>()))>;
   using sycl_type = SyclTy;
   using pointer = value_type *;
-  static_assert(std::is_same_v<reference, value_type &>);
+  using reference = value_type &;
 
   variadic_iterator(const variadic_iterator &) = default;
   variadic_iterator(variadic_iterator &&) = default;
@@ -79,7 +77,16 @@ class variadic_iterator {
   decltype(auto) operator*() {
     return std::visit(
         [](auto &&It) -> decltype(auto) {
-          return DereferenceImpl::dereference(*It);
+          decltype(auto) Elem = *It;
+          using Ty = std::decay_t<decltype(Elem)>;
+          static_assert(!std::is_same_v<Ty, decltype(Elem)>);
+          if constexpr (std::is_same_v<Ty, sycl_type>) {
+            return *getSyclObjImpl(Elem);
+          } else if constexpr (std::is_same_v<Ty, value_type>) {
+            return Elem;
+          } else {
+            return *Elem;
+          }
         },
         It);
   }