fix temporary vector going out of scope; update tests

mwinn2 · Mar 7, 2024 · c944a62 · c944a62
1 parent ae6bfc1
commit c944a62
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 31 deletions.
diff --git a/Framework/Core/include/Framework/ASoA.h b/Framework/Core/include/Framework/ASoA.h
@@ -2954,10 +2954,10 @@ class FilteredBase : public T
 
   auto rawSlice(uint64_t start, uint64_t end) const
   {
-    auto s = this->asArrowTable()->Slice(start, end - start + 1);
-    SelectionVector newSelection{static_cast<int64_t>(end - start + 1)};
-    std::iota(newSelection.begin(), newSelection.end(), 0);
-    return self_t{{s}, newSelection, start};
+    SelectionVector newSelection;
+    newSelection.resize(static_cast<int64_t>(end - start + 1));
+    std::iota(newSelection.begin(), newSelection.end(), start);
+    return self_t{{this->asArrowTable()}, std::move(newSelection), 0};
   }
 
   auto emptySlice() const
@@ -3227,12 +3227,14 @@ class Filtered : public FilteredBase<T>
     return it;
   }
 
+  using FilteredBase<T>::getSelectedRows;
+
   auto rawSlice(uint64_t start, uint64_t end) const
   {
-    auto s = this->asArrowTable()->Slice(start, end - start + 1);
-    SelectionVector newSelection{static_cast<int64_t>(end - start + 1)};
-    std::iota(newSelection.begin(), newSelection.end(), 0);
-    return self_t{{s}, newSelection, start};
+    SelectionVector newSelection;
+    newSelection.resize(static_cast<int64_t>(end - start + 1));
+    std::iota(newSelection.begin(), newSelection.end(), start);
+    return self_t{{this->asArrowTable()}, std::move(newSelection), 0};
   }
 
   auto emptySlice() const
@@ -3401,10 +3403,10 @@ class Filtered<Filtered<T>> : public FilteredBase<typename T::table_t>
 
   auto rawSlice(uint64_t start, uint64_t end) const
   {
-    auto s = this->asArrowTable()->Slice(start, end - start + 1);
-    SelectionVector newSelection{static_cast<int64_t>(end - start + 1)};
-    std::iota(newSelection.begin(), newSelection.end(), 0);
-    return self_t{{s}, newSelection, start};
+    SelectionVector newSelection;
+    newSelection.resize(static_cast<int64_t>(end - start + 1));
+    std::iota(newSelection.begin(), newSelection.end(), start);
+    return self_t{{this->asArrowTable()}, std::move(newSelection), 0};
   }
 
   auto emptySlice() const

diff --git a/Framework/Core/test/test_ASoA.cxx b/Framework/Core/test/test_ASoA.cxx
@@ -1039,20 +1039,16 @@ TEST_CASE("TestSelfIndexRecursion")
   // FIXME: only 4 levels of recursive self-index dereference are tested
   // self-index binding should stay the same for recursive dereferences
   for (auto& p : fp) {
-    auto bp = std::is_same_v<std::decay_t<decltype(p)>, FullPoints::iterator>;
-    REQUIRE(bp);
+    REQUIRE(std::is_same_v<std::decay_t<decltype(p)>, FullPoints::iterator>);
     auto ops = p.pointSeq_as<FullPoints>();
     for (auto& pp : ops) {
-      auto bpp = std::is_same_v<std::decay_t<decltype(pp)>, FullPoints::iterator>;
-      REQUIRE(bpp);
+      REQUIRE(std::is_same_v<std::decay_t<decltype(pp)>, FullPoints::iterator>);
       auto opps = pp.pointSeq_as<FullPoints>();
       for (auto& ppp : opps) {
-        auto bppp = std::is_same_v<std::decay_t<decltype(ppp)>, FullPoints::iterator>;
-        REQUIRE(bppp);
+        REQUIRE(std::is_same_v<std::decay_t<decltype(ppp)>, FullPoints::iterator>);
         auto oppps = ppp.pointSeq_as<FullPoints>();
         for (auto& pppp : oppps) {
-          auto bpppp = std::is_same_v<std::decay_t<decltype(pppp)>, FullPoints::iterator>;
-          REQUIRE(bpppp);
+          REQUIRE(std::is_same_v<std::decay_t<decltype(pppp)>, FullPoints::iterator>);
           auto opppps = pppp.pointSeq_as<FullPoints>();
         }
       }
@@ -1077,21 +1073,16 @@ TEST_CASE("TestSelfIndexRecursion")
 
   // Filter should not interfere with self-index and the binding should stay the same
   for (auto& p : ffp) {
-    using T1 = std::decay_t<decltype(p)>;
-    auto bp = std::is_same_v<T1, FilteredPoints::iterator>;
-    REQUIRE(bp);
-    auto ops = p.pointSeq_as<typename T1::parent_t>();
+    REQUIRE(std::is_same_v<std::decay_t<decltype(p)>, FilteredPoints::iterator>);
+    auto ops = p.pointSeq_as<typename std::decay_t<decltype(p)>::parent_t>();
     for (auto& pp : ops) {
-      auto bpp = std::is_same_v<std::decay_t<decltype(pp)>, FullPoints::iterator>;
-      REQUIRE(bpp);
+      REQUIRE(std::is_same_v<std::decay_t<decltype(pp)>::parent_t, FilteredPoints>);
       auto opps = pp.pointSeq_as<FilteredPoints>();
       for (auto& ppp : opps) {
-        auto bppp = std::is_same_v<std::decay_t<decltype(ppp)>, FullPoints::iterator>;
-        REQUIRE(bppp);
+        REQUIRE(std::is_same_v<std::decay_t<decltype(ppp)>, FilteredPoints::iterator>);
         auto oppps = ppp.pointSeq_as<FilteredPoints>();
         for (auto& pppp : oppps) {
-          auto bpppp = std::is_same_v<std::decay_t<decltype(pppp)>, FullPoints::iterator>;
-          REQUIRE(bpppp);
+          REQUIRE(std::is_same_v<std::decay_t<decltype(pppp)>, FilteredPoints::iterator>);
           auto opppps = pppp.pointSeq_as<FilteredPoints>();
         }
       }
@@ -1100,7 +1091,7 @@ TEST_CASE("TestSelfIndexRecursion")
 
   auto const& ffpa = ffp;
 
-  // rawIteratorAt() should create an unfiltered iterator, unline begin() and iteratorAt()
+  // rawIteratorAt() should create an unfiltered iterator, unlike begin() and iteratorAt()
   for (auto& it1 : ffpa) {
     [[maybe_unused]] auto it2 = ffpa.rawIteratorAt(0);
     [[maybe_unused]] auto it3 = ffpa.iteratorAt(0);