More empty body checks

Mark Mitchell mark@codesourcery.com
Sun May 6 23:24:00 GMT 2007


Mike Stump wrote:

Below, I've provided some technical feedback.

> +/* We check for a ) immediatly followed by ; with no whitespacing

Typo: "immediately".

> +   between.  This is used to issue a warning for while (1); and for
> +   (;;);, as the semicolon is probably extraneous.

Hard to read: "and for ..." does not make clear that the "for" is a
keyword, rather than something for which we issue a warning.  Write as:

  a warning for:

     while (...);

  and for:

     for (...);

The use of "1" and empty conditions for the "for" is misleading; this
warning has nothing to do with the triviality of the loop.

> +   On parse errors, the next tokenmight not be a ), so do nothing in
> +   that case. */

Typo: space between "token" and "might".

> +  close_paren = cp_lexer_peek_token (parser->lexer);
> +  if (close_paren->type != CPP_CLOSE_PAREN)
> +    return;

This should be folded into the callers, to avoid the redundant token
peeking:

  if (cp_parser_require (CPP_CLOSE_PAREN))
    check_empty_body (...);

> +  token = cp_lexer_peek_nth_token (parser->lexer, 2);
> +  semi_loc =  expand_location (token->location);
> +  if (token->type == CPP_SEMICOLON
> +      && !(token->flags & PREV_WHITE)
> +      && !(close_loc.line != semi_loc.line)
> +#ifdef USE_MAPPED_LOCATION
> +      && !(close_loc.column+1 != semi_loc.column)
> +#endif
> +      )

More efficient as:

  token = cp_lexer_peek_nth_token (parser->lexer, 2);
  if (token->type != CPP_SEMICOLON
       || (token->flags & PREV_WHITE))
    return;
  semi_loc = expand_location (...)
  ...

> +    warning (OPT_Wempty_body,
> +	     "suggest a space before %<;%> or explicit braces around empty "
> +	     "body in %<%s%> statement",
> +	     type);

You also need to update doc/invoke.texi to reflect that you're warning
about this in C++ for for/while statements.

And, you should try to add the same functionality to the C front end;
there's no reason to have this difference in behavior between the front
ends.  It may not be possible to share the code, but there's no reason
you can't implement the same functionality there.  You don't have to do
this to check in the patch, but I hope you will consider it.

OK with all of those changes.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713



More information about the Gcc-patches mailing list