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