[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

[Greg Clayton via Phabricator via lldb-commits]

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