This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Support for %d$c format specifier in diagnostics.c
- From: Gabriel Dos Reis <gdr at integrable-solutions dot net>
- To: Ishikawa <ishikawa at yk dot rim dot or dot jp>
- Cc: "Joseph S. Myers" <jsm28 at cam dot ac dot uk>, Zack Weinberg <zack at codesourcery dot com>, Neil Booth <neil at daikokuya dot co dot uk>, gcc-patches at gcc dot gnu dot org
- Date: 21 Jul 2003 10:25:22 +0200
- Subject: Re: Support for %d$c format specifier in diagnostics.c
- Organization: Integrable Solutions
- References: <m19WKhW-000H3mC@standard.erephon><20030628193135.GA1767@daikokuya.co.uk><87llvlk3gi.fsf@egil.codesourcery.com><3EFE65DC.52576EC7@yk.rim.or.jp><87vfupiglu.fsf@egil.codesourcery.com><3EFEE0EF.C560ED1A@yk.rim.or.jp><87isqoiwng.fsf@egil.codesourcery.com><3F061172.814BA7A0@yk.rim.or.jp><Pine.LNX.4.56.0307051127540.6302@kern.srcf.societies.cam.ac.uk><3F0CA938.1FA02D3E@yk.rim.or.jp><m3vfuayizy.fsf@uniton.integrable-solutions.net><3F1A1654.4476090C@yk.rim.or.jp>
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.