diff mbox

[PATCHv4,2/5] gdb: New API for tracking innermost block

Message ID 30e9656515ca65bd2ee5bf98f5d2e28def3b68e8.1508418720.git.andrew.burgess@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess Oct. 19, 2017, 1:27 p.m. UTC
This commit is preparation for a later change, at this point there
should be no user visible change.

We currently maintain a global innermost_block which tracks the most
inner block encountered when parsing an expression.

This commit wraps the innermost_block into a new class, and switches all
direct accesses to the variable to use the class API.

gdb/ChangeLog:

	* ada-exp.y (write_var_from_sym): Switch to innermost_block API.
	* ada-lang.c (resolve_subexp): Likewise.
	* breakpoint.c (set_breakpoint_condition) Likewise.
	(watch_command_1) Likewise.
	* c-exp.y (variable): Likewise.
	* d-exp.y (PrimaryExpression): Likewise.
	* f-exp.y (variable): Likewise.
	* go-exp.y (variable): Likewise.
	* m2-exp.y (variable): Likewise.
	* objfiles.c (objfile::~objfile): Likewise.
	* p-exp.y (variable): Likewise.
	* parse.c (innermost_block): Change type.
	* parser-defs.h (class innermost_block_tracker): New.
	(innermost_block): Change to innermost_block_tracker.
	* printcmd.c (display_command): Switch to innermost_block API.
	(do_one_display): Likewise.
	* rust-exp.y (do_one_display): Likewise.
	* symfile.c (clear_symtab_users): Likewise.
	* varobj.c (varobj_create): Switch to innermost_block API, replace
	use of innermost_block with block stored on varobj object.
---
 gdb/ChangeLog     | 23 +++++++++++++++++++++++
 gdb/ada-exp.y     |  6 +-----
 gdb/ada-lang.c    |  8 ++------
 gdb/breakpoint.c  | 12 ++++++------
 gdb/c-exp.y       | 20 ++++----------------
 gdb/d-exp.y       | 11 ++---------
 gdb/f-exp.y       |  7 +------
 gdb/go-exp.y      |  7 +------
 gdb/m2-exp.y      | 14 ++------------
 gdb/objfiles.c    |  2 +-
 gdb/p-exp.y       | 12 ++----------
 gdb/parse.c       | 13 ++++++++++++-
 gdb/parser-defs.h | 36 ++++++++++++++++++++++++++++++++++--
 gdb/printcmd.c    |  8 ++++----
 gdb/rust-exp.y    |  8 +++-----
 gdb/symfile.c     |  2 +-
 gdb/varobj.c      |  6 +++---
 17 files changed, 102 insertions(+), 93 deletions(-)

Comments

Simon Marchi Nov. 12, 2017, 4:26 p.m. UTC | #1
On 2017-10-19 09:27 AM, Andrew Burgess wrote:
> This commit is preparation for a later change, at this point there
> should be no user visible change.
> 
> We currently maintain a global innermost_block which tracks the most
> inner block encountered when parsing an expression.
> 
> This commit wraps the innermost_block into a new class, and switches all
> direct accesses to the variable to use the class API.

Hi Andrew,

I think this is a nice cleanup on its own right, removing a lot of duplicated
code.  LGTM with a nit:

> diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
> index f7ba7f088c..d9de10dda8 100644
> --- a/gdb/parser-defs.h
> +++ b/gdb/parser-defs.h
> @@ -63,9 +63,41 @@ extern const struct block *expression_context_block;
>     then look up the macro definitions active at that point.  */
>  extern CORE_ADDR expression_context_pc;
>  
> +/* When parsing expressions we track the innermost block that was
> +   referenced.  These functions are described in more detail at their
> +   definition site.  */
> +class innermost_block_tracker
> +{
> +public:
> +  innermost_block_tracker ()
> +    : innermost_block (NULL)
> +  { /* Nothing.  */ }
> +
> +  void reset ()
> +  {
> +    innermost_block = NULL;
> +  }
> +
> +  void update (const struct block *b);
> +
> +  void update (const struct block_symbol &bs)
> +  {
> +    update (bs.block);
> +  }
> +
> +  const struct block *block () const
> +  {
> +    return innermost_block;
> +  }
> +
> +private:
> +  const struct block *innermost_block;

Private members should be prefixed with m_ (m_innermost_block).

Simon
diff mbox

Patch

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index 4c1ff7b493..ff5650672c 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -761,11 +761,7 @@  write_var_from_sym (struct parser_state *par_state,
 		    struct symbol *sym)
 {
   if (orig_left_context == NULL && symbol_read_needs_frame (sym))
-    {
-      if (innermost_block == 0
-	  || contained_in (block, innermost_block))
-	innermost_block = block;
-    }
+    innermost_block.update (block);
 
   write_exp_elt_opcode (par_state, OP_VAR_VALUE);
   write_exp_elt_block (par_state, block);
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 7409496ce7..2b47e7ee6d 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -3509,9 +3509,7 @@  resolve_subexp (struct expression **expp, int *pos, int deprocedure_p,
 
           exp->elts[pc + 1].block = candidates[i].block;
           exp->elts[pc + 2].symbol = candidates[i].symbol;
-          if (innermost_block == NULL
-              || contained_in (candidates[i].block, innermost_block))
-            innermost_block = candidates[i].block;
+	  innermost_block.update (candidates[i]);
         }
 
       if (deprocedure_p
@@ -3554,9 +3552,7 @@  resolve_subexp (struct expression **expp, int *pos, int deprocedure_p,
 
             exp->elts[pc + 4].block = candidates[i].block;
             exp->elts[pc + 5].symbol = candidates[i].symbol;
-            if (innermost_block == NULL
-                || contained_in (candidates[i].block, innermost_block))
-              innermost_block = candidates[i].block;
+	    innermost_block.update (candidates[i]);
           }
       }
       break;
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 32ceea7c9b..665ca3e76d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -905,12 +905,12 @@  set_breakpoint_condition (struct breakpoint *b, const char *exp,
 	{
 	  struct watchpoint *w = (struct watchpoint *) b;
 
-	  innermost_block = NULL;
+	  innermost_block.reset ();
 	  arg = exp;
 	  w->cond_exp = parse_exp_1 (&arg, 0, 0, 0);
 	  if (*arg)
 	    error (_("Junk at end of expression"));
-	  w->cond_exp_valid_block = innermost_block;
+	  w->cond_exp_valid_block = innermost_block.block ();
 	}
       else
 	{
@@ -10777,7 +10777,7 @@  watch_command_1 (const char *arg, int accessflag, int from_tty,
   /* Parse the rest of the arguments.  From here on out, everything
      is in terms of a newly allocated string instead of the original
      ARG.  */
-  innermost_block = NULL;
+  innermost_block.reset ();
   std::string expression (arg, exp_end - arg);
   exp_start = arg = expression.c_str ();
   expression_up exp = parse_exp_1 (&arg, 0, 0, 0);
@@ -10799,7 +10799,7 @@  watch_command_1 (const char *arg, int accessflag, int from_tty,
       error (_("Cannot watch constant value `%.*s'."), len, exp_start);
     }
 
-  exp_valid_block = innermost_block;
+  exp_valid_block = innermost_block.block ();
   mark = value_mark ();
   fetch_subexp_value (exp.get (), &pc, &val, &result, NULL, just_location);
 
@@ -10837,13 +10837,13 @@  watch_command_1 (const char *arg, int accessflag, int from_tty,
   toklen = end_tok - tok;
   if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
     {
-      innermost_block = NULL;
+      innermost_block.reset ();
       tok = cond_start = end_tok + 1;
       parse_exp_1 (&tok, 0, 0, 0);
 
       /* The watchpoint expression may not be local, but the condition
 	 may still be.  E.g.: `watch global if local > 0'.  */
-      cond_exp_valid_block = innermost_block;
+      cond_exp_valid_block = innermost_block.block ();
 
       cond_end = tok;
     }
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 18c74d8a17..c5df14f252 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -962,12 +962,8 @@  variable:	block COLONCOLON name
 			    error (_("No symbol \"%s\" in specified context."),
 				   copy_name ($3));
 			  if (symbol_read_needs_frame (sym.symbol))
-			    {
-			      if (innermost_block == 0
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
-			    }
+
+			    innermost_block.update (sym);
 
 			  write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			  write_exp_elt_block (pstate, sym.block);
@@ -1056,12 +1052,7 @@  variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
+				innermost_block.update (sym);
 
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);
@@ -1073,10 +1064,7 @@  variable:	name_not_typename
 			      /* C++: it hangs off of `this'.  Must
 			         not inadvertently convert from a method call
 				 to data ref.  */
-			      if (innermost_block == 0
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
+			      innermost_block.update (sym);
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, STRUCTOP_PTR);
diff --git a/gdb/d-exp.y b/gdb/d-exp.y
index 00c96764d6..c1f11ad1fa 100644
--- a/gdb/d-exp.y
+++ b/gdb/d-exp.y
@@ -422,12 +422,7 @@  PrimaryExpression:
 		  if (sym.symbol && SYMBOL_CLASS (sym.symbol) != LOC_TYPEDEF)
 		    {
 		      if (symbol_read_needs_frame (sym.symbol))
-			{
-			  if (innermost_block == 0
-			      || contained_in (sym.block, innermost_block))
-			    innermost_block = sym.block;
-			}
-
+			innermost_block.update (sym);
 		      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 		      write_exp_elt_block (pstate, sym.block);
 		      write_exp_elt_sym (pstate, sym.symbol);
@@ -437,9 +432,7 @@  PrimaryExpression:
 		     {
 		      /* It hangs off of `this'.  Must not inadvertently convert from a
 			 method call to data ref.  */
-		      if (innermost_block == 0
-			  || contained_in (sym.block, innermost_block))
-			innermost_block = sym.block;
+		      innermost_block.update (sym);
 		      write_exp_elt_opcode (pstate, OP_THIS);
 		      write_exp_elt_opcode (pstate, OP_THIS);
 		      write_exp_elt_opcode (pstate, STRUCTOP_PTR);
diff --git a/gdb/f-exp.y b/gdb/f-exp.y
index 8dcc811b68..6e01e8d442 100644
--- a/gdb/f-exp.y
+++ b/gdb/f-exp.y
@@ -460,12 +460,7 @@  variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
+				innermost_block.update (sym);
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);
 			      write_exp_elt_sym (pstate, sym.symbol);
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index 629093a118..5d808fe9dc 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -554,12 +554,7 @@  variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
+				innermost_block.update (sym);
 
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);
diff --git a/gdb/m2-exp.y b/gdb/m2-exp.y
index 02dc36f480..5fc43d08f5 100644
--- a/gdb/m2-exp.y
+++ b/gdb/m2-exp.y
@@ -548,12 +548,7 @@  variable:	block COLONCOLON NAME
 			    error (_("No symbol \"%s\" in specified context."),
 				   copy_name ($3));
 			  if (symbol_read_needs_frame (sym.symbol))
-			    {
-			      if (innermost_block == 0
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
-			    }
+			    innermost_block.update (sym);
 
 			  write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			  write_exp_elt_block (pstate, sym.block);
@@ -574,12 +569,7 @@  variable:	NAME
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
-				  if (innermost_block == 0 ||
-				      contained_in (sym.block,
-						    innermost_block))
-				    innermost_block = sym.block;
-				}
+				innermost_block.update (sym);
 
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index d8fe88b136..0a6398e54d 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -704,7 +704,7 @@  objfile::~objfile ()
      FIXME: It's not clear which of these are supposed to persist
      between expressions and which ought to be reset each time.  */
   expression_context_block = NULL;
-  innermost_block = NULL;
+  innermost_block.reset ();
 
   /* Check to see if the current_source_symtab belongs to this objfile,
      and if so, call clear_current_source_symtab_and_line.  */
diff --git a/gdb/p-exp.y b/gdb/p-exp.y
index eee4fa94b3..68433a90b4 100644
--- a/gdb/p-exp.y
+++ b/gdb/p-exp.y
@@ -709,12 +709,7 @@  variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
+				innermost_block.update (sym);
 
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);
@@ -728,10 +723,7 @@  variable:	name_not_typename
 			      /* Object pascal: it hangs off of `this'.  Must
 			         not inadvertently convert from a method call
 				 to data ref.  */
-			      if (innermost_block == 0
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
+			      innermost_block.update (sym);
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, STRUCTOP_PTR);
diff --git a/gdb/parse.c b/gdb/parse.c
index 6bbf25f699..945ef295e6 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -68,7 +68,7 @@  const struct exp_descriptor exp_descriptor_standard =
 /* Global variables declared in parser-defs.h (and commented there).  */
 const struct block *expression_context_block;
 CORE_ADDR expression_context_pc;
-const struct block *innermost_block;
+innermost_block_tracker innermost_block;
 int arglist_len;
 static struct type_stack type_stack;
 const char *lexptr;
@@ -121,6 +121,17 @@  static expression_up parse_exp_in_context_1 (const char **, CORE_ADDR,
 					     const struct block *, int,
 					     int, int *);
 
+
+/* Update the stored innermost block if the new block is more inner than
+   the currently stored block, or if no block is stored yet.  */
+
+void
+innermost_block_tracker::update (const struct block *b)
+{
+  if (innermost_block == NULL || contained_in (b, innermost_block))
+    innermost_block = b;
+}
+
 /* Data structure for saving values of arglist_len for function calls whose
    arguments contain other function calls.  */
 
diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
index f7ba7f088c..d9de10dda8 100644
--- a/gdb/parser-defs.h
+++ b/gdb/parser-defs.h
@@ -63,9 +63,41 @@  extern const struct block *expression_context_block;
    then look up the macro definitions active at that point.  */
 extern CORE_ADDR expression_context_pc;
 
+/* When parsing expressions we track the innermost block that was
+   referenced.  These functions are described in more detail at their
+   definition site.  */
+class innermost_block_tracker
+{
+public:
+  innermost_block_tracker ()
+    : innermost_block (NULL)
+  { /* Nothing.  */ }
+
+  void reset ()
+  {
+    innermost_block = NULL;
+  }
+
+  void update (const struct block *b);
+
+  void update (const struct block_symbol &bs)
+  {
+    update (bs.block);
+  }
+
+  const struct block *block () const
+  {
+    return innermost_block;
+  }
+
+private:
+  const struct block *innermost_block;
+};
+
 /* The innermost context required by the stack and register variables
-   we've encountered so far.  */
-extern const struct block *innermost_block;
+   we've encountered so far.  This should be cleared before parsing an
+   expression, and queried once the parse is complete.  */
+extern innermost_block_tracker innermost_block;
 
 /* Number of arguments seen so far in innermost function call.  */
 extern int arglist_len;
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 77a05e5d9f..c443b5b945 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1721,14 +1721,14 @@  display_command (char *arg, int from_tty)
       fmt.raw = 0;
     }
 
-  innermost_block = NULL;
+  innermost_block.reset ();
   expression_up expr = parse_expression (exp);
 
   newobj = new display ();
 
   newobj->exp_string = xstrdup (exp);
   newobj->exp = std::move (expr);
-  newobj->block = innermost_block;
+  newobj->block = innermost_block.block ();
   newobj->pspace = current_program_space;
   newobj->number = ++display_number;
   newobj->format = fmt;
@@ -1889,9 +1889,9 @@  do_one_display (struct display *d)
 
       TRY
 	{
-	  innermost_block = NULL;
+	  innermost_block.reset ();
 	  d->exp = parse_expression (d->exp_string);
-	  d->block = innermost_block;
+	  d->block = innermost_block.block ();
 	}
       CATCH (ex, RETURN_MASK_ALL)
 	{
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index 0cb185c512..403c5cbfdb 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -1049,15 +1049,13 @@  super_name (const struct rust_op *ident, unsigned int n_supers)
 		   ident->right.params);
 }
 
-/* A helper that updates innermost_block as appropriate.  */
+/* A helper that updates the innermost block as appropriate.  */
 
 static void
 update_innermost_block (struct block_symbol sym)
 {
-  if (symbol_read_needs_frame (sym.symbol)
-      && (innermost_block == NULL
-	  || contained_in (sym.block, innermost_block)))
-    innermost_block = sym.block;
+  if (symbol_read_needs_frame (sym.symbol))
+    innermost_block.update (sym);
 }
 
 /* A helper to look up a Rust type, or fail.  This only works for
diff --git a/gdb/symfile.c b/gdb/symfile.c
index a7d8553bb0..40c087fa4d 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2890,7 +2890,7 @@  clear_symtab_users (symfile_add_flags add_flags)
      FIXME: It's not clear which of these are supposed to persist
      between expressions and which ought to be reset each time.  */
   expression_context_block = NULL;
-  innermost_block = NULL;
+  innermost_block.reset ();
 
   /* Varobj may refer to old symbols, perform a cleanup.  */
   varobj_invalidate ();
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f2232888c4..5b5ffd2baf 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -324,7 +324,7 @@  varobj_create (const char *objname,
 	}
 
       p = expression;
-      innermost_block = NULL;
+      innermost_block.reset ();
       /* Wrap the call to parse expression, so we can 
          return a sensible error.  */
       TRY
@@ -351,7 +351,7 @@  varobj_create (const char *objname,
 	}
 
       var->format = variable_default_display (var);
-      var->root->valid_block = innermost_block;
+      var->root->valid_block = innermost_block.block ();
       var->name = expression;
       /* For a root var, the name and the expr are the same.  */
       var->path_expr = expression;
@@ -360,7 +360,7 @@  varobj_create (const char *objname,
          we must select the appropriate frame before parsing
          the expression, otherwise the value will not be current.
          Since select_frame is so benign, just call it for all cases.  */
-      if (innermost_block)
+      if (var->root->valid_block)
 	{
 	  /* User could specify explicit FRAME-ADDR which was not found but
 	     EXPRESSION is frame specific and we would not be able to evaluate