[PATCH] support: Add TEST_COMPARE macro
Paul Eggert
eggert@cs.ucla.edu
Fri Dec 1 19:06:00 GMT 2017
On 12/01/2017 07:53 AM, Florian Weimer wrote:
> +/* Compare the two integers LEFT and RIGHT and report failure if they
> + are different. */
> +#define TEST_COMPARE(left, right) \
> + ({ \
> + typedef __typeof__ (left) __left_type; \
> + typedef __typeof__ (right) __right_type; \
> + __left_type __left_value = (left); \
> + __right_type __right_value = (right); \
> + /* Prevent use with floating-point and boolean types. */ \
Why prevent booleans? They don't have problems when compared as integers.
> + _Static_assert ((__left_type) 0.1 == (__left_type) 0, \
> + "left value has floating-point type"); \
I suggest changing this to something like "(__left_type) 1.5 ==
(__left_type) 1", so that boolean types are allowed, and so that the
expression matches the string.
> + /* Make sure that type promotion does not alter the sign. */ \
> + enum \
> + { \
> + __left_is_unsigned = (__left_type) -1 > 0, \
> + __right_is_unsigned = (__right_type) -1 > 0, \
> + __left_promotes_to_right = __left_is_unsigned \
> + && sizeof (__left_value) < sizeof (__right_value), \
> + __right_promotes_to_left = __right_is_unsigned \
> + && sizeof (__right_value) < sizeof (__left_value) \
> + }; \
> + _Static_assert (__left_is_unsigned == __right_is_unsigned \
> + || __left_promotes_to_right \
> + || __right_promotes_to_left, \
> + "integer promotion may alter sign of operands"); \
This confuses integer promotion (which always converts to int or to
unsigned) with the usual integer conversions (which can convert to a
type wider than int or unsigned int). Integer promotion does not cause
any problems that TEST_COMPARE is trying to detect; on the contrary,
promotion helps to prevent such problems.
For example, if __left_value is signed char and __right_value is
unsigned short and unsigned short is narrower than int, the static
assertion will fail even though the comparison is safe because both
operands are promoted to int before comparison.
Please see the proposal in
<https://sourceware.org/ml/libc-alpha/2017-11/msg00903.html>, which
should do the right thing for such cases.
More information about the Libc-alpha
mailing list