This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCHv2] Call braced_list_to_string after array size is fixed


On 08/26/2018 01:45 AM, Bernd Edlinger wrote:

>>> cp:
>>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>
>>> 	* decl.c (eval_check_narrowing): Remove.
>>> 	(check_initializer): Move call to braced_list_to_string from here ...
>>> 	* typeck2.c (store_init_value): ... to here.
>>> 	(digest_init_r): Remove handing of signed/unsigned char strings.
>>>
>>> testsuite:
>>> 2018-08-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>
>>> 	* c-c++-common/array-init.c: New test.
>>> 	* g++.dg/init/string2.C: Remove xfail.
>> My concern here is that you removed code that was explicitly added
>> during the initial review work of the patch that turned braced
>> initializers into strings.
>>
> 
> Yes, you mean BRACE_ENCLOSED_INITIALIZER_P
> which is (TREE_CODE (NODE) == CONSTRUCTOR && TREE_TYPE (NODE) == init_list_type_node)
I was referring to the callback into the eval routine from within
braced_list_to_string which is related to the concern where you dropped
the c++98_only selector.  Sorry I wasn't clear about that.

[ snip ]

> 
>>
>>> -    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
>>> +    a[] = { 1, 2, 333, 0 };         // { dg-warning "\\\[\(-Wnarrowing|-Woverflow\)" }
>> This is not an XFAIL, this is a selector.  Essentially it says that the
>> diagnostic is appropriate when not in c++98 mode.
>>
>> You can see that to be the case if you compile the test in c++98 mode
>> without your change.  It will compile with no errors or warnings.
>>
>> However, after your change it issues a warning for the narrowing
>> conversion in c++98 mode, which AFAICT it should not do.
>>
> 
> This just restores the state _before_ Martin's braced initializer patch.
> So I have the impression that is actually a regression due to the
> original braced initializer patch.
> It is unfortunate that Martin did not check that.
The ChangeLog said it was removing an xfail, so naturally when I saw it
was removing a selector I assumed that the selector was right
(particularly since it made sense given current behavior) and you'd made
a minor goof.

But I can confirm that prior to Martin's changes we did issue a
diagnostic when in C++98 mode:

j.C: In instantiation of ‘int tmplen() [with T = char]’:
j.C:61:27:   required from here
j.C:57:5: warning: overflow in conversion from ‘int’ to ‘char’ changes
value from ‘333’ to ‘'M'’ [-Woverflow]
57 |     a[] = { 1, 2, 333, 0 };         // { dg-warning
"\\\[\(-Wnarrowing|-Woverflow\)" "" { target { ! c++98_only } } }
   |     ^

You can see that with the trunk just before Martin's change or with
older compilers (I also tested with 6.4.1 as I had it handy).

So I'll withdraw my objection to this change.  I'll dig into your
updated patch momentarily.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]