Special handling of "%H" (Re: Support for %d$c format specifierin diagnostics.c)

[Joseph S. Myers]

On Thu, 31 Jul 2003, Ishikawa wrote:

> This is the latest cut for the patch to support positional format
> specifiers for study/review.

Please take care to follow the GNU Coding Standards.  I'll point out some
of the issues below, but only point out a particular issue once although
it may appear elsewhere in the code as well; for each issue you should go
through the code and fix it everywhere it occurs.

> !   /* fprintf(stderr,"c_tree_printer: a =%d\n", a);*/

Don't include this sort of commented out code.

> !   if(a == PRINT_ARG_VALUE_AS_BEFORE /* good old printing as before. */
> !      || a == PRINT_ARG_VALUE )	/* positional parameter support. */

Space after the "if" and before the open parenthesis; no space before the
close parenthesis.  Comments should be proper sentences, but in this case
aren't needed: comments just repeating the code in words are unnecessary,
they should give a higher-level overview.

> + #if 0

Don't include #if 0 code; it will just cause trouble later for someone
trying to work out why it is there.  If you want to say "we don't do
things this way because ...", say it in a comment rather than #if 0 code.  
Slow code for checking for internal errors can go under ENABLE_CHECKING.  
CVS is for "we used to do things this way..." but that shouldn't be
necessary at all with new code like this.

> !    Now pp_format_text() no longer formats/prints the first %H reference.

Describe what the code does and why, but not a temporal reference such as 
"Now" or "no longer" which in five years' time will look very dated.

> + /* for isdigit */
> + #include <ctype.h>

(a) GCC uses safe-ctype.h from libiberty to avoid locale-dependence.  The 
digits in positional parameters are always ASCII ones, not locale-specific 
ones.

(b) That comment is the sort of unnecessary trivial comment.

> + /*
> +  * Aborting with error message and the format string that caused it.
> +  */

An asterisk on each link is not the standard style of comments.  Please
follow the GNU Coding Standards and the style of the existing code.

> + static
> + void

Nor is the use of separate lines for "static" and "void" proper style.

> + /**

Nor is two asterisks proper style.

> +       fprintf(stderr, "pretty-print.c: Specifying incompatible types for argument (at %d) twice\n", i);
> +       fprintf(stderr,"format : <<%s>>\n", text->arg_array[0].v.cptr);
> + #ifdef ENABLE_CHECKING
> +       abort();      	
> + #endif

For this sort of consistency check, I think you should simply use
internal_error () rather than attempting to continue.  And remember the
spaces before the open parenthesis, and after the comma.

-- 
Joseph S. Myers
jsm@polyomino.org.uk