clayborg added inline comments.
================ Comment at: unittests/Utility/VMRangeTest.cpp:42 + VMRange range1(0x100, 0x200); + VMRange range2(0x100, 0x200); + EXPECT_EQ(range1, range2); ---------------- Seems like a few more edge cases might be good: ``` EXPECT_NE(range1, VMRange(0x100, 0x1ff)); EXPECT_NE(range1, VMRange(0x100, 0x201)); EXPECT_NE(range1, VMRange(0x0ff, 0x200)); EXPECT_NE(range1, VMRange(0x101 0x200)); ``` ================ Comment at: unittests/Utility/VMRangeTest.cpp:71 + EXPECT_FALSE(range.Contains(0x00)); + EXPECT_FALSE(range.Contains(0xFE)); + EXPECT_FALSE(range.Contains(0xFF)); ---------------- Probably don't need the 0xFE as 0xFF will test the condition of an address before 0x100 ================ Comment at: unittests/Utility/VMRangeTest.cpp:78 + EXPECT_FALSE(range.Contains(0x201)); + EXPECT_FALSE(range.Contains(0xFFF)); +} ---------------- Should probably test the UINT64_MAX address instead of just 0xfff as the last edge case ================ Comment at: unittests/Utility/VMRangeTest.cpp:88 + EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101))); + EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105))); + EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105))); ---------------- add a zero sized range check where the address is inside the other: ``` EXPECT_FALSE(range.Contains(VMRange(0x100, 0x100))); ``` ================ Comment at: unittests/Utility/VMRangeTest.cpp:91 + EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF))); + EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200))); + EXPECT_FALSE(range.Contains(VMRange(0x105, 0x208))); ---------------- Add a test to see that an equal range is also contained: ``` EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)); ``` ================ Comment at: unittests/Utility/VMRangeTest.cpp:92 + EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200))); + EXPECT_FALSE(range.Contains(VMRange(0x105, 0x208))); + EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201))); ---------------- change 0x208 to 0x201 so it is just outside the range ================ Comment at: unittests/Utility/VMRangeTest.cpp:92 + EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200))); + EXPECT_FALSE(range.Contains(VMRange(0x105, 0x208))); + EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201))); ---------------- clayborg wrote: > change 0x208 to 0x201 so it is just outside the range Add check for MAX address: EXPECT_FALSE(range.Contains(VMRange(0x105, UINT64_MAX))); ================ Comment at: unittests/Utility/VMRangeTest.cpp:103 + VMRange range3(0x100, 0x200); + VMRange range4(0xFFF, 0x1000); + ---------------- Test ordering of empty ranges? Not sure if that makes sense https://reviews.llvm.org/D49415 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
