On Tue, Nov 5, 2019 at 2:15 PM Aldy Hernandez <aldyh@redhat.com> wrote:
The function range_int_cst_p only works with VR_RANGE's at the moment.
This is silly because VR_ANTI_RANGE and even VR_VARYING can contain
numeric bounds. I have fixed this oversight and have made the function
return the bounds in MIN/MAX. This simplifies a lot of code, because
there is no longer a need to special case VR_VARYING and VR_ANTI_RANGE,
as well as pick at the individual range components outside of the API.
The patch has the pleasant side-effect of bringing more things into the
API fold. Basically, any access to either value_range::min(), max(), or
kind(), is suspect and a big hint that the code should be rewritten to
use the API (contains_p, varying_p, zero_p, etc).
One of the primary culprits of API noncompliance is the sprintf and
strlen warning code. Mind you, not due to negligence on the author, but
because we had no value-range API when Martin added the passes. I
realize it's nobody's responsibility to fix older value-range code, and
I'll probably end up doing it myself (next cycle??), but I could
definitely use a hand from the experts, as it's intricate and delicate code.
Speak of which, in converting dump_strlen_info() to use the new
range_int_cst_p, I noticed a lot of the code disappeared if we used the
API. Martin, if you'd prefer not to dump varying, undefined, etc, let
me know and we can gate that call to vr.dump(). I took the liberty
because it was simple, clean, and hidden away in an internal debugging
helper.
OK for trunk?
No. It's a semantic change, no? Don't we for VR_ANTI_RANGE
always get [-INF, +INF] back then? Likewise for VARYING?
What do we get for UNDEFINED? I think callers are not prepared
for this and expect it to return true for "useful" ranges only.
If you want this, use a new name, like get_range_bounds ().
Also not sure why min/max need to be INTEGER_CST, why
not _always_ return something (that is, the fucntion should never
need to return false).
The patch doesn't look like an improvement, it just adds to confusion.
Richard.