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

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

On 08/31/18 18:45, Jeff Law wrote:
> 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.
> 

Ah, that you mean.

I think it should be fine, since the main purpose of eval_check_narrowing
was to call check_narrowing on the value, which is normally done by reshape_init
but this is by-passed if braced_list_to_string is successful.

It is much smarter to call braced_list_to_string that after digest_init completed.


> 
>>
>>>
>>>> -    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.
> 

Yes, indeed, the change log is wrong, it should be

* g++.dg/init/string2.C: Adjust test expectations.


> 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
> 


Thanks
Bernd.