Remove std::optional<std::wrapped_reference>> from .at()

There was a desire to move to use either a pointer or a
normal reference.  The original .at() returned a normal
reference but did not allow for offsets that were out
of bounds.  I went back to this original and used an
abort for the invalid offset.  I added a death test in
util_unittest to verify that was working.

BUG=b:271591258
TEST=USE="coverage" FEATURES="test noclean" emerge-brya chromeos-base/gestures

Change-Id: Ie6a6716f23d5c8516dab7801098e7f4d60b00bee
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/gestures/+/4357694
Reviewed-by: Harry Cutts <hcutts@chromium.org>
Code-Coverage: Henry Barnor <hbarnor@chromium.org>
Auto-Submit: Denis Brockus <dbrockus@chromium.org>
Tested-by: Denis Brockus <dbrockus@chromium.org>
Commit-Queue: Denis Brockus <dbrockus@chromium.org>

9 files changed, 64 insertions, 56 deletions

diff --git a/include/lookahead_filter_interpreter.h b/include/lookahead_filter_interpreter.h
index d3d4077..c37f170 100644
--- a/include/lookahead_filter_interpreter.h
+++ b/include/lookahead_filter_interpreter.h
@@ -6,7 +6,6 @@
 #include <list>
 #include <map>
 #include <memory>
-#include <optional>
 
 #include <gtest/gtest.h>  // For FRIEND_TEST
 
@@ -108,12 +107,9 @@ class LookaheadFilterInterpreter : public FilterInterpreter {
 
   stime_t ExtraVariableDelay() const;
 
-  typedef std::list<QState> QStateListType;
-  typedef std::optional<std::reference_wrapper<QState>> OptionalRefQState;
-
-  struct QStateList : public QStateListType {
-    OptionalRefQState at(int offset) {
-      return ListAt<OptionalRefQState, QStateListType>(*this, offset);
+  struct QStateList : public std::list<QState> {
+    QState& at(int offset) {
+      return ListAt<QState>(*this, offset);
     }
   } queue_;
 
diff --git a/include/metrics_filter_interpreter.h b/include/metrics_filter_interpreter.h
index b58aabf..9dc1978 100644
--- a/include/metrics_filter_interpreter.h
+++ b/include/metrics_filter_interpreter.h
@@ -5,7 +5,6 @@
 #include <gtest/gtest.h>  // for FRIEND_TEST
 #include <list>
 #include <map>
-#include <optional>
 
 #include "include/filter_interpreter.h"
 #include "include/finger_metrics.h"
@@ -55,12 +54,10 @@ class MetricsFilterInterpreter : public FilterInterpreter {
 
   // struct for one finger's data of one frame.
   typedef State<FingerState, 3> MState;
-  typedef std::list<MState> MStateListType;
-  typedef std::optional<std::reference_wrapper<MState>> OptionalRefMState;
 
-  struct FingerHistory : public MStateListType {
-    OptionalRefMState at(int offset) {
-      return ListAt<OptionalRefMState, MStateListType>(*this, offset);
+  struct FingerHistory : public std::list<MState> {
+    MState& at(int offset) {
+      return ListAt<MState>(*this, offset);
     }
   };
 
diff --git a/include/trend_classifying_filter_interpreter.h b/include/trend_classifying_filter_interpreter.h
index 1de25cd..6c05073 100644
--- a/include/trend_classifying_filter_interpreter.h
+++ b/include/trend_classifying_filter_interpreter.h
@@ -5,7 +5,6 @@
 #include <gtest/gtest.h>  // for FRIEND_TEST
 #include <list>
 #include <map>
-#include <optional>
 #include <set>
 
 #include "include/filter_interpreter.h"
@@ -154,16 +153,12 @@ private:
     }
   };
 
-  typedef std::list<KState> KStateListType;
-  typedef std::optional<std::reference_wrapper<KState>> OptionalRefKState;
-
-  struct FingerHistory : public KStateListType {
-    OptionalRefKState at(int offset) {
-      return ListAt<OptionalRefKState, KStateListType>(*this, offset);
+  struct FingerHistory : public std::list<KState> {
+    KState& at(int offset) {
+      return ListAt<KState>(*this, offset);
     }
   };
 
-
   // Trend types for internal use
   enum TrendType {
     TREND_NONE,
diff --git a/include/util.h b/include/util.h
index 0e1d89a..afaf61a 100644
--- a/include/util.h
+++ b/include/util.h
@@ -7,7 +7,6 @@
 
 #include <list>
 #include <map>
-#include <optional>
 #include <set>
 
 #include <math.h>
@@ -95,24 +94,24 @@ inline bool SetContainsValue(const Set& the_set,
   return the_set.find(elt) != the_set.end();
 }
 
-template<typename R, typename L>
-R ListAt(L& the_list, int offset) {
+template<typename Elem>
+Elem& ListAt(std::list<Elem>& the_list, int offset) {
   // Traverse to the appropriate offset
   if (offset < 0) {
     // negative offset is from end to begin
     for (auto iter = the_list.rbegin(); iter != the_list.rend(); ++iter) {
       if (++offset == 0)
-        return R(*iter);
+        return *iter;
     }
   } else {
     // positive offset is from begin to end
     for (auto iter = the_list.begin(); iter != the_list.end(); ++iter) {
       if (offset-- == 0)
-        return R(*iter);
+        return *iter;
     }
   }
   // Invalid offset
-  return std::nullopt;
+  abort();
 }
 
 }  // namespace gestures
diff --git a/src/lookahead_filter_interpreter.cc b/src/lookahead_filter_interpreter.cc
index 69156f5..fea2c47 100644
--- a/src/lookahead_filter_interpreter.cc
+++ b/src/lookahead_filter_interpreter.cc
@@ -147,11 +147,11 @@ void LookaheadFilterInterpreter::AssignTrackingIds() {
     return;
   }
 
-  auto& tail = queue_.at(-1)->get();
+  auto& tail = queue_.at(-1);
   HardwareState* hs = &tail.state_;
-  QState* prev_qs = queue_.size() < 2 ? NULL : &(queue_.at(-2)->get());
+  QState* prev_qs = queue_.size() < 2 ? NULL : &(queue_.at(-2));
   HardwareState* prev_hs = prev_qs ? &prev_qs->state_ : NULL;
-  QState* prev2_qs = queue_.size() < 3 ? NULL : &(queue_.at(-3)->get());
+  QState* prev2_qs = queue_.size() < 3 ? NULL : &(queue_.at(-3));
   HardwareState* prev2_hs = prev2_qs ? &prev2_qs->state_ : NULL;
 
   RemoveMissingIdsFromMap(&tail.output_ids_, *hs);
@@ -352,7 +352,7 @@ void LookaheadFilterInterpreter::TapDownOccurringGesture(stime_t now) {
     return;  // We didn't push a new hardware state now
   // See if latest hwstate has finger that previous doesn't.
   // Get the state_ of back->prev
-  HardwareState& prev_hs = queue_.at(-2)->get().state_;
+  HardwareState& prev_hs = queue_.at(-2).state_;
   if (hs.finger_cnt > prev_hs.finger_cnt) {
     // Finger was added.
     ProduceGesture(Gesture(kGestureFling, prev_hs.timestamp, hs.timestamp,
@@ -388,8 +388,8 @@ short LookaheadFilterInterpreter::NextTrackingId() {
 void LookaheadFilterInterpreter::AttemptInterpolation() {
   if (queue_.size() < 2)
     return;
-  QState& new_node = queue_.at(-1)->get();
-  QState& prev = queue_.at(-2)->get();
+  QState& new_node = queue_.at(-1);
+  QState& prev = queue_.at(-2);
   if (new_node.state_.timestamp - prev.state_.timestamp <
       split_min_period_.val_) {
     return;  // Nodes came in too quickly to need interpolation
diff --git a/src/lookahead_filter_interpreter_unittest.cc b/src/lookahead_filter_interpreter_unittest.cc
index ee99a0f..1345daf 100644
--- a/src/lookahead_filter_interpreter_unittest.cc
+++ b/src/lookahead_filter_interpreter_unittest.cc
@@ -821,9 +821,9 @@ TEST(LookaheadFilterInterpreterTest, QuickMoveTest) {
 
   // Expecting Quick movement detected and ID correction 5 -> 4.
   wrapper.SyncInterpret(&hs[6], &timeout);
-  EXPECT_EQ(interpreter->queue_.at(-1)->get().fs_[0].tracking_id, 4);
-  EXPECT_EQ(interpreter->queue_.at(-2)->get().fs_[0].tracking_id, 4);
-  EXPECT_EQ(interpreter->queue_.at(-3)->get().fs_[0].tracking_id, 4);
+  EXPECT_EQ(interpreter->queue_.at(-1).fs_[0].tracking_id, 4);
+  EXPECT_EQ(interpreter->queue_.at(-2).fs_[0].tracking_id, 4);
+  EXPECT_EQ(interpreter->queue_.at(-3).fs_[0].tracking_id, 4);
 }
 
 struct QuickSwipeTestInputs {
diff --git a/src/metrics_filter_interpreter.cc b/src/metrics_filter_interpreter.cc
index 39c9766..a19b814 100644
--- a/src/metrics_filter_interpreter.cc
+++ b/src/metrics_filter_interpreter.cc
@@ -151,9 +151,9 @@ bool MetricsFilterInterpreter::DetectNoisyGround(FingerHistory& history) {
   if (history.size() < 3)
     return false;
 
-  auto current = history.at(-1)->get();
-  auto past_1 = history.at(-2)->get();
-  auto past_2 = history.at(-3)->get();
+  auto current = history.at(-1);
+  auto past_1 = history.at(-2);
+  auto past_2 = history.at(-3);
   // Noise pattern needs to happen in a short period of time
   if (current.timestamp - past_2.timestamp > noisy_ground_time_threshold_.val_)
     return false;
diff --git a/src/trend_classifying_filter_interpreter.cc b/src/trend_classifying_filter_interpreter.cc
index 2bfa156..714cb68 100644
--- a/src/trend_classifying_filter_interpreter.cc
+++ b/src/trend_classifying_filter_interpreter.cc
@@ -79,7 +79,7 @@ void TrendClassifyingFilterInterpreter::AddNewStateToBuffer(
   auto& current = history.emplace_back(fs);
   if (history.size() == 1)
     return;
-  auto& previous_end = history.at(-2)->get();
+  auto& previous_end = history.at(-2);
 
   current.DxAxis()->val = current.XAxis()->val - previous_end.XAxis()->val;
   current.DyAxis()->val = current.YAxis()->val - previous_end.YAxis()->val;
diff --git a/src/util_unittest.cc b/src/util_unittest.cc
index 9784d70..9d6bd40 100644
--- a/src/util_unittest.cc
+++ b/src/util_unittest.cc
@@ -27,12 +27,9 @@ TEST(UtilTest, ListAtTest) {
     int x;
   };
 
-  typedef std::list<element> ElemListType;
-  typedef std::optional<std::reference_wrapper<element>> OptionalRefElem;
-
-  struct ElemList : public ElemListType {
-    OptionalRefElem at(int offset) {
-      return ListAt<OptionalRefElem, ElemListType>(*this, offset);
+  struct ElemList : public std::list<element> {
+    element& at(int offset) {
+      return ListAt<element>(*this, offset);
     }
   } list;
 
@@ -40,25 +37,49 @@ TEST(UtilTest, ListAtTest) {
     auto& elem = list.emplace_back();
     elem.x = i;
   }
-  EXPECT_EQ(list.at(-1)->get().x, list.at(list.size() - 1)->get().x);
+  EXPECT_EQ(list.at(-1).x, list.at(list.size() - 1).x);
 
   for (auto i = 0; i < kMaxElements; ++i) {
     for (auto j = 0; j < kMaxElements; ++j) {
       if (i == j) {
-        EXPECT_EQ(list.at(i)->get().x, list.at(j)->get().x);
-        EXPECT_EQ(&(list.at(i)->get()), &(list.at(j)->get()));
+        EXPECT_EQ(list.at(i).x, list.at(j).x);
+        EXPECT_EQ(&(list.at(i)), &(list.at(j)));
       } else {
-        EXPECT_NE(list.at(i)->get().x, list.at(j)->get().x);
-        EXPECT_NE(&(list.at(i)->get()), &(list.at(j)->get()));
+        EXPECT_NE(list.at(i).x, list.at(j).x);
+        EXPECT_NE(&(list.at(i)), &(list.at(j)));
       }
     }
   }
+}
+
+TEST(UtilTest, ListAtDeathForwardTest) {
+  const int kMaxElements = 3;
+
+  struct IntList : public std::list<int> {
+    int& at(int offset) {
+      return ListAt<int>(*this, offset);
+    }
+  } list;
+
+  for (auto i = 0; i < kMaxElements; ++i) {
+    list.emplace_back(i);
+  }
+  EXPECT_DEATH(list.at(kMaxElements+1), "");
+}
 
-  int list_count = list.size();
-  EXPECT_EQ(list.at(list_count), std::nullopt);
-  EXPECT_NE(list.at(list_count - 1), std::nullopt);
-  EXPECT_EQ(list.at(-list_count - 1), std::nullopt);
-  EXPECT_NE(list.at(-list_count), std::nullopt);
+TEST(UtilTest, ListAtDeathBackwardTest) {
+  const int kMaxElements = 3;
+
+  struct IntList : public std::list<int> {
+    int& at(int offset) {
+      return ListAt<int>(*this, offset);
+    }
+  } list;
+
+  for (auto i = 0; i < kMaxElements; ++i) {
+    list.emplace_back(i);
+  }
+  EXPECT_DEATH(list.at(-(kMaxElements+1)), "");
 }
 
 }  // namespace gestures