New scope checking patch

New scope checking patch

[Rob Quill]

[-- Attachment #1: Type: text/plain, Size: 2872 bytes --]

Hi all,

The attached patch adds the ability to evaluate whether expressions
are in scope without giving an error message. For example, if you were
using a command file to control GDB then it it may not be ideal that
the script stop executing due to a variable not being in scope at
given position.

This patch allows you to say (for example)

while(the program is not finished)
{
    if($in_scope(x)) print x;
    step;
}

which would step through he whole program and print all the values of
x where x is valid, and just continue stepping where it is not valid.

The bulk of the patch is changes to the evaluate_subexp functions in eval.c

The following cases are cases which the core is written for, but have
not been tested yet:

case OP_SCOPE:
case OP_REGISTER:
case OP_STRING:
case OP_OBJC_NSSTRING:		/* Objective C Foundation
case OP_BITSTRING:
case UNOP_MEMVAL:
case UNOP_MEMVAL_TLS:
case OP_OBJC_SELF

These cases have not been tested as I am unsure of code or commands
which would allow me to test them. If anyone has any suggestions then
that is greatly appreciated.

The following cases are unimplemented as I am unsure of how to check
the scope of these expressions. Any advice you have is greatly
appreciated.

case OP_ARRAY:
case TERNOP_SLICE:
case TERNOP_SLICE_COUNT:
case TERNOP_COND:
case OP_OBJC_SELECTOR:
case OP_OBJC_MSGCALL:
case OP_FUNCALL:
case OP_F77_UNDETERMINED_ARGLIST:
case STRUCTOP_PTR:
case STRUCTOP_MEMBER:
case STRUCTOP_MPTR:
//	case TYPE_CODE_METHODPTR:
//	case TYPE_CODE_MEMBERPTR:
case MULTI_SUBSCRIPT:
case UNOP_IND:

For example, I think for a function call the way to check scope should
be to check that there is a function in scope with that name, and that
all the parameters are in scope. However, I am unsure how to implement
this.

TYPE_CODE_METHODPTR and TYPE_CODE_MEMBERPTR are commented out as they
are supposedly redefinitions of two other cases (greater than and less
than operators, I believe)

I should also point out that expression are evaluated without side
effects, so that, for example $in_scope(a = b) will return 1 if a and
b are in scope, but will not change the value of a.

There is a caveat with the patch which is as follows. If a variable is
not in scope then the expression parser in c-exp.y inserts a value 0
into the expression tree. When the scope checking function sees a 0
expression in the tree it will return 0, to indicate not in scope.
However, in the general case it should be true that all literals are
in scope. However, due to the above caveat $in_scope(0) will return 0,
any any other literal will return 1.

This also affects compound expressions that involve the literal 0. For
example, $in_scope(0+3) will return 0, as an expression with a binary
operator is considering in scope iff both of its operands are in
scope.

Any thoughts on the patch are greatly appreciated.

Thanks.

Rob Quill

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: new_scope_check.patch --]
[-- Type: text/x-patch; name=new_scope_check.patch, Size: 18345 bytes --]

diff -urN ./clean/src/gdb/29k-share/CVS/Entries ./modified/src/gdb/29k-share/CVS/Entries
--- ./clean/src/gdb/29k-share/CVS/Entries	2007-11-11 17:08:33.000000000 +0000
+++ ./modified/src/gdb/29k-share/CVS/Entries	2007-11-11 16:27:06.000000000 +0000
@@ -1 +1 @@
-D/udi////
+D
diff -urN ./clean/src/gdb/29k-share/CVS/Entries.Log ./modified/src/gdb/29k-share/CVS/Entries.Log
--- ./clean/src/gdb/29k-share/CVS/Entries.Log	1970-01-01 01:00:00.000000000 +0100
+++ ./modified/src/gdb/29k-share/CVS/Entries.Log	2007-11-11 16:27:06.000000000 +0000
@@ -0,0 +1 @@
+A D/udi////
diff -urN ./clean/src/gdb/c-exp.y ./modified/src/gdb/c-exp.y
--- ./clean/src/gdb/c-exp.y	2007-11-11 17:08:42.000000000 +0000
+++ ./modified/src/gdb/c-exp.y	2007-11-11 17:05:35.000000000 +0000
@@ -102,6 +102,9 @@
 #define yytable	 c_yytable
 #define yycheck	 c_yycheck
 
+/* Global variable denoting whether we are only interested in scope, not value */
+int check_scope = 0;
+
 #ifndef YYDEBUG
 #define	YYDEBUG 1		/* Default to yydebug support */
 #endif
@@ -207,6 +210,8 @@
 %token TRUEKEYWORD
 %token FALSEKEYWORD
 
+/* $in_scope opperator */
+%left IN_SCOPE
 
 %left ','
 %left ABOVE_COMMA
@@ -250,6 +255,11 @@
 	;
 
 /* Expressions, not including the comma operator.  */
+exp	:	IN_SCOPE
+			{ check_scope = 1; }
+		'(' exp ')'
+	;
+
 exp	:	'*' exp    %prec UNARY
 			{ write_exp_elt_opcode (UNOP_IND); }
 	;
@@ -596,8 +606,25 @@
 					       VAR_DOMAIN, (int *) NULL,
 					       (struct symtab **) NULL);
 			  if (sym == 0)
-			    error ("No symbol \"%s\" in specified context.",
+			  {
+			    /* Case for scope checking. If scope is being checked and
+				   the symbol is not in scope, return an expresison of
+				   value 0. */
+			    if(check_scope == 0)
+			    {
+			       error ("No symbol \"%s\" in specified context.",
 				   copy_name ($3));
+			    }
+			    else
+			    {
+				   YYSTYPE val;
+			 	   parse_number ("0", 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);
+			    }
+			  }
 
 			  write_exp_elt_opcode (OP_VAR_VALUE);
 			  /* block_found is set by lookup_symbol.  */
@@ -676,8 +703,22 @@
 			    if (!have_full_symbols () && !have_partial_symbols ())
 			      error ("No symbol table is loaded.  Use the \"file\" command.");
 			    else
-			      error ("No symbol \"%s\" in current context.", name);
-			}
+			    {
+			      if(check_scope == 0)
+			      {
+			    	error ("No symbol \"%s\" in current context.", name);
+			      }
+			      else
+			      {
+                     YYSTYPE val;
+                     parse_number ("0", 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);
+			      }
+			    }
+}
 	;
 
 variable:	name_not_typename
@@ -731,8 +772,19 @@
 			      else if (!have_full_symbols () && !have_partial_symbols ())
 				error ("No symbol table is loaded.  Use the \"file\" command.");
 			      else
-				error ("No symbol \"%s\" in current context.",
-				       copy_name ($1.stoken));
+			      {
+			        if(check_scope == 0)
+				       error ("No symbol \"%s\" in current context.", copy_name ($1.stoken));
+			        else
+			        {
+				       YYSTYPE val;
+			 	       parse_number ("0", 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);
+				    }
+			      }
 			    }
 			}
 	;
@@ -1310,6 +1362,11 @@
   enum exp_opcode opcode;
 };
 
+static const struct token tokentab9[] =
+  {
+    {"$in_scope", IN_SCOPE, BINOP_END}
+  };
+
 static const struct token tokentab3[] =
   {
     {">>=", ASSIGN_MODIFY, BINOP_RSH},
@@ -1372,6 +1429,15 @@
   prev_lexptr = lexptr;
 
   tokstart = lexptr;
+  /* 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;
+      }
   /* See if it is a special token of length 3.  */
   for (i = 0; i < sizeof tokentab3 / sizeof tokentab3[0]; i++)
     if (strncmp (tokstart, tokentab3[i].operator, 3) == 0)
diff -urN ./clean/src/gdb/eval.c ./modified/src/gdb/eval.c
--- ./clean/src/gdb/eval.c	2007-11-11 17:08:42.000000000 +0000
+++ ./modified/src/gdb/eval.c	2007-11-12 15:54:37.000000000 +0000
@@ -44,11 +44,15 @@
 /* This is defined in valops.c */
 extern int overload_resolution;
 
+/* Variable denoting is scope is being checked */
+extern int check_scope;
+
 /* JYG: lookup rtti type of STRUCTOP_PTR when this is set to continue
    on with successful lookup for member/method of the rtti type. */
 extern int objectprint;
 
 /* Prototypes for local functions. */
+static struct value *evaluate_subexp_for_scope (struct expression *, int *);
 
 static struct value *evaluate_subexp_for_sizeof (struct expression *, int *);
 
@@ -162,7 +166,24 @@
 evaluate_expression (struct expression *exp)
 {
   int pc = 0;
-  return evaluate_subexp (NULL_TYPE, exp, &pc, EVAL_NORMAL);
+
+  /* Modifications so that if we are checking scope we can 
+     reset the value of the variable to 0, so we don't check 
+     scope when we don't want to */
+  struct value *val = NULL;
+  if(check_scope == 1)
+  {
+    val = evaluate_subexp_for_scope(exp, &pc);
+  }
+  else
+  {
+	val = evaluate_subexp (NULL_TYPE, exp, &pc, EVAL_NORMAL);
+  }
+
+  if(check_scope == 1)
+    check_scope = 0;
+
+  return val;
 }
 
 /* Evaluate an expression, avoiding all memory references
@@ -443,12 +464,15 @@
 				  &exp->elts[pc + 3].string,
 				  0, noside);
       if (arg1 == NULL)
-	error (_("There is no field named %s"), &exp->elts[pc + 3].string);
+      {
+	  error (_("There is no field named %s"), &exp->elts[pc + 3].string);
+      }
+
       return arg1;
 
     case OP_LONG:
       (*pos) += 3;
-      return value_from_longest (exp->elts[pc + 1].type,
+        return value_from_longest (exp->elts[pc + 1].type,
 				 exp->elts[pc + 2].longconst);
 
     case OP_DOUBLE:
@@ -461,7 +485,7 @@
       return value_from_decfloat (expect_type, exp->elts[pc + 1].type,
 				exp->elts[pc + 2].decfloatconst);
 
-    case OP_VAR_VALUE:
+	case OP_VAR_VALUE:
       (*pos) += 3;
       if (noside == EVAL_SKIP)
 	goto nosideret;
@@ -495,7 +519,7 @@
 
 	return ret;
       }
-
+ 
     case OP_LAST:
       (*pos) += 2;
       return
@@ -523,20 +547,22 @@
       }
     case OP_BOOL:
       (*pos) += 2;
-      return value_from_longest (LA_BOOL_TYPE,
-				 exp->elts[pc + 1].longconst);
+        return value_from_longest (LA_BOOL_TYPE,
+				   exp->elts[pc + 1].longconst);
+
 
     case OP_INTERNALVAR:
       (*pos) += 2;
-      return value_of_internalvar (exp->elts[pc + 1].internalvar);
+        return value_of_internalvar (exp->elts[pc + 1].internalvar);
+
 
     case OP_STRING:
       tem = longest_to_int (exp->elts[pc + 1].longconst);
       (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
       if (noside == EVAL_SKIP)
 	goto nosideret;
-      return value_string (&exp->elts[pc + 2].string, tem);
-
+	  return value_string (&exp->elts[pc + 2].string, tem);
+      
     case OP_OBJC_NSSTRING:		/* Objective C Foundation Class NSString constant.  */
       tem = longest_to_int (exp->elts[pc + 1].longconst);
       (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
@@ -544,6 +570,7 @@
 	{
 	  goto nosideret;
 	}
+
       return (struct value *) value_nsstring (&exp->elts[pc + 2].string, tem + 1);
 
     case OP_BITSTRING:
@@ -552,6 +579,7 @@
 	+= 3 + BYTES_TO_EXP_ELEM ((tem + HOST_CHAR_BIT - 1) / HOST_CHAR_BIT);
       if (noside == EVAL_SKIP)
 	goto nosideret;
+
       return value_bitstring (&exp->elts[pc + 2].string, tem);
       break;
 
@@ -1337,7 +1365,9 @@
     case STRUCTOP_STRUCT:
       tem = longest_to_int (exp->elts[pc + 1].longconst);
       (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
+
       arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
+
       if (noside == EVAL_SKIP)
 	goto nosideret;
       if (noside == EVAL_AVOID_SIDE_EFFECTS)
@@ -1438,8 +1468,11 @@
     case BINOP_CONCAT:
       arg1 = evaluate_subexp_with_coercion (exp, pos, noside);
       arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
+
+	return value_binop (arg1, arg2, BINOP_LOGICAL_AND);
+
       if (noside == EVAL_SKIP)
-	goto nosideret;
+        goto nosideret;
       if (binop_user_defined_p (op, arg1, arg2))
 	return value_x_binop (arg1, arg2, op, OP_NULL, noside);
       else
@@ -1450,32 +1483,36 @@
       arg2 = evaluate_subexp (value_type (arg1), exp, pos, noside);
 
       if (noside == EVAL_SKIP || noside == EVAL_AVOID_SIDE_EFFECTS)
-	return arg1;
+        return arg1;
       if (binop_user_defined_p (op, arg1, arg2))
-	return value_x_binop (arg1, arg2, op, OP_NULL, noside);
+        return value_x_binop (arg1, arg2, op, OP_NULL, noside);
       else
 	return value_assign (arg1, arg2);
-
+     
+      
     case BINOP_ASSIGN_MODIFY:
       (*pos) += 2;
       arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
       arg2 = evaluate_subexp (value_type (arg1), exp, pos, noside);
+
       if (noside == EVAL_SKIP || noside == EVAL_AVOID_SIDE_EFFECTS)
-	return arg1;
+        return arg1;
+ 
       op = exp->elts[pc + 1].opcode;
       if (binop_user_defined_p (op, arg1, arg2))
-	return value_x_binop (arg1, arg2, BINOP_ASSIGN_MODIFY, op, noside);
+        return value_x_binop (arg1, arg2, BINOP_ASSIGN_MODIFY, op, noside);
       else if (op == BINOP_ADD)
-	arg2 = value_add (arg1, arg2);
+        arg2 = value_add (arg1, arg2);
       else if (op == BINOP_SUB)
-	arg2 = value_sub (arg1, arg2);
+        arg2 = value_sub (arg1, arg2);
       else
-	arg2 = value_binop (arg1, arg2, op);
+        arg2 = value_binop (arg1, arg2, op);
       return value_assign (arg1, arg2);
 
     case BINOP_ADD:
       arg1 = evaluate_subexp_with_coercion (exp, pos, noside);
       arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
+
       if (noside == EVAL_SKIP)
 	goto nosideret;
       if (binop_user_defined_p (op, arg1, arg2))
@@ -1486,6 +1523,7 @@
     case BINOP_SUB:
       arg1 = evaluate_subexp_with_coercion (exp, pos, noside);
       arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
+
       if (noside == EVAL_SKIP)
 	goto nosideret;
       if (binop_user_defined_p (op, arg1, arg2))
@@ -1505,6 +1543,7 @@
     case BINOP_BITWISE_XOR:
       arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
       arg2 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
+
       if (noside == EVAL_SKIP)
 	goto nosideret;
       if (binop_user_defined_p (op, arg1, arg2))
@@ -1518,6 +1557,7 @@
     case BINOP_RANGE:
       arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
       arg2 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
+
       if (noside == EVAL_SKIP)
 	goto nosideret;
       error (_("':' operator used in invalid context"));
@@ -1525,6 +1565,7 @@
     case BINOP_SUBSCRIPT:
       arg1 = evaluate_subexp_with_coercion (exp, pos, noside);
       arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
+
       if (noside == EVAL_SKIP)
 	goto nosideret;
       if (binop_user_defined_p (op, arg1, arg2))
@@ -1556,6 +1597,7 @@
     case BINOP_IN:
       arg1 = evaluate_subexp_with_coercion (exp, pos, noside);
       arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
+
       if (noside == EVAL_SKIP)
 	goto nosideret;
       return value_in (arg1, arg2);
@@ -1741,10 +1783,12 @@
       else
 	{
 	  tem = value_logical_not (arg1);
-	  arg2 = evaluate_subexp (NULL_TYPE, exp, pos,
+
+      arg2 = evaluate_subexp (NULL_TYPE, exp, pos,
 				  (!tem ? EVAL_SKIP : noside));
+
 	  return value_from_longest (LA_BOOL_TYPE,
-			     (LONGEST) (!tem || !value_logical_not (arg2)));
+			       (LONGEST) (!tem || !value_logical_not (arg2)));
 	}
 
     case BINOP_EQUAL:
@@ -2085,7 +2129,202 @@
 nosideret:
   return value_from_longest (builtin_type_long, (LONGEST) 1);
 }
-\f
+
+/* Evaluate a subexpression to see if the expression
+ * is in scope. Some special case scope checking
+ * remains in evaluate_subexp_standard(). */
+static struct value *
+evaluate_subexp_for_scope (struct expression *exp, int *pos)
+{
+	struct value *arg1 = NULL;
+	struct value *arg2 = NULL;
+	
+	int tem;
+  	int pc = (*pos)++;
+    enum exp_opcode op = exp->elts[pc].opcode;
+
+    switch(op)
+	{
+	case OP_SCOPE:
+      tem = longest_to_int (exp->elts[pc + 2].longconst);
+      (*pos) += 4 + BYTES_TO_EXP_ELEM (tem + 1);
+
+	  arg1 = value_aggregate_elt (exp->elts[pc + 1].type,
+				  &exp->elts[pc + 3].string,
+				  0, EVAL_NORMAL);
+      if (arg1 == NULL)
+        return value_from_longest (builtin_type_int, (LONGEST) 0);
+
+    case OP_LONG:
+    case OP_DOUBLE:
+	case OP_DECFLOAT:
+	case OP_VAR_VALUE:
+      (*pos) += 3;
+
+	  /* Special case, if a variable is not in scope the parser
+	   * puts an expression of value 0. If the expression is
+	   * of value 0, return one aof 0, else return a vlaue of 1.
+	   */
+	if(exp->elts[pc + 2].longconst == 0)
+	  return value_from_longest (builtin_type_int, (LONGEST) 0);
+	else
+	  return value_from_longest (builtin_type_int, (LONGEST) 1);
+
+    case OP_LAST:
+	case OP_INTERNALVAR:
+      (*pos) += 2;
+      return value_from_longest (builtin_type_int, (LONGEST) 1);
+
+    case OP_REGISTER:
+	  {
+	const char *name = &exp->elts[pc + 2].string;
+	int regno;
+	struct value *val;
+
+	(*pos) += 3 + BYTES_TO_EXP_ELEM (exp->elts[pc + 1].longconst + 1);
+	regno = frame_map_name_to_regnum (deprecated_safe_get_selected_frame (),
+					  name, strlen (name));
+
+	if (regno == -1)
+      return value_from_longest (builtin_type_int, (LONGEST) 0);
+
+	  val = value_of_register (regno, get_selected_frame (NULL));
+	
+  	if (val == NULL)
+	  return value_from_longest (builtin_type_int, (LONGEST) 0);
+	else
+	  return value_from_longest (builtin_type_int, (LONGEST) 1);
+	  }
+    case OP_STRING:
+    case OP_OBJC_NSSTRING:		/* Objective C Foundation Class NSString constant.  */
+      tem = longest_to_int (exp->elts[pc + 1].longconst);
+      (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
+
+  	  return value_from_longest (builtin_type_int, (LONGEST) 1);
+
+    case OP_BITSTRING:
+      tem = longest_to_int (exp->elts[pc + 1].longconst);
+      (*pos)
+	+= 3 + BYTES_TO_EXP_ELEM ((tem + HOST_CHAR_BIT - 1) / HOST_CHAR_BIT);
+
+      return value_from_longest (builtin_type_int, (LONGEST) 1);
+
+	/* FIXME: Unsure of how to check scope for these, so deal wiht them normally */
+    case OP_ARRAY:
+    case TERNOP_SLICE:
+    case TERNOP_SLICE_COUNT:
+    case TERNOP_COND:
+    case OP_OBJC_SELECTOR:
+    case OP_OBJC_MSGCALL:
+	case OP_FUNCALL:
+	case OP_F77_UNDETERMINED_ARGLIST:
+    case STRUCTOP_PTR:
+    case STRUCTOP_MEMBER:
+    case STRUCTOP_MPTR:
+/*	case TYPE_CODE_METHODPTR:
+	case TYPE_CODE_MEMBERPTR: */
+    case MULTI_SUBSCRIPT:
+    case UNOP_IND:
+	  return evaluate_subexp(NULL_TYPE, exp, pos, EVAL_NORMAL);
+
+    case STRUCTOP_STRUCT:
+	
+      tem = longest_to_int (exp->elts[pc + 1].longconst);
+      (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
+
+      /* Evaluate the sub-expression without scope so that the
+	   * correct type of value is returned */
+      arg1 = evaluate_subexp (NULL_TYPE, exp, pos, EVAL_NORMAL);
+    {
+ 	  struct value *temp;
+	  temp = arg1;
+	  return value_struct_elt (&temp, NULL, &exp->elts[pc + 2].string,
+				   NULL, "structure");
+	}
+	/* An expression which involves a binary operator is in scope
+	 * iff it's two sub expressions are in scope. OP_COMPLEX
+	 * is also regared as a binary operator in this respect. */
+    case BINOP_CONCAT:
+    case BINOP_ASSIGN:
+    case BINOP_ASSIGN_MODIFY:
+    case BINOP_ADD:
+    case BINOP_SUB:
+    case BINOP_EXP:
+    case BINOP_MUL:
+    case BINOP_DIV:
+    case BINOP_REM:
+    case BINOP_MOD:
+    case BINOP_LSH:
+    case BINOP_RSH:
+    case BINOP_BITWISE_AND:
+    case BINOP_BITWISE_IOR:
+    case BINOP_BITWISE_XOR:
+    case BINOP_RANGE:
+    case BINOP_SUBSCRIPT:
+    case BINOP_IN:
+    case BINOP_LOGICAL_AND:
+    case BINOP_LOGICAL_OR:
+    case BINOP_EQUAL:
+    case BINOP_NOTEQUAL:
+    case BINOP_LESS:
+    case BINOP_GTR:
+    case BINOP_GEQ:
+    case BINOP_LEQ:
+    case BINOP_REPEAT:
+    case BINOP_COMMA:
+    case OP_COMPLEX:
+  	  arg1 = evaluate_subexp_for_scope (exp, pos);
+      arg2 = evaluate_subexp_for_scope (exp, pos);
+
+	  return value_binop (arg1, arg2, BINOP_LOGICAL_AND);
+
+    case UNOP_PLUS:
+    case UNOP_NEG:
+    case UNOP_COMPLEMENT:
+    case UNOP_LOGICAL_NOT:
+    case UNOP_ADDR:
+    case UNOP_SIZEOF:
+    case UNOP_PREINCREMENT:
+    case UNOP_PREDECREMENT:
+    case UNOP_POSTINCREMENT:
+    case UNOP_POSTDECREMENT:
+ 	  arg1 = evaluate_subexp_for_scope (exp, pos);
+	  return arg1;
+
+    case UNOP_CAST:
+    case UNOP_MEMVAL:
+      (*pos) += 2;
+      arg1 = evaluate_subexp_for_scope (exp, pos);
+	  return arg1;
+
+    case UNOP_MEMVAL_TLS:
+      (*pos) += 3;
+      arg1 = evaluate_subexp_for_scope (exp, pos);
+	  return arg1;
+
+  	case OP_THIS:
+      (*pos) += 1;
+      return value_from_longest (builtin_type_int, (LONGEST) 1);
+
+    case OP_OBJC_SELF:
+      (*pos) += 1;
+      return value_from_longest (builtin_type_int, (LONGEST) 1);
+
+    default:
+      /* Removing this case and compiling with gcc -Wall reveals that
+         a lot of cases are hitting this case.  Some of these should
+         probably be removed from expression.h; others are legitimate
+         expressions which are (apparently) not fully implemented.
+
+         If there are any cases landing here which mean a user error,
+         then they should be separate cases, with more descriptive
+         error messages.  */
+
+      error (_("\
+GDB does not (yet) know how to evaluate that kind of expression for scope"));
+    }
+}
+
 /* Evaluate a subexpression of EXP, at index *POS,
    and return the address of that subexpression.
    Advance *POS over the subexpression.

Re: New scope checking patch

[Michael Snyder]

On Mon, 2007-11-12 at 16:29 +0000, Rob Quill wrote:
> Hi all,
> 
> The attached patch adds the ability to evaluate whether expressions
> are in scope without giving an error message. For example, if you were
> using a command file to control GDB then it it may not be ideal that
> the script stop executing due to a variable not being in scope at
> given position.

That's great!  I've often wanted something like that.

> 
> This patch allows you to say (for example)
> 
> while(the program is not finished)
> {
>     if($in_scope(x)) print x;
>     step;
> }
> 
> which would step through he whole program and print all the values of
> x where x is valid, and just continue stepping where it is not valid.
> 
> The bulk of the patch is changes to the evaluate_subexp functions in eval.c
> 
> The following cases are cases which the core is written for, but have
> not been tested yet:
> 
> case OP_SCOPE:
> case OP_REGISTER:
> case OP_STRING:
> case OP_OBJC_NSSTRING:		/* Objective C Foundation
> case OP_BITSTRING:
> case UNOP_MEMVAL:
> case UNOP_MEMVAL_TLS:
> case OP_OBJC_SELF
> 
> These cases have not been tested as I am unsure of code or commands
> which would allow me to test them. If anyone has any suggestions then
> that is greatly appreciated.
> 
> The following cases are unimplemented as I am unsure of how to check
> the scope of these expressions. Any advice you have is greatly
> appreciated.
> 
> case OP_ARRAY:
> case TERNOP_SLICE:
> case TERNOP_SLICE_COUNT:
> case TERNOP_COND:
> case OP_OBJC_SELECTOR:
> case OP_OBJC_MSGCALL:
> case OP_FUNCALL:
> case OP_F77_UNDETERMINED_ARGLIST:
> case STRUCTOP_PTR:
> case STRUCTOP_MEMBER:
> case STRUCTOP_MPTR:
> //	case TYPE_CODE_METHODPTR:
> //	case TYPE_CODE_MEMBERPTR:
> case MULTI_SUBSCRIPT:
> case UNOP_IND:
> 
> For example, I think for a function call the way to check scope should
> be to check that there is a function in scope with that name, and that
> all the parameters are in scope. However, I am unsure how to implement
> this.
> 
> TYPE_CODE_METHODPTR and TYPE_CODE_MEMBERPTR are commented out as they
> are supposedly redefinitions of two other cases (greater than and less
> than operators, I believe)
> 
> I should also point out that expression are evaluated without side
> effects, so that, for example $in_scope(a = b) will return 1 if a and
> b are in scope, but will not change the value of a.
> 
> There is a caveat with the patch which is as follows. If a variable is
> not in scope then the expression parser in c-exp.y inserts a value 0
> into the expression tree. When the scope checking function sees a 0
> expression in the tree it will return 0, to indicate not in scope.
> However, in the general case it should be true that all literals are
> in scope. However, due to the above caveat $in_scope(0) will return 0,
> any any other literal will return 1.
> 
> This also affects compound expressions that involve the literal 0. For
> example, $in_scope(0+3) will return 0, as an expression with a binary
> operator is considering in scope iff both of its operands are in
> scope.
> 
> Any thoughts on the patch are greatly appreciated.
> 
> Thanks.
> 
> Rob Quill

Re: New scope checking patch

[Jim Blandy]

"Rob Quill" <rob.quill at gmail.com> writes:
> The attached patch adds the ability to evaluate whether expressions
> are in scope without giving an error message. For example, if you were
> using a command file to control GDB then it it may not be ideal that
> the script stop executing due to a variable not being in scope at
> given position.

Hi, Rob.  I'm sorry for dropping the review here.

Some mechanical points:

- Patches should have ChangeLog entries.  For example, take a look at
  this post:
  http://sourceware.org/ml/gdb-patches/2008-01/msg00144.html

- Use 'cvs diff' to get the difference between your current sources
  and the public file revisions they're based on; patches that affect
  CVS metadata like "CVS/Entries" files can be treacherous if applied
  unawares.  :)

- GDB's code ought to follow the GNU Coding Standards; in particular,
  lines shouldn't be longer than eighty characters.  (c-exp.y in
  particular has a lot of particularly horrible formatting, but we've
  got to fight back; new patches should not make things worse.)  Also,
  indentation should follow GNU style, for consistency with the rest
  of the code.

- The patch includes a number of hunks that are strictly whitespace
  changes in code that's not otherwise involved in the change; they
  require reviewers to check the 'before' and 'after' text by hand, to
  see whether anything has actually changed.  Try to keep extraneous
  hunks out of patches.

As to the patch:

The functionality I originally suggested was to have an operator
$in_scope(X), where X is an identifier.  What you've implemented is a
change that allows X to be an arbitrary expression, such that
$in_scope (X) is true if all the identifiers in X are bound.  I gather
this was more useful for your original purpose, scripting GDB.  That's
fine, but it does make the change a bit harder.

Having the $in_scope operator set a global flag which is cleared by
evaluate_expression isn't the ideal approach; global flags often have
effects beyond what was intended.  For example:

- If I enter the expression '$in_scope (x) + y', the patch will
  evaluate the entire expression using evaluate_for_scope, not just
  the '$in_scope (x)' part.  So $in_scope doesn't act like an operator
  checking its operands; its presence affects the evaluation of the
  whole expression.

- GDB sometimes parses an expression, and then evaluates it many times
  over a long period of time (say, conditions for breakpoints).
  Sometimes it parses an expression, and then doesn't evaluate it for
  a while (say, 'display' expressions set when the program isn't
  running).  So setting check_scope when the expression is parsed, and
  then checking and clearing it when the expression is evaluated,
  won't work --- check_scope will be clear for expressions that need
  it (in the first case) and set for expressions that don't (the
  second case).

The patch also contains what looks like some abandoned experiments,
or changes that belong in evaluate_subexp_for_scope but escaped into
other functions.  Please be careful with this:

@@ -1438,8 +1468,11 @@ evaluate_subexp_standard (struct type *e
     case BINOP_CONCAT:
       arg1 = evaluate_subexp_with_coercion (exp, pos, noside);
       arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
-      if (noside == EVAL_SKIP)
-	goto nosideret;
+
+	return value_binop (arg1, arg2, BINOP_LOGICAL_AND);
+
+      if (noside == EVAL_SKIP)
+        goto nosideret;
       if (binop_user_defined_p (op, arg1, arg2))
 	return value_x_binop (arg1, arg2, op, OP_NULL, noside);
       else

> This also affects compound expressions that involve the literal 0. For
> example, $in_scope(0+3) will return 0, as an expression with a binary
> operator is considering in scope iff both of its operands are in
> scope.

If you want to have $in_scope take an arbitrary expression as its
operand, then cases like this will need to be handled properly.
Otherwise, people won't be able to reliably take whatever expressions
they have at hand and apply $in_scope to them.

If restricting $in_scope (X) to the case where X is a single
identifier is useful to you, the patch can be much simpler.  Have the
grammar rule be:

   exp: IN_SCOPE '(' name ')' { ... }

where the '...' calls lookup_symbol itself, the way the 'variable'
non-terminal does, and then produces an OP_LONG whose value is zero or
one, depending on whether lookup_symbol found anything?  That way, no
changes are needed to eval.c at all.

Re: New scope checking patch

[Rob Quill]

Hi,

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.

Rob

2008-01-11   Rob Quill <rob.quill@gmail.com>
        * c-exp.y : Add $in_scope as a type of expression.

Index: gdb/c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -r1.42 c-exp.y
210a211,212
> /* $in_scope opperator */
> %left IN_SCOPE
253a256,296
> exp	: IN_SCOPE '(' name_not_typename ')'
> 	{
> 	  YYSTYPE val;
> 	  struct symbol *sym = $3.sym;
>
> 	  if (sym)
> 	  {
> 	    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);
> 	  }
> 	  else
> 	  {
> 	    struct minimal_symbol *msymbol;
> 	    char *arg = copy_name ($3.stoken);
>
> 	    msymbol = lookup_minimal_symbol (arg, NULL, NULL);
> 	    if (msymbol != NULL)
> 	    {
> 		  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);
> 	    }
> 	    else if (!have_full_symbols () && !have_partial_symbols ())
> 		  error ("No symbol table is loaded.  Use the \"file\" command.");
> 	    else
> 	    {
> 		  parse_number ("0", 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);
> 	    }
> 	  }
> 	}
> 	;
>
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;
>     }

Re: New scope checking patch

[Jim Blandy]

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

Re: New scope checking patch

[Michael Snyder]

On Fri, 2008-01-11 at 14:50 -0800, Jim Blandy wrote:

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

In a previous thread, we sort of reached (or approached)
a loose consensus that "diff -up" is preferred.

You didn't mention -p, but folks didn't like -p without the -u
anyways...

Re: New scope checking patch

[Jim Blandy]

Michael Snyder <msnyder at specifix.com> writes:
> On Fri, 2008-01-11 at 14:50 -0800, Jim Blandy wrote:
>
>> 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.
>
> In a previous thread, we sort of reached (or approached)
> a loose consensus that "diff -up" is preferred.
>
> You didn't mention -p, but folks didn't like -p without the -u
> anyways...

Sounds good to me.

Re: New scope checking patch

[Rob Quill]

[-- Attachment #1: Type: text/plain, Size: 3573 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: in_scope.patch --]
[-- Type: text/x-diff; name=in_scope.patch, Size: 1544 bytes --]

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;

Re: New scope checking patch

[Jim Blandy]

"Rob Quill" <rob.quill at gmail.com> writes:
> @@ -208,6 +208,8 @@
>  %token TRUEKEYWORD
>  %token FALSEKEYWORD
>  
> +/* $in_scope opperator */
> +%left IN_SCOPE
>  
>  %left ','
>  %left ABOVE_COMMA

Doesn't it work to treat $in_scope like sizeof, declared with %token?
$in_scope isn't really left-associative in any meaningful sense.

> +	  {
> +        write_exp_elt_longcst ((LONGEST) 1);
> +	  }

You should leave out the braces here, and in the 'else', since there's
only one statement.

Also, please be sure that the indentation follows the GNU coding
conventions.  Substatements should be indented by two spaces.
(c-exp.y is not a great place to look for examples, since it's a mess,
but look at, say, frame.c.)

From looking at your patch as it arrived through my mailer, it seemed
that the code block for the new $in_scope grammar rule was not
indented in the same way as the other blocks.  These should all be
consistent.

Re: New scope checking patch

[Rob Quill]

On 17/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
> Also, please be sure that the indentation follows the GNU coding
> conventions.  Substatements should be indented by two spaces.
> (c-exp.y is not a great place to look for examples, since it's a mess,
> but look at, say, frame.c.)
>
> From looking at your patch as it arrived through my mailer, it seemed
> that the code block for the new $in_scope grammar rule was not
> indented in the same way as the other blocks.  These should all be
> consistent.

Hey,

Sorry about getting the formatting consistently wrong. I've
reformatted it and it looks to me like it matches the other cases,
although it is hard to tell where to use tabs and where to use spaces,
and I'm never sure it's right as what if I have a different tab size
to you etc. I tried putting it through indent, but that just made a
mess, although as a rule is it OK to use that if it is a C file?

http://www.gnu.org/prep/standards/standards.html#Formatting states:

"The rest of this section gives our recommendations for other aspects
of C formatting style, which is also the default style of the indent
program in version 1.2 and newer."

Anyway, hopefully this version will have the formatting right.

Rob


2008-01-17   Rob Quill <rob.quill@gmail.com>
       * c-exp.y : Add $in_scope as a type of expression.

Index: gdb/c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -u -p -r1.42 c-exp.y
--- gdb/c-exp.y	9 Jan 2008 19:27:15 -0000	1.42
+++ gdb/c-exp.y	17 Jan 2008 20:59:33 -0000
@@ -208,6 +208,8 @@ static int parse_number (char *, int, in
 %token TRUEKEYWORD
 %token FALSEKEYWORD

+/* $in_scope opperator */
+%token IN_SCOPE

 %left ','
 %left ABOVE_COMMA
@@ -251,6 +253,30 @@ exp1	:	exp
 	;

 /* Expressions, not including the comma operator.  */
+exp	:	IN_SCOPE '(' name_not_typename ')'
+			{
+			  struct type *int_type;
+			  struct minimal_symbol *min_symbol;
+
+			  /* 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);
+
+			  min_symbol =
+			    lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL);
+			  if ($3.sym || min_symbol)
+			    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 +1704,9 @@ yylex ()
   /* 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;

Re: New scope checking patch

[Jim Blandy]

"Rob Quill" <rob.quill at gmail.com> writes:
> On 17/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
>> Also, please be sure that the indentation follows the GNU coding
>> conventions.  Substatements should be indented by two spaces.
>> (c-exp.y is not a great place to look for examples, since it's a mess,
>> but look at, say, frame.c.)
>>
>> From looking at your patch as it arrived through my mailer, it seemed
>> that the code block for the new $in_scope grammar rule was not
>> indented in the same way as the other blocks.  These should all be
>> consistent.
>
> Hey,
>
> Sorry about getting the formatting consistently wrong. I've
> reformatted it and it looks to me like it matches the other cases,
> although it is hard to tell where to use tabs and where to use spaces,
> and I'm never sure it's right as what if I have a different tab size
> to you etc. I tried putting it through indent, but that just made a
> mess, although as a rule is it OK to use that if it is a C file?

I'm told Open Source projects avoid tabs, but it seems that GDB
permits them, with tab stops every 8 columns.  The GNU coding
standards don't say much about tab use or width, beyond saying that
error messages that include line and column numbers should assume tab
stops every 8 columns when computing column numbers.

We don't generally use indent; if you look through the mailing list
archives, you can see the arguments.  I can only remember the
arguments that made sense to me, so I'm not sure I can accurately
explain the reasoning.  :)

If you use GNU Emacs C mode with the default settings, then TAB, C-j,
and C-M-q will do the right thing.

> +			  if (!have_full_symbols () && !have_partial_symbols ())
> +				error ("No symbol table is loaded.  Use the \"file\" command.");

Too much indentation?


> +
> +			  /* 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);
> +
> +			  min_symbol =
> +			    lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL);
> +			  if ($3.sym || min_symbol)
> +			    write_exp_elt_longcst ((LONGEST) 1);
> +			  else
> +			    write_exp_elt_longcst ((LONGEST) 0);



> +
> +			  write_exp_elt_opcode (OP_LONG); }

Closing brace should get its own line, since the opening brace did.

> @@ -1678,6 +1704,9 @@ yylex ()
>    /* 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;

When I view this (again, with 8-column tab stops), the 'case' is not
lined up with the other cases, and the 'return' is not indented two
spaces within the 'if'.

Re: New scope checking patch

[Doug Evans]

On Jan 17, 2008 1:57 PM, Jim Blandy <jimb@codesourcery.com> wrote:
> I'm told Open Source projects avoid tabs,

My theory on this is that this is done to cater to folks who errantly
thought implementing indentation level by setting the tab stop was a
good idea.  i.e. want an indentation level of 4 - set your tab
character to 4 :-(.  Oops.

> but it seems that GDB
> permits them, with tab stops every 8 columns.

Not unreasonable IMO.

Re: New scope checking patch

[Daniel Jacobowitz]

On Thu, Jan 17, 2008 at 01:57:58PM -0800, Jim Blandy wrote:
> I'm told Open Source projects avoid tabs, but it seems that GDB
> permits them, with tab stops every 8 columns.  The GNU coding
> standards don't say much about tab use or width, beyond saying that
> error messages that include line and column numbers should assume tab
> stops every 8 columns when computing column numbers.

GDB in fact requires them.  Leading eight spaces should always be
tabs.  Remaining spaces should be spaces and tabs in the middle of
lines are generally frowned upon.

This is, not coincidentally, what Emacs does.

You can use the gdb_indent.sh script, but if the file has not
previously been indented it may make a mess.  That just runs GNU
Indent with some magic options.

-- 
Daniel Jacobowitz
CodeSourcery

Re: New scope checking patch

[Rob Quill]

[-- Attachment #1: Type: text/plain, Size: 3914 bytes --]

On 17/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
>
> "Rob Quill" <rob.quill at gmail.com> writes:
> > On 17/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
> >> Also, please be sure that the indentation follows the GNU coding
> >> conventions.  Substatements should be indented by two spaces.
> >> (c-exp.y is not a great place to look for examples, since it's a mess,
> >> but look at, say, frame.c.)
> >>
> >> From looking at your patch as it arrived through my mailer, it seemed
> >> that the code block for the new $in_scope grammar rule was not
> >> indented in the same way as the other blocks.  These should all be
> >> consistent.
> >
> > Hey,
> >
> > Sorry about getting the formatting consistently wrong. I've
> > reformatted it and it looks to me like it matches the other cases,
> > although it is hard to tell where to use tabs and where to use spaces,
> > and I'm never sure it's right as what if I have a different tab size
> > to you etc. I tried putting it through indent, but that just made a
> > mess, although as a rule is it OK to use that if it is a C file?
>
> I'm told Open Source projects avoid tabs, but it seems that GDB
> permits them, with tab stops every 8 columns.  The GNU coding
> standards don't say much about tab use or width, beyond saying that
> error messages that include line and column numbers should assume tab
> stops every 8 columns when computing column numbers.
>
> We don't generally use indent; if you look through the mailing list
> archives, you can see the arguments.  I can only remember the
> arguments that made sense to me, so I'm not sure I can accurately
> explain the reasoning.  :)
>
> If you use GNU Emacs C mode with the default settings, then TAB, C-j,
> and C-M-q will do the right thing.
>
> > +                       if (!have_full_symbols () && !have_partial_symbols ())
> > +                             error ("No symbol table is loaded.  Use the \"file\" command.");
>
> Too much indentation?

Well, you would think so, but I've changed the tab spacing in vim to 8
and the way I have formatted it matches how everything else is
formatted.

> > +
> > +                       /* 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);
> > +
> > +                       min_symbol =
> > +                         lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL);
> > +                       if ($3.sym || min_symbol)
> > +                         write_exp_elt_longcst ((LONGEST) 1);
> > +                       else
> > +                         write_exp_elt_longcst ((LONGEST) 0);
>
>
>
> > +
> > +                       write_exp_elt_opcode (OP_LONG); }
>
> Closing brace should get its own line, since the opening brace did.

This is what I thought too, but if you look at the rest of the file
(for example the case for "exp : exp ARROW name") the open brace
always starts on a new line and always ends on the same line.

>
> > @@ -1678,6 +1704,9 @@ yylex ()
> >    /* 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;
>
> When I view this (again, with 8-column tab stops), the 'case' is not
> lined up with the other cases, and the 'return' is not indented two
> spaces within the 'if'.

Sorry, missed that. I've attached the fixed copy. I believe that the
amount of indentation matches the layout of the current file, and that
even though it looks like a lot in the context of the patch, all the
other grammar rules in the file use the same large amount of
indentation.

Rob

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: in_scope.patch --]
[-- Type: text/x-diff; name=in_scope.patch, Size: 1687 bytes --]

Index: gdb/c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -u -p -r1.42 c-exp.y
--- gdb/c-exp.y	9 Jan 2008 19:27:15 -0000	1.42
+++ gdb/c-exp.y	18 Jan 2008 03:32:41 -0000
@@ -208,6 +208,8 @@ static int parse_number (char *, int, in
 %token TRUEKEYWORD
 %token FALSEKEYWORD
 
+/* $in_scope opperator */
+%token IN_SCOPE
 
 %left ','
 %left ABOVE_COMMA
@@ -251,6 +253,30 @@ exp1	:	exp
 	;
 
 /* Expressions, not including the comma operator.  */
+exp	:	IN_SCOPE '(' name_not_typename ')'
+			{
+			  struct type *int_type;
+			  struct minimal_symbol *min_symbol;
+
+			  /* 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);
+
+			  min_symbol = 
+			    lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL);
+			  if ($3.sym || min_symbol)
+			    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 +1704,9 @@ yylex ()
   /* 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;

Re: New scope checking patch

[Jim Blandy]

"Rob Quill" <rob.quill at gmail.com> writes:
>> > +                       if (!have_full_symbols () && !have_partial_symbols ())
>> > +                             error ("No symbol table is loaded.  Use the \"file\" command.");
>>
>> Too much indentation?
>
> Well, you would think so, but I've changed the tab spacing in vim to 8
> and the way I have formatted it matches how everything else is
> formatted.

I swear I'm not harrassing you.  :) Once we can get these minor things
taken care of, the patch looks ready to me.

The error should be indented two columns within the 'if'.  Here's what
I see in most every other use of 'error' in the grammar section of the
file.

		  if (new_type == NULL)
		    error ("No type \"%s\" within class or namespace \"%s\".",
			   ncopy, TYPE_NAME (type));

>> Closing brace should get its own line, since the opening brace did.
>
> This is what I thought too, but if you look at the rest of the file
> (for example the case for "exp : exp ARROW name") the open brace
> always starts on a new line and always ends on the same line.

Right you are.

>> When I view this (again, with 8-column tab stops), the 'case' is not
>> lined up with the other cases, and the 'return' is not indented two
>> spaces within the 'if'.
>
> Sorry, missed that. I've attached the fixed copy. I believe that the
> amount of indentation matches the layout of the current file, and that
> even though it looks like a lot in the context of the patch, all the
> other grammar rules in the file use the same large amount of
> indentation.

This code block looks fine to me in your latest patch.

The only other point left is the ChangeLog entry.  The one you posted
was:

2008-01-17   Rob Quill <rob.quill@gmail.com>
      * c-exp.y : Add $in_scope as a type of expression.

This needs a little rearrangement, and a bit more detail.  A model to
follow might be:

2008-01-07  Vladimir Prus  <vladimir@codesourcery.com>

	Ignore change in name of dynamic linker during
	execution on Solaris.  This also unbreaks pending breakpoints.

	* solist.h (struct target_so_ops): New field same.
        * solib-svr4.c (svr4_same): New.
        (_initialize_svr4_solib): Register svr4_same.
        * solib.c (update_solib_list): Use ops->same, if available.

If the change merits it, it's nice to have a (brief) summary sentence
at the top (substantial explanation belongs in comments in the code).
Your "Add $in_scope ..." might go there.  Then, each change is
attributed to a particular function, top-level declaration, or
whatever grouping makes sense (in documentation, we put the section
name in the parens).

So your ChangeLog entry would need to mention IN_SCOPE as a new token
you're defining, 'exp' (I guess) as the non-terminal symbol to which
you're adding a grammar rule, and 'yylex' as the function to which
you're adding a bit of code.

2008-01-17   Rob Quill <rob.quill@gmail.com>

	Add $in_scope as a type of expression.
        * c-exp.y (IN_SCOPE): ...
        (exp): ...
        (yylex): ...

Re: New scope checking patch

[Rob Quill]

On 18/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
>
> "Rob Quill" <rob.quill at gmail.com> writes:
> >> > +                       if (!have_full_symbols () && !have_partial_symbols ())
> >> > +                             error ("No symbol table is loaded.  Use the \"file\" command.");
> >>
> >> Too much indentation?
> >
> > Well, you would think so, but I've changed the tab spacing in vim to 8
> > and the way I have formatted it matches how everything else is
> > formatted.
>
> I swear I'm not harrassing you.  :) Once we can get these minor things
> taken care of, the patch looks ready to me.

Sorry about that. I was just frustrated because it feels like every
time I submit this wrong it makes me look bad :)

> The error should be indented two columns within the 'if'.  Here's what
> I see in most every other use of 'error' in the grammar section of the
> file.
>
>                   if (new_type == NULL)
>                     error ("No type \"%s\" within class or namespace \"%s\".",
>                            ncopy, TYPE_NAME (type));
>
> >> Closing brace should get its own line, since the opening brace did.
> >
> > This is what I thought too, but if you look at the rest of the file
> > (for example the case for "exp : exp ARROW name") the open brace
> > always starts on a new line and always ends on the same line.
>
> Right you are.
>
> >> When I view this (again, with 8-column tab stops), the 'case' is not
> >> lined up with the other cases, and the 'return' is not indented two
> >> spaces within the 'if'.
> >
> > Sorry, missed that. I've attached the fixed copy. I believe that the
> > amount of indentation matches the layout of the current file, and that
> > even though it looks like a lot in the context of the patch, all the
> > other grammar rules in the file use the same large amount of
> > indentation.
>
> This code block looks fine to me in your latest patch.
>
> The only other point left is the ChangeLog entry.  The one you posted
> was:
>
> 2008-01-17   Rob Quill <rob.quill@gmail.com>
>       * c-exp.y : Add $in_scope as a type of expression.
>
> This needs a little rearrangement, and a bit more detail.  A model to
> follow might be:
>
> 2008-01-07  Vladimir Prus  <vladimir@codesourcery.com>
>
>         Ignore change in name of dynamic linker during
>         execution on Solaris.  This also unbreaks pending breakpoints.
>
>         * solist.h (struct target_so_ops): New field same.
>         * solib-svr4.c (svr4_same): New.
>         (_initialize_svr4_solib): Register svr4_same.
>         * solib.c (update_solib_list): Use ops->same, if available.
>
> If the change merits it, it's nice to have a (brief) summary sentence
> at the top (substantial explanation belongs in comments in the code).
> Your "Add $in_scope ..." might go there.  Then, each change is
> attributed to a particular function, top-level declaration, or
> whatever grouping makes sense (in documentation, we put the section
> name in the parens).
>
> So your ChangeLog entry would need to mention IN_SCOPE as a new token
> you're defining, 'exp' (I guess) as the non-terminal symbol to which
> you're adding a grammar rule, and 'yylex' as the function to which
> you're adding a bit of code.

Ok, here we go.

Rob

2008-01-18   Rob Quill <rob.quill@gmail.com>

         Add $in_scope as a type of expression.
         * c-exp.y (IN_SCOPE): New token.
         (exp): IN_SCOPE (name): evaluates to 1 if name is in scope, 0
otherwise.
         (yylex): Match "$in_scope" as IN_SCOPE token

Index: gdb/c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.42
diff -u -p -r1.42 c-exp.y
--- gdb/c-exp.y	9 Jan 2008 19:27:15 -0000	1.42
+++ gdb/c-exp.y	18 Jan 2008 22:41:05 -0000
@@ -208,6 +208,8 @@ static int parse_number (char *, int, in
 %token TRUEKEYWORD
 %token FALSEKEYWORD

+/* $in_scope opperator */
+%token IN_SCOPE

 %left ','
 %left ABOVE_COMMA
@@ -251,6 +253,30 @@ exp1	:	exp
 	;

 /* Expressions, not including the comma operator.  */
+exp	:	IN_SCOPE '(' name_not_typename ')'
+			{
+			  struct type *int_type;
+			  struct minimal_symbol *min_symbol;
+
+			  /* 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);
+
+			  min_symbol =
+			    lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL);
+			  if ($3.sym || min_symbol)
+			    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 +1704,9 @@ yylex ()
   /* 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;

Re: New scope checking patch

[Jim Blandy]

"Rob Quill" <rob.quill at gmail.com> writes:
> 2008-01-18   Rob Quill <rob.quill@gmail.com>
>
>          Add $in_scope as a type of expression.
>          * c-exp.y (IN_SCOPE): New token.
>          (exp): IN_SCOPE (name): evaluates to 1 if name is in scope, 0
> otherwise.
>          (yylex): Match "$in_scope" as IN_SCOPE token

This looks good.

I see you've got your copyright paperwork filed with the FSF.

If you haven't already (and only if you haven't already), you'll need
to visit:

http://sourceware.org/cgi-bin/pdw/ps_form.cgi

to get added to the gdb project; list me as your approver.

Since this is a user-visible feature, it needs documentation.  Write
up a patch to doc/gdb.texinfo, make sure it looks okay in .info and
PDF form, and post the patch to the .texinfo file here.  I think this
also needs an entry in gdb/NEWS.

Re: New scope checking patch

[Michael Snyder]

On Fri, 2008-01-18 at 22:43 +0000, Rob Quill wrote:
> On 18/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
> >
> > "Rob Quill" <rob.quill at gmail.com> writes:
> > >> > +                       if (!have_full_symbols () && !have_partial_symbols ())
> > >> > +                             error ("No symbol table is loaded.  Use the \"file\" command.");
> > >>
> > >> Too much indentation?
> > >
> > > Well, you would think so, but I've changed the tab spacing in vim to 8
> > > and the way I have formatted it matches how everything else is
> > > formatted.
> >
> > I swear I'm not harrassing you.  :) Once we can get these minor things
> > taken care of, the patch looks ready to me.
> 
> Sorry about that. I was just frustrated because it feels like every
> time I submit this wrong it makes me look bad :)

Naaaahh!!!

We're famously fussy.   ;-)

Re: New scope checking patch

[Rob Quill]

On 19/01/2008, Jim Blandy <jimb@codesourcery.com> wrote:
>
> "Rob Quill" <rob.quill at gmail.com> writes:
> > 2008-01-18   Rob Quill <rob.quill@gmail.com>
> >
> >          Add $in_scope as a type of expression.
> >          * c-exp.y (IN_SCOPE): New token.
> >          (exp): IN_SCOPE (name): evaluates to 1 if name is in scope, 0
> > otherwise.
> >          (yylex): Match "$in_scope" as IN_SCOPE token
>
> This looks good.
>
> I see you've got your copyright paperwork filed with the FSF.
>
> If you haven't already (and only if you haven't already), you'll need
> to visit:
>
> http://sourceware.org/cgi-bin/pdw/ps_form.cgi
>
> to get added to the gdb project; list me as your approver.
>
> Since this is a user-visible feature, it needs documentation.  Write
> up a patch to doc/gdb.texinfo, make sure it looks okay in .info and
> PDF form, and post the patch to the .texinfo file here.  I think this
> also needs an entry in gdb/NEWS.

Hi,

I have attached the docs patch below. I wasn't sure how much to put in
the docs, so I just put a small paragraph in the place which seemed
most appropriate. Any feedback is much appreciated.

Rob

2008-01-18   Rob Quill <rob.quill@gmail.com>

	* gdb.texinfo (Program Variables): Add a small paragraph about
	  the $in_scope operator.
	* NEWS: Mentions $in_scope.

Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.254
diff -u -p -r1.254 NEWS
--- gdb/NEWS	30 Jan 2008 00:51:49 -0000	1.254
+++ gdb/NEWS	30 Jan 2008 11:56:12 -0000
@@ -3,6 +3,12 @@

 *** Changes since GDB 6.7

+* New expression type
+
+$in_scope(...)
+  The value of this expression is 1 if the variable within the
+  parentheses is within the current scope, 0 otherwise.
+
 * New native configurations

 NetBSD/hppa			hppa*-*netbsd*
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.463
diff -u -p -r1.463 gdb.texinfo
--- gdb/doc/gdb.texinfo	30 Jan 2008 04:27:11 -0000	1.463
+++ gdb/doc/gdb.texinfo	30 Jan 2008 11:56:35 -0000
@@ -5676,6 +5676,13 @@ scope resolution operator in @value{GDBN
 @c FIXME: Um, so what happens in one of those rare cases where it's in
 @c conflict??  --mew

+The @code{$in_scope} operator can be used to check if a variable is in scope,
+returning 1 if it is and 0 if it is not. This is most useful when scripting GDB
+as it means that the script will ot stop executing if a variable is
not in scope,
+provided that you check if it is in scope before you test it's value.
The operator
+only works on variables and will not work on structure members or
array elements
+for instance.
+
 @cindex wrong values
 @cindex variable values, wrong
 @cindex function entry/exit, wrong values of variables

Re: New scope checking patch

[Jim Blandy]

On Jan 30, 2008 4:05 AM, Rob Quill <rob.quill@gmail.com> wrote:
> I have attached the docs patch below. I wasn't sure how much to put in
> the docs, so I just put a small paragraph in the place which seemed
> most appropriate. Any feedback is much appreciated.

I think you can be more expansive than that --- don't be shy!  :)
There are two points of view to consider when you write documentation:

1) Think of someone who is writing scripts and needs your feature.
Where would they look for it in the manual?  What terms would they
look up in the index?

2) Think of someone who is aware of the feature, but wants full
details on how it works.  The GDB manual is also the reference manual
--- how should a reference manual describe this feature?  What does a
script author need to know to decide whether the feature does what
they want, and whether they've used it correctly?

If the feature should be mentioned in more than one place, then there
should be one complete description, and the others should reference
it.

Re: New scope checking patch

[Eli Zaretskii]

> Date: Wed, 30 Jan 2008 12:05:35 +0000
> From: "Rob Quill" <rob.quill@gmail.com>
> 
> I have attached the docs patch below.

Thanks.

> I wasn't sure how much to put in the docs

Imagine yourself reading the manual and put there as much stuff as you
would like to find if you were reading about this feature for the
first time.

> Any feedback is much appreciated.

I have some comments below.

>	* gdb.texinfo (Program Variables): Add a small paragraph about
>	  the $in_scope operator.

The name in parentheses should be the name of the node, not the
chapter/section.

I actually think that this feature should be described in the node
"Convenience Vars", not in "Variables".  The latter describes how to
refer to variables in the program being debugged; while $in_scope is
related (and should perhaps be cross-referenced from "Variables"),
that is not the right place for describing GDB built-ins.

> Index: gdb/NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.254
> diff -u -p -r1.254 NEWS
> --- gdb/NEWS	30 Jan 2008 00:51:49 -0000	1.254
> +++ gdb/NEWS	30 Jan 2008 11:56:12 -0000
> @@ -3,6 +3,12 @@
> 
>  *** Changes since GDB 6.7
> 
> +* New expression type
> +
> +$in_scope(...)
> +  The value of this expression is 1 if the variable within the
> +  parentheses is within the current scope, 0 otherwise.

This is okay.

> +The @code{$in_scope} operator can be used to check if a variable is in scope,
> +returning 1 if it is and 0 if it is not. This is most useful when scripting GDB

Two spaces after a period that ends a sentence, please.

Also, please use @value{GDBN} instead of a literal "GDB", as we do
elsewhere in the manual.

Finally, please add index entries for this feature.  I suggest these:

  @vindex $in_scope
  @cindex variable in scope, testing

Re: New scope checking patch

[Jim Blandy]

On Jan 30, 2008 10:20 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> I actually think that this feature should be described in the node
> "Convenience Vars", not in "Variables".  The latter describes how to
> refer to variables in the program being debugged; while $in_scope is
> related (and should perhaps be cross-referenced from "Variables"),
> that is not the right place for describing GDB built-ins.

That's kind of an interesting point.  This is an entirely new kind of
entity: it's not a convenience variable, it's a convenience function,
or perhaps a new operator.  I'm not sure where it should go.  I'd
expect the primary description to be in "Expressions", in the part
that says, "GDB supports these operators, in addition to those common
to programming languages".

Re: New scope checking patch

[Eli Zaretskii]

> Date: Wed, 30 Jan 2008 15:57:06 -0800
> From: "Jim Blandy" <jimb@red-bean.com>
> Cc: "Rob Quill" <rob.quill@gmail.com>, gdb-patches@sourceware.org
> 
> I'd expect the primary description to be in "Expressions", in the
> part that says, "GDB supports these operators, in addition to those
> common to programming languages".

Fine with me.

Re: New scope checking patch

[Michael Snyder]

On Wed, 2008-01-30 at 15:57 -0800, Jim Blandy wrote:
> On Jan 30, 2008 10:20 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > I actually think that this feature should be described in the node
> > "Convenience Vars", not in "Variables".  The latter describes how to
> > refer to variables in the program being debugged; while $in_scope is
> > related (and should perhaps be cross-referenced from "Variables"),
> > that is not the right place for describing GDB built-ins.
> 
> That's kind of an interesting point.  This is an entirely new kind of
> entity: it's not a convenience variable, it's a convenience function,
> or perhaps a new operator.  I'm not sure where it should go.  I'd
> expect the primary description to be in "Expressions", in the part
> that says, "GDB supports these operators, in addition to those common
> to programming languages".

Maybe we could have a discussion about what sorts of operators
gdb might provide in this context.

The canonical example for me of a missing functionality is
"offsetof" (in the sense of "sizeof").  I suppose in that
spirit, I might like to have "typeof", and "scopeof".

Maybe for C++ or objective-C, "classof"...

Re: New scope checking patch

[Rob Quill]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

2008/1/31 Eli Zaretskii <eliz@gnu.org>:
>> Date: Wed, 30 Jan 2008 15:57:06 -0800
>> From: "Jim Blandy" <jimb@red-bean.com>
>> Cc: "Rob Quill" <rob.quill@gmail.com>, gdb-patches@sourceware.org
>>
>> I'd expect the primary description to be in "Expressions", in the
>> part that says, "GDB supports these operators, in addition to those
>> common to programming languages".
>
> Fine with me.
>

Hi all,

This patch (like the remove deprecated_set_value_type one) has been a
long time coming. Please find attached what I believe should be the
final version of this patch.

Rob

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: in_scope.patch --]
[-- Type: text/x-diff; name=in_scope.patch, Size: 4260 bytes --]

2008-07-27   Rob Quill <rob.quill@gmail.com>

	Add $in_scope as a type of expression.
	* c-exp.y (IN_SCOPE): New token.
	  (exp): IN_SCOPE (name): evaluates to 1 if name is in scope,
	  0 otherwise.
	(yylex): Match "$in_scope" as IN_SCOPE token
       * gdb.texinfo (expressions): Add a small paragraph (with an
	 example) about the $in_scope operator.
       * NEWS: Mentions $in_scope.

Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.282
diff -u -p -r1.282 NEWS
--- gdb/NEWS	18 Jul 2008 20:55:32 -0000	1.282
+++ gdb/NEWS	27 Jul 2008 23:02:48 -0000
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 6.8
 
+* New expression type
+
+$in_scope(...)
+  The value of this expression is 1 if the variable within the
+  parentheses is within the current scope, 0 otherwise.
+
 * Commands `set debug-file-directory', `set solib-search-path' and `set args'
 now complete on file names.
 
Index: gdb/c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.46
diff -u -p -r1.46 c-exp.y
--- gdb/c-exp.y	25 Jun 2008 15:49:20 -0000	1.46
+++ gdb/c-exp.y	27 Jul 2008 23:02:49 -0000
@@ -209,6 +209,8 @@ static int parse_number (char *, int, in
 %token TRUEKEYWORD
 %token FALSEKEYWORD
 
+/* $in_scope opperator */
+%token IN_SCOPE
 
 %left ','
 %left ABOVE_COMMA
@@ -252,6 +254,30 @@ exp1	:	exp
 	;
 
 /* Expressions, not including the comma operator.  */
+exp    :       IN_SCOPE '(' name_not_typename ')'
+                       {
+                         struct type *int_type;
+                         struct minimal_symbol *min_symbol;
+
+                         /* 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);
+
+                         min_symbol =
+                           lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL);
+                         if ($3.sym || min_symbol)
+                           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); }
 	;
@@ -1739,6 +1765,9 @@ yylex ()
   /* 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;
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.509
diff -u -p -r1.509 gdb.texinfo
--- gdb/doc/gdb.texinfo	18 Jul 2008 20:55:33 -0000	1.509
+++ gdb/doc/gdb.texinfo	27 Jul 2008 23:03:11 -0000
@@ -5750,6 +5750,24 @@ memory.  @var{addr} may be any expressio
 pointer (but parentheses are required around binary operators, just as in
 a cast).  This construct is allowed regardless of what kind of data is
 normally supposed to reside at @var{addr}.
+
+@vindex $in_scope
+@cindex variable in scope, testing
+@item $in_scope
+@samp{$in_scope} allows you to check if a variable is in scope,
+returning 1 if it is and 0 if it is not.  This is most useful when scripting @value{GDBN}
+as it means that the script will not stop executing if a variable is not in scope,
+provided that you check if it is in scope before you test it's value.  The operator
+only works on variables and will not work on structure members or array elements
+for instance.
+
+An example usage from a GDB script may be:
+
+@smallexample
+if ($in_scope(a) == 1 && $in_scope(b) == 1)
+  print a+b  
+@end smallexample
+
 @end table
 
 @node Ambiguous Expressions

Re: New scope checking patch

[Eli Zaretskii]

> Date: Mon, 28 Jul 2008 00:45:03 +0100
> From: "Rob Quill" <rob.quill@gmail.com>
> Cc: "Jim Blandy" <jimb@red-bean.com>, gdb-patches@sourceware.org
> 
> This patch (like the remove deprecated_set_value_type one) has been a
> long time coming. Please find attached what I believe should be the
> final version of this patch.

Thanks.

> --- gdb/NEWS	18 Jul 2008 20:55:32 -0000	1.282
> +++ gdb/NEWS	27 Jul 2008 23:02:48 -0000
> @@ -3,6 +3,12 @@
>  
>  *** Changes since GDB 6.8
>  
> +* New expression type
> +
> +$in_scope(...)
> +  The value of this expression is 1 if the variable within the
> +  parentheses is within the current scope, 0 otherwise.

This is okay.

> +provided that you check if it is in scope before you test it's value.  The operator
                                                             ^^^^
"its"

Otherwise, okay.

Re: New scope checking patch

[Rob Quill]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

2008/7/28 Eli Zaretskii <eliz@gnu.org>:
>> Date: Mon, 28 Jul 2008 00:45:03 +0100
>> From: "Rob Quill" <rob.quill@gmail.com>
>> Cc: "Jim Blandy" <jimb@red-bean.com>, gdb-patches@sourceware.org
>>
>> This patch (like the remove deprecated_set_value_type one) has been a
>> long time coming. Please find attached what I believe should be the
>> final version of this patch.
>
> Thanks.
>
>> --- gdb/NEWS  18 Jul 2008 20:55:32 -0000      1.282
>> +++ gdb/NEWS  27 Jul 2008 23:02:48 -0000
>> @@ -3,6 +3,12 @@
>>
>>  *** Changes since GDB 6.8
>>
>> +* New expression type
>> +
>> +$in_scope(...)
>> +  The value of this expression is 1 if the variable within the
>> +  parentheses is within the current scope, 0 otherwise.
>
> This is okay.
>
>> +provided that you check if it is in scope before you test it's value.  The operator
>                                                             ^^^^
> "its"

I thought about that before I submitted it but couldn't make up my
mind as to which was right, as the scope belongs to the variable so
maybe it needed an apostrophe. Anyway, I've attached the fixed patch.

Rob

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: in_scope.patch --]
[-- Type: text/x-diff; name=in_scope.patch, Size: 4259 bytes --]

2008-07-27   Rob Quill <rob.quill@gmail.com>

	Add $in_scope as a type of expression.
	* c-exp.y (IN_SCOPE): New token.
	  (exp): IN_SCOPE (name): evaluates to 1 if name is in scope,
	  0 otherwise.
	(yylex): Match "$in_scope" as IN_SCOPE token
       * gdb.texinfo (expressions): Add a small paragraph (with an
	 example) about the $in_scope operator.
       * NEWS: Mentions $in_scope.

Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.282
diff -u -p -r1.282 NEWS
--- gdb/NEWS	18 Jul 2008 20:55:32 -0000	1.282
+++ gdb/NEWS	27 Jul 2008 23:02:48 -0000
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 6.8
 
+* New expression type
+
+$in_scope(...)
+  The value of this expression is 1 if the variable within the
+  parentheses is within the current scope, 0 otherwise.
+
 * Commands `set debug-file-directory', `set solib-search-path' and `set args'
 now complete on file names.
 
Index: gdb/c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.46
diff -u -p -r1.46 c-exp.y
--- gdb/c-exp.y	25 Jun 2008 15:49:20 -0000	1.46
+++ gdb/c-exp.y	27 Jul 2008 23:02:49 -0000
@@ -209,6 +209,8 @@ static int parse_number (char *, int, in
 %token TRUEKEYWORD
 %token FALSEKEYWORD
 
+/* $in_scope opperator */
+%token IN_SCOPE
 
 %left ','
 %left ABOVE_COMMA
@@ -252,6 +254,30 @@ exp1	:	exp
 	;
 
 /* Expressions, not including the comma operator.  */
+exp    :       IN_SCOPE '(' name_not_typename ')'
+                       {
+                         struct type *int_type;
+                         struct minimal_symbol *min_symbol;
+
+                         /* 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);
+
+                         min_symbol =
+                           lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL);
+                         if ($3.sym || min_symbol)
+                           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); }
 	;
@@ -1739,6 +1765,9 @@ yylex ()
   /* 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;
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.509
diff -u -p -r1.509 gdb.texinfo
--- gdb/doc/gdb.texinfo	18 Jul 2008 20:55:33 -0000	1.509
+++ gdb/doc/gdb.texinfo	27 Jul 2008 23:03:11 -0000
@@ -5750,6 +5750,24 @@ memory.  @var{addr} may be any expressio
 pointer (but parentheses are required around binary operators, just as in
 a cast).  This construct is allowed regardless of what kind of data is
 normally supposed to reside at @var{addr}.
+
+@vindex $in_scope
+@cindex variable in scope, testing
+@item $in_scope
+@samp{$in_scope} allows you to check if a variable is in scope,
+returning 1 if it is and 0 if it is not.  This is most useful when scripting @value{GDBN}
+as it means that the script will not stop executing if a variable is not in scope,
+provided that you check if it is in scope before you test its value.  The operator
+only works on variables and will not work on structure members or array elements
+for instance.
+
+An example usage from a GDB script may be:
+
+@smallexample
+if ($in_scope(a) == 1 && $in_scope(b) == 1)
+  print a+b  
+@end smallexample
+
 @end table
 
 @node Ambiguous Expressions

Re: New scope checking patch

[Eli Zaretskii]

> Date: Mon, 28 Jul 2008 11:31:21 +0100
> From: "Rob Quill" <rob.quill@gmail.com>
> Cc: jimb@red-bean.com, gdb-patches@sourceware.org
> 
> >> +provided that you check if it is in scope before you test it's value.  The operator
> >                                                             ^^^^
> > "its"
> 
> I thought about that before I submitted it but couldn't make up my
> mind as to which was right, as the scope belongs to the variable so
> maybe it needed an apostrophe.

"it's" == "it is", which is not what you want here.  You want "its" ==
"belonging to it".

Re: New scope checking patch

[Tom Tromey]

>>>>> "Rob" == Rob Quill <rob.quill@gmail.com> writes:

Rob> 2008-07-27   Rob Quill <rob.quill@gmail.com>
Rob> 	Add $in_scope as a type of expression.

This seems like a good candidate for an internal function.  The syntax
is nearly identical.

The only difference is that, at the GCC Summit, we agreed that
arguments to internal functions would be expressions.  So, instead of
$in_scope(x) you would have to write $in_scope("x").

Internal functions are only in the python repository right now.  The
patch consists of two parts: an internals-only part, and the part that
exposes the functionality to the Python layer.

I think we can submit the internals part as a separate patch.  AFAIK
the only reason we haven't is just that there's been no need for it
outside the Python work.

Tom

Re: New scope checking patch

[Rob Quill]

2008/7/29 Tom Tromey <tromey@redhat.com>:
>>>>>> "Rob" == Rob Quill <rob.quill@gmail.com> writes:
>
> Rob> 2008-07-27   Rob Quill <rob.quill@gmail.com>
> Rob>    Add $in_scope as a type of expression.
>
> This seems like a good candidate for an internal function.  The syntax
> is nearly identical.

Hi Tom,

Are you saying that there is something that I need to do before this
can be committed? If so, could you explain a bit further? Where can I
find out about internal functions?

> The only difference is that, at the GCC Summit, we agreed that
> arguments to internal functions would be expressions.  So, instead of
> $in_scope(x) you would have to write $in_scope("x").

With regards to expressions, the patch in it's current form can only
determine the scope of variables. I do have a very rough patch with
some fairly major caveats which can deal with expressions. If this
patch it implemented as an internal function, what is to stop someone
passing an expression to $in_scope(), in which case, does the patch
need to be able to determine if a whole expression is in scope?
Because I ran into some problems when doing that before. I believe
they may even be discussed earlier in this thread.

Rob

> Internal functions are only in the python repository right now.  The
> patch consists of two parts: an internals-only part, and the part that
> exposes the functionality to the Python layer.
>
> I think we can submit the internals part as a separate patch.  AFAIK
> the only reason we haven't is just that there's been no need for it
> outside the Python work.
>
> Tom
>

Re: New scope checking patch

[Tom Tromey]

>>>>> "Rob" == Rob Quill <rob.quill@gmail.com> writes:

Tom> This seems like a good candidate for an internal function.  The syntax
Tom> is nearly identical.

Rob> Are you saying that there is something that I need to do before this
Rob> can be committed?

Sorry -- to be completely clear, I am not a gdb maintainer.
So, you are free to ignore what I say.

Rob> Where can I find out about internal functions?

There's been some discussion on the various lists.
Otherwise, the python-gdb git repository.

Or, I suppose one of us can send out a patch.

Tom> The only difference is that, at the GCC Summit, we agreed that
Tom> arguments to internal functions would be expressions.  So, instead of
Tom> $in_scope(x) you would have to write $in_scope("x").

Rob> If this patch it implemented as an internal function, what is to
Rob> stop someone passing an expression to $in_scope(), in which case,
Rob> does the patch need to be able to determine if a whole expression
Rob> is in scope?

In the internal-function form, in_scope would take a string-valued
argument.  So, the expression would be evaluated first -- just like
any other function argument -- and then passed to the in_scope
primitive.

I guess this would prevent easy checking of whether an entire
expression is in scope.  (BTW does that mean just checking all the
names in the expression?)

Tom

Re: New scope checking patch

[Rob Quill]

2008/7/29 Tom Tromey <tromey@redhat.com>:
> Rob> If this patch it implemented as an internal function, what is to
> Rob> stop someone passing an expression to $in_scope(), in which case,
> Rob> does the patch need to be able to determine if a whole expression
> Rob> is in scope?
>
> In the internal-function form, in_scope would take a string-valued
> argument.  So, the expression would be evaluated first -- just like
> any other function argument -- and then passed to the in_scope
> primitive.
>
> I guess this would prevent easy checking of whether an entire
> expression is in scope.  (BTW does that mean just checking all the
> names in the expression?)

Yep. Exactly. I just checked and found I had discussed the problem I
had in the first post in this thread.

"There is a caveat with the patch which is as follows. If a variable is
not in scope then the expression parser in c-exp.y inserts a value 0
into the expression tree. When the scope checking function sees a 0
expression in the tree it will return 0, to indicate not in scope.
However, in the general case it should be true that all literals are
in scope. However, due to the above caveat $in_scope(0) will return 0,
any any other literal will return 1.

This also affects compound expressions that involve the literal 0. For
example, $in_scope(0+3) will return 0, as an expression with a binary
operator is considering in scope iff both of its operands are in
scope."

Obviously this disappears though with the current patch as it can't
deal with compound expressions.

Rob

Re: New scope checking patch

[Tom Tromey]

>>>>> "Rob" == Rob Quill <rob.quill@gmail.com> writes:

Rob> Where can I find out about internal functions?

I've appended the patch.  It adds a new sort of value, which is an
internal function.  These are just like ordinary functions in an
expression but they run inside gdb -- not the inferior.  The user sees
them as convenience variables of "internal function" type.

This patch builds but, because it is divorced from the code that uses
it, I did not test it.  So, if you try it, let me know how it fails :)

BTW -- I noticed that with your patch, an expression using $in_scope
will not print properly.  That is because in_scope is turned into a
constant in the expression structure.


Thiago -- FYI, I took the patch from the python-patches branch,
deleted stuff relying on things not yet in cvs (command destructors
and Python), and then wrote comments and a ChangeLog entry.  You may
want the latter :)

Tom

b/gdb/ChangeLog:
2008-07-29  Tom Tromey  <tromey@redhat.com>

	* value.h (lookup_only_internalvar, create_internalvar,
	lookup_internalvar): Update.
	(internal_function_fn, add_internal_function,
	call_internal_function, value_internal_function_name): Declare.
	* value.c (struct internal_function): New struct.
	(functionlist): New global.
	(internal_fn_type): Likewise.
	(lookup_only_internalvar): Made argument const.
	(create_internalvar): Likewise.
	(lookup_internalvar): Likewise.
	(value_create_internal_function): New function.
	(value_internal_function_name): Likewise.
	(call_internal_function): Likewise.
	(function_command): Likewise.
	(add_internal_function): Likewise.
	(_initialize_values): Add the "function" command.  Initialize
	internal_fn_type.
	* valprint.c (value_check_printable): Handle
	TYPE_CODE_INTERNAL_FUNCTION.
	* gdbtypes.h (enum type_code) <TYPE_CODE_INTERNAL_FUNCTION>: New
	constant.
	* eval.c: (evaluate_subexp_standard) <OP_FUNCALL>: Handle
	TYPE_CODE_INTERNAL_FUNCTION.

projecttype:gdb
email:tromey@gcc.gnu.org
revision:edd41e881c9f658211f1ebc08126a0222488e3a4
configure:
make:
check:

diff --git a/gdb/eval.c b/gdb/eval.c
index bbd7539..7ee910f 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1293,6 +1293,9 @@ evaluate_subexp_standard (struct type *expect_type,
 	  else
 	    error (_("Expression of type other than \"Function returning ...\" used as function"));
 	}
+      if (TYPE_CODE (value_type (argvec[0])) == TYPE_CODE_INTERNAL_FUNCTION)
+	return call_internal_function (argvec[0], nargs, argvec + 1);
+
       return call_function_by_hand (argvec[0], nargs, argvec + 1);
       /* pai: FIXME save value from call_function_by_hand, then adjust pc by adjust_fn_pc if +ve  */
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 7ef7d67..365707b 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -133,7 +133,10 @@ enum type_code
 
     TYPE_CODE_NAMESPACE,	/* C++ namespace.  */
 
-    TYPE_CODE_DECFLOAT		/* Decimal floating point.  */
+    TYPE_CODE_DECFLOAT,		/* Decimal floating point.  */
+
+    /* Internal function type.  */
+    TYPE_CODE_INTERNAL_FUNCTION
   };
 
 /* For now allow source to use TYPE_CODE_CLASS for C++ classes, as an
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 99c376f..617a3f4 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -259,6 +259,13 @@ value_check_printable (struct value *val, struct ui_file *stream)
       return 0;
     }
 
+  if (TYPE_CODE (value_type (val)) == TYPE_CODE_INTERNAL_FUNCTION)
+    {
+      fprintf_filtered (stream, _("<internal function %s>"),
+			value_internal_function_name (val));
+      return 0;
+    }
+
   return 1;
 }
 
diff --git a/gdb/value.c b/gdb/value.c
index b38bae0..93436e3 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -41,6 +41,24 @@
 
 void _initialize_values (void);
 
+/* Definition of a user function.  */
+struct internal_function
+{
+  /* The name of the function.  It is a bit odd to have this in the
+     function itself -- the user might use a differently-named
+     convenience variable to hold the function.  */
+  char *name;
+
+  /* The handler.  */
+  internal_function_fn handler;
+
+  /* User data for the handler.  */
+  void *cookie;
+};
+
+/* Command list holding internal functions.  */
+static struct cmd_list_element *functionlist;
+
 struct value
 {
   /* Type of value; either not an lval, or one of the various
@@ -203,6 +221,10 @@ struct value_history_chunk
 static struct value_history_chunk *value_history_chain;
 
 static int value_history_count;	/* Abs number of last entry stored */
+
+/* The type of internal functions.  */
+
+static struct type *internal_fn_type;
 \f
 /* List of all value objects currently allocated
    (except for those released by calls to release_value)
@@ -747,7 +769,7 @@ init_if_undefined_command (char* args, int from_tty)
    the return value is NULL.  */
 
 struct internalvar *
-lookup_only_internalvar (char *name)
+lookup_only_internalvar (const char *name)
 {
   struct internalvar *var;
 
@@ -763,7 +785,7 @@ lookup_only_internalvar (char *name)
    NAME should not normally include a dollar sign.  */
 
 struct internalvar *
-create_internalvar (char *name)
+create_internalvar (const char *name)
 {
   struct internalvar *var;
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
@@ -784,7 +806,7 @@ create_internalvar (char *name)
    one is created, with a void value.  */
 
 struct internalvar *
-lookup_internalvar (char *name)
+lookup_internalvar (const char *name)
 {
   struct internalvar *var;
 
@@ -888,6 +910,81 @@ internalvar_name (struct internalvar *var)
   return var->name;
 }
 
+/* Create a value which represents an internal function.
+   NAME is the name of the function.
+   HANDLER is a pointer to a function which is called when this
+   internal function is invoked during expression evaluation.
+   COOKIE is a pointer which is passed verbatim to HANDLER.  */
+static struct value *
+value_create_internal_function (const char *name,
+				internal_function_fn handler,
+				void *cookie)
+{
+  struct value *result = allocate_value (internal_fn_type);
+  gdb_byte *addr = value_contents_writeable (result);
+  struct internal_function **fnp = (struct internal_function **) addr;
+  /* The internal_function object is leaked here -- to make it truly
+     deletable, we would have to reference count it and add special
+     code to value_free and value_copy.  */
+  struct internal_function *ifn = XNEW (struct internal_function);
+  ifn->name = xstrdup (name);
+  ifn->handler = handler;
+  ifn->cookie = cookie;
+  *fnp = ifn;
+  return result;
+}
+
+/* Return the name of the value VAL, which is assumed to be of
+   "internal function" type.  The result is not copied and should not
+   be freed or modified.  */
+char *
+value_internal_function_name (struct value *val)
+{
+  gdb_byte *addr = value_contents_writeable (val);
+  struct internal_function *ifn = * (struct internal_function **) addr;
+  return ifn->name;
+}
+
+/* Call an internal function.  FUNC is a value which is assumed to be
+   of "internal function" type.  ARGC is the number of arguments to
+   pass to it, and ARGV is a vector holding the arguments.  */
+struct value *
+call_internal_function (struct value *func, int argc, struct value **argv)
+{
+  gdb_byte *addr = value_contents_writeable (func);
+  struct internal_function *ifn = * (struct internal_function **) addr;
+  return (*ifn->handler) (ifn->cookie, argc, argv);
+}
+
+/* The 'function' command.  This does nothing -- it is just a
+   placeholder to let "help function NAME" work.  This is also used as
+   the implementation of the sub-command that is created when
+   registering an internal function.  */
+static void
+function_command (char *command, int from_tty)
+{
+  /* Do nothing.  */
+}
+
+/* Add a new internal function.  NAME is the name of the function; DOC
+   is a documentation string describing the function.  HANDLER is
+   called when the function is invoked.  COOKIE is an arbitrary
+   pointer which is passed to HANDLER and is intended for "user
+   data".  */
+void
+add_internal_function (const char *name, const char *doc,
+		       internal_function_fn handler,
+		       void *cookie)
+{
+  struct internalvar *var = lookup_internalvar (name);
+  struct value *fnval = value_create_internal_function (name, handler, cookie);
+  release_value (fnval);
+  set_internalvar (var, fnval);
+
+  add_cmd (xstrdup (name), no_class, function_command, (char *) doc,
+	   &functionlist);
+}
+
 /* Update VALUE before discarding OBJFILE.  COPIED_TYPES is used to
    prevent cycles / duplicates.  */
 
@@ -1780,4 +1877,13 @@ init-if-undefined VARIABLE = EXPRESSION\n\
 Set an internal VARIABLE to the result of the EXPRESSION if it does not\n\
 exist or does not contain a value.  The EXPRESSION is not evaluated if the\n\
 VARIABLE is already initialized."));
+
+  add_prefix_cmd ("function", no_class, function_command, _("\
+Placeholder command for showing help on convenience functions."),
+		  &functionlist, "function ", 0, &cmdlist);
+
+  internal_fn_type = alloc_type (NULL);
+  TYPE_CODE (internal_fn_type) = TYPE_CODE_INTERNAL_FUNCTION;
+  TYPE_LENGTH (internal_fn_type) = sizeof (struct internal_function *);
+  TYPE_NAME (internal_fn_type) = "<internal function>";
 }
diff --git a/gdb/value.h b/gdb/value.h
index 2aac9b2..bb597b2 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -451,11 +451,11 @@ extern void set_internalvar_component (struct internalvar *var,
 				       int bitpos, int bitsize,
 				       struct value *newvalue);
 
-extern struct internalvar *lookup_only_internalvar (char *name);
+extern struct internalvar *lookup_only_internalvar (const char *name);
 
-extern struct internalvar *create_internalvar (char *name);
+extern struct internalvar *create_internalvar (const char *name);
 
-extern struct internalvar *lookup_internalvar (char *name);
+extern struct internalvar *lookup_internalvar (const char *name);
 
 extern int value_equal (struct value *arg1, struct value *arg2);
 
@@ -570,4 +570,19 @@ extern struct value *value_allocate_space_in_inferior (int);
 extern struct value *value_of_local (const char *name, int complain);
 
 extern struct value * value_subscripted_rvalue (struct value *array, struct value *idx, int lowerbound);
+
+/* User function handler.  */
+
+typedef struct value *(*internal_function_fn) (void *cookie,
+					       int argc,
+					       struct value **argv);
+
+void add_internal_function (const char *name, const char *doc,
+			    internal_function_fn handler, void *cookie);
+
+struct value *call_internal_function (struct value *function,
+				      int argc, struct value **argv);
+
+char *value_internal_function_name (struct value *);
+
 #endif /* !defined (VALUE_H) */

Convenience functions (was: Re: New scope checking patch)

[Daniel Jacobowitz]

[Preachy interlude: splitting up patches good!  posting unrelated
patches at the bottom of threads bad!  Until Tom mentioned it on IRC
this week I'd completely forgotten this patch had been posted.]

On Tue, Jul 29, 2008 at 09:33:30PM -0600, Tom Tromey wrote:
> >>>>> "Rob" == Rob Quill <rob.quill@gmail.com> writes:
> 
> Rob> Where can I find out about internal functions?
> 
> I've appended the patch.  It adds a new sort of value, which is an
> internal function.  These are just like ordinary functions in an
> expression but they run inside gdb -- not the inferior.  The user sees
> them as convenience variables of "internal function" type.

I looked through this patch, since I have another use for it too.  It
looks fine to me, except that of course it is missing documentation
and test cases.  For test cases we'd need a function to test with;
perhaps Rob's?

-- 
Daniel Jacobowitz
CodeSourcery

Re: Convenience functions

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> [Preachy interlude: splitting up patches good!  posting unrelated
Daniel> patches at the bottom of threads bad!  Until Tom mentioned it on IRC
Daniel> this week I'd completely forgotten this patch had been posted.]

Yeah, well...  I sent this as a convenience to Rob, not as a real
submission.  It isn't actually useful for anything in this form.

Daniel> I looked through this patch, since I have another use for it too.  It
Daniel> looks fine to me, except that of course it is missing documentation
Daniel> and test cases.  For test cases we'd need a function to test with;
Daniel> perhaps Rob's?

Exactly :)

Tom

Re: Convenience functions

[Daniel Jacobowitz]

On Thu, Oct 23, 2008 at 09:15:52AM -0600, Tom Tromey wrote:
> Daniel> I looked through this patch, since I have another use for it too.  It
> Daniel> looks fine to me, except that of course it is missing documentation
> Daniel> and test cases.  For test cases we'd need a function to test with;
> Daniel> perhaps Rob's?
> 
> Exactly :)

Maybe I can convince one of you to submit that pair of patches...

-- 
Daniel Jacobowitz
CodeSourcery

Re: Convenience functions

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> On Thu, Oct 23, 2008 at 09:15:52AM -0600, Tom Tromey wrote:
Daniel> I looked through this patch, since I have another use for it too.  It
Daniel> looks fine to me, except that of course it is missing documentation
Daniel> and test cases.  For test cases we'd need a function to test with;
Daniel> perhaps Rob's?

Tom> Exactly :)

Daniel> Maybe I can convince one of you to submit that pair of patches...

Sure, I'd be happy to.

Tom

Re: Convenience functions

[Tom Tromey]

Daniel> Maybe I can convince one of you to submit that pair of patches...

I looked at this a little bit today.

There are two problems.

First, in_scope ought to take a string argument.  However, converting
a string value to a char* is a pain.  At least, I couldn't find any
predefined way to do this.  This area could use a lot of work, I
think.

Second, Rob's in_scope patch uses lookup_minimal_symbol.  This
function seems misnamed, in that to me "scope" implies scoping -- but
this is just checking for global symbols.

I did write some docs.  And, you can see the rest in the appended.

I wouldn't recommend moving forward with this patch, unless somebody
wants to clean it up.  I think a different convenience function would
be a better starting point.  I probably won't work on this any more
for the time being.

Tom

2008-10-23  Tom Tromey  <tromey@redhat.com>

	* symtab.c (in_scope_internal_function): New function.
	(_initialize_symtab): Register it.
	* value.h (lookup_only_internalvar, create_internalvar,
	lookup_internalvar): Update.
	(internal_function_fn, add_internal_function,
	call_internal_function, value_internal_function_name): Declare.
	* value.c (struct internal_function): New struct.
	(functionlist): New global.
	(internal_fn_type): Likewise.
	(lookup_only_internalvar): Made argument const.
	(create_internalvar): Likewise.
	(lookup_internalvar): Likewise.
	(value_create_internal_function): New function.
	(value_internal_function_name): Likewise.
	(call_internal_function): Likewise.
	(function_command): Likewise.
	(add_internal_function): Likewise.
	(_initialize_values): Add the "function" command.  Initialize
	internal_fn_type.
	* valprint.c (value_check_printable): Handle
	TYPE_CODE_INTERNAL_FUNCTION.
	* gdbtypes.h (enum type_code) <TYPE_CODE_INTERNAL_FUNCTION>: New
	constant.
	* eval.c: (evaluate_subexp_standard) <OP_FUNCALL>: Handle
	TYPE_CODE_INTERNAL_FUNCTION.

2008-10-23  Tom Tromey  <tromey@redhat.com>

	* gdb.texinfo (Convenience Vars): Document convenience functions.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c003cd4..baa5993 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7390,6 +7390,29 @@ On HP-UX systems, if you refer to a function or variable name that
 begins with a dollar sign, @value{GDBN} searches for a user or system
 name first, before it searches for a convenience variable.
 
+@cindex convenience functions
+@value{GDBN} also supplies some @dfn{convenience functions}.  These
+have a syntax similar to convenience variables.  A convenience
+function can be used in an expression just like an ordinary function;
+however, a convenience function is implemented internally to
+@value{GDBN}.
+
+@table @code
+@item help function
+@kindex help function
+@cindex show all convenience functions
+Print a list of all convenience functions.
+@end table
+
+Currently there is a single defined convenience function:
+
+@table @code
+@kindex $in_scope
+@item $in_scope (@var{name})
+Evaluate to @samp{1} if the symbol @var{name} is defined in the
+program, or @samp{0} otherwise.
+@end table
+
 @node Registers
 @section Registers
 
diff --git a/gdb/eval.c b/gdb/eval.c
index cf3e876..2d4551d 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1515,6 +1515,9 @@ evaluate_subexp_standard (struct type *expect_type,
 	  else
 	    error (_("Expression of type other than \"Function returning ...\" used as function"));
 	}
+      if (TYPE_CODE (value_type (argvec[0])) == TYPE_CODE_INTERNAL_FUNCTION)
+	return call_internal_function (argvec[0], nargs, argvec + 1);
+
       return call_function_by_hand (argvec[0], nargs, argvec + 1);
       /* pai: FIXME save value from call_function_by_hand, then adjust pc by adjust_fn_pc if +ve  */
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 2a41c5b..dc944b1 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -133,7 +133,10 @@ enum type_code
 
     TYPE_CODE_NAMESPACE,	/* C++ namespace.  */
 
-    TYPE_CODE_DECFLOAT		/* Decimal floating point.  */
+    TYPE_CODE_DECFLOAT,		/* Decimal floating point.  */
+
+    /* Internal function type.  */
+    TYPE_CODE_INTERNAL_FUNCTION
   };
 
 /* For now allow source to use TYPE_CODE_CLASS for C++ classes, as an
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 3af3e8a..514bc97 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4578,6 +4578,36 @@ expand_line_sal (struct symtab_and_line sal)
 }
 
 
+/* The implementation of the "in_scope" internal function.  */
+static struct value *
+in_scope_internal_function (void *ignore, int argc, struct value **argv)
+{
+  const gdb_byte *contents;
+  struct type *type;
+  struct minimal_symbol *minsym;
+
+  if (argc != 1)
+    error (_("in_scope requires a single argument"));
+
+  type = check_typedef (value_type (argv[0]));
+  if (TYPE_CODE (type) == TYPE_CODE_STRING
+      || (TYPE_CODE (type) == TYPE_CODE_ARRAY
+	  && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CHAR
+	      || (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_INT
+		  && TYPE_LENGTH (TYPE_TARGET_TYPE (type)) == 1))))
+    {
+      /* Ok.  */
+    }
+  else
+    error (_("in_scope argument must have string type"));
+
+  contents = value_contents (argv[0]);
+  /* FIXME: host/target charset confusion here.  */
+  minsym = lookup_minimal_symbol ((char *) contents, NULL, NULL);
+  return value_from_longest (builtin_type (current_gdbarch)->builtin_int,
+			     minsym != 0);
+}
+
 void
 _initialize_symtab (void)
 {
@@ -4625,6 +4655,13 @@ Show how the debugger handles ambiguities in expressions."), _("\
 Valid values are \"ask\", \"all\", \"cancel\", and the default is \"all\"."),
                         NULL, NULL, &setlist, &showlist);
 
+  add_internal_function ("in_scope", _("\
+Test whether a name is defined in the current scope.\n\
+This function takes a single argument, a string.  It returns one\n\
+if a symbol of that name is available, or zero if no symbol of that\n\
+name appears."),
+			 in_scope_internal_function, NULL);
+
   /* Initialize the one built-in type that isn't language dependent... */
   builtin_type_error = init_type (TYPE_CODE_ERROR, 0, 0,
 				  "<unknown type>", (struct objfile *) NULL);
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 99c376f..617a3f4 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -259,6 +259,13 @@ value_check_printable (struct value *val, struct ui_file *stream)
       return 0;
     }
 
+  if (TYPE_CODE (value_type (val)) == TYPE_CODE_INTERNAL_FUNCTION)
+    {
+      fprintf_filtered (stream, _("<internal function %s>"),
+			value_internal_function_name (val));
+      return 0;
+    }
+
   return 1;
 }
 
diff --git a/gdb/value.c b/gdb/value.c
index 0b530f0..b5aa853 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -43,6 +43,24 @@
 
 void _initialize_values (void);
 
+/* Definition of a user function.  */
+struct internal_function
+{
+  /* The name of the function.  It is a bit odd to have this in the
+     function itself -- the user might use a differently-named
+     convenience variable to hold the function.  */
+  char *name;
+
+  /* The handler.  */
+  internal_function_fn handler;
+
+  /* User data for the handler.  */
+  void *cookie;
+};
+
+/* Command list holding internal functions.  */
+static struct cmd_list_element *functionlist;
+
 struct value
 {
   /* Type of value; either not an lval, or one of the various
@@ -205,6 +223,10 @@ struct value_history_chunk
 static struct value_history_chunk *value_history_chain;
 
 static int value_history_count;	/* Abs number of last entry stored */
+
+/* The type of internal functions.  */
+
+static struct type *internal_fn_type;
 \f
 /* List of all value objects currently allocated
    (except for those released by calls to release_value)
@@ -774,7 +796,7 @@ init_if_undefined_command (char* args, int from_tty)
    the return value is NULL.  */
 
 struct internalvar *
-lookup_only_internalvar (char *name)
+lookup_only_internalvar (const char *name)
 {
   struct internalvar *var;
 
@@ -790,7 +812,7 @@ lookup_only_internalvar (char *name)
    NAME should not normally include a dollar sign.  */
 
 struct internalvar *
-create_internalvar (char *name)
+create_internalvar (const char *name)
 {
   struct internalvar *var;
   var = (struct internalvar *) xmalloc (sizeof (struct internalvar));
@@ -811,7 +833,7 @@ create_internalvar (char *name)
    one is created, with a void value.  */
 
 struct internalvar *
-lookup_internalvar (char *name)
+lookup_internalvar (const char *name)
 {
   struct internalvar *var;
 
@@ -915,6 +937,81 @@ internalvar_name (struct internalvar *var)
   return var->name;
 }
 
+/* Create a value which represents an internal function.
+   NAME is the name of the function.
+   HANDLER is a pointer to a function which is called when this
+   internal function is invoked during expression evaluation.
+   COOKIE is a pointer which is passed verbatim to HANDLER.  */
+static struct value *
+value_create_internal_function (const char *name,
+				internal_function_fn handler,
+				void *cookie)
+{
+  struct value *result = allocate_value (internal_fn_type);
+  gdb_byte *addr = value_contents_writeable (result);
+  struct internal_function **fnp = (struct internal_function **) addr;
+  /* The internal_function object is leaked here -- to make it truly
+     deletable, we would have to reference count it and add special
+     code to value_free and value_copy.  */
+  struct internal_function *ifn = XNEW (struct internal_function);
+  ifn->name = xstrdup (name);
+  ifn->handler = handler;
+  ifn->cookie = cookie;
+  *fnp = ifn;
+  return result;
+}
+
+/* Return the name of the value VAL, which is assumed to be of
+   "internal function" type.  The result is not copied and should not
+   be freed or modified.  */
+char *
+value_internal_function_name (struct value *val)
+{
+  gdb_byte *addr = value_contents_writeable (val);
+  struct internal_function *ifn = * (struct internal_function **) addr;
+  return ifn->name;
+}
+
+/* Call an internal function.  FUNC is a value which is assumed to be
+   of "internal function" type.  ARGC is the number of arguments to
+   pass to it, and ARGV is a vector holding the arguments.  */
+struct value *
+call_internal_function (struct value *func, int argc, struct value **argv)
+{
+  gdb_byte *addr = value_contents_writeable (func);
+  struct internal_function *ifn = * (struct internal_function **) addr;
+  return (*ifn->handler) (ifn->cookie, argc, argv);
+}
+
+/* The 'function' command.  This does nothing -- it is just a
+   placeholder to let "help function NAME" work.  This is also used as
+   the implementation of the sub-command that is created when
+   registering an internal function.  */
+static void
+function_command (char *command, int from_tty)
+{
+  /* Do nothing.  */
+}
+
+/* Add a new internal function.  NAME is the name of the function; DOC
+   is a documentation string describing the function.  HANDLER is
+   called when the function is invoked.  COOKIE is an arbitrary
+   pointer which is passed to HANDLER and is intended for "user
+   data".  */
+void
+add_internal_function (const char *name, const char *doc,
+		       internal_function_fn handler,
+		       void *cookie)
+{
+  struct internalvar *var = lookup_internalvar (name);
+  struct value *fnval = value_create_internal_function (name, handler, cookie);
+  release_value (fnval);
+  set_internalvar (var, fnval);
+
+  add_cmd (xstrdup (name), no_class, function_command, (char *) doc,
+	   &functionlist);
+}
+
 /* Update VALUE before discarding OBJFILE.  COPIED_TYPES is used to
    prevent cycles / duplicates.  */
 
@@ -1795,4 +1892,13 @@ init-if-undefined VARIABLE = EXPRESSION\n\
 Set an internal VARIABLE to the result of the EXPRESSION if it does not\n\
 exist or does not contain a value.  The EXPRESSION is not evaluated if the\n\
 VARIABLE is already initialized."));
+
+  add_prefix_cmd ("function", no_class, function_command, _("\
+Placeholder command for showing help on convenience functions."),
+		  &functionlist, "function ", 0, &cmdlist);
+
+  internal_fn_type = alloc_type (NULL);
+  TYPE_CODE (internal_fn_type) = TYPE_CODE_INTERNAL_FUNCTION;
+  TYPE_LENGTH (internal_fn_type) = sizeof (struct internal_function *);
+  TYPE_NAME (internal_fn_type) = "<internal function>";
 }
diff --git a/gdb/value.h b/gdb/value.h
index f53d333..e1ea5dc 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -465,11 +465,11 @@ extern void set_internalvar_component (struct internalvar *var,
 				       int bitpos, int bitsize,
 				       struct value *newvalue);
 
-extern struct internalvar *lookup_only_internalvar (char *name);
+extern struct internalvar *lookup_only_internalvar (const char *name);
 
-extern struct internalvar *create_internalvar (char *name);
+extern struct internalvar *create_internalvar (const char *name);
 
-extern struct internalvar *lookup_internalvar (char *name);
+extern struct internalvar *lookup_internalvar (const char *name);
 
 extern int value_equal (struct value *arg1, struct value *arg2);
 
@@ -585,4 +585,19 @@ extern struct value *value_allocate_space_in_inferior (int);
 extern struct value *value_of_local (const char *name, int complain);
 
 extern struct value * value_subscripted_rvalue (struct value *array, struct value *idx, int lowerbound);
+
+/* User function handler.  */
+
+typedef struct value *(*internal_function_fn) (void *cookie,
+					       int argc,
+					       struct value **argv);
+
+void add_internal_function (const char *name, const char *doc,
+			    internal_function_fn handler, void *cookie);
+
+struct value *call_internal_function (struct value *function,
+				      int argc, struct value **argv);
+
+char *value_internal_function_name (struct value *);
+
 #endif /* !defined (VALUE_H) */

Re: Convenience functions

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Tom Tromey <tromey@redhat.com>
> Date: Thu, 23 Oct 2008 13:13:15 -0600
> 
> I did write some docs.

Thanks.  I have two small requests for the documentation part:

> +Currently there is a single defined convenience function:

I would replace this with something more general, because we do hope
the list will grow.  Once somebody adds another function, this
sentence will become obsolete, but we might forget to change it.
OTOH, everybody can count to 1, so in its current form the sentence is
redundant.

> +@item $in_scope (@var{name})
> +Evaluate to @samp{1} if the symbol @var{name} is defined in the
> +program, or @samp{0} otherwise.

I suggest to change @var{name} to @var{symbol}, and then the text
could be a little bit more concise:

  Evaluate to @samp{1} if @var{symbol} is defined in the program, ...

By the way, is "defined in the program" accurate enough?  The name
"in_scope" does mean ``in scope'', right?  If so, I'd modify that
sentence to use ``in scope'' explicitly.  "Defined in the program"
could mean defined somewhere outside the scope of $pc.

Thanks.

Re: Convenience functions (was: Re: New scope checking patch)

[Thiago Jung Bauermann]

El jue, 23-10-2008 a las 09:41 -0400, Daniel Jacobowitz escribió:
> [Preachy interlude: splitting up patches good!  posting unrelated
> patches at the bottom of threads bad!  Until Tom mentioned it on IRC
> this week I'd completely forgotten this patch had been posted.]
> 
> On Tue, Jul 29, 2008 at 09:33:30PM -0600, Tom Tromey wrote:
> > >>>>> "Rob" == Rob Quill <rob.quill@gmail.com> writes:
> > 
> > Rob> Where can I find out about internal functions?
> > 
> > I've appended the patch.  It adds a new sort of value, which is an
> > internal function.  These are just like ordinary functions in an
> > expression but they run inside gdb -- not the inferior.  The user sees
> > them as convenience variables of "internal function" type.
> 
> I looked through this patch, since I have another use for it too.  It
> looks fine to me, except that of course it is missing documentation
> and test cases.  For test cases we'd need a function to test with;
> perhaps Rob's?

Since Tromey mentioned that Rob's patch is not correct in its current
form, I'm thinking of implementing it in python and use it to direct my
choice of next python patches to post here (i.e., my next set of python
patches would have the goal of making a scope checking script work).
Then I could work on testcases and documentation. What do you think?

The upside is that this will help focus the python submissions to
enabling a useful functionality as soon as possible. The downside (or
maybe it's not a downside at all) is that a scope checking feature is
being asked by users, so it would mean that such python script would
preferably be shipped in GDB releases...
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: Convenience functions (was: Re: New scope checking patch)

[Daniel Jacobowitz]

On Tue, Nov 04, 2008 at 07:35:59PM -0200, Thiago Jung Bauermann wrote:
> Since Tromey mentioned that Rob's patch is not correct in its current
> form, I'm thinking of implementing it in python and use it to direct my
> choice of next python patches to post here (i.e., my next set of python
> patches would have the goal of making a scope checking script work).
> Then I could work on testcases and documentation. What do you think?

Sounds reasonable to me.

> The upside is that this will help focus the python submissions to
> enabling a useful functionality as soon as possible. The downside (or
> maybe it's not a downside at all) is that a scope checking feature is
> being asked by users, so it would mean that such python script would
> preferably be shipped in GDB releases...

Yes - we've got to get the mechanism to do that in place anyway.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Convenience functions

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Thiago> The upside is that this will help focus the python submissions to
Thiago> enabling a useful functionality as soon as possible. The downside (or
Thiago> maybe it's not a downside at all) is that a scope checking feature is
Thiago> being asked by users, so it would mean that such python script would
Thiago> preferably be shipped in GDB releases...

Daniel> Yes - we've got to get the mechanism to do that in place anyway.

If you mean the "shipping python script" functionality, I'm hoping to
piggyback on the part of Sérgio's syscall work that deal with datadir.

Thiago, just FYI, this is one patch we may have to pull from trunk to
the python branch.

Tom