[PATCH] D59959: [ConstantRange] Add unsigned and signed intersection type

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 6 08:36:26 PDT 2019


lebedev.ri added a comment.

In D59959#1457356 <https://reviews.llvm.org/D59959#1457356>, @nikic wrote:

> I think I'd like to make two more changes here: Rename `IntersectionType` to `OperationType` or similar, as the Smallest/Unsigned/Signed classification will be useful in other places as well (e.g. it is also directly applicable to unions).


So i agree that `IntersectionType` will be misleading for other ops. But operation is a bit too generic maybe?
We are saying "please intersect these two ranges, and out of all the intersections (common elements? overlapped elements?, ???), //try// to pick smallest/one with no wrap/...".
So i wonder if it would be better not to talk about *operation*, but about the result of operation?
How does e.g. `DesiredRangeVariant` sound?

> Furthermore I'd like to change the enum class to an enum, as I don't think that `ConstantRange::OperationType::Smallest` adds anything over just `ConstantRange::Smallest`, beyond being unwieldy.

As long as it stays within `ConstantRange` prefix, i think that could be fine.


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

https://reviews.llvm.org/D59959





More information about the llvm-commits mailing list