[PATCH] D59716: [ConstantRange] Add full() + empty() named constructors (NFC)

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 23 06:01:06 PDT 2019


nikic added a comment.

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

> Looks like a nice cleanup! The naming choice is a bit controversial i think.
>  Is it fully obvious that `full()`/`empty()` don't check that it's full/empty and return bool, but return new full/empty range?
>  Any more obvious choices? Or am i imagining this?


I think the static constructors `ConstantRange::full(BW)` and `ConstantRange::empty(BW)` are unambiguous, they can't really be interpreted as a boolean check. The `CR.full()` and `CR.empty()` methods do look like boolean checks when written in that way. This is part of the reason why I made those methods private: Inside `ConstantRange.cpp` it's clear (imho) that something like `return empty()` returns an empty range, while this would not necessarily be obvious outside of it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59716





More information about the llvm-commits mailing list