diff mbox

[C++] PR 50810 (new try)

Message ID 4EA5503C.7070202@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Oct. 24, 2011, 11:47 a.m. UTC
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.

Booted and tested x96_64-linux.

Ok?

Thanks,
Paolo.

PS: alternately, I'm attaching a simpler variant which leaves 
-Wno-narrowing active in C++0x mode.

/////////////////////
/c-family
2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* c-opts.c (c_common_handle_option): Do not enable -Wc++0x-compat
	as part of -Wall; handle -Wc++0x-compat.
	(c_common_post_options): -std=c++0x enables -Wnarrowing, can be
	disabled only with -Wno-c++0x-compat.
	* c.opt ([Wc++0x-compat], [Wnarrowing]): Update.

/cp
2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* typeck2.c (check_narrowing): Adjust OPT_Wnarrowing diagnostics.
	(digest_init_r): Call check_narrowing irrespective of the C++ dialect.
	* decl.c (check_initializer): Likewise.
	* semantics.c (finish_compound_literal): Likewise.

/testsuite
2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* g++.dg/cpp0x/warn_cxx0x2.C: New.
	* g++.dg/cpp0x/warn_cxx0x3.C: Likewise.
	* g++.dg/cpp0x/warn_cxx0x4.C: Likewise.
	* g++.dg/cpp0x/initlist55.C: Adjust.

2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* doc/invoke.texi ([-Wall], [-Wnarrowing], [-Wc++0x-compat]): Update.

/c-family
2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* c-opts.c (c_common_handle_option): Do not enable -Wc++0x-compat
	as part of -Wall; handle -Wc++0x-compat.
	(c_common_post_options): -std=c++0x enables -Wnarrowing.
	* c.opt ([Wnarrowing]): Update.

/cp
2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* typeck2.c (check_narrowing): Adjust OPT_Wnarrowing diagnostics.
	(digest_init_r): Call check_narrowing irrespective of the C++ dialect.
	* decl.c (check_initializer): Likewise.
	* semantics.c (finish_compound_literal): Likewise.

/testsuite
2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* g++.dg/cpp0x/warn_cxx0x2.C: New.
	* g++.dg/cpp0x/warn_cxx0x3.C: Likewise.

2011-10-24  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/50810
	* doc/invoke.texi ([-Wall], [-Wnarrowing], [-Wc++0x-compat]): Update.
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 180348)
+++ 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-narrowing} suppresses 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 180348)
+++ c-family/c.opt	(working copy)
@@ -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 180348)
+++ 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,18 @@ 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;
+    {
+      /* If we're allowing C++0x constructs, don't warn about C++98
+	 identifiers which are keywords in C++0x.  */
+      warn_cxx0x_compat = 0;
 
+      if (warn_narrowing == -1)
+	warn_narrowing = 1;
+    }
+  else 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,4 @@
+// PR c++/50810
+// { dg-options "-std=gnu++98 -Wc++0x-compat" }
+
+signed char data[] = { 0xff }; // { 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,4 @@
+// PR c++/50810
+// { dg-options "-std=gnu++98 -Wc++0x-compat -Wno-narrowing" }
+
+signed char data[] = { 0xff };
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 180348)
+++ 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 180348)
+++ 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 180348)
+++ 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

Comments

Gabriel Dos Reis Oct. 24, 2011, 12:17 p.m. UTC | #1
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.
Paolo Carlini Oct. 24, 2011, 12:18 p.m. UTC | #2
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.
Gabriel Dos Reis Oct. 24, 2011, 12:57 p.m. UTC | #3
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! :-)
Jason Merrill Oct. 24, 2011, 1:06 p.m. UTC | #4
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
Jason Merrill Oct. 24, 2011, 1:10 p.m. UTC | #5
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
Gabriel Dos Reis Oct. 24, 2011, 1:26 p.m. UTC | #6
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
Jason Merrill Oct. 24, 2011, 1:29 p.m. UTC | #7
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
Paolo Carlini Oct. 24, 2011, 1:31 p.m. UTC | #8
.. 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.
Gabriel Dos Reis Oct. 24, 2011, 1:49 p.m. UTC | #9
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.
Jason Merrill Oct. 24, 2011, 2:10 p.m. UTC | #10
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
Gabriel Dos Reis Oct. 24, 2011, 2:39 p.m. UTC | #11
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
Jason Merrill Oct. 24, 2011, 2:53 p.m. UTC | #12
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
Gabriel Dos Reis Oct. 24, 2011, 5:21 p.m. UTC | #13
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.
Jason Merrill Oct. 24, 2011, 5:46 p.m. UTC | #14
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
Gabriel Dos Reis Oct. 24, 2011, 6:13 p.m. UTC | #15
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.

>
Jason Merrill Oct. 24, 2011, 6:17 p.m. UTC | #16
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
Gabriel Dos Reis Oct. 24, 2011, 6:47 p.m. UTC | #17
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
Jason Merrill Oct. 24, 2011, 7:05 p.m. UTC | #18
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
Gabriel Dos Reis Oct. 24, 2011, 9:09 p.m. UTC | #19
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.
diff mbox

Patch

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