Re: [PATCH] Recognize -Wextra, update manual.

[Phil Edwards <phil at jaj dot com>]

Zack and Joseph, thank you very much for reviewing this.


On Fri, Jun 07, 2002 at 02:26:44PM -0700, Zack Weinberg wrote:
> >    /* Warn about unused parms if extra warnings were specified.  */
> > -  /* Either ``-W -Wunused'' or ``-Wunused-parameter'' enables this
> > +  /* Either ``-Wextra -Wunused'' or ``-Wunused-parameter'' enables this
> >       warning.  WARN_UNUSED_PARAMETER is negative when set by
> >       -Wunused.  */
> 
> It is not clear from this that -Wall implies -Wunused and that
> therefore -Wall -Wextra will get you unused-parameter warnings.

I will add some text (in both places that you pointed out).


> > +  {"extra", &extra_warnings, 1,
> > +   N_("Print extra warnings") },
> 
> This is not real clear -- I'd like to see some mention of the higher
> false-positive rate, but nothing comes to mind just now.

Me neither.  I waffled around for a few minutes and then chickened out.


> >  understood by @command{gcc}.  If the @option{-v} option is also specified
> >  then @option{--help} will also be passed on to the various processes
> >  invoked by @command{gcc}, so that they can display the command line options
> > -they accept.  If the @option{-W} option is also specified then command
> > +they accept.  If the @option{-Wextra} option is also specified then command
> >  line options which have no documentation associated with them will also
> >  be displayed.
> 
> Why on earth is this a function of -Wextra?

*shrug*  Don't look at me, I'm just updating the text.  :-)

As a user, I'd rather have the -Wextra functionality here on all the time;
just because we don't have a short description for an option is no reason
not to at least print the option name.  (The user might just want to check
their spealling.)

I'd prefer to handle the rename separately (and before) going back and
deciding whether extra_warnings should have been used in the first place
for a particular piece of code.


> > +@item
> > +A variable is declared without a type specifier (``implicit int'').
> 
> Does this mean without any type specifiers at all ("foo;"), or with a
> modifier only ("unsigned foo;")?  The warning for the former should be
> a bit more prominent; the feature was removed (not deprecated,
> removed) in C99.

I'll update this text with Joseph's comments.


> > +A pointer is compared against integer zero with @samp{<}, @samp{<=},
> > +@samp{>}, or @samp{>=}.
> 
> Does this warning not trigger for <=> (pointer type) 0?  It probably
> should, and "integer zero" replaced with "null".

Will investigate.


> > +@item @r{(C++ only)}
> > +Ambiguous virtual bases.
> 
> What's an ambiguous virtual base, asks the reader not terribly
> familiar with C++?

That's the part where I asked for help constructing an example and a
better explanation.  :-)  I don't know how to put it into words without
just quoting chunks fo the standard.


> > +@item @r{(C++ only)}
> > +Subscripting an array which has been declared @samp{register}.
> > +
> > +@item @r{(C++ only)}
> > +Taking the address of a variable which has been declared @samp{register}.
> 
> It seems to me that these warnings should be issued by the C front end
> too, and at -Wall and/or -pedantic as well as -Wextra -- the main use
> of register these days is to point out that a variable isn't supposed
> to have its address taken.  Any use of a register-declared array is
> undefined behavior (C99 6.3.2.1p3, and footnote 100 in 6.7.1);
> applying & to a register variable is a constraint violation (6.5.3.2p1).
> 
> This is strictly a separate issue from your patch, but it seems like a
> good time to bring it up.

I agree with both of those paragraphs.


Phil

-- 
If ye love wealth greater than liberty, the tranquility of servitude greater
than the animating contest for freedom, go home and leave us in peace.  We seek
not your counsel, nor your arms.  Crouch down and lick the hand that feeds you;
and may posterity forget that ye were our countrymen.            - Samuel Adams