Re: Support for %d$c format specifier in diagnostics.c

[Gabriel Dos Reis <gdr at integrable-solutions dot net>]

Ishikawa <ishikawa@yk.rim.or.jp> writes:

[...]

|    The proposed patch is not thread-safe.  But thread-safety is not
|    required in this particular case.  The new routine uses a
|    statically assigned array to store argument info.  (It won't be

I've worked to eliminate globals and local statics from the diagnostic
framework, please in a revised patch don't introduce any.  Consider
for example making the array a member of the output_buffer structure.  


| CAVEAT: 
| 	
|    I found out that precision for integer print implicitly defines the
|    used type (%d, %ld, %lld, ... ).

implicitly?  The are listed explicitly.

|    output_integer_with_precision() macro uses the following
|    precision/types.
| 
| 	%d   %u
| 	%ld  %lu
| 	%lld %llu

there are also %wd and %wu

|    This must be observed by my patch also.
|    A new output_integer_with_precision_NEW() macro was introduced.
|    This new macro takes the index to the argument array.

That macro is not necessary, once you make the array a member of
output_buffer. 

|     Routines in diagnostic.c requires prior envrionment initialization
|     and I was not quite sure how to do that. Also, I found out
|     that diagnostic.o requires linking of various
|     modules.

Currently, there are "tree"-bits there.  I'm working on removing those
dependencies.  diagnostic.c should not be a place where people should
put bits when they don't have an idea where to put them.

[...]

| [] Implementation Strategy.
| 
|     To support the above, we need to scan the whole format string to find
|     out the type of each i-th value passed to output_format.
| 
|     So when we see ONE format specifier in the form of %1$d format (1
|     could be 2, 3, 4...), we scan the whole format string and we build an
|     internal array that defines the type of i-th argument.  Afterward we
|     scan the argument list and record the value of argument themselves.
| 
|     (UNLESS one such reference to a format specifier in the form of
|     %n$c is done, we won't have the overhead of the array construction.)
| 
|     Requirements/Caution:
| 
|     The size of paramaters pushed on the stack can vary (say, int vs long
|     long, and/or 64 bits pointer vs 32 bit int, etc.), so we must KNOW the
|     size of each argument before constructing the argument value
|     list.  (This is why we use a two pass algorithm below.  one for the
|     format string to look for the type of each argument, 
|     and then the scan of the argument list itself for the value.)

We should simply require that the type of "n" in "%n%c" be an int.
That simplifies varibility in precision and other kind of can of worms.

|     Passing the correct types in the format string is and has always been
|     the responsibility of the caller.

Sure, but now, we got a format-checker.  You might want to coordinate
that part with Kaveh.

| [] %d$n format specifier usage rule: 
| 
|     Initially, I was not entirely sure of the usage
|     rule of %n$c format specifier.
| 
|     Standard turns out to be clear. My reading was insufficient.
|     We must either use %n$ form specifiers for ALL the specifiers, or
|     NONE at all.  (Thanks to Zack and Joseph Myers for pointing this out.)
| 
| 	========================================
| 	standard-wise 
| 	----------------------------------------
| 	OK	           %1$d %3$c %2$s %6$d %5$s %4$c
| 
| 	NOT OK             %d %s %c %6$d %5$s %4$c
| 
| 	NOT OK ambiguous   %d %s %c %6$d %s %4$c
| 			   1  2  3   4   5  6
| 	In the last example, against which argument is the format specifier
| 	above (the fifth one) should be used?  
| 	----------------------------------------
| 
|     So I only permit ALL or NONE usage.

That makes sense.  Remember, this is not a general purpose library.
It is designed to be used specifically in the GCC project.

Your patch would probably for the complete removal of xxx_with_decl,
or else you get another level of complication. 

[...]

|     BTW, Does anyone use the format_decoder function?

Yes, a lots.