Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Gerald Pfeifer]

Is it possible that this is responsible for a bootstrap failure introduced 
in the last 27 hours or so?

/scratch/tmp/gerald/gcc-HEAD/gcc/tree-object-size.c:44:59: error: narrowing conversion of '-0x00000000000000001' from 'int' to 'long unsigned int' inside { } [-Werror=narrowing]
/scratch/tmp/gerald/gcc-HEAD/gcc/tree-object-size.c:44:59: error: narrowing conversion of '-0x00000000000000001' from 'int' to 'long unsigned int' inside { } [-Werror=narrowing]
cc1plus: all warnings being treated as errors
gmake[3]: *** [tree-object-size.o] Error 1
gmake[3]: Leaving directory `/local0/scratch/gerald/OBJ-1023-1848/gcc'
gmake[2]: *** [all-stage2-gcc] Error 2
gmake[2]: Leaving directory `/local0/scratch/gerald/OBJ-1023-1848'
gmake[1]: *** [stage2-bubble] Error 2

The code in question is

  static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };

This is on amd64-unknown-freebsd8.0, though I am puzzled it does not
seem to trigger for other 64-bit platforms?

I also filed PR 50841 for the bootstrap failure, especially if it's
not yours.

Gerald


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

        PR c++/50810
        * c-opts.c (c_common_handle_option): Enable -Wnarrowing as part
        of -Wall; include -Wnarrowing in -Wc++0x-compat; adjust default
        Wnarrowing for C++0x and C++98.
        * c.opt ([Wnarrowing]): Update.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

On 10/23/2011 10:07 PM, Gerald Pfeifer wrote:
> Is it possible that this is responsible for a bootstrap failure introduced
> in the last 27 hours or so?
>
> /scratch/tmp/gerald/gcc-HEAD/gcc/tree-object-size.c:44:59: error: narrowing conversion of '-0x00000000000000001' from 'int' to 'long unsigned int' inside { } [-Werror=narrowing]
> /scratch/tmp/gerald/gcc-HEAD/gcc/tree-object-size.c:44:59: error: narrowing conversion of '-0x00000000000000001' from 'int' to 'long unsigned int' inside { } [-Werror=narrowing]
So, to be clear, this is for bootstrapping with a C++ compiler, right? 
Honestly, didn't try that... It's definitely possible that there are 
glitches in the tree wrt -Wnarrowing in C++.

Paolo.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

> The code in question is
>
>   static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };
>
> This is on amd64-unknown-freebsd8.0, though I am puzzled it does not
> seem to trigger for other 64-bit platforms?

It does trigger on Linux.  I guess the patch wasn't bootstrapped.

There is another problem in Ada.  Fixed thusly.


2011-10-23  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc-interface/decl.c (create_concat_name): Add explicit cast.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 480 bytes --]

Index: gcc-interface/decl.c
===================================================================
--- gcc-interface/decl.c	(revision 180235)
+++ gcc-interface/decl.c	(working copy)
@@ -8976,7 +8976,7 @@ create_concat_name (Entity_Id gnat_entit
 
   if (suffix)
     {
-      String_Template temp = {1, strlen (suffix)};
+      String_Template temp = {1, (int) strlen (suffix)};
       Fat_Pointer fp = {suffix, &temp};
       Get_External_Name_With_Suffix (gnat_entity, fp);
     }

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Eric Botcazou]

> So, to be clear, this is for bootstrapping with a C++ compiler, right?
> Honestly, didn't try that... It's definitely possible that there are
> glitches in the tree wrt -Wnarrowing in C++.

Bootstrapping with the C++ compiler has been the default for months...

-- 
Eric Botcazou

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

On 10/23/2011 10:19 PM, Eric Botcazou wrote:
>> So, to be clear, this is for bootstrapping with a C++ compiler, right?
>> Honestly, didn't try that... It's definitely possible that there are
>> glitches in the tree wrt -Wnarrowing in C++.
> Bootstrapping with the C++ compiler has been the default for months...
>
Oh my, I thought I was till using C here... Ok, I'll fix that.

Paolo.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Eric Botcazou]

> Oh my, I thought I was till using C here... Ok, I'll fix that.

The base compiler is a C compiler, stage 2/3 are built with the C++ compiler.

-- 
Eric Botcazou

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]

On 10/23/2011 10:25 PM, Eric Botcazou wrote:
>> Oh my, I thought I was till using C here... Ok, I'll fix that.
> The base compiler is a C compiler, stage 2/3 are built with the C++ compiler.
Yes, yes. Sorry about this.

Anyway, the below appears to work for me. Eric shall I commit it?

Thanks,
Paolo.

/////////////////


[-- Attachment #2: p --]
[-- Type: text/plain, Size: 1178 bytes --]

Index: tree-ssa-ccp.c
===================================================================
--- tree-ssa-ccp.c	(revision 180346)
+++ tree-ssa-ccp.c	(working copy)
@@ -2011,7 +2011,9 @@ ccp_visit_stmt (gimple stmt, edge *taken_edge_p, t
      Mark them VARYING.  */
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_ALL_DEFS)
     {
-      prop_value_t v = { VARYING, NULL_TREE, { -1, (HOST_WIDE_INT) -1 } };
+      prop_value_t v =
+	{ VARYING, NULL_TREE, { (unsigned HOST_WIDE_INT) -1,
+				(HOST_WIDE_INT) -1 } };
       set_lattice_value (def, v);
     }
 
Index: tree-object-size.c
===================================================================
--- tree-object-size.c	(revision 180346)
+++ tree-object-size.c	(working copy)
@@ -41,7 +41,9 @@ struct object_size_info
   unsigned int *stack, *tos;
 };
 
-static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };
+static unsigned HOST_WIDE_INT unknown[4]
+= { (unsigned HOST_WIDE_INT)-1, (unsigned HOST_WIDE_INT)-1,
+    (unsigned HOST_WIDE_INT)0, (unsigned HOST_WIDE_INT)0 };
 
 static tree compute_object_offset (const_tree, const_tree);
 static unsigned HOST_WIDE_INT addr_object_size (struct object_size_info *,

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Eric Botcazou]

> Anyway, the below appears to work for me. Eric shall I commit it?

I have other errors for config/i386/i386.c on my x86-64 machine.  But are we 
sure that we want to warn on

static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };

with -Wall?  This seems overly picky to me.

-- 
Eric Botcazou

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing

[Paolo Carlini]

On 10/23/2011 10:39 PM, Paolo Carlini wrote:
> On 10/23/2011 10:25 PM, Eric Botcazou wrote:
>>> Oh my, I thought I was till using C here... Ok, I'll fix that.
>> The base compiler is a C compiler, stage 2/3 are built with the C++ 
>> compiler.
> Yes, yes. Sorry about this.
>
> Anyway, the below appears to work for me. Eric shall I commit it?
Nope, doesn't work, there are *many* more issues in gcc/config.

I'm afraid we are not ready yet to enable this, target maintainer have 
to help cleaning up gcc/config first, I'm going to revert my patch.

Paolo.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing

[Paolo Carlini]

On 10/23/2011 10:47 PM, Paolo Carlini wrote:
> On 10/23/2011 10:39 PM, Paolo Carlini wrote:
>> On 10/23/2011 10:25 PM, Eric Botcazou wrote:
>>>> Oh my, I thought I was till using C here... Ok, I'll fix that.
>>> The base compiler is a C compiler, stage 2/3 are built with the C++ 
>>> compiler.
>> Yes, yes. Sorry about this.
>>
>> Anyway, the below appears to work for me. Eric shall I commit it?
> Nope, doesn't work, there are *many* more issues in gcc/config.
>
> I'm afraid we are not ready yet to enable this, target maintainer have 
> to help cleaning up gcc/config first, I'm going to revert my patch.
Done.

Paolo.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Gabriel Dos Reis]

On Sun, Oct 23, 2011 at 3:45 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Anyway, the below appears to work for me. Eric shall I commit it?
>
> I have other errors for config/i386/i386.c on my x86-64 machine.  But are we
> sure that we want to warn on
>
> static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };
>
> with -Wall?  This seems overly picky to me.
>

The warning probably should not be in -Wall.  It is fairly recent in C++, and I
think we should allow users to adapt before enabling it by default.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

