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

[Ishikawa <ishikawa at yk dot rim dot or dot jp>]

"Joseph S. Myers" wrote:
> 
> On Sun, 27 Jul 2003, Zack Weinberg wrote:
> 
> > We do not have established lines of communication with the Translation
> > Project, so I do not want to try to explain to the translators that
> > they need to ignore the %H and count from the next argument.  I'd
> 
> Also, this notion (that %H consumes an argument, but operand numbers count
> from the next argument) is entirely unknown to the format checking code.
> That doesn't matter as long as the rule is followed that operand numbers
> are only ever used in .po files, but it might be possible to write a
> script that uses GCC's built-in format checking to validate .po files (by
> substituting the strings directly into GCC's source and building the
> compiler), which would fail if the .po files start having formats not
> comprehensible to the format checking code.
> 
> I agree that since %H is a format that does consume an argument, it should
> become %1$H when operand numbers are used (with special handling for %1$H
> at the start, if necessary) and the subsequent operands should be counted
> from 2 as usual.
> 

It is dawning on me now slowly.

So it would mean if we take out the text_specifies_location() processing
somehow from our way, we are better off.

Let us review the processing again


void
diagnostic_set_info (diagnostic_info *diagnostic, const char *msgid,
		     va_list *args, location_t location,
		     diagnostic_t kind)
{
  diagnostic->message.err_no = errno;
  diagnostic->message.args_ptr = args;
  diagnostic->message.format_spec = _(msgid);
  /* If the diagnostic message doesn't specify a location,
     use LOCATION.  */
  if (!text_specifies_location (&diagnostic->message,
&diagnostic->location))
    diagnostic->location = location;
  diagnostic->kind = kind;
}

If diagnostic->message contains leading %H then
diagnostic->location is set by picking up the value from
the first argument in va_list *args.
If not, diagnostic->location is set to location passed
as part of the parameter.

I checked the caller of diagnostic_set_info().

1	diagnostic_set_info {diagnostic.c 147}
2		error {diagnostic.c 527}
3		fatal_error {diagnostic.c 557}
4		inform {diagnostic.c 478}
5		internal_error {diagnostic.c 576}
6		pedwarn {diagnostic.c 512}
7		sorry {diagnostic.c 542}
8		warning {diagnostic.c 492}

Callers of diagnostic_set_info() shares the same usage pattern.

  va_start (ap, msgid);
  diagnostic_set_info (&diagnostic, msgid, &ap, input_location,
DK_ERROR);
  report_diagnostic (&diagnostic);
  va_end (ap);

Zack's suggestion would defer the setting of locus information deduced
by the leading "%H" (or "%1$H")
until pp_format_text() is called within report_diagnostic().

report_diagnostic is actually diagnostic_report_diagnostic().

Its essential part reads as follows.
 
      (*diagnostic_starter (context)) (context, diagnostic);
      pp_format_text (context->printer, &diagnostic->message);
      (*diagnostic_finalizer (context)) (context, diagnostic);
      pp_flush (context->printer);
      diagnostic_action_after_output (context, diagnostic);

Now the problem with deferred processing of "%H" within pp_format_text
and the subsequent setting of locus value there is
we have trouble if (*diagnostic_starter) tries to refer
to diagnostic->location.
I am afraid it does. ( I searched for "->location" and
it is used a few times in diagnostic.c.). 

After thinking over, now I have an idea that might solve 
the problem at hand.

Instead of letting 
text_specifies_location (&diagnostic->message, &diagnostic->location))
strip off "%H" or "%1$H" after it finds it, and set diagnostic->location
from the value in diagnostic->arg_ptr and bumps the variable argument
pointer, we let it
 - set the value of diagnostic->location accordingly (as before),
 - restore the diagnostic->arg_ptr to the original value after
   bumping it to obtain the locus value,
 - not rip  "%H" or "%1$H" from the format string.

With this modification, if the locus value obtained via
text_specifies_location()
via "%H..." is used with (*diagnostic_starter), the value
is available as before.

But now, pp_format_text() DOES RECEIVE the "%H" or "%1$H" at
the beginning of the format string. (and variable argument
list contains the corresponding value.)

Now a modification to pp_format_text() is in order.  When it sees "%H"
or "%1%H" as the first format specifier in the format string, we let
it bump the variable argument pointer to obtain the location_t value,
but we don't produce an output string from it.  No formatted printing
is done for "%H" or "%1$H" aside from the stack pointer manipulation.

With this change, we can have
the following processing.

   warning ("%H'%D' might be used uninitialized in this function",
   -> As before for a gedanken experiment, someone 
      prepares the following translation.
      "%1$H'%2$D' might be used uninitialized in this function",
   -> This is substitued in  diagnostic->message.format_spec = _(msgid);
   -> Now this message is passed to the now modified 
      text_specifies_loation() that leaves  %1$H intact.
      (diagnostic->location is set to the locus value by using
      *locus = *va_arg (*text->args_ptr, location_t *);

      Now text_specifies_location() restores the variable argument
pointer
      so that we can obtain location_t value again from the
      variable argument list.

   -> diagnostic->message.format_spec now remains as
      "%1$H'%2$D' might be used uninitialized in this function"

   -> pp_text_format() receives this format string and the
      unmodified variable argument list.
      No problem this time around.	
      The positional argument support mechanism 
      process this normally, and since
      the leading %H (%1$H) is not going to produce the
      output (the new modification), 
      the output remains the same as original (!).

Brilliant idea if I have not missed something :-)

The change to text_specifies_location would be something like as
follows: Please adivise me how the value related to va_arg processing
should be saved and restored. (I am a little confused about this.)

static bool
text_specifies_location (text_info *text, location_t *locus)
{
  const char *p;
  /* Skip any leading text.  */
  for (p = text->format_spec; *p && *p != '%'; ++p)
    ;

  /* Extract the location information if any.  */
  if (*p == '%' && *++p == 'H')
    {
 
      va_list old_arg_ptr; // addition.
      old_arg_ptr = *text->args_ptr; //add. should this be
**text->args_ptr? 
      *locus = *va_arg (*text->args_ptr, location_t *);
      // remove   text->format_spec = p + 1;
      *text->args_ptr = old_arg_ptr  // add. Should LHS be
**text->args_ptr?
      return true;
    }

  return false;
}


Happy Hacking,


-- 
int main(void){int j=2003;/*(c)2003 cishikawa. */
char t[] ="<CI> @abcdefghijklmnopqrstuvwxyz.,\n\"";
char *i ="g>qtCIuqivb,gCwe\np@.ietCIuqi\"tqkvv is>dnamz";
while(*i)((j+=strchr(t,*i++)-(int)t),(j%=sizeof t-1),
(putchar(t[j])));return 0;}/* under GPL */