Re: New scope checking patch

[Jim Blandy <jimb at codesourcery dot com>]

"Rob Quill" <rob.quill at gmail.com> writes:
> Please find attached the revised and much simpler patch. One of the
> drawbacks of the new patch is that you cannot check the scope of
> structure members, which is quite inconvenient, but I can't really fix
> that without resorting to the old style of patch.
>
> I think that for the moment this patch is sufficient and definitely
> provides and improvement, but I think in future it would be good to be
> able to check arbitrary expressions, but as you say in your previous
> email, this requires some significant reworking of the patch. It also
> depends on how much people will find it useful. Michael, how do you
> feel about this? (As you seemed interested in the previous attempt) If
> there is enough interest then I am more than happy to spend some time
> reworking the old patch at a later date.

Unfortunately, I can't try out this patch.  Absurdly, the default
output format for 'diff' can't be used reliably by patch, because it
doesn't include any context lines (unchanged text surrounding the
changed text), which 'patch' uses to ensure that the change doesn't
conflict with other changes made to the code.

Use 'cvs diff -u' or 'cvs diff -c' to produce unified or context
diffs.  Unified diffs seem to be the more widely preferred form these
days.  All patches posted to this list (or pretty much to any other
open source list) should be in unified or context diff form.

I have the following in my ~/.cvsrc file, so I don't have to remember
to type '-u' all the time:

        diff -u

>> 	    parse_number ("1", 1, 0, &val);
>> 	    write_exp_elt_opcode (OP_LONG);
>> 	    write_exp_elt_type (val.typed_val_int.type);
>> 	    write_exp_elt_longcst ((LONGEST)val.typed_val_int.val);
>> 	    write_exp_elt_opcode (OP_LONG);

You've written out this code three times; that's quite a chunk.  I'd
recommend first deciding whether the symbol is in scope (checking
'$3.sym', and then calling lookup_minimal_symbol, as you do now), and
computing a one or a zero from that.

Then, did you know that you can get away with, simply:

          struct type *int_type = builtin_type (current_gdbarch)->builtin_int;
          write_exp_elt_opcode (OP_LONG);
          write_exp_elt_type (int_type);
          write_exp_elt_longcst (<flag value>);
          write_exp_elt_opcode (OP_LONG);

So there's no need to parse the number from a string.

> 1313a1357,1361
>> static const struct token tokentab9[] =
>>   {
>>     {"$in_scope", IN_SCOPE, BINOP_END}
>>   };
>>
> 1375a1424,1432
>>   /* Code for recognising the $in_scope token. */
>>   /* See if it is a special token of length 9.  */
>>   for (i = 0; i < sizeof tokentab9 / sizeof tokentab9[0]; i++)
>>     if (strncmp (tokstart, tokentab9[i].operator, 9) == 0)
>>     {
>> 	    lexptr += 9;
>> 	    yylval.opcode = tokentab9[i].opcode;
>> 	    return tokentab9[i].token;
>>     }

I think I would rather see $in_scope recognized like the other
keywords (see "Catch specific keywords.") than at this point as a
symbolic token.