On 10/23/2011 11:05 PM, Gabriel Dos Reis wrote:
> On Sun, Oct 23, 2011 at 3:45 PM, Eric Botcazou<ebotcazou@adacore.com>  wrote:
>>> Anyway, the below appears to work for me. Eric shall I commit it?
>> I have other errors for config/i386/i386.c on my x86-64 machine.  But are we
>> sure that we want to warn on
>>
>> static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };
>>
>> with -Wall?  This seems overly picky to me.
>>
> The warning probably should not be in -Wall.  It is fairly recent in C++, and I
> think we should allow users to adapt before enabling it by default.
The issue is that we wanted -Wconversion to be enabled by -Wc++0x-compat 
(after all, it's what the PR asks) but the latter is *already* in -Wall.

Personally, I would be in favor of taking -Wc++0x-compat out of -Wall.

Paolo.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Gabriel Dos Reis]

On Sun, Oct 23, 2011 at 4:28 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 10/23/2011 11:05 PM, Gabriel Dos Reis wrote:
>>
>> On Sun, Oct 23, 2011 at 3:45 PM, Eric Botcazou<ebotcazou@adacore.com>
>>  wrote:
>>>>
>>>> Anyway, the below appears to work for me. Eric shall I commit it?
>>>
>>> I have other errors for config/i386/i386.c on my x86-64 machine.  But are
>>> we
>>> sure that we want to warn on
>>>
>>> static unsigned HOST_WIDE_INT unknown[4] = { -1, -1, 0, 0 };
>>>
>>> with -Wall?  This seems overly picky to me.
>>>
>> The warning probably should not be in -Wall.  It is fairly recent in C++,
>> and I
>> think we should allow users to adapt before enabling it by default.
>
> The issue is that we wanted -Wconversion to be enabled by -Wc++0x-compat
> (after all, it's what the PR asks) but the latter is *already* in -Wall.

yes.

>
> Personally, I would be in favor of taking -Wc++0x-compat out of -Wall.
>

Patch pre-approved.
It makes sense though that -Wextra implies -Wc++0x-compat.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

Hi,
>> Personally, I would be in favor of taking -Wc++0x-compat out of -Wall.
>>
> Patch pre-approved.
Thanks.
> It makes sense though that -Wextra implies -Wc++0x-compat.
Indeed, it would. However, unfortunately, we are using -W to bootstrap 
(it just failed on me). Thus I'm bootstrapping and testing the below, 
which just takes -Wc++0x-compat out from -Wall without adding it to -Wextra.

I'll wait anyway until tomorrow in case of further comments.

Thanks again,
Paolo.

//////////////////////////////

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 47 bytes --]

... and the patch ;)

Paolo.

////////////////

[-- Attachment #2: CL_50810_4 --]
[-- Type: text/plain, Size: 884 bytes --]

/c-family
2011-10-23  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-23  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-23  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-23  Paolo Carlini  <paolo.carlini@oracle.com>

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

[-- Attachment #3: patch_50810_4 --]
[-- Type: text/plain, Size: 6583 bytes --]

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

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Gabriel Dos Reis]

On Sun, Oct 23, 2011 at 7:56 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> ... and the patch ;)

I am bit puzzled by this:

+This flag is included in @option{-Wc++0x-compat}.
+With -std=c++0x, @option{-Wno-narrowing} suppresses the diagnostic
+required by the standard.

and this:
-  /* 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;
+    }




We do not use -W or -Wno- to suppressed *required* diagnostics.  So,
when -std=c++0x,
-Wno-narrowing should not have any effect.  However with
-Wc++0x-compat, it could
make sense to have -Wno-narrowing suppress the diagnostic.

The point is this:  we do not use -W flags to change a standards semantics.
But we use -W to make suggestions (e.g. warnings) so a suggesting should be
suppressed only in the context of another suggestion (-Wc++0x-compat.)

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

Hi,

On 10/24/2011 03:30 AM, Gabriel Dos Reis wrote:
> We do not use -W or -Wno- to suppressed *required* diagnostics. So, 
> when -std=c++0x, -Wno-narrowing should not have any effect.
Personally, I have no problem with this, but note, I'm not inventing 
anything new here, the behavior you are discussing *pre*-dates my patch 
and I feel a little nervous about changing it. If you think you can 
approve this part of rhe patch, I'll change it as you want and resend.

Paolo.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Gabriel Dos Reis]

On Sun, Oct 23, 2011 at 8:48 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 10/24/2011 03:30 AM, Gabriel Dos Reis wrote:
>>
>> We do not use -W or -Wno- to suppressed *required* diagnostics. So, when
>> -std=c++0x, -Wno-narrowing should not have any effect.
>
> Personally, I have no problem with this, but note, I'm not inventing
> anything new here, the behavior you are discussing *pre*-dates my patch and
> I feel a little nervous about changing it. If you think you can approve this
> part of rhe patch, I'll change it as you want and resend.

Let me quote again the part of the patch under discussion:

-  /* 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;
+

Before the patch, -std=c++0x effectively put off -Wc++0x-compat because we
are compiling c++98/c++03 code, so we can only *warn* about possible
compatibility conflict with C++11.   However, the narrowing diagnostic required
by C++11 is NOT a warning.  It is a diagnostic.  The way we alter a standard
mandate is through some -fflag, e.g. -fpermissive.

What the above patch fragment is doing is to turn on a *warning*.
When -std=c++0x
is in effect, narrowing is no longer a warning.  It is an error by default.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Paolo Carlini]

On 10/24/2011 04:10 AM, Gabriel Dos Reis wrote:
> Before the patch, -std=c++0x effectively put off -Wc++0x-compat 
> because we are compiling c++98/c++03 code, so we can only *warn* about 
> possible compatibility conflict with C++11. However, the narrowing 
> diagnostic required by C++11 is NOT a warning. It is a diagnostic. The 
> way we alter a standard mandate is through some -fflag, e.g. 
> -fpermissive. What the above patch fragment is doing is to turn on a 
> *warning*. When -std=c++0x is in effect, narrowing is no longer a 
> warning. It is an error by default.
I'm missing your point, I'm sorry: I maintain that *before* and after 
the patch -Wno-narrowing in C++0x mode was able to suppress the 
narrowing warnings. I'm 100% sure, I double checked for you one second 
ago. Are we on the same page on this? If we are, and you think gcc 
should do something new, I have no problem changing my patch to, eg:

   if (cxx_dialect == cxx0x)
     {
       /* If we're allowing C++0x constructs, don't warn about C++98
      identifiers which are keywords in C++0x.  */
       warn_cxx0x_compat = 0;
       warn_narrowing = 1;
     }
   else if (warn_narrowing == -1)
     warn_narrowing = 0;

Paolo.

Re: Bootstrap failure in tree-object-size.c due to -Wnarrowing (was: [C++ Patch] PR 50810)

[Gabriel Dos Reis]

On Sun, Oct 23, 2011 at 9:16 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 10/24/2011 04:10 AM, Gabriel Dos Reis wrote:
>>
>> Before the patch, -std=c++0x effectively put off -Wc++0x-compat because we
>> are compiling c++98/c++03 code, so we can only *warn* about possible
>> compatibility conflict with C++11. However, the narrowing diagnostic
>> required by C++11 is NOT a warning. It is a diagnostic. The way we alter a
>> standard mandate is through some -fflag, e.g. -fpermissive. What the above
>> patch fragment is doing is to turn on a *warning*. When -std=c++0x is in
>> effect, narrowing is no longer a warning. It is an error by default.
>
> I'm missing your point, I'm sorry: I maintain that *before* and after the
> patch -Wno-narrowing in C++0x mode was able to suppress the narrowing
> warnings.

and I am saying that is a bug.

> 'm 100% sure, I double checked for you one second ago. Are we on
> the same page on this? If we are, and you think gcc should do something new,

It is new only in the sense that a bug will be fixed.  Otherwise no,
it is not new.

> I have no problem changing my patch to, eg:
>
>  if (cxx_dialect == cxx0x)
>    {
>      /* If we're allowing C++0x constructs, don't warn about C++98
>     identifiers which are keywords in C++0x.  */
>      warn_cxx0x_compat = 0;
>      warn_narrowing = 1;
>    }

Yes, a -Wno-narrowing should not suppress narrowing in C++11 mode.

>  else if (warn_narrowing == -1)
>    warn_narrowing = 0;
>

OK.