This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Wextra digest (fixing PR7651)


"Manuel LÃpez-IbÃÃez" <lopezibanez@gmail.com> writes:

> On 11 Jan 2007 15:48:36 -0800, Ian Lance Taylor <iant@google.com> wrote:
> 
> > These issues are tricky because on the one hand we don't want too many
> > different options, and on the other hand we want to give people the
> > control they are asking for with regard to turning off warnings.
> >
> 
> Yeah, I know but I thought it was a goal that every warning is
> controlled by some option. I also think (and is under discussion
> elsewhere) that super-options (as Wall) should only enable other
> options and don't have associated any warning to themselves.

Yes, I agree with that.  What I was trying to say is that, given that
goal, we have a choice between giving each warning its own option, or
grouping warnings together.  When warnings are grouped, people can
only disable or enable the entire group, which makes it harder for
them to control the specific warning they are interested in.  When
warnings are not grouped, there are too many options.  We need to
strike a balance between those extremes, and that is tricky.


> > > * Subscripting an array which has been declared register.
> > >
> > > * Taking the address of a variable which has been declared register.
> >
> > Hmmm.  In the C frontend these are pedwarns.  But the C++ frontend
> > doesn't have pedwarns.  And maybe C++ doesn't require these warnings
> > anyhow, I don't know.  -Winvalid-register anyone?
> 
> If we don't require them, we could drop them.  Also, there is already
> a patch in the queue [1] to name this -Waddress-of-register. I could
> update it with Winvalid-register.
> 
> [1] http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01676.html

-Waddress-of-register works for me.


> > > * A non-static reference or non-static const member appears in a class
> > > without constructors.
> >
> > -Wbad-code.  Or, I dunno, how about -Wmissing-field-initializers?
> >
> 
> Wbad-code is vague. -Wmissing-field-initializers seems fine and also
> -Wuninitialized. Any other opinion?

-Wbad-code was a joke, but we have so many warning options that it
seems erious.  I think -Wuninitialized is a good suggestion.


> > > * Ambiguous virtual bases (virtual base inaccessible due to
> > > ambiguity). (There is also an unconditional warning for direct base
> > > inaccessible due to ambiguity)
> >
> > What do you think of -Woverloaded-virtual?
> 
> Fine to me but I am no C++ expert and we have to think if the users of
> Woverloaded-virtual will be happy with this additional warning.

In my opinion, both the current -Woverloaded-virtual and an
inaccessible virtual base are almost always mistakes.  And they are
the same sort of mistake: something in a base class is hidden in a
child class.  So I personally think using the same option here is
reasonable.


> > > * An enumerator and a non-enumerator both appear in a conditional
> > > expression. (There is also an unconditional warning for two different
> > > enumeral types used in a conditional expression).
> >
> > Maybe we should rename -Wswitch-enum to -Wenum and wrap these warnings
> > under there as well.
> 
> I don't think they have much to do with each other. Moreover, this
> warning applies only to C++. What about -Wconversion ?

-Wconversion is good.


> > > * A function can return either with or without a value.
> >
> > I give up.
> 
> :-) None said it was going to be easy. We have a warning similar to this:
> 
> -Wreturn-type
>            warn about any "return" statement with no return-value in a
> function whose return-type is not "void".
> 
> However, this is enabled by Wall and the one from Wextra surely
> produces false positives (I guess that is the reason it is in Wextra).
> -Wreturn-type-may ?

I tend to think that having some returns with a value and some without
is an error.  Maybe grouping them in -Wreturn-type makes sense,
although the name is not quite right.


> > > * An unsigned value is compared against zero with < or >=.
> > > Walways-true claims to warn for this but it doesn't. There is also an
> > > unconditional warning for expressions that are always true or false
> > > due to the range of types.
> >
> > -Walways-true should warn for this.
> 
> No, I think it shouldn't but let's leave this for now, please. I need
> to do a bit more of research and archive archaeology to properly
> justify why I think that this is a Really Bad Idea (TM). Or we could
> just drop the idea @ kernel.org and see what happens. Any brave
> volunteer?

-Wunsigned-compare?

> > > Finally, the manual page claims that Wextra warns for any of several
> > > floating-point events that often indicate errors, such as overflow,
> > > underflow, loss of precision, etc. I wasn't able to find any instance
> > > of this. I am fairly sure that Wextra doesn't do such thing.
> >
> > I have no idea what that refers to.
> 
> So we just remove it from doc/invoke.texi ?

Yes.

Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]