[gnu.gcc.bug] Summary of proposed new warnings

paul@UUNET.UU.NET (Paul Hudson) (01/10/89)

This is a summary of the responses I got to my request for desired warnings
for gcc, plus some suggested in the ANSI standard. I've also incorporated
many of the warnings that lint gives. 

Thanks to all those who responded. Further suggestions welcome.

It's almost certain that each of you reading this will disagree with
at least one of the suggested warnings. Remember however that each if
these warnings has been thought desirable by at least one person. I'd still
like to know which you disagree with, since if a significant number of people
object to a particular warning I'll control it with it's own -W flag.

Because of this I haven't decided on the flags to use yet. I'll wait a few
days for reaction before starting implementation.


Null loop body.

Null loop bodies occur often in valid C code. Nevertheless sometimes they are
unintended, so there should be an option to warn about them. There were
several requests to warn about

	while(<cond>);
	  {
          <statements>
	  }

only if the indentation indicated the <statements> were intended to form
a loop body (as above). I think this is too difficult in practice, and, as
always in C, would be defeated if the loop was part of a macro.

Null bodies will be warned about always if the condition has no
side-effect.  This is almost certainly an error. There will be a flag
to force all null bodies to be warned about.




Null if-parts.

Similar to the previous case. Again I have seen code like

	if (<cond>)
	  ;
	else
	  <statements>

which is meaningful if rather bad style (in *my* opinion, guys!). This
case is easy to spot, and not warn about.

	if (<cond>);	/* and no else */

should always be warned about. Even if the <cond> has side effects, the if ()
is redundant.



Semi-colon (ie null statement) after compound statement.

This is most likely to arise as a result of macro-expansion, and can cause
some very difficult to track down errors with if-else nesting.



Assignments in conditionals.

This and the next are to catch two common errors, using '=' where '==' was
intended and vice versa.

A warning will be given if the result of an assignment is being used for a
truth value. For example

	if (a = 1) ...

will provoke an error.
It's common in C to see

	if (a = f()) ...

intended. It's bad style, and wil provoke a warning. It's effect can
be got (without a waring, and with the same instructions generated) by
	
	if ((a = f()) != 0)

In fact this is precisely what the compiler does to the former form
internally.



Statements with no effect.

The inverse to the previous example. This warns about statements (or parts of
statements) with no apparent effects. Among these are

	i == 1;

which I used to type frequently when learning C, having taught my fingers
to type '==' everywhere. I think that

	i == f();

should also be warned about. Here the expression as a whole has a side-effect,
but it's still probably unintentional. I expect some problems with expressions
resulting from macros with this however (eg ignoring the return value from
putc()).



Old-style (ie not prototype) function declarations.

This was added to a previous version of gcc here, and proved very
useful during the conversion of existing C code to ANSI-C, ensuring
that prototypes were used to the full. It certainly shouldn't be on by
default, because most old code will produce hundreds of warnings. It
would be nice to turn it off for system header files too, since converting 
/usr/include to ANSI style is a long job ;-). Perhaps the first use of
a #pragma in gcc? (other than running hack, of course).



Function parameter assigned to before use.

This error is uncommon in C, but when a language with var or value
parameters is used it can indicates a value parameter was thought to
be a var parameter. Anyway, it's easy to add.



Unused label.

As the recent posting of the result of linting gcc has shown, this can happen
to the best of us! (For those who missed it, the "case" was omitted before
a case label which was an enumeration constant, thus defining a label).



Local variable not initialised before use.

Really not initialised, not possibly not initialised. This is however
difficult to add, needing more flow-analysis.



Local variable initialised but not used.

I've seen this one surprisingly often. Again, requires collecting more
information.



Static function/variable not used.

Obvious.



extern function declaration (not at top level) not used.

The neat example I was sent is

f()
{
	extern strclo(); /* s/lo/py/ */
	s = strcpy(one, two);
}



Implicit narrowing conversion.

The infamous "assignment of int from long may lose precision" only more so.
Yes, I know GNU products assume longs and ints are 32 bits, but the real
world alas is more complex. Anyway, it's bad style.



Value assigned to enumeration variable is not enumeration variable or
enumeration constant.

If you, like me, dislike the sloppy interpretation ANSI put on enumeration
types, this will help you use them properly.



Conditional is constant.

Again, used intentionally sometimes, but warning should be available.



extern declarations not used.

It's in lint, so someone must like it. I've not found it very useful.



Case label not an enumeration constant (when switch-value is).

If you're using enumerations strictly, this is probably unintentional.



'&' applied to function or array.

Probably indicates confusion.


Division by (constant) 0.

Lint appears to notice this. It's better to spot this at compilation time
rather than run-time.


Definition of '%s' hides previous definition.

In a limited way this is already done. I'd like to see it extended (having
just accidently hidden a top-level static with a local).

Strange comparisions

Such as (ordered) comparisions of unsigned quantities with zero.

	unsigned int u;

	if (u >= 0)

is pointless.

	if (u > 0)

is dubious. Comparing an unsigned quantity to a negative constant should
be an optional warning (since constants can now be indicated as unsigned).



It would be nice if gcc built up a "prototype" from the first call of
an undeclared (or old-style declared) function. Each of the formal args
would be taken as the actual arg promoted via the default promotions.