This is an archive of the discontinued LLVM Phabricator instance.

Added test for Core/Range class.
ClosedPublic

Authored by teemperor on Aug 12 2018, 10:29 PM.

Details

Summary

We can optimize and refactor some of the classes in RangeMap.h, but first
we should have some tests for all the data structures in there. This adds a first
batch of tests for the Range class itself.

There are some unexpected results happening when mixing invalid and valid ranges, so
I added some FIXME's for that in the tests.

Diff Detail

Repository
rL LLVM

Event Timeline

teemperor created this revision.Aug 12 2018, 10:29 PM

(Also we maybe might want to merge the functionalities of Range and VMRange in the future).

  • Fixed typo in CMakeLists
vsk accepted this revision.Aug 13 2018, 11:27 AM

Thanks, looks good with nitpicks.

unittests/Core/RangeTest.cpp
139 ↗(On Diff #160286)

Yeah, this looks wrong.

177 ↗(On Diff #160286)

Yep, the first two results are at least inconsistent with the third, and look wrong to me.

250 ↗(On Diff #160286)

Drop this comment?

253 ↗(On Diff #160286)

Might help to have a helper lambda here, like expectOrderedLessThan = [](r1, r2) { expect_true(r1 < r2); expect_false(r2 < r1); }.

This revision is now accepted and ready to land.Aug 13 2018, 11:27 AM
teemperor updated this revision to Diff 160439.Aug 13 2018, 1:42 PM
  • Addressed Vedant's comments (thanks!)
This revision was automatically updated to reflect the committed changes.