Introduce expression::evaluate

Message ID 20210217154712.498439-1-tom@tromey.com
State New
Headers show
Series
  • Introduce expression::evaluate
Related show

Commit Message

Tom Tromey Feb. 17, 2021, 3:47 p.m.
This introduces a new method, expression::evaluate, and changes the
top-level expression-evaluation functions to use it.  Stack temporary
handling is moved into this new method, which makes sense because that
handling was only done when "*pos == 0".

This patch avoids some temporary regressions related to stack
temporary in the larger expression rewrite series.  I've pulled it out
separately because it seems like a reasonable change in its own right,
and because it's better to avoid making that series even longer.

Regression tested on x86-64 Fedora 32.

gdb/ChangeLog
2021-02-17  Tom Tromey  <tom@tromey.com>

	* expression.h (struct expression) <evaluate>: Declare method.
	* eval.c (evaluate_subexp): Simplify.
	(expression::evaluate): New method.
	(evaluate_expression, evaluate_type): Use expression::evaluate.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/eval.c       | 47 +++++++++++++++++++++++++----------------------
 gdb/expression.h |  5 +++++
 3 files changed, 37 insertions(+), 22 deletions(-)

-- 
2.26.2

Comments

Andrew Burgess Feb. 18, 2021, 10:26 a.m. | #1
* Tom Tromey <tom@tromey.com> [2021-02-17 08:47:12 -0700]:

> This introduces a new method, expression::evaluate, and changes the

> top-level expression-evaluation functions to use it.  Stack temporary

> handling is moved into this new method, which makes sense because that

> handling was only done when "*pos == 0".

> 

> This patch avoids some temporary regressions related to stack

> temporary in the larger expression rewrite series.  I've pulled it out

> separately because it seems like a reasonable change in its own right,

> and because it's better to avoid making that series even longer.

> 

> Regression tested on x86-64 Fedora 32.

> 

> gdb/ChangeLog

> 2021-02-17  Tom Tromey  <tom@tromey.com>

> 

> 	* expression.h (struct expression) <evaluate>: Declare method.

> 	* eval.c (evaluate_subexp): Simplify.

> 	(expression::evaluate): New method.

> 	(evaluate_expression, evaluate_type): Use expression::evaluate.


LGTM.

Thanks,
Andrew


> ---

>  gdb/ChangeLog    |  7 +++++++

>  gdb/eval.c       | 47 +++++++++++++++++++++++++----------------------

>  gdb/expression.h |  5 +++++

>  3 files changed, 37 insertions(+), 22 deletions(-)

> 

> diff --git a/gdb/eval.c b/gdb/eval.c

> index e63511b7005..8256fdea148 100644

> --- a/gdb/eval.c

> +++ b/gdb/eval.c

> @@ -61,22 +61,8 @@ struct value *

>  evaluate_subexp (struct type *expect_type, struct expression *exp,

>  		 int *pos, enum noside noside)

>  {

> -  struct value *retval;

> -

> -  gdb::optional<enable_thread_stack_temporaries> stack_temporaries;

> -  if (*pos == 0 && target_has_execution ()

> -      && exp->language_defn->la_language == language_cplus

> -      && !thread_stack_temporaries_enabled_p (inferior_thread ()))

> -    stack_temporaries.emplace (inferior_thread ());

> -

> -  retval = (*exp->language_defn->expression_ops ()->evaluate_exp)

> -    (expect_type, exp, pos, noside);

> -

> -  if (stack_temporaries.has_value ()

> -      && value_in_thread_stack_temporaries (retval, inferior_thread ()))

> -    retval = value_non_lval (retval);

> -

> -  return retval;

> +  return ((*exp->language_defn->expression_ops ()->evaluate_exp)

> +	  (expect_type, exp, pos, noside));

>  }

>  

>  /* Parse the string EXP as a C expression, evaluate it,

> @@ -121,14 +107,33 @@ parse_to_comma_and_eval (const char **expp)

>  }

>  

>  

> +/* See expression.h.  */

> +

> +struct value *

> +expression::evaluate (struct type *expect_type, enum noside noside)

> +{

> +  gdb::optional<enable_thread_stack_temporaries> stack_temporaries;

> +  if (target_has_execution ()

> +      && language_defn->la_language == language_cplus

> +      && !thread_stack_temporaries_enabled_p (inferior_thread ()))

> +    stack_temporaries.emplace (inferior_thread ());

> +

> +  int pos = 0;

> +  struct value *retval = evaluate_subexp (expect_type, this, &pos, noside);

> +

> +  if (stack_temporaries.has_value ()

> +      && value_in_thread_stack_temporaries (retval, inferior_thread ()))

> +    retval = value_non_lval (retval);

> +

> +  return retval;

> +}

> +

>  /* See value.h.  */

>  

>  struct value *

>  evaluate_expression (struct expression *exp, struct type *expect_type)

>  {

> -  int pc = 0;

> -

> -  return evaluate_subexp (expect_type, exp, &pc, EVAL_NORMAL);

> +  return exp->evaluate (expect_type, EVAL_NORMAL);

>  }

>  

>  /* Evaluate an expression, avoiding all memory references

> @@ -137,9 +142,7 @@ evaluate_expression (struct expression *exp, struct type *expect_type)

>  struct value *

>  evaluate_type (struct expression *exp)

>  {

> -  int pc = 0;

> -

> -  return evaluate_subexp (nullptr, exp, &pc, EVAL_AVOID_SIDE_EFFECTS);

> +  return exp->evaluate (nullptr, EVAL_AVOID_SIDE_EFFECTS);

>  }

>  

>  /* Evaluate a subexpression, avoiding all memory references and

> diff --git a/gdb/expression.h b/gdb/expression.h

> index e70169e9b3e..397a0af9aba 100644

> --- a/gdb/expression.h

> +++ b/gdb/expression.h

> @@ -120,6 +120,11 @@ struct expression

>        return elts[0].opcode;

>    }

>  

> +  /* Evaluate the expression.  EXPECT_TYPE is the context type of the

> +     expression; normally this should be nullptr.  NOSIDE controls how

> +     evaluation is performed.  */

> +  struct value *evaluate (struct type *expect_type, enum noside noside);

> +

>    /* Language it was entered in.  */

>    const struct language_defn *language_defn;

>    /* Architecture it was parsed in.  */

> -- 

> 2.26.2

>

Patch

diff --git a/gdb/eval.c b/gdb/eval.c
index e63511b7005..8256fdea148 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -61,22 +61,8 @@  struct value *
 evaluate_subexp (struct type *expect_type, struct expression *exp,
 		 int *pos, enum noside noside)
 {
-  struct value *retval;
-
-  gdb::optional<enable_thread_stack_temporaries> stack_temporaries;
-  if (*pos == 0 && target_has_execution ()
-      && exp->language_defn->la_language == language_cplus
-      && !thread_stack_temporaries_enabled_p (inferior_thread ()))
-    stack_temporaries.emplace (inferior_thread ());
-
-  retval = (*exp->language_defn->expression_ops ()->evaluate_exp)
-    (expect_type, exp, pos, noside);
-
-  if (stack_temporaries.has_value ()
-      && value_in_thread_stack_temporaries (retval, inferior_thread ()))
-    retval = value_non_lval (retval);
-
-  return retval;
+  return ((*exp->language_defn->expression_ops ()->evaluate_exp)
+	  (expect_type, exp, pos, noside));
 }
 
 /* Parse the string EXP as a C expression, evaluate it,
@@ -121,14 +107,33 @@  parse_to_comma_and_eval (const char **expp)
 }
 
 
+/* See expression.h.  */
+
+struct value *
+expression::evaluate (struct type *expect_type, enum noside noside)
+{
+  gdb::optional<enable_thread_stack_temporaries> stack_temporaries;
+  if (target_has_execution ()
+      && language_defn->la_language == language_cplus
+      && !thread_stack_temporaries_enabled_p (inferior_thread ()))
+    stack_temporaries.emplace (inferior_thread ());
+
+  int pos = 0;
+  struct value *retval = evaluate_subexp (expect_type, this, &pos, noside);
+
+  if (stack_temporaries.has_value ()
+      && value_in_thread_stack_temporaries (retval, inferior_thread ()))
+    retval = value_non_lval (retval);
+
+  return retval;
+}
+
 /* See value.h.  */
 
 struct value *
 evaluate_expression (struct expression *exp, struct type *expect_type)
 {
-  int pc = 0;
-
-  return evaluate_subexp (expect_type, exp, &pc, EVAL_NORMAL);
+  return exp->evaluate (expect_type, EVAL_NORMAL);
 }
 
 /* Evaluate an expression, avoiding all memory references
@@ -137,9 +142,7 @@  evaluate_expression (struct expression *exp, struct type *expect_type)
 struct value *
 evaluate_type (struct expression *exp)
 {
-  int pc = 0;
-
-  return evaluate_subexp (nullptr, exp, &pc, EVAL_AVOID_SIDE_EFFECTS);
+  return exp->evaluate (nullptr, EVAL_AVOID_SIDE_EFFECTS);
 }
 
 /* Evaluate a subexpression, avoiding all memory references and
diff --git a/gdb/expression.h b/gdb/expression.h
index e70169e9b3e..397a0af9aba 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -120,6 +120,11 @@  struct expression
       return elts[0].opcode;
   }
 
+  /* Evaluate the expression.  EXPECT_TYPE is the context type of the
+     expression; normally this should be nullptr.  NOSIDE controls how
+     evaluation is performed.  */
+  struct value *evaluate (struct type *expect_type, enum noside noside);
+
   /* Language it was entered in.  */
   const struct language_defn *language_defn;
   /* Architecture it was parsed in.  */