[PATCH] D60662: [ConstantRange] Simplify unittests after getSetSize was removed

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 14 01:47:45 PDT 2019


nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM. The end result here looks better to me than the original getSetSize() based code, so we'd want to do these changes even independently of that removal.



================
Comment at: unittests/IR/ConstantRangeTest.cpp:1463-1464
     // Loop over all N1 in CR1 and N2 in CR2 and check whether any of the
     // operations have overflow / have no overflow. These loops are based
     // on Size1/Size2 to properly handle empty/full ranges.
     bool RangeHasOverflow = false;
----------------
The "These loops..." etc part of this comment should be dropped, as it's no longer relevant with the current implementation.


================
Comment at: unittests/IR/ConstantRangeTest.cpp:1468
+    ForeachNumInConstantRange(CR1, [&](const APInt &N1) {
+      assert(CR1.contains(N1));
+      ForeachNumInConstantRange(CR2, [&](const APInt &N2) {
----------------
I think we should drop these two asserts (or at least put them inside ForeachNumInConstantRange, but I don't think it's worthwhile to have them).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60662/new/

https://reviews.llvm.org/D60662





More information about the llvm-commits mailing list