Re: [rfa] linespec.c, part 5

[David Carlton <carlton at math dot stanford dot edu>]

On Thu, 5 Dec 2002 12:12:15 -0500, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:

>> Maybe I'll replace the these comments right after the end of
>> decode_line_1:
>> 
>> /* Now, still more helper functions.  */
>> 
>> /* NOTE: carlton/2002-11-07: Some of these have non-obvious side
>> effects.  In particular, if a function is passed ARGPTR as an
>> argument, it modifies what ARGPTR points to.  (Typically, it
>> advances *ARGPTR past whatever substring it has just looked
>> at.)  */
>> 
>> with a comment saying:
>> 
>> /* Now, more helper functions for decode_line_1.  Some conventions
>> that these functions follow:
>> 
>> Decode_line_1 typically passes along some of its arguments or local
>> variables to the subfunctions.  It passes the variables by
>> reference if they are modified by the subfunction, and by value
>> otherwise.
>> 
>> Some of the functions have side effects that don't arise from
>> variables that are passed by reference.  In particular, if a
>> function is passed ARGPTR as an argument, it modifies what ARGPTR
>> points to; typically, it advances *ARGPTR past whatever substring
>> it has just looked at.  (If it doesn't modify *ARGPTR, then the
>> function gets passed *ARGPTR instead, which is then called ARG: see
>> set_flags, for example.)  Also, functions that return a struct
>> symtabs_and_lines may modify CANONICAL, as in the description of
>> decode_line_1.
>> 
>> If a function returns a struct symtabs_and_lines, then that struct
>> will immediately make its way up the call chain to be returned by
>> decode_line_1.  In particular, all of the functions decode_XXX
>> calculate the appropriate struct symtabs_and_lines, under the
>> assumption that their argument is of the form XXX.  */
>> 
>> Is that clearer?

> better, yes.

> BTW, I did a diff -w of decode_compound and the code you removed, and
> I noticed this (ignore the line numbers):

> @@ -211,7 +231,6 @@
>           *argptr = (*p == '\'') ? p + 1 : p;
>           /* Look up entire name */
>           sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
> -         s = (struct symtab *) 0;
>           if (sym)
>             return symbol_found (funfirstline, canonical, copy, sym,
>                                  NULL, sym_symtab);

Oh, sorry, I should have explained that.  As you noticed, the code
once looked something like

  s = (struct symtab *) 0;
  if (sym)
    goto symbol_found;

(there was probably other stuff there).

Then my first patch replaced all of the "goto symbol_found"'s by
"return symbol_found (a bunch of arguments, including s)".  So that
would have turned the code into

  s = (struct symtab *) 0;
  if (sym)
    return symbol_found (funfirstline, canonical, copy, sym,
                         s, sym_symtab);

except that, since we know that s is 0 there, I replaced the s by
NULL, giving us

  s = (struct symtab *) 0;
  if (sym)
    return symbol_found (funfirstline, canonical, copy, sym,
                         NULL, sym_symtab);

But at the time of my first patch, I couldn't say for sure if another
flow of control might be affected by the "s = (struct symtab *) 0;"
line, so I left it in there.

This patch, however, allows us to answer that question: the only place
in the new decode_compound function where 's' is referred to is in
that line that is deleted, and since decode_line_1 immediately returns
the value of decode_compound, setting 's' doesn't have any effect.  So
I deleted it.

> OK check it in.

Thanks, will do (with the expanded comment mentioned above).

David Carlton
carlton@math.stanford.edu