diff mbox

RFC: PATCH to adjust warning flags for C++

Message ID 4EB0324F.9030505@redhat.com
State New
Headers show

Commit Message

Jason Merrill Nov. 1, 2011, 5:54 p.m. UTC
Paolo Carlini's patch to add -Wnarrowing to -Wc++0x-compat (and thus 
-Wall) broke bootstrap because of narrowing warnings, so I'd like to add 
-Wno-narrowing to the stage 2+ warning flags.  Is this the best way to 
do that?

Jason

Comments

Gabriel Dos Reis Nov. 1, 2011, 7:48 p.m. UTC | #1
On Tue, Nov 1, 2011 at 12:54 PM, Jason Merrill <jason@redhat.com> wrote:
> Paolo Carlini's patch to add -Wnarrowing to -Wc++0x-compat (and thus -Wall)
> broke bootstrap because of narrowing warnings, so I'd like to add
> -Wno-narrowing to the stage 2+ warning flags.  Is this the best way to do
> that?

why do we want to include -Wc++0x-compat in -Wall?
Jason Merrill Nov. 2, 2011, 1:11 a.m. UTC | #2
On 11/01/2011 03:48 PM, Gabriel Dos Reis wrote:
> On Tue, Nov 1, 2011 at 12:54 PM, Jason Merrill<jason@redhat.com>  wrote:
>> Paolo Carlini's patch to add -Wnarrowing to -Wc++0x-compat (and thus -Wall)
>> broke bootstrap because of narrowing warnings, so I'd like to add
>> -Wno-narrowing to the stage 2+ warning flags.  Is this the best way to do
>> that?
>
> why do we want to include -Wc++0x-compat in -Wall?

It's already included.  And I think that "your code won't work in C++11" 
is a warning that most C++ programmers will be interested in if they are 
asking for warnings.

Jason
Gabriel Dos Reis Nov. 2, 2011, 4:05 a.m. UTC | #3
On Tue, Nov 1, 2011 at 8:11 PM, Jason Merrill <jason@redhat.com> wrote:
> On 11/01/2011 03:48 PM, Gabriel Dos Reis wrote:
>>
>> On Tue, Nov 1, 2011 at 12:54 PM, Jason Merrill<jason@redhat.com>  wrote:
>>>
>>> Paolo Carlini's patch to add -Wnarrowing to -Wc++0x-compat (and thus
>>> -Wall)
>>> broke bootstrap because of narrowing warnings, so I'd like to add
>>> -Wno-narrowing to the stage 2+ warning flags.  Is this the best way to do
>>> that?
>>
>> why do we want to include -Wc++0x-compat in -Wall?
>
> It's already included.

yes, that is why I asked.  E.g. it isn't obvious that -Wc++0x-compat
 ought to be in -Wall at this stage or 4.6.x.

>  And I think that "your code won't work in C++11" is
> a warning that most C++ programmers will be interested in if they are asking
> for warnings.

Even when -std=c++03 -Wall or -std=c++98 -Wall?

I would suggest we do this:
    1. Include -Wc++0x-compat in -W or -Wextra for THIS release.
    2.  leave -Wnarrowing in -Wc++0x-compat by default.
    3.  Make a release note that -Wc++0x-compat will be activated in
the very major release.
Jason Merrill Nov. 2, 2011, 5:40 a.m. UTC | #4
On 11/02/2011 12:05 AM, Gabriel Dos Reis wrote:
>>   And I think that "your code won't work in C++11" is
>> a warning that most C++ programmers will be interested in if they are asking
>> for warnings.
>
> Even when -std=c++03 -Wall or -std=c++98 -Wall?

Yes.  -Wc++0x-compat has been part of -Wall for almost 5 years.  If 
people don't want narrowing warnings, they can use -Wno-narrowing, which 
is helpfully mentioned in the warnings themselves.

Jason
Paolo Bonzini Nov. 2, 2011, 9:40 a.m. UTC | #5
On 11/01/2011 06:54 PM, Jason Merrill wrote:
> Paolo Carlini's patch to add -Wnarrowing to -Wc++0x-compat (and thus
> -Wall) broke bootstrap because of narrowing warnings, so I'd like to add
> -Wno-narrowing to the stage 2+ warning flags.  Is this the best way to
> do that?

Is this a C++-only warning?  Also, how did you test the patch?

Paolo
Gabriel Dos Reis Nov. 2, 2011, 12:40 p.m. UTC | #6
On Wed, Nov 2, 2011 at 12:40 AM, Jason Merrill <jason@redhat.com> wrote:
> On 11/02/2011 12:05 AM, Gabriel Dos Reis wrote:
>>>
>>>  And I think that "your code won't work in C++11" is
>>> a warning that most C++ programmers will be interested in if they are
>>> asking
>>> for warnings.
>>
>> Even when -std=c++03 -Wall or -std=c++98 -Wall?
>
> Yes.  -Wc++0x-compat has been part of -Wall for almost 5 years.  If people
> don't want narrowing warnings, they can use -Wno-narrowing, which is
> helpfully mentioned in the warnings themselves.
>

narrowing is just one issue.

Originally, we were checking mostly for clashes in identifiers and the like.
And we should have been more cautious then.  Now that C++11 support
is far better, I think it is still time to correct what we should have done.
At least not warning when the standard is explicitly selected, as opposed
to whatever the default is.  That is, if -std=c++03 or -std=c++98 is
explicitly given
on the command line, we should avoid including -Wc++0x-compat in -Wall.
Jason Merrill Nov. 2, 2011, 1:47 p.m. UTC | #7
On 11/02/2011 05:40 AM, Paolo Bonzini wrote:
> Is this a C++-only warning? Also, how did you test the patch?

It is, but the flag is accepted and ignored by the C front end.

I tested it with a bootstrap.

Jason
Paolo Bonzini Nov. 2, 2011, 1:50 p.m. UTC | #8
On 11/02/2011 02:47 PM, Jason Merrill wrote:
>
>> Is this a C++-only warning? Also, how did you test the patch?
>
> It is, but the flag is accepted and ignored by the C front end.

Then please adjust libcpp/configure.ac too; otherwise the patch is okay 
to fix bootstrap.  It can be reverted later if c++0x-compat is made less 
stringent.

Paolo
diff mbox

Patch

diff --git a/gcc/configure.ac b/gcc/configure.ac
index d63acea..6f036bd 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -329,10 +329,11 @@  GCC_STDINT_TYPES
 # * 'long long'
 # * variadic macros
 # * overlong strings
+# * C++11 narrowing conversions in { }
 # So, we only use -pedantic if we can disable those warnings.
 
 ACX_PROG_CC_WARNING_OPTS(
-	m4_quote(m4_do([-W -Wall -Wwrite-strings -Wcast-qual])), [loose_warn])
+	m4_quote(m4_do([-W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual])), [loose_warn])
 ACX_PROG_CC_WARNING_OPTS(
 	m4_quote(m4_do([-Wstrict-prototypes -Wmissing-prototypes])),
 	[c_loose_warn])