vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.
Thanks, looks good with nitpicks.
================
Comment at: unittests/Core/RangeTest.cpp:139
+ RangeT r;
+ // FIXME: This is probably not intended.
+ EXPECT_TRUE(r.ContainsEndInclusive(0));
----------------
Yeah, this looks wrong.
================
Comment at: unittests/Core/RangeTest.cpp:177
+ EXPECT_TRUE(r.Contains(RangeT(3, 0)));
+ EXPECT_TRUE(r.Contains(RangeT(4, 0)));
+ EXPECT_FALSE(r.Contains(RangeT(8, 0)));
----------------
Yep, the first two results are at least inconsistent with the third, and look
wrong to me.
================
Comment at: unittests/Core/RangeTest.cpp:250
+
+// For less than, we should
+TEST(RangeTest, LessThan) {
----------------
Drop this comment?
================
Comment at: unittests/Core/RangeTest.cpp:253
+ RangeT r(10, 20);
+
+ // Equal range.
----------------
Might help to have a helper lambda here, like `expectOrderedLessThan = [](r1,
r2) { expect_true(r1 < r2); expect_false(r2 < r1); }`.
https://reviews.llvm.org/D50620
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
