This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 06/20/2017 10:59 AM, Martin Sebor wrote:
On 06/20/2017 02:41 AM, Aldy Hernandez wrote:On 05/23/2017 03:26 PM, Martin Sebor wrote:On 05/23/2017 04:48 AM, Aldy Hernandez wrote:+ void Union (wide_int x, wide_int y); + bool Union (const irange &r); + bool Union (const irange &r1, const irange &r2); + + // THIS = THIS ^ [X,Y]. Return TRUE if result is non-empty. + bool Intersect (wide_int x, wide_int y, bool readonly = false); + // THIS = THIS ^ R. Return TRUE if result is non-empty. + // THIS = R1 ^ R2. Return TRUE if result is non-empty. + bool Intersect (const irange &r1, const irange &r2, bool readonly = false); + // Return TRUE if THIS ^ R will be non-empty. + bool Intersect_p (const irange &r) + { return Intersect (r, /*readonly=*/true); } I would suggest the following changes to Union, Intersect, and Not: 1) Define all three members without the readonly argument and returning irange& (i.e., *this). The return value can be used wherever irange& is expected, and the is_empty() member function can be called on it to obtain the same result. E.g., Intersect A with B, storing the result in A: irange A, B; if (A.Intersect (B).is_empty ()) { ... } 2) Add non-members like so: irange range_union (const irange &lhs, const irange &rhs) { return irange (lhs).Union (rhs); } and find out if the union of A or B is empty without modifying either argument: irange A, B; if (range_union (A, B).is_empty ()) { ... }Perhaps we could provide an implicit conversion from irange to bool such that we could write: if (range_union (A, B)) { ... } as well as being able to write: if (!range_union (A, B).is_empty ()) { ... } That is, have range_union() return an irange as suggested, but have a bool overload (or whatever the C++ nomenclature is) such that converting an irange to a bool is interpreted as ``nitems != 0''. Is this acceptable C++ practice?Implicit conversion to bool is a common way of testing validity but I don't think it would be too surprising to use it as a test for non-emptiness. An alternative to consider is to provide an implicit conversion to an unsigned integer (instead of num_ranges()(*)) and have it return the number of ranges. That will make it possible to do the same thing as above while also simplifying the API.
I really like this.I have added an implicit conversion to unsigned that returns the number of sub-ranges. However, I have also left the empty_p() method for a cleaner interface within the class itself. It feels wrong to type "if (!*this)" to represent emptiness within the class.
Martin [*] FWIW, there's nothing wrong with the name num_ranges() but those familiar with the C++ standard library are going to be accustomed to size() as the name of a function that returns the number of elements in a container. Since the irange class is an ordered sequence of ranges, size() would work for it too.
In my upcoming patch I have renamed num_ranges() to num_pairs() which I thought was clearer, but if you still feel strongly about this, I can rename to size().
PS Thinking of the irange class as a container of ranges suggests the design might benefit from introducing a simple lower-level abstraction (class) for a single contiguous range.
Hmmm, indeed. Perhaps when the dust settles with the patch I'm about to post :).
Thanks for your suggestions. Aldy
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |