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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D60662#1465673 <https://reviews.llvm.org/D60662#1465673>, @lebedev.ri wrote:

> In D60662#1465668 <https://reviews.llvm.org/D60662#1465668>, @MaskRay wrote:
>
> > > The commit message was
> > > 
> > >> The last use of this helper method was removed by rL302385 <https://reviews.llvm.org/rL302385>.
> > > 
> > > Which is clearly not true, as this commit removed more uses of that method, that weren't testing the method itself, but using it. Therefore i'm questioning the whole removal. Moving it into a static function in tests is another, unrelated matter.
> >
> > That commit didn't remove more uses of that method. The last library use was removed two years ago and no use has been added since then. I deleted it, and then realized there was a unittest relying on it.
>
>
> That is precisely my point, it was still used, in test.
>  As you have just wrote, that wasn't considered when dropping the method.


When my first commit was made, it was indeed used in the unittest (I don't index `/(clang|lld|llvm)/(test|unittests)/` with my language server so I missed the references in unittests, it seems I should), but I soon fixed that (the second commit). It wasn't elegant and I saw your complaint, so I created this revision.

>>> and the way the test was rewritten to avoid it looks more complicated than it was with the function, at least to me..
>>> 
>>> Please just revert those two commits.
>> 
>> I agree that the last commit made it more complex than it should. I apologize for that. So I created this revision. However, "more complicated than it was with the function" is your opinion and I'm not convinced of that judgement. So, I'd like to get more input from others, especially those who removed the last few library uses.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D60662