PATCH COMMITTED: Don't break tests for enum in range

[Ian Lance Taylor]

Eric Botcazou <ebotcazou@adacore.com> writes:

> > I noticed that gcc was in some cases breaking tests for whether a
> > value was in range for an enum.  An example is this, in C++:
> >
> > enum E { V = 1 };
> > static const E E_MIN = V;
> > static const E E_MAX = V;
> >
> > bool valid(E v) { return v >= E_MIN && v <= E_MAX; }
> >
> > int main() { return valid(E(2)); }
> >
> > Here the test in main was returning 1 when one would expect it to
> > return 0.
> 
> Quite interesting, this is precisely THE situation where VRP breaks Ada if 
> counter-measures a la VIEW_CONVERT_EXPR are not used.  AFAIK the C++ compiler 
> doesn't generate VIEW_CONVERT_EXPR in this case, so can't VRP draw the same 
> wrong conclusion too?

I *think* that for a case like this VRP will tend to draw the right
conclusion: that valid should return false.


> > Looking at the code, the cases where range_successor and
> > range_predecessor return NULL are cases where the ranges being merged
> > are contradictory.  For example, in the above test case it happens
> > when merging [L1,H1] and [L2,H2] such that L1 == L2 and H1 < H2.  The
> > code asks for the successor of H1, and fails.  But note that H1 < H2,
> > so if H1 has no successor, where did H2 come from (in truth it came
> > from TYPE_MAX_VALUE of type_for_mode)?  Rather than return a nonsense
> > result, I think it's better for merge_ranges to simply punt.  And
> > making that change fixes the bug.
> 
> Could you clarify in which case we are exactly for merge_ranges?

make_range is dealing with "(int) v <= E_MAX".  The enum is unsigned,
so when handling the NOP_EXPR it enter this code in make_range:

	  if (!TYPE_UNSIGNED (exp_type) && TYPE_UNSIGNED (arg0_type))

Then low == NULL, so it does this one:

		  /* Otherwise, "or" the range with the range of the input
		     that will be interpreted as negative.  */
		  if (! merge_ranges (&n_in_p, &n_low, &n_high,
				      0, n_low, n_high, 1,
				      fold_convert (arg0_type,
						    integer_zero_node),
				      high_positive))

Here n_low == NULL and n_high == 1 in type E.  high_positive is
0x7fffffff, also in type E (note this is out of range according to
TYPE_MAX_VALUE, but not according to TYPE_PRECISION which is 32).

In merge_ranges, low0 is NULL, high0 is 1, low1 is 0, high1 is
0x7fffffff, in0_p is false, in1_p is true, lowequal is false,
highequal is false, no_overlap is false, subset is false.  We come to
this code:

	  low = range_successor (high0);
	  high = high1;
	  in_p = (low != 0);

range_successor (high0) returns NULL.  We wind up setting *plow to
NULL, *phigh to 0x7fffffff, and *pin_p to 0.  Since we have an
unsigned type, this is the range [0x80000000, 0xffffffff].  This is
meaningless.  The range we actually want is [2, 0x7fffffff].
Alternatively it would probably work to return an empty range, if we
had a way to represent that.  Returning [0x80000000, 0xffffffff] gives
us garbage.  When that range is merged with the range from the other
side of the conditional, we get the wrong answer.

Ian