gdb: New API for tracking innermost block

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

@@ -1,3 +1,26 @@
+2018-01-21  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* 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.
+
 2018-01-21  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* expression.h (innermost_block): Remove declaration.

gdb/ada-exp.y

@@ -757,11 +757,7 @@ write_var_from_sym (struct parser_state *par_state,
 		    struct symbol *sym)
 {
   if (symbol_read_needs_frame (sym))
-    {
+    innermost_block.update (block);
-      if (innermost_block == 0
-	  || contained_in (block, innermost_block))
-	innermost_block = block;
-    }
 
   write_exp_elt_opcode (par_state, OP_VAR_VALUE);
   write_exp_elt_block (par_state, block);

gdb/ada-lang.c

@@ -3507,9 +3507,7 @@ resolve_subexp (expression_up *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
+	  innermost_block.update (candidates[i]);
-              || contained_in (candidates[i].block, innermost_block))
-            innermost_block = candidates[i].block;
         }
 
       if (deprocedure_p
@@ -3554,9 +3552,7 @@ resolve_subexp (expression_up *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
+	    innermost_block.update (candidates[i]);
-                || contained_in (candidates[i].block, innermost_block))
-              innermost_block = candidates[i].block;
           }
       }
       break;

gdb/breakpoint.c

@@ -879,12 +879,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
 	{
@@ -10717,7 +10717,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);
@@ -10739,7 +10739,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);
 
@@ -10777,13 +10777,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;
     }

gdb/c-exp.y

@@ -949,12 +949,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
+			    innermost_block.update (sym);
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
-			    }
 
 			  write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			  write_exp_elt_block (pstate, sym.block);
@@ -1043,12 +1039,7 @@ variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
+				innermost_block.update (sym);
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
 
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);
@@ -1060,10 +1051,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
+			      innermost_block.update (sym);
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, STRUCTOP_PTR);

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))
-			{
+			innermost_block.update (sym);
-			  if (innermost_block == 0
-			      || contained_in (sym.block, innermost_block))
-			    innermost_block = sym.block;
-			}
-
 		      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
+		      innermost_block.update (sym);
-			  || contained_in (sym.block, innermost_block))
-			innermost_block = sym.block;
 		      write_exp_elt_opcode (pstate, OP_THIS);
 		      write_exp_elt_opcode (pstate, OP_THIS);
 		      write_exp_elt_opcode (pstate, STRUCTOP_PTR);

gdb/f-exp.y

@@ -461,12 +461,7 @@ variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
+				innermost_block.update (sym);
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);
 			      write_exp_elt_sym (pstate, sym.symbol);

gdb/go-exp.y

@@ -552,12 +552,7 @@ variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
+				innermost_block.update (sym);
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
 
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);

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))
-			    {
+			    innermost_block.update (sym);
-			      if (innermost_block == 0
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
-			    }
 
 			  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))
-				{
+				innermost_block.update (sym);
-				  if (innermost_block == 0 ||
-				      contained_in (sym.block,
-						    innermost_block))
-				    innermost_block = sym.block;
-				}
 
 			      write_exp_elt_opcode (pstate, OP_VAR_VALUE);
 			      write_exp_elt_block (pstate, sym.block);

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

gdb/p-exp.y

@@ -709,12 +709,7 @@ variable:	name_not_typename
 			  if (sym.symbol)
 			    {
 			      if (symbol_read_needs_frame (sym.symbol))
-				{
+				innermost_block.update (sym);
-				  if (innermost_block == 0
-				      || contained_in (sym.block,
-						       innermost_block))
-				    innermost_block = sym.block;
-				}
 
 			      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
+			      innermost_block.update (sym);
-				  || contained_in (sym.block,
-						   innermost_block))
-				innermost_block = sym.block;
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, OP_THIS);
 			      write_exp_elt_opcode (pstate, STRUCTOP_PTR);

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,15 @@ static expression_up parse_exp_in_context_1 (const char **, CORE_ADDR,
 					     const struct block *, int,
 					     int, int *);
 
+/* Documented at it's declaration.  */
+
+void
+innermost_block_tracker::update (const struct block *b)
+{
+  if (m_innermost_block == NULL || contained_in (b, m_innermost_block))
+    m_innermost_block = b;
+}
+
 /* Data structure for saving values of arglist_len for function calls whose
    arguments contain other function calls.  */
 

gdb/parser-defs.h

@@ -75,9 +75,51 @@ extern const struct block *expression_context_block;
    then look up the macro definitions active at that point.  */
 extern CORE_ADDR expression_context_pc;
 
-/* The innermost context required by the stack and register variables
+/* When parsing expressions we track the innermost block that was
-   we've encountered so far.  */
+   referenced.  */
-extern const struct block *innermost_block;
+
+class innermost_block_tracker
+{
+public:
+  innermost_block_tracker ()
+    : m_innermost_block (NULL)
+  { /* Nothing.  */ }
+
+  /* Reset the currently stored innermost block.  Usually called before
+     parsing a new expression.  */
+  void reset ()
+  {
+    m_innermost_block = nullptr;
+  }
+
+  /* Update the stored innermost block if the new block B is more inner
+     than the currently stored block, or if no block is stored yet.  */
+  void update (const struct block *b);
+
+  /* Overload of main UPDATE method which extracts the block from BS.  */
+  void update (const struct block_symbol &bs)
+  {
+    update (bs.block);
+  }
+
+  /* Return the stored innermost block.  Can be nullptr if no symbols or
+     registers were found during an expression parse, and so no innermost
+     block was defined.  */
+  const struct block *block () const
+  {
+    return m_innermost_block;
+  }
+
+private:
+  /* The currently stored innermost block found while parsing an
+     expression.  */
+  const struct block *m_innermost_block;
+};
+
+/* The innermost context required by the stack and register variables 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;

gdb/printcmd.c

@@ -1723,14 +1723,14 @@ display_command (const 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;
@@ -1891,9 +1891,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)
 	{

gdb/rust-exp.y

@@ -1044,15 +1044,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)
+  if (symbol_read_needs_frame (sym.symbol))
-      && (innermost_block == NULL
+    innermost_block.update (sym);
-	  || contained_in (sym.block, innermost_block)))
-    innermost_block = sym.block;
 }
 
 /* A helper to look up a Rust type, or fail.  This only works for

gdb/symfile.c

@@ -2900,7 +2900,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 ();

gdb/varobj.c

@@ -311,7 +311,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
@@ -336,7 +336,7 @@ varobj_create (const char *objname,
 	}
 
       var->format = variable_default_display (var.get ());
-      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;
@@ -345,7 +345,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