Message ID | 4EA5503C.7070202@oracle.com |
---|---|
State | New |
Headers | show |
On Mon, Oct 24, 2011 at 6:47 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Hi, > > the below is a new variant removing -Wc++0x-compat from -Wall (cannot be > added to -Wextra either because bootstrap passes -W) and also, as requested > by Gaby, preventing -Wno-narrowing from suppressing the warning in C++0x > mode (if the user really needs to silence it, -Wno-c++0x-compat works). I > also added a new testcase for that. > OK with a minor correction. This bit +With -std=c++0x, @option{-Wno-c++0x-compat} can be used to suppress +the diagnostic required by the standard. should not be there. It is currently an accident of implementation detail as opposed to a feature. It needs no advertisement.
Hi, > On Mon, Oct 24, 2011 at 6:47 AM, Paolo Carlini<paolo.carlini@oracle.com> wrote: >> Hi, >> >> the below is a new variant removing -Wc++0x-compat from -Wall (cannot be >> added to -Wextra either because bootstrap passes -W) and also, as requested >> by Gaby, preventing -Wno-narrowing from suppressing the warning in C++0x >> mode (if the user really needs to silence it, -Wno-c++0x-compat works). I >> also added a new testcase for that. >> > OK with a minor correction. This bit > > +With -std=c++0x, @option{-Wno-c++0x-compat} can be used to suppress > +the diagnostic required by the standard. > > should not be there. It is currently an accident of implementation > detail as opposed to a feature. It needs no advertisement. Ok. But I actively made it possible, if you want I can remove the possibility altogether, the patch also becomes cleaner ;) Paolo.
On Mon, Oct 24, 2011 at 7:18 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Hi, >> >> On Mon, Oct 24, 2011 at 6:47 AM, Paolo Carlini<paolo.carlini@oracle.com> >> wrote: >>> >>> Hi, >>> >>> the below is a new variant removing -Wc++0x-compat from -Wall (cannot be >>> added to -Wextra either because bootstrap passes -W) and also, as >>> requested >>> by Gaby, preventing -Wno-narrowing from suppressing the warning in C++0x >>> mode (if the user really needs to silence it, -Wno-c++0x-compat works). I >>> also added a new testcase for that. >>> >> OK with a minor correction. This bit >> >> +With -std=c++0x, @option{-Wno-c++0x-compat} can be used to suppress >> +the diagnostic required by the standard. >> >> should not be there. It is currently an accident of implementation >> detail as opposed to a feature. It needs no advertisement. > > Ok. But I actively made it possible, if you want I can remove the > possibility altogether, the patch also becomes cleaner ;) > Yes, I have been saying all long that -Wflag is not the way to suppress a standard semantics. So, if you can make the patch cleaner without that, then the better! :-)
On 10/24/2011 07:47 AM, Paolo Carlini wrote: > the below is a new variant removing -Wc++0x-compat from -Wall (cannot be > added to -Wextra either because bootstrap passes -W) I don't understand the rationale for this. If the warning is problematic for bootstrap, why not just add -Wno-narrowing to the bootstrap warning flags? I haven't read the whole discussion thread yet, though. > and also, as > requested by Gaby, preventing -Wno-narrowing from suppressing the > warning in C++0x mode (if the user really needs to silence it, > -Wno-c++0x-compat works). I also added a new testcase for that. No. I added -Wno-narrowing specifically to suppress the diagnostic in C++0x mode; see c++/49793. There are several diagnostics required by standards that can be suppressed by -Wno- flags, such as -Wno-long-long. Jason
On 10/24/2011 09:06 AM, Jason Merrill wrote: > On 10/24/2011 07:47 AM, Paolo Carlini wrote: >> the below is a new variant removing -Wc++0x-compat from -Wall (cannot be >> added to -Wextra either because bootstrap passes -W) > > I don't understand the rationale for this. If the warning is problematic > for bootstrap, why not just add -Wno-narrowing to the bootstrap warning > flags? I haven't read the whole discussion thread yet, though. OK, I read it and still think this is the right solution. >> and also, as >> requested by Gaby, preventing -Wno-narrowing from suppressing the >> warning in C++0x mode (if the user really needs to silence it, >> -Wno-c++0x-compat works). I also added a new testcase for that. > > No. I added -Wno-narrowing specifically to suppress the diagnostic in > C++0x mode; see c++/49793. There are several diagnostics required by > standards that can be suppressed by -Wno- flags, such as -Wno-long-long. ...but I'm not strongly opposed to renaming the option, so long as its function remains. Jason
On Mon, Oct 24, 2011 at 8:06 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/24/2011 07:47 AM, Paolo Carlini wrote: [...] >> and also, as >> requested by Gaby, preventing -Wno-narrowing from suppressing the >> warning in C++0x mode (if the user really needs to silence it, >> -Wno-c++0x-compat works). I also added a new testcase for that. > > No. I added -Wno-narrowing specifically to suppress the diagnostic in C++0x > mode; see c++/49793. There are several diagnostics required by standards > that can be suppressed by -Wno- flags, such as -Wno-long-long. I do not think I follow. The way we suppress a standard feature is through a non-W flag. -Wno-long-long should not have any effect at all in C++11. It may have an effect in C++03. -- Gaby
On 10/24/2011 09:26 AM, Gabriel Dos Reis wrote: > On Mon, Oct 24, 2011 at 8:06 AM, Jason Merrill<jason@redhat.com> wrote: >> No. I added -Wno-narrowing specifically to suppress the diagnostic in C++0x >> mode; see c++/49793. There are several diagnostics required by standards >> that can be suppressed by -Wno- flags, such as -Wno-long-long. > > I do not think I follow. The way we suppress a standard feature is through > a non-W flag. -Wno-long-long should not have any effect at all in C++11. > It may have an effect in C++03. Right, -Wno-long-long is only useful in C++03 and C90. But it does in fact suppress a standard diagnostic. Jason
.. just to let you know guys, I'm already unassigned from the PR, but today I wanted to give it one (actually 3) more try. Given the controversy, I don't feel like further following the issue, it just makes me nervous. Eventually, feel free to adjust my patches to your likes. Paolo.
On Mon, Oct 24, 2011 at 8:29 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/24/2011 09:26 AM, Gabriel Dos Reis wrote: >> >> On Mon, Oct 24, 2011 at 8:06 AM, Jason Merrill<jason@redhat.com> wrote: > >>> No. I added -Wno-narrowing specifically to suppress the diagnostic in >>> C++0x >>> mode; see c++/49793. There are several diagnostics required by standards >>> that can be suppressed by -Wno- flags, such as -Wno-long-long. >> >> I do not think I follow. The way we suppress a standard feature is >> through >> a non-W flag. -Wno-long-long should not have any effect at all in C++11. >> It may have an effect in C++03. > > Right, -Wno-long-long is only useful in C++03 and C90. But it does in fact > suppress a standard diagnostic. a diagnostic of an extension :-) Similarly, -Wno-narrowing suppresses diagnostic in C++03 when -Wc++0x-compat is in effect (therefore C++03). However, just like -Wno-long-long, it should not have any effect when -std=c++0x or -std=c++11.
On 10/24/2011 09:49 AM, Gabriel Dos Reis wrote: > On Mon, Oct 24, 2011 at 8:29 AM, Jason Merrill<jason@redhat.com> wrote: >> Right, -Wno-long-long is only useful in C++03 and C90. But it does in fact >> suppress a standard diagnostic. > > a diagnostic of an extension :-) I'm not going to argue semantics any further. What change do you suggest that still allows users to suppress narrowing diagnostics in C++11? Jason
On Mon, Oct 24, 2011 at 9:10 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/24/2011 09:49 AM, Gabriel Dos Reis wrote: >> >> On Mon, Oct 24, 2011 at 8:29 AM, Jason Merrill<jason@redhat.com> wrote: >>> >>> Right, -Wno-long-long is only useful in C++03 and C90. But it does in >>> fact >>> suppress a standard diagnostic. >> >> a diagnostic of an extension :-) > > I'm not going to argue semantics any further. What change do you suggest > that still allows users to suppress narrowing diagnostics in C++11? > Hmm, the narrowing semantics also affects SFINAE, not just simple declaration. If we want a flag that can also affect the outcome of overload resolution, it should one of the the -fflags, such as -fpermissive. -- Gaby
On 10/24/2011 10:39 AM, Gabriel Dos Reis wrote: > Hmm, the narrowing semantics also affects SFINAE, not just simple declaration. > If we want a flag that can also affect the outcome of overload > resolution, it should one of the the -fflags, such as -fpermissive. I don't want the option to affect SFINAE, just suppress the diagnostic when tf_error is set. There are a number of similar places in the compiler where if tf_error is set we give a pedwarn and accept the code, but if tf_error is not set we fail. Jason
On Mon, Oct 24, 2011 at 9:53 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/24/2011 10:39 AM, Gabriel Dos Reis wrote: >> >> Hmm, the narrowing semantics also affects SFINAE, not just simple >> declaration. >> If we want a flag that can also affect the outcome of overload >> resolution, it should one of the the -fflags, such as -fpermissive. > > I don't want the option to affect SFINAE, So, let me recap: 1. narrowing conversion is ill-formed in C++11; therefore a diagnostic is required. This has two implications: a. "Normal" scenario: a diagnostic is required and the program rejected. This is the one people complain about. b. SFINAE context: because the narrowing is ill-formed, the offernding expression (rather, the offending function) would just be silently ignored; no diagnostic is actually output. 2. -Wnarrowing warns about narrowing conversion, but does not reject the code. This is only for C++98, C++03, with -Wc++0x-compat While 1.b. looks like -Wnarrowing in sfinae context, it is not -Wno-narrowing because with -Wnarrowing we still accept the expression (as opposed to rejecting.) So, if you make -Wno-narrowing meaningful in C++11 mode then how can it not affect sfinae (case 1.b.) and still be consistent with the other case where a diagnostic is required the expression accepted? > just suppress the diagnostic when > tf_error is set. There are a number of similar places in the compiler where > if tf_error is set we give a pedwarn and accept the code, but if tf_error is > not set we fail.
On 10/24/2011 01:21 PM, Gabriel Dos Reis wrote: > On Mon, Oct 24, 2011 at 9:53 AM, Jason Merrill<jason@redhat.com> wrote: > So, if you make -Wno-narrowing meaningful in C++11 mode then how can > it not affect sfinae (case 1.b.) and still be consistent with the > other case where a diagnostic is required the expression accepted? Right, they will be inconsistent. But that consistency isn't relevant for legacy code, which can't have list-initialization in SFINAE context. Jason
On Mon, Oct 24, 2011 at 12:46 PM, Jason Merrill <jason@redhat.com> wrote: > On 10/24/2011 01:21 PM, Gabriel Dos Reis wrote: >> >> On Mon, Oct 24, 2011 at 9:53 AM, Jason Merrill<jason@redhat.com> wrote: >> So, if you make -Wno-narrowing meaningful in C++11 mode then how can >> it not affect sfinae (case 1.b.) and still be consistent with the >> other case where a diagnostic is required the expression accepted? > > Right, they will be inconsistent. But that consistency isn't relevant for > legacy code, which can't have list-initialization in SFINAE context. yes, but how does the compiler distinguish a "legacy code" compiled under C++11 from non-legacy C++11 code? I have no problem with C++03 codes. I do not think they are affected. The problem is with C++11 codes. There is no reason for them to be subjected to the inconsistency, especially for codes in header files that are upgraded (beyond control of the end user) and included in "legacy" codes. The "legacy" code may not have list-initialization in sfinae context, but the upgraded header file may have, without the end user knowing. It is wrong for a -Wflag to introduce that inconsistency in new codes. >
On 10/24/2011 02:13 PM, Gabriel Dos Reis wrote: > yes, but how does the compiler distinguish a "legacy code" compiled > under C++11 from non-legacy C++11 code? It doesn't. > The problem is with C++11 codes. There is no reason for them to be subjected > to the inconsistency, especially for codes in header files that are > upgraded (beyond control of the end user) and included in "legacy" codes. > The "legacy" code may not have list-initialization in sfinae context, but > the upgraded header file may have, without the end user knowing. And that header is unaffected. Only initialization outside SFINAE context is affected. > It is wrong for a -Wflag to introduce that inconsistency in new codes. What would you suggest instead? Jason
On Mon, Oct 24, 2011 at 1:17 PM, Jason Merrill <jason@redhat.com> wrote: > On 10/24/2011 02:13 PM, Gabriel Dos Reis wrote: >> The problem is with C++11 codes. There is no reason for them to be >> subjected >> to the inconsistency, especially for codes in header files that are >> upgraded (beyond control of the end user) and included in "legacy" codes. >> The "legacy" code may not have list-initialization in sfinae context, but >> the upgraded header file may have, without the end user knowing. > > And that header is unaffected. Only initialization outside SFINAE context > is affected. I am afraid I do not understand why the header will not be affected. What about (testcase) int f(char); double f(...); const int n = sizeof f({257}); ? >> It is wrong for a -Wflag to introduce that inconsistency in new codes. > > What would you suggest instead? An -fflag. If -fpermissive is too broad, then -flegacy-init or -flegacy
On 10/24/2011 02:47 PM, Gabriel Dos Reis wrote: > What about (testcase) > > int f(char); > double f(...); > > const int n = sizeof f({257}); > > ? The narrowing conversion would be marked as 'bad' and therefore the second overload chosen. As before, the objective is to only change the diagnostic, not the meaning of valid code. Jason
On Mon, Oct 24, 2011 at 2:05 PM, Jason Merrill <jason@redhat.com> wrote: > On 10/24/2011 02:47 PM, Gabriel Dos Reis wrote: >> >> What about (testcase) >> >> int f(char); >> double f(...); >> >> const int n = sizeof f({257}); >> >> ? > > The narrowing conversion would be marked as 'bad' and therefore the second > overload chosen. As before, the objective is to only change the diagnostic, > not the meaning of valid code. > OK. I think we may need better documentation of the behavior.
Index: doc/invoke.texi =================================================================== --- doc/invoke.texi (revision 180373) +++ doc/invoke.texi (working copy) @@ -2365,17 +2365,18 @@ an instance of a derived class through a pointer t base class does not have a virtual destructor. This warning is enabled by @option{-Wall}. -@item -Wno-narrowing @r{(C++ and Objective-C++ only)} +@item -Wnarrowing @r{(C++ and Objective-C++ only)} @opindex Wnarrowing @opindex Wno-narrowing -With -std=c++0x, suppress the diagnostic required by the standard for -narrowing conversions within @samp{@{ @}}, e.g. +Warn when a narrowing conversion occurs within @samp{@{ @}}, e.g. @smallexample int i = @{ 2.2 @}; // error: narrowing from double to int @end smallexample -This flag can be useful for compiling valid C++98 code in C++0x mode +This flag is included in @option{-Wc++0x-compat}. +With -std=c++0x, @option{-Wno-c++0x-compat} can be used to suppress +the diagnostic required by the standard. @item -Wnoexcept @r{(C++ and Objective-C++ only)} @opindex Wnoexcept @@ -2993,7 +2994,6 @@ Options} and @ref{Objective-C and Objective-C++ Di @gccoptlist{-Waddress @gol -Warray-bounds @r{(only with} @option{-O2}@r{)} @gol --Wc++0x-compat @gol -Wchar-subscripts @gol -Wenum-compare @r{(in C/Objc; this is on by default in C++)} @gol -Wimplicit-int @r{(C and Objective-C only)} @gol @@ -4066,7 +4066,7 @@ ISO C and ISO C++, e.g.@: request for implicit con @item -Wc++0x-compat @r{(C++ and Objective-C++ only)} Warn about C++ constructs whose meaning differs between ISO C++ 1998 and ISO C++ 200x, e.g., identifiers in ISO C++ 1998 that will become keywords -in ISO C++ 200x. This warning is enabled by @option{-Wall}. +in ISO C++ 200x. This warning turns on @option{-Wnarrowing}. @item -Wcast-qual @opindex Wcast-qual Index: c-family/c.opt =================================================================== --- c-family/c.opt (revision 180373) +++ c-family/c.opt (working copy) @@ -288,7 +288,7 @@ C ObjC Var(warn_cxx_compat) Warning Warn about C constructs that are not in the common subset of C and C++ Wc++0x-compat -C++ ObjC++ Var(warn_cxx0x_compat) Warning +C++ ObjC++ Var(warn_cxx0x_compat) Init(-1) Warning Warn about C++ constructs whose meaning differs between ISO C++ 1998 and ISO C++ 200x Wcast-qual @@ -490,8 +490,8 @@ C ObjC C++ ObjC++ Warning Warn about use of multi-character character constants Wnarrowing -C ObjC C++ ObjC++ Warning Var(warn_narrowing) Init(1) --Wno-narrowing In C++0x mode, ignore ill-formed narrowing conversions within { } +C ObjC C++ ObjC++ Warning Var(warn_narrowing) Init(-1) Warning +Warn about ill-formed narrowing conversions within { } Wnested-externs C ObjC Var(warn_nested_externs) Warning Index: c-family/c-opts.c =================================================================== --- c-family/c-opts.c (revision 180373) +++ c-family/c-opts.c (working copy) @@ -404,7 +404,6 @@ c_common_handle_option (size_t scode, const char * /* C++-specific warnings. */ warn_sign_compare = value; warn_reorder = value; - warn_cxx0x_compat = value; warn_delnonvdtor = value; } @@ -436,6 +435,10 @@ c_common_handle_option (size_t scode, const char * cpp_opts->warn_cxx_operator_names = value; break; + case OPT_Wc__0x_compat: + warn_narrowing = value; + break; + case OPT_Wdeprecated: cpp_opts->cpp_warn_deprecated = value; break; @@ -997,11 +1000,22 @@ c_common_post_options (const char **pfilename) if (warn_implicit_function_declaration == -1) warn_implicit_function_declaration = flag_isoc99; - /* If we're allowing C++0x constructs, don't warn about C++0x - compatibility problems. */ if (cxx_dialect == cxx0x) - warn_cxx0x_compat = 0; + { + warn_narrowing = warn_cxx0x_compat; + /* If we're allowing C++0x constructs, don't warn about C++98 + identifiers which are keywords in C++0x. */ + warn_cxx0x_compat = 0; + } + else + { + if (warn_cxx0x_compat == -1) + warn_cxx0x_compat = 0; + if (warn_narrowing == -1) + warn_narrowing = 0; + } + if (flag_preprocess_only) { /* Open the output now. We must do so even if flag_no_output is Index: testsuite/g++.dg/cpp0x/warn_cxx0x2.C =================================================================== --- testsuite/g++.dg/cpp0x/warn_cxx0x2.C (revision 0) +++ testsuite/g++.dg/cpp0x/warn_cxx0x2.C (revision 0) @@ -0,0 +1,5 @@ +// PR c++/50810 +// { dg-options "-std=gnu++98 -Wc++0x-compat" } + +int i; +float data[] = { i }; // { dg-warning "narrowing" } Index: testsuite/g++.dg/cpp0x/warn_cxx0x3.C =================================================================== --- testsuite/g++.dg/cpp0x/warn_cxx0x3.C (revision 0) +++ testsuite/g++.dg/cpp0x/warn_cxx0x3.C (revision 0) @@ -0,0 +1,5 @@ +// PR c++/50810 +// { dg-options "-std=gnu++98 -Wc++0x-compat -Wno-narrowing" } + +int i; +float data[] = { i }; Index: testsuite/g++.dg/cpp0x/warn_cxx0x4.C =================================================================== --- testsuite/g++.dg/cpp0x/warn_cxx0x4.C (revision 0) +++ testsuite/g++.dg/cpp0x/warn_cxx0x4.C (revision 0) @@ -0,0 +1,5 @@ +// PR c++/50810 +// { dg-options "-std=c++0x -Wno-c++0x-compat" } + +int i; +float data[] = { i }; Index: testsuite/g++.dg/cpp0x/initlist55.C =================================================================== --- testsuite/g++.dg/cpp0x/initlist55.C (revision 180373) +++ testsuite/g++.dg/cpp0x/initlist55.C (working copy) @@ -2,4 +2,4 @@ // { dg-options "-std=c++0x -pedantic-errors -Wno-narrowing" } int i; -float d = { i }; +float d = { i }; // { dg-error "narrowing" } Index: cp/decl.c =================================================================== --- cp/decl.c (revision 180373) +++ cp/decl.c (working copy) @@ -5523,7 +5523,7 @@ check_initializer (tree decl, tree init, int flags else { init = reshape_init (type, init, tf_warning_or_error); - if (cxx_dialect >= cxx0x && SCALAR_TYPE_P (type)) + if (SCALAR_TYPE_P (type)) check_narrowing (type, init); } } Index: cp/typeck2.c =================================================================== --- cp/typeck2.c (revision 180373) +++ cp/typeck2.c (working copy) @@ -803,8 +803,10 @@ check_narrowing (tree type, tree init) } if (!ok) - pedwarn (input_location, OPT_Wnarrowing, "narrowing conversion of %qE " - "from %qT to %qT inside { }", init, ftype, type); + emit_diagnostic ((cxx_dialect != cxx98) ? DK_PEDWARN : DK_WARNING, + input_location, OPT_Wnarrowing, + "narrowing conversion of %qE from %qT to %qT inside { }", + init, ftype, type); } /* Process the initializer INIT for a variable of type TYPE, emitting @@ -901,7 +903,7 @@ digest_init_r (tree type, tree init, bool nested, { tree *exp; - if (cxx_dialect != cxx98 && nested) + if (nested) check_narrowing (type, init); init = convert_for_initialization (0, type, init, flags, ICR_INIT, NULL_TREE, 0, Index: cp/semantics.c =================================================================== --- cp/semantics.c (revision 180373) +++ cp/semantics.c (working copy) @@ -2369,7 +2369,7 @@ finish_compound_literal (tree type, tree compound_ && check_array_initializer (NULL_TREE, type, compound_literal)) return error_mark_node; compound_literal = reshape_init (type, compound_literal, complain); - if (cxx_dialect >= cxx0x && SCALAR_TYPE_P (type) + if (SCALAR_TYPE_P (type) && !BRACE_ENCLOSED_INITIALIZER_P (compound_literal)) check_narrowing (type, compound_literal); if (TREE_CODE (type) == ARRAY_TYPE