Re: New scope checking patch

["Rob Quill" <rob dot quill at gmail dot com>]

On 11/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
>
> "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
>

Sorry about that, the attached patch is done with cvs diff -up

> >>          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.
>

Thanks for the tip :) I think the only example I had seen of parsing
actual values was where it used the parse number function, so I
assumed that was the right thing to use.

> > 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.

My mistake, I didn't realise before that keywords were declared here.

Rob

Index: gdb/c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -u -r1.42 c-exp.y
--- gdb/c-exp.y	9 Jan 2008 19:27:15 -0000	1.42
+++ gdb/c-exp.y	13 Jan 2008 20:22:26 -0000
@@ -208,6 +208,8 @@
 %token TRUEKEYWORD
 %token FALSEKEYWORD
 
+/* $in_scope opperator */
+%left IN_SCOPE
 
 %left ','
 %left ABOVE_COMMA
@@ -251,6 +253,32 @@
 	;
 
 /* Expressions, not including the comma operator.  */
+exp	: IN_SCOPE '(' name_not_typename ')'
+	{
+	  struct type *int_type;
+
+	  /* If there are no symbols then just stop right away */
+	  if (!have_full_symbols () && !have_partial_symbols ())
+		error ("No symbol table is loaded.  Use the \"file\" command.");
+
+	  /* Otherwise, prepare to write out the value */
+      int_type = builtin_type (current_gdbarch)->builtin_int;
+      write_exp_elt_opcode (OP_LONG);
+      write_exp_elt_type (int_type);
+
+	  if ($3.sym || lookup_minimal_symbol(copy_name($3.stoken), NULL, NULL))
+	  {
+        write_exp_elt_longcst ((LONGEST) 1);
+	  }
+	  else
+	  {
+	    write_exp_elt_longcst ((LONGEST) 0);
+	  }
+
+	  write_exp_elt_opcode (OP_LONG);
+	}
+	;
+
 exp	:	'*' exp    %prec UNARY
 			{ write_exp_elt_opcode (UNOP_IND); }
 	;
@@ -1678,6 +1706,9 @@
   /* Catch specific keywords.  Should be done with a data structure.  */
   switch (namelen)
     {
+	case 9:
+	  if (strncmp (tokstart, "$in_scope", 9) == 0)
+	return IN_SCOPE;
     case 8:
       if (strncmp (tokstart, "unsigned", 8) == 0)
 	return UNSIGNED;