[PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Chen Gang]

The maximize 64-bits integer decimal string length excluding NUL is 20 (
'-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT.

2014-11-16  Chen Gang  <gang.chen.5i5j@gmail.com>

	* c-family/c-cppbuiltin.c (builtin_define_with_int_value):  Use
	20 instead of 18 for the maximize 64-bits integer decimal
	string length
---
 gcc/c-family/c-cppbuiltin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index 26fabc2..8e8cec4 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1345,7 +1345,7 @@ builtin_define_with_int_value (const char *macro, HOST_WIDE_INT value)
 {
   char *buf;
   size_t mlen = strlen (macro);
-  size_t vlen = 18;
+  size_t vlen = 20; /* maximize value length: -9223372036854775808 */
   size_t extra = 2; /* space for = and NUL.  */
 
   buf = (char *) alloca (mlen + vlen + extra);
-- 
1.9.3

Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Joseph Myers]

On Sun, 16 Nov 2014, Chen Gang wrote:

> The maximize 64-bits integer decimal string length excluding NUL is 20 (
> '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT.
> 
> 2014-11-16  Chen Gang  <gang.chen.5i5j@gmail.com>
> 
> 	* c-family/c-cppbuiltin.c (builtin_define_with_int_value):  Use
> 	20 instead of 18 for the maximize 64-bits integer decimal
> 	string length

OK.  (Though it's not a good idea to use builtin_define_with_int_value 
with large arguments, as it won't generate any suffixes for arguments 
outside the range of target int, and -9223372036854775808 would actually 
need to be expressed as (-9223372036854775807LL-1).  I think it's OK that 
it doesn't parenthesize negative numbers when outputting them - that the 
only cases for which the lack of parentheses could affect the parse are 
invalid for other reasons - though parentheses around negative numbers 
output might be a good idea anyway to make it obvious the output is OK, 
and would accord with how some macros such as __*_MIN_EXP__ are output; 
those values should probably use builtin_define_with_int_value.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Chen Gang]

On 11/19/14 0:44, Joseph Myers wrote:
> On Sun, 16 Nov 2014, Chen Gang wrote:
> 
>> The maximize 64-bits integer decimal string length excluding NUL is 20 (
>> '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT.
>>
>> 2014-11-16  Chen Gang  <gang.chen.5i5j@gmail.com>
>>
>> 	* c-family/c-cppbuiltin.c (builtin_define_with_int_value):  Use
>> 	20 instead of 18 for the maximize 64-bits integer decimal
>> 	string length
> 
> OK.  (Though it's not a good idea to use builtin_define_with_int_value 
> with large arguments, as it won't generate any suffixes for arguments 
> outside the range of target int, and -9223372036854775808 would actually 
> need to be expressed as (-9223372036854775807LL-1).  I think it's OK that 
> it doesn't parenthesize negative numbers when outputting them - that the 
> only cases for which the lack of parentheses could affect the parse are 
> invalid for other reasons - though parentheses around negative numbers 
> output might be a good idea anyway to make it obvious the output is OK, 
> and would accord with how some macros such as __*_MIN_EXP__ are output; 
> those values should probably use builtin_define_with_int_value.)
> 

OK, thanks, what you said sounds reasonable to me. We need '(' and ')'
for negative members, and "LL" for the members which is larger than 32
bits.

For me, for simplify thinking, can let all numbers have '(', ')' and
"LL" (whether it is positive numbers or negative numbers, whether it is
large numbers or small numbers), and one sprintf() is enough:

  sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC")LL", macro, value);

And excuse me, I do not understand why use "(-9223372036854775807LL-1)"
instead of "(-9223372036854775808LL)": compiler will report warning for
it, but for me, it is more likely the compiler's own issue:

  bash-3.2# cat ./test.c 
  #include <stdio.h>
  
  int main ()
  {
    long long i = (-9223372036854775808LL);
    long long j = 0x8000000000000000LL;
    long long k = 0x7fffffffffffffffLL;
  
    printf("\ni: %lld, j: %lld, k: %lld\n", i, j, k);
  
    return 0;
  }
  bash-3.2# gcc -o test test.c
  test.c:5:19: warning: integer constant is larger than the largest signed integer type
    long long i = (-9223372036854775808LL);
                    ^
  1 warning generated.
  bash-3.2# ./test 
  
  i: -9223372036854775808, j: -9223372036854775808, k: 9223372036854775807



Thanks
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Chen Gang]

On 11/19/14 11:03, Chen Gang wrote:
> On 11/19/14 0:44, Joseph Myers wrote:
>> On Sun, 16 Nov 2014, Chen Gang wrote:
>>
>>> The maximize 64-bits integer decimal string length excluding NUL is 20 (
>>> '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT.
>>>
>>> 2014-11-16  Chen Gang  <gang.chen.5i5j@gmail.com>
>>>
>>> 	* c-family/c-cppbuiltin.c (builtin_define_with_int_value):  Use
>>> 	20 instead of 18 for the maximize 64-bits integer decimal
>>> 	string length
>>
>> OK.  (Though it's not a good idea to use builtin_define_with_int_value 
>> with large arguments, as it won't generate any suffixes for arguments 
>> outside the range of target int, and -9223372036854775808 would actually 
>> need to be expressed as (-9223372036854775807LL-1).  I think it's OK that 
>> it doesn't parenthesize negative numbers when outputting them - that the 
>> only cases for which the lack of parentheses could affect the parse are 
>> invalid for other reasons - though parentheses around negative numbers 
>> output might be a good idea anyway to make it obvious the output is OK, 
>> and would accord with how some macros such as __*_MIN_EXP__ are output; 
>> those values should probably use builtin_define_with_int_value.)
>>
> 
> OK, thanks, what you said sounds reasonable to me. We need '(' and ')'
> for negative members, and "LL" for the members which is larger than 32
> bits.
> 
> For me, for simplify thinking, can let all numbers have '(', ')' and
> "LL" (whether it is positive numbers or negative numbers, whether it is
> large numbers or small numbers), and one sprintf() is enough:
> 
>   sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC")LL", macro, value);

Oh, sorry, need be:

  sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC"LL)", macro, value);

> 
> And excuse me, I do not understand why use "(-9223372036854775807LL-1)"
> instead of "(-9223372036854775808LL)": compiler will report warning for
> it, but for me, it is more likely the compiler's own issue:
> 
>   bash-3.2# cat ./test.c 
>   #include <stdio.h>
>   
>   int main ()
>   {
>     long long i = (-9223372036854775808LL);
>     long long j = 0x8000000000000000LL;
>     long long k = 0x7fffffffffffffffLL;
>   
>     printf("\ni: %lld, j: %lld, k: %lld\n", i, j, k);
>   
>     return 0;
>   }
>   bash-3.2# gcc -o test test.c
>   test.c:5:19: warning: integer constant is larger than the largest signed integer type
>     long long i = (-9223372036854775808LL);
>                     ^
>   1 warning generated.
>   bash-3.2# ./test 
>   
>   i: -9223372036854775808, j: -9223372036854775808, k: 9223372036854775807
> 
> 
> 
> Thanks
> 

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Joseph Myers]

On Wed, 19 Nov 2014, Chen Gang wrote:

> OK, thanks, what you said sounds reasonable to me. We need '(' and ')'
> for negative members, and "LL" for the members which is larger than 32
> bits.

I don't think LL is a good idea when not needed - quite possibly some of 
the macros are expected or required to have type int.  Really I think it 
would be better to require that this function is only used for values that 
fit in target int (given appropriate checks on all the users to make sure 
they fit in with that), and put a corresponding assertion there.

> And excuse me, I do not understand why use "(-9223372036854775807LL-1)"
> instead of "(-9223372036854775808LL)": compiler will report warning for
> it, but for me, it is more likely the compiler's own issue:

Because - and 9223372036854775808LL are separate tokens, and the latter is 
not a valid long long value.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Chen Gang]

On 11/20/14 1:20, Joseph Myers wrote:
> On Wed, 19 Nov 2014, Chen Gang wrote:
> 
>> OK, thanks, what you said sounds reasonable to me. We need '(' and ')'
>> for negative members, and "LL" for the members which is larger than 32
>> bits.
> 
> I don't think LL is a good idea when not needed - quite possibly some of 
> the macros are expected or required to have type int.

OK, thanks. I guess your meaning is:

 - If the value is small enough to be expressed by type 'int', we should
   not provide 'LL'.

 - If the value is positive number, we should not provide '(' and ')'.

If what I guess is correct, I shall try to send patch v2 for it within
this month.

>                                                       Really I think it 
> would be better to require that this function is only used for values that 
> fit in target int (given appropriate checks on all the users to make sure 
> they fit in with that), and put a corresponding assertion there.
> 

Excuse me, I am not quite familiar with "target int", could you provide
more details for it? And if necessary, please help send patch v2 instead
of me:

 - We need provide the best code for upstream. I guess, at present, you
   can, but I can't (I am not quite familiar with the related things).

 - I have planned to send another v2 patches for gcc, and also analyze
   some issues within gcc. I should finish them within this month (it
   seems, at present, I may delay again).

 - If can bear me, still let me send patch for it, I may send patch v2
   in next month.

Thanks.

>> And excuse me, I do not understand why use "(-9223372036854775807LL-1)"
>> instead of "(-9223372036854775808LL)": compiler will report warning for
>> it, but for me, it is more likely the compiler's own issue:
> 
> Because - and 9223372036854775808LL are separate tokens, and the latter is 
> not a valid long long value.
> 

OK, thanks. But for the user (e.g. a normal programmer like me), will
feel that:

 - (-9223372036854775808LL) is a valid value, since it prints correctly.

 - If it is reasonable to report warning for (-9223372036854775808LL),
   why do not also report warning for 0x8000000000000000LL.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Joseph Myers]

On Thu, 20 Nov 2014, Chen Gang wrote:

> OK, thanks. I guess your meaning is:
> 
>  - If the value is small enough to be expressed by type 'int', we should
>    not provide 'LL'.

Yes - and values not small enough should probably not be accepted by this 
function.

>  - If the value is positive number, we should not provide '(' and ')'.

Yes.

> >                                                       Really I think it 
> > would be better to require that this function is only used for values that 
> > fit in target int (given appropriate checks on all the users to make sure 
> > they fit in with that), and put a corresponding assertion there.
> > 
> 
> Excuse me, I am not quite familiar with "target int", could you provide
> more details for it? And if necessary, please help send patch v2 instead
> of me:

"target int" is the type int on the target, as opposed to on the host 
(with precision TYPE_PRECISION (integer_type_node)).  I think you can use 
wi::fits_to_tree_p (n, integer_type_node) to test if n is in range for 
target int.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length

[Chen Gang]

OK, thank you for your details description.

I shall send patch v2 for it, within this week.

Send from Lenovo A788t.

Joseph Myers <joseph@codesourcery.com> wrote:

>On Thu, 20 Nov 2014, Chen Gang wrote:
>
>> OK, thanks. I guess your meaning is:
>> 
>>  - If the value is small enough to be expressed by type 'int', we should
>>    not provide 'LL'.
>
>Yes - and values not small enough should probably not be accepted by this 
>function.
>
>>  - If the value is positive number, we should not provide '(' and ')'.
>
>Yes.
>
>> >                                                       Really I think it 
>> > would be better to require that this function is only used for values that 
>> > fit in target int (given appropriate checks on all the users to make sure 
>> > they fit in with that), and put a corresponding assertion there.
>> > 
>> 
>> Excuse me, I am not quite familiar with "target int", could you provide
>> more details for it? And if necessary, please help send patch v2 instead
>> of me:
>
>"target int" is the type int on the target, as opposed to on the host 
>(with precision TYPE_PRECISION (integer_type_node)).  I think you can use 
>wi::fits_to_tree_p (n, integer_type_node) to test if n is in range for 
>target int.
>
>-- 
>Joseph S. Myers
>joseph@codesourcery.com