[PATCH] D23178: [ADT] Add relation operators for Optional

[David Blaikie via llvm-commits]

dblaikie added a comment.

This seems a bit more complicated than what we were discussing - perhaps it's for good reason. But to give a sense, this is sort of what I was picturing.

(some notes:

- I don't think it's worth checking that comparisons with None do not invoke the underlying type's comparison operators - yes, it's a possible bug, but seems like the sort of excess testing that's easy to fall into with mocking because we can
- did find one 'bug' in the implementation, op==(const T&, const Optional<T>&) inverted its arguments and called op==(const Optional<T>&, const T&) which isn't technically how it's spec'd, but one could argue the spec is overly narrow. I'm kind of OK 'fixing' that and having the test be a bit more narrow for simplicity...
- It's still annoying to have the EXPECT lines in another function since gtest doesn't give a backtrace on the EXPECT failure. But there's nothing really to be done about that short of writing out all the EXPECT lines, which seems lame/annoying/difficult/verbose/not worth it. But all the more reason that splitting this into one test per operator like this is at least a little helpful.)

  template <typename OperatorT, typename T> void CheckRelation(const Optional<T> &L, const Optional<T> &R, bool Expected) { EXPECT_EQ(Expected, OperatorT::apply(L, R));

  if (L) EXPECT_EQ(Expected, OperatorT::apply(*L, R)); else EXPECT_EQ(Expected, OperatorT::apply(None, R));

  if (R) EXPECT_EQ(Expected, OperatorT::apply(L, *R)); else EXPECT_EQ(Expected, OperatorT::apply(L, None)); }



  struct EqualityMock {};
  
  const Optional<EqualityMock> NoneEq, EqualityLhs((EqualityMock())),
      EqualityRhs((EqualityMock()));
  bool IsEqual;
  
  bool operator==(const EqualityMock &Lhs, const EqualityMock &Rhs) {
    EXPECT_EQ(&Lhs, &*EqualityLhs);
    EXPECT_EQ(&Rhs, &*EqualityRhs);
    return IsEqual;
  }
  
  TEST_F(OptionalTest, OperatorEqual) {
    CheckRelation<EqualTo>(NoneEq, NoneEq, true);
    CheckRelation<EqualTo>(NoneEq, EqualityLhs, false);
    CheckRelation<EqualTo>(EqualityLhs, NoneEq, false);
  
    IsEqual = false;
    CheckRelation<EqualTo>(EqualityLhs, EqualityRhs, false);
    IsEqual = true;
    CheckRelation<EqualTo>(EqualityLhs, EqualityRhs, true);
  }
  
  TEST_F(OptionalTest, OperatorNotEqual) {
    CheckRelation<NotEqualTo>(NoneEq, NoneEq, false);
    CheckRelation<NotEqualTo>(NoneEq, EqualityLhs, true);
    CheckRelation<NotEqualTo>(EqualityLhs, NoneEq, true);
  
    IsEqual = false;
    CheckRelation<NotEqualTo>(EqualityLhs, EqualityRhs, true);
    IsEqual = true;
    CheckRelation<NotEqualTo>(EqualityLhs, EqualityRhs, false);
  }
  
  struct InequalityMock { };
  
  const Optional<InequalityMock> NoneIneq, InequalityLhs((InequalityMock())),
      InequalityRhs((InequalityMock()));
  bool IsLess;
  
  bool operator<(const InequalityMock &Lhs, const InequalityMock &Rhs) {
    EXPECT_EQ(&Lhs, &*InequalityLhs);
    EXPECT_EQ(&Rhs, &*InequalityRhs);
    return IsLess;
  }
  
  TEST_F(OptionalTest, OperatorLess) {
    CheckRelation<Less>(NoneIneq, NoneIneq, false);
    CheckRelation<Less>(NoneIneq, InequalityRhs, true);
    CheckRelation<Less>(InequalityLhs, NoneIneq, false);
  
    IsLess = false;
    CheckRelation<Less>(InequalityLhs, InequalityRhs, false);
    IsLess = true;
    CheckRelation<Less>(InequalityLhs, InequalityRhs, true);
  }
  
  TEST_F(OptionalTest, OperatorGreater) {
    CheckRelation<Greater>(NoneIneq, NoneIneq, false);
    CheckRelation<Greater>(NoneIneq, InequalityRhs, false);
    CheckRelation<Greater>(InequalityLhs, NoneIneq, true);
  
    IsLess = false;
    CheckRelation<Greater>(InequalityRhs, InequalityLhs, false);
    IsLess = true;
    CheckRelation<Greater>(InequalityRhs, InequalityLhs, true);
  }
  
  TEST_F(OptionalTest, OperatorLessEqual) {
    CheckRelation<LessEqual>(NoneIneq, NoneIneq, true);
    CheckRelation<LessEqual>(NoneIneq, InequalityRhs, true);
    CheckRelation<LessEqual>(InequalityLhs, NoneIneq, false);
  
    IsLess = false;
    CheckRelation<LessEqual>(InequalityRhs, InequalityLhs, true);
    IsLess = true;
    CheckRelation<LessEqual>(InequalityRhs, InequalityLhs, false);
  }
  
  TEST_F(OptionalTest, OperatorGreaterEqual) {
    CheckRelation<GreaterEqual>(NoneIneq, NoneIneq, true);
    CheckRelation<GreaterEqual>(NoneIneq, InequalityRhs, false);
    CheckRelation<GreaterEqual>(InequalityLhs, NoneIneq, true);
  
    IsLess = false;
    CheckRelation<GreaterEqual>(InequalityLhs, InequalityRhs, true);
    IsLess = true;
    CheckRelation<GreaterEqual>(InequalityLhs, InequalityRhs, false);
  }


https://reviews.llvm.org/D23178