Re: [PATCHv3 2/3] Refactor tst-strtod-round.c for type-generic-ness

["Paul E. Murphy" <murphyp at linux dot vnet dot ibm dot com>]

On 05/24/2016 10:25 AM, Joseph Myers wrote:
> On Mon, 23 May 2016, Paul E. Murphy wrote:
> 
>> +/* Define the long double choose (LDC) macro
>> +   to select the appropriate generated long double
>> +   value from the generated test data.  */
>> +#if LDBL_MANT_DIG == 53 && LDBL_MAX_EXP == 1024
>> +/* This is for the long double == double format.  */
>> +# define LDC(a, ...) a
> 
>> +/* This macro is used in conjunction with the output from the
>> +   gen-tst-strtod-round utility to select the appropriately
>> +   rounded long double value for a given format.  */
>> +#define TEST(s,							\
> 
>> +struct test_exactness
>> +  {
>> +  STRUCT_FOREACH_TYPE ()
>> +  };
>> +
>> +struct test_results
>> +  {
>> +  STRUCT_FOREACH_TYPE ()
>> +  };
>> +
>> +struct test {
>> +  const char *s;
>> +  struct test_exactness exact;
>> +  struct test_results r[4];
>> +};
> 
> All this code seems extremely specific to the tst-strtod-round test.  I 
> think it belongs there, not in a header that's supposed to be about common 
> code for making multiple strtod tests type-generic.

My intent for the header is to move all the type specific bits into
one file to make the test themselves type-generic.  If we want it to be
exclusively common code supporting type generic tests, I should push
all of the changes to tst-strtod.h back into tst-strtod-round.c.

My thinking was that in the future, gen-tst-strtod-round could be
extended slightly to generate full test cases for strtod, and thus
replace many of the existing tests in their entirety.  Though, such
tests would probably just be added to tst-strtod-round anyways.

> 
> I think the only code this patch adds to tst-strtod.h that clearly belongs 
> there is the definition of STRUCT_FOREACH_TYPE.  Even the rounding mode 
> handling seems very specific to tst-strtod-round (to the extent of having 
> a comment about "This test").
> 
> Also, struct test_exactness previously had all its elements bool.  By 
> making it use STRUCT_FOREACH_TYPE you're giving floating-point types to 
> what are logically booleans, which I don't think is appropriate.