From 0fdfa876d478623f6f9c812fa561b7e0cbf973fa Mon Sep 17 00:00:00 2001
From: Andrey Prokopenko <prokopenkoav@ornl.gov>
Date: Fri, 15 Sep 2023 11:42:18 -0400
Subject: [PATCH] Fix passing temporary const ref in indexable getter

If both DefaultIndexableGetter has const ref pair argument, and
Indexables have decltype(auto) return, this ends up in a wrong result
(likely to the handling of temporaries). This leads to a
default-initialized scene bounding box, which leads to same Morton
codes, which leads to f-d up hierarchy.

Providing a second operator()(PairIndexVolume<Geometry> &&pair) operator
in DefaultIndexableGetter fixes that.
---
 src/details/ArborX_DetailsLegacy.hpp   |  2 +-
 src/details/ArborX_IndexableGetter.hpp |  6 ++++++
 test/tstDetailsTreeConstruction.cpp    | 13 +++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/details/ArborX_DetailsLegacy.hpp b/src/details/ArborX_DetailsLegacy.hpp
index 0f599d31c..e5dc0e6f1 100644
--- a/src/details/ArborX_DetailsLegacy.hpp
+++ b/src/details/ArborX_DetailsLegacy.hpp
@@ -36,7 +36,7 @@ class LegacyValues
   {}
 
   KOKKOS_FUNCTION
-  decltype(auto) operator()(size_type i) const
+  auto operator()(size_type i) const
   {
     if constexpr (std::is_same_v<BoundingVolume,
                                  typename AccessTraitsHelper<Access>::type>)
diff --git a/src/details/ArborX_IndexableGetter.hpp b/src/details/ArborX_IndexableGetter.hpp
index 462504ece..d109ba655 100644
--- a/src/details/ArborX_IndexableGetter.hpp
+++ b/src/details/ArborX_IndexableGetter.hpp
@@ -38,6 +38,12 @@ struct DefaultIndexableGetter
   {
     return pair.bounding_volume;
   }
+
+  template <typename Geometry>
+  KOKKOS_FUNCTION Geometry operator()(PairIndexVolume<Geometry> &&pair) const
+  {
+    return pair.bounding_volume;
+  }
 };
 
 template <typename Values, typename IndexableGetter>
diff --git a/test/tstDetailsTreeConstruction.cpp b/test/tstDetailsTreeConstruction.cpp
index 80c388d84..16331e5af 100644
--- a/test/tstDetailsTreeConstruction.cpp
+++ b/test/tstDetailsTreeConstruction.cpp
@@ -79,6 +79,19 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(assign_morton_codes, DeviceType,
   BOOST_TEST(ArborX::Details::equals(
       scene_host, {{{0.0, 0.0, 0.0}}, {{(float)N, (float)N, (float)N}}}));
 
+  ArborX::Details::LegacyValues<decltype(boxes), ArborX::Box> values{boxes};
+  ArborX::Details::Indexables<decltype(values),
+                              ArborX::Details::DefaultIndexableGetter>
+      indexables{values, ArborX::Details::DefaultIndexableGetter{}};
+
+  // Test for a bug where missing move ref operator() in DefaultIndexableGetter
+  // results in a default initialized indexable used in scene box calucation.
+  scene_host = ArborX::Box{};
+  ArborX::Details::TreeConstruction::calculateBoundingBoxOfTheScene(
+      space, indexables, scene_host);
+  BOOST_TEST(ArborX::Details::equals(
+      scene_host, {{{0.0, 0.0, 0.0}}, {{(float)N, (float)N, (float)N}}}));
+
   Kokkos::View<unsigned long long *, DeviceType> morton_codes("morton_codes",
                                                               n);
   ArborX::Details::TreeConstruction::projectOntoSpaceFillingCurve(