[PATCH 0/3] Minor expression cleanups

[PATCH 0/3] Minor expression cleanups

[Tom Tromey]

While looking at some parser-related changes for DAP, I found a few
things that could be cleaned up independently.

Regression tested on x86-64 Fedora 36.

---
Tom Tromey (3):
      Remove op_name
      Remove evaluate_expression
      Remove evaluate_type

 gdb/ada-lang.c        |  4 ++--
 gdb/arc-tdep.c        |  2 +-
 gdb/breakpoint.c      |  2 +-
 gdb/cli/cli-script.c  |  4 ++--
 gdb/darwin-nat-info.c |  2 +-
 gdb/dtrace-probe.c    |  4 ++--
 gdb/eval.c            | 25 ++++---------------------
 gdb/expprint.c        | 41 +++++++++++++++++------------------------
 gdb/expression.h      | 12 +++++++-----
 gdb/gnu-v3-abi.c      |  2 +-
 gdb/linux-thread-db.c |  2 +-
 gdb/mi/mi-main.c      |  6 +++---
 gdb/objc-lang.c       |  4 +---
 gdb/printcmd.c        | 12 ++++++------
 gdb/stack.c           |  2 +-
 gdb/stap-probe.c      |  2 +-
 gdb/tracepoint.c      |  2 +-
 gdb/typeprint.c       |  4 ++--
 gdb/value.c           |  2 +-
 gdb/value.h           | 11 -----------
 gdb/varobj.c          | 14 +++++++-------
 21 files changed, 62 insertions(+), 97 deletions(-)
---
base-commit: 1f7f972f59470104734ec705c985ab319eb580db
change-id: 20230428-minor-expr-cleanuups-77d8f8d15594

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/3] Remove op_name

[Tom Tromey]

op_name is only needed in a single place, so remove it and inline it
there.
---
 gdb/expprint.c   | 41 +++++++++++++++++------------------------
 gdb/expression.h |  4 ----
 2 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/gdb/expprint.c b/gdb/expprint.c
index e0ec25573c2..b996c9faf8b 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -36,29 +36,6 @@
 
 #include <ctype.h>
 
-/* Default name for the standard operator OPCODE (i.e., one defined in
-   the definition of enum exp_opcode).  */
-
-const char *
-op_name (enum exp_opcode opcode)
-{
-  switch (opcode)
-    {
-    default:
-      {
-	static char buf[30];
-
-	xsnprintf (buf, sizeof (buf), "<unknown %d>", opcode);
-	return buf;
-      }
-#define OP(name)	\
-    case name:		\
-      return #name ;
-#include "std-operator.def"
-#undef OP
-    }
-}
-
 /* Meant to be used in debug sessions, so don't export it in a header file.  */
 extern void ATTRIBUTE_USED debug_exp (struct expression *exp);
 
@@ -84,7 +61,23 @@ check_objfile (const struct block *block, struct objfile *objfile)
 void
 dump_for_expression (struct ui_file *stream, int depth, enum exp_opcode op)
 {
-  gdb_printf (stream, _("%*sOperation: %s\n"), depth, "", op_name (op));
+  gdb_printf (stream, _("%*sOperation: "), depth, "");
+
+  switch (op)
+    {
+    default:
+      gdb_printf (stream, "<unknown %d>", op);
+      break;
+
+#define OP(name)	\
+    case name:		\
+      gdb_puts (#name, stream); \
+      break;
+#include "std-operator.def"
+#undef OP
+    }
+
+  gdb_puts ("\n", stream);
 }
 
 void
diff --git a/gdb/expression.h b/gdb/expression.h
index c314fc24e1b..77949ddaf4b 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -284,10 +284,6 @@ extern struct value *evaluate_subexp_do_call (expression *exp,
 					      const char *function_name,
 					      type *default_return_type);
 
-/* From expprint.c */
-
-extern const char *op_name (enum exp_opcode opcode);
-
 /* In an OP_RANGE expression, either bound could be empty, indicating
    that its value is by default that of the corresponding bound of the
    array or string.  Also, the upper end of the range can be exclusive

-- 
2.39.1

[PATCH 2/3] Remove evaluate_expression

[Tom Tromey]

evaluate_expression is just a little wrapper for a method on
expression.  Removing it also removes a lot of ugly (IMO) calls to
get().
---
 gdb/ada-lang.c        |  4 ++--
 gdb/arc-tdep.c        |  2 +-
 gdb/breakpoint.c      |  2 +-
 gdb/cli/cli-script.c  |  4 ++--
 gdb/darwin-nat-info.c |  2 +-
 gdb/dtrace-probe.c    |  2 +-
 gdb/eval.c            | 16 ++++------------
 gdb/expression.h      |  3 ++-
 gdb/linux-thread-db.c |  2 +-
 gdb/mi/mi-main.c      |  4 ++--
 gdb/objc-lang.c       |  4 +---
 gdb/printcmd.c        | 12 ++++++------
 gdb/stack.c           |  2 +-
 gdb/stap-probe.c      |  2 +-
 gdb/tracepoint.c      |  2 +-
 gdb/value.c           |  2 +-
 gdb/value.h           |  9 ---------
 gdb/varobj.c          | 12 ++++++------
 18 files changed, 34 insertions(+), 52 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 3a9f554ce4c..50d5c6f9697 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4339,7 +4339,7 @@ ada_read_renaming_var_value (struct symbol *renaming_sym,
 
   sym_name = renaming_sym->linkage_name ();
   expression_up expr = parse_exp_1 (&sym_name, 0, block, 0);
-  return evaluate_expression (expr.get ());
+  return expr->evaluate ();
 }
 \f
 
@@ -12318,7 +12318,7 @@ should_stop_exception (const struct bp_location *bl)
   try
     {
       scoped_value_mark mark;
-      stop = value_true (evaluate_expression (ada_loc->excep_cond_expr.get ()));
+      stop = value_true (ada_loc->excep_cond_expr->evaluate ());
     }
   catch (const gdb_exception_error &ex)
     {
diff --git a/gdb/arc-tdep.c b/gdb/arc-tdep.c
index bbf4846a8f8..0b541c6335a 100644
--- a/gdb/arc-tdep.c
+++ b/gdb/arc-tdep.c
@@ -2434,7 +2434,7 @@ dump_arc_instruction_command (const char *args, int from_tty)
 {
   struct value *val;
   if (args != NULL && strlen (args) > 0)
-    val = evaluate_expression (parse_expression (args).get ());
+    val = parse_expression (args)->evaluate ();
   else
     val = access_value_history (0);
   val->record_latest ();
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 46287da5f87..20e016e8cb3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4984,7 +4984,7 @@ static bool
 breakpoint_cond_eval (expression *exp)
 {
   scoped_value_mark mark;
-  return value_true (evaluate_expression (exp));
+  return value_true (exp->evaluate ());
 }
 
 /* Allocate a new bpstat.  Link it to the FIFO list by BS_LINK_POINTER.  */
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 294a5f18fe6..b96dd74330c 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -567,7 +567,7 @@ execute_control_command_1 (struct command_line *cmd, int from_tty)
 	    /* Evaluate the expression.  */
 	    {
 	      scoped_value_mark mark;
-	      value *val = evaluate_expression (expr.get ());
+	      value *val = expr->evaluate ();
 	      cond_result = value_true (val);
 	    }
 
@@ -622,7 +622,7 @@ execute_control_command_1 (struct command_line *cmd, int from_tty)
 	/* Evaluate the conditional.  */
 	{
 	  scoped_value_mark mark;
-	  value *val = evaluate_expression (expr.get ());
+	  value *val = expr->evaluate ();
 
 	  /* Choose which arm to take commands from based on the value
 	     of the conditional expression.  */
diff --git a/gdb/darwin-nat-info.c b/gdb/darwin-nat-info.c
index 9e562074cf8..d5f633bccae 100644
--- a/gdb/darwin-nat-info.c
+++ b/gdb/darwin-nat-info.c
@@ -723,7 +723,7 @@ info_mach_region_command (const char *exp, int from_tty)
   struct inferior *inf;
 
   expression_up expr = parse_expression (exp);
-  val = evaluate_expression (expr.get ());
+  val = expr->evaluate ();
   if (TYPE_IS_REFERENCE (val->type ()))
     {
       val = value_ind (val);
diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index 70fa1a9413e..dad15df5b99 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -714,7 +714,7 @@ dtrace_probe::evaluate_argument (unsigned n,
   struct dtrace_probe_arg *arg;
 
   arg = this->get_arg_by_number (n, gdbarch);
-  return evaluate_expression (arg->expr.get (), arg->type);
+  return arg->expr->evaluate (arg->type);
 }
 
 /* Implementation of the compile_to_ax method.  */
diff --git a/gdb/eval.c b/gdb/eval.c
index 6b362f46424..a8d5c78e09e 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -53,7 +53,7 @@ parse_and_eval_address (const char *exp)
 {
   expression_up expr = parse_expression (exp);
 
-  return value_as_address (evaluate_expression (expr.get ()));
+  return value_as_address (expr->evaluate ());
 }
 
 /* Like parse_and_eval_address, but treats the value of the expression
@@ -63,7 +63,7 @@ parse_and_eval_long (const char *exp)
 {
   expression_up expr = parse_expression (exp);
 
-  return value_as_long (evaluate_expression (expr.get ()));
+  return value_as_long (expr->evaluate ());
 }
 
 struct value *
@@ -71,7 +71,7 @@ parse_and_eval (const char *exp)
 {
   expression_up expr = parse_expression (exp);
 
-  return evaluate_expression (expr.get ());
+  return expr->evaluate ();
 }
 
 /* Parse up to a comma (or to a closeparen)
@@ -83,7 +83,7 @@ parse_to_comma_and_eval (const char **expp)
 {
   expression_up expr = parse_exp_1 (expp, 0, nullptr, 1);
 
-  return evaluate_expression (expr.get ());
+  return expr->evaluate ();
 }
 \f
 
@@ -116,14 +116,6 @@ expression::evaluate (struct type *expect_type, enum noside noside)
   return retval;
 }
 
-/* See value.h.  */
-
-struct value *
-evaluate_expression (struct expression *exp, struct type *expect_type)
-{
-  return exp->evaluate (expect_type, EVAL_NORMAL);
-}
-
 /* Evaluate an expression, avoiding all memory references
    and getting a value whose type alone is correct.  */
 
diff --git a/gdb/expression.h b/gdb/expression.h
index 77949ddaf4b..5bfb051a393 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -222,7 +222,8 @@ struct expression
   /* 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);
+  struct value *evaluate (struct type *expect_type = nullptr,
+			  enum noside noside = EVAL_NORMAL);
 
   /* Language it was entered in.  */
   const struct language_defn *language_defn;
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 0e190f94266..71a81aa0cb9 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -694,7 +694,7 @@ check_thread_db_callback (const td_thrhandle_t *th, void *arg)
       switch_to_thread (thread_info);
 
       expression_up expr = parse_expression ("(int) errno");
-      struct value *val = evaluate_expression (expr.get ());
+      struct value *val = expr->evaluate ();
 
       if (tdb_testinfo->log_progress)
 	{
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0013e5dfafd..683d36d93d5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1189,7 +1189,7 @@ mi_cmd_data_evaluate_expression (const char *command, char **argv, int argc)
 
   expression_up expr = parse_expression (argv[0]);
 
-  val = evaluate_expression (expr.get ());
+  val = expr->evaluate ();
 
   string_file stb;
 
@@ -2468,7 +2468,7 @@ print_variable_or_computed (const char *expression, enum print_values values)
   if (values == PRINT_SIMPLE_VALUES)
     val = evaluate_type (expr.get ());
   else
-    val = evaluate_expression (expr.get ());
+    val = expr->evaluate ();
 
   gdb::optional<ui_out_emit_tuple> tuple_emitter;
   if (values != PRINT_NO_VALUES)
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 4a9dee44dd7..ccbe7c19729 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1164,9 +1164,7 @@ print_object_command (const char *args, int from_tty)
   {
     expression_up expr = parse_expression (args);
 
-    object
-      = evaluate_expression (expr.get (),
-			     builtin_type (expr->gdbarch)->builtin_data_ptr);
+    object = expr->evaluate (builtin_type (expr->gdbarch)->builtin_data_ptr);
   }
 
   /* Validate the address for sanity.  */
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index dd92e31d31b..693d1a8a93c 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1316,7 +1316,7 @@ process_print_command_args (const char *args, value_print_options *print_opts,
       /* VOIDPRINT is true to indicate that we do want to print a void
 	 value, so invert it for parse_expression.  */
       expression_up expr = parse_expression (exp, nullptr, !voidprint);
-      return evaluate_expression (expr.get ());
+      return expr->evaluate ();
     }
 
   return access_value_history (0);
@@ -1495,7 +1495,7 @@ output_command (const char *exp, int from_tty)
 
   expression_up expr = parse_expression (exp);
 
-  val = evaluate_expression (expr.get ());
+  val = expr->evaluate ();
 
   annotate_value_begin (val->type ());
 
@@ -1534,7 +1534,7 @@ set_command (const char *exp, int from_tty)
 	(_("Expression is not an assignment (and might have no effect)"));
     }
 
-  evaluate_expression (expr.get ());
+  expr->evaluate ();
 }
 
 static void
@@ -1900,7 +1900,7 @@ x_command (const char *exp, int from_tty)
 	 command's definition.  */
       if (from_tty)
 	set_repeat_arguments ("");
-      val = evaluate_expression (expr.get ());
+      val = expr->evaluate ();
       if (TYPE_IS_REFERENCE (val->type ()))
 	val = coerce_ref (val);
       /* In rvalue contexts, such as this, functions are coerced into
@@ -2188,7 +2188,7 @@ do_one_display (struct display *d)
 	  struct value *val;
 	  CORE_ADDR addr;
 
-	  val = evaluate_expression (d->exp.get ());
+	  val = d->exp->evaluate ();
 	  addr = value_as_address (val);
 	  if (d->format.format == 'i')
 	    addr = gdbarch_addr_bits_remove (d->exp->gdbarch, addr);
@@ -2226,7 +2226,7 @@ do_one_display (struct display *d)
 	{
 	  struct value *val;
 
-	  val = evaluate_expression (d->exp.get ());
+	  val = d->exp->evaluate ();
 	  print_formatted (val, d->format.size, &opts, gdb_stdout);
 	}
       catch (const gdb_exception_error &ex)
diff --git a/gdb/stack.c b/gdb/stack.c
index 03e903d901b..b1b25aa1c7e 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2722,7 +2722,7 @@ return_command (const char *retval_exp, int from_tty)
 
       /* Compute the return value.  Should the computation fail, this
 	 call throws an error.  */
-      return_value = evaluate_expression (retval_expr.get ());
+      return_value = retval_expr->evaluate ();
 
       /* Cast return value to the return type of the function.  Should
 	 the cast fail, this call throws an error.  */
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index c7ad35db06f..756e90d2192 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1444,7 +1444,7 @@ stap_probe::evaluate_argument (unsigned n, frame_info_ptr frame)
   struct gdbarch *gdbarch = get_frame_arch (frame);
 
   arg = this->get_arg_by_number (n, gdbarch);
-  return evaluate_expression (arg->aexpr.get (), arg->atype);
+  return arg->aexpr->evaluate (arg->atype);
 }
 
 /* Compile the probe's argument N (indexed from 0) to agent expression.
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c8bac7a0122..ffda4a6439e 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1375,7 +1375,7 @@ encode_actions_1 (struct command_line *action,
 		    case UNOP_MEMVAL:
 		      {
 			/* Safe because we know it's a simple expression.  */
-			tempval = evaluate_expression (exp.get ());
+			tempval = exp->evaluate ();
 			addr = tempval->address ();
 			expr::unop_memval_operation *memop
 			  = (gdb::checked_static_cast<expr::unop_memval_operation *>
diff --git a/gdb/value.c b/gdb/value.c
index eab1933b256..01c30eb3f93 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1896,7 +1896,7 @@ init_if_undefined_command (const char* args, int from_tty)
   /* Only evaluate the expression if the lvalue is void.
      This may still fail if the expression is invalid.  */
   if (intvar->kind == INTERNALVAR_VOID)
-    evaluate_expression (expr.get ());
+    expr->evaluate ();
 }
 
 
diff --git a/gdb/value.h b/gdb/value.h
index e13f92c397b..f6092aba5a4 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1299,15 +1299,6 @@ extern int using_struct_return (struct gdbarch *gdbarch,
 				struct value *function,
 				struct type *value_type);
 
-/* Evaluate the expression EXP.  If set, EXPECT_TYPE is passed to the
-   outermost operation's evaluation.  This is ignored by most
-   operations, but may be used, e.g., to determine the type of an
-   otherwise untyped symbol.  The caller should not assume that the
-   returned value has this type.  */
-
-extern struct value *evaluate_expression (struct expression *exp,
-					  struct type *expect_type = nullptr);
-
 extern struct value *evaluate_type (struct expression *exp);
 
 extern value *evaluate_var_value (enum noside noside, const block *blk,
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 5a5e55c2d82..75b4d44b109 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -364,12 +364,12 @@ varobj_create (const char *objname,
 	  select_frame (fi);	 
 	}
 
-      /* We definitely need to catch errors here.
-	 If evaluate_expression succeeds we got the value we wanted.
-	 But if it fails, we still go on with a call to evaluate_type().  */
+      /* We definitely need to catch errors here.  If evaluation of
+	 the expression succeeds, we got the value we wanted.  But if
+	 it fails, we still go on with a call to evaluate_type().  */
       try
 	{
-	  value = evaluate_expression (var->root->exp.get ());
+	  value = var->root->exp->evaluate ();
 	}
       catch (const gdb_exception_error &except)
 	{
@@ -979,7 +979,7 @@ varobj_set_value (struct varobj *var, const char *expression)
   expression_up exp = parse_exp_1 (&s, 0, 0, 0);
   try
     {
-      value = evaluate_expression (exp.get ());
+      value = exp->evaluate ();
     }
 
   catch (const gdb_exception_error &except)
@@ -1995,7 +1995,7 @@ value_of_root_1 (struct varobj **var_handle)
 	 expression fails we want to just return NULL.  */
       try
 	{
-	  new_val = evaluate_expression (var->root->exp.get ());
+	  new_val = var->root->exp->evaluate ();
 	}
       catch (const gdb_exception_error &except)
 	{

-- 
2.39.1

[PATCH 3/3] Remove evaluate_type

[Tom Tromey]

Like evaluate_expression, evaluate_type is also just a simple wrapper.
Removing it makes the code a little nicer.
---
 gdb/dtrace-probe.c | 2 +-
 gdb/eval.c         | 9 ---------
 gdb/expression.h   | 5 +++++
 gdb/gnu-v3-abi.c   | 2 +-
 gdb/mi/mi-main.c   | 2 +-
 gdb/typeprint.c    | 4 ++--
 gdb/value.h        | 2 --
 gdb/varobj.c       | 2 +-
 8 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index dad15df5b99..a742c3c25e3 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -494,7 +494,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
 	    }
 
 	  if (expr != NULL && expr->first_opcode () == OP_TYPE)
-	    type = evaluate_type (expr.get ())->type ();
+	    type = expr->evaluate_type ()->type ();
 
 	  args.emplace_back (type, std::move (type_str), std::move (expr));
 	}
diff --git a/gdb/eval.c b/gdb/eval.c
index a8d5c78e09e..a75bddcadfd 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -116,15 +116,6 @@ expression::evaluate (struct type *expect_type, enum noside noside)
   return retval;
 }
 
-/* Evaluate an expression, avoiding all memory references
-   and getting a value whose type alone is correct.  */
-
-struct value *
-evaluate_type (struct expression *exp)
-{
-  return exp->evaluate (nullptr, EVAL_AVOID_SIDE_EFFECTS);
-}
-
 /* Find the current value of a watchpoint on EXP.  Return the value in
    *VALP and *RESULTP and the chain of intermediate and final values
    in *VAL_CHAIN.  RESULTP and VAL_CHAIN may be NULL if the caller does
diff --git a/gdb/expression.h b/gdb/expression.h
index 5bfb051a393..e6e4bec2e80 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -225,6 +225,11 @@ struct expression
   struct value *evaluate (struct type *expect_type = nullptr,
 			  enum noside noside = EVAL_NORMAL);
 
+  /* Evaluate an expression, avoiding all memory references
+     and getting a value whose type alone is correct.  */
+  struct value *evaluate_type ()
+  { return evaluate (nullptr, EVAL_AVOID_SIDE_EFFECTS); }
+
   /* Language it was entered in.  */
   const struct language_defn *language_defn;
   /* Architecture it was parsed in.  */
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 12bc5b09a93..687bf7e89bd 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -1211,7 +1211,7 @@ gnuv3_get_type_from_type_info (struct value *type_info_ptr)
      internal form to reconstruct the type somehow.  */
   std::string type_name = gnuv3_get_typename_from_type_info (type_info_ptr);
   expression_up expr (parse_expression (type_name.c_str ()));
-  struct value *type_val = evaluate_type (expr.get ());
+  struct value *type_val = expr->evaluate_type ();
   return type_val->type ();
 }
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 683d36d93d5..19cdf47a283 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2466,7 +2466,7 @@ print_variable_or_computed (const char *expression, enum print_values values)
   expression_up expr = parse_expression (expression);
 
   if (values == PRINT_SIMPLE_VALUES)
-    val = evaluate_type (expr.get ());
+    val = expr->evaluate_type ();
   else
     val = expr->evaluate ();
 
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index 06ecd2c4776..6dbd44eb4cd 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -511,7 +511,7 @@ whatis_exp (const char *exp, int show)
 	 "whatis" prints the type of the expression without stripping
 	 any typedef level.  "ptype" always strips all levels of
 	 typedefs.  */
-      val = evaluate_type (expr.get ());
+      val = expr->evaluate_type ();
       type = val->type ();
 
       if (show == -1 && expr->first_opcode () == OP_TYPE)
@@ -708,7 +708,7 @@ maintenance_print_type (const char *type_name, int from_tty)
   if (type_name != NULL)
     {
       expression_up expr = parse_expression (type_name);
-      struct value *val = evaluate_type (expr.get ());
+      struct value *val = expr->evaluate_type ();
       struct type *type = val->type ();
 
       if (type != nullptr)
diff --git a/gdb/value.h b/gdb/value.h
index f6092aba5a4..d042d816409 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1299,8 +1299,6 @@ extern int using_struct_return (struct gdbarch *gdbarch,
 				struct value *function,
 				struct type *value_type);
 
-extern struct value *evaluate_type (struct expression *exp);
-
 extern value *evaluate_var_value (enum noside noside, const block *blk,
 				  symbol *var);
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 75b4d44b109..dd39784a21c 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -375,7 +375,7 @@ varobj_create (const char *objname,
 	{
 	  /* Error getting the value.  Try to at least get the
 	     right type.  */
-	  struct value *type_only_value = evaluate_type (var->root->exp.get ());
+	  struct value *type_only_value = var->root->exp->evaluate_type ();
 
 	  var->type = type_only_value->type ();
 	}

-- 
2.39.1

Re: [PATCH 0/3] Minor expression cleanups

[Keith Seitz]

On 4/28/23 09:02, Tom Tromey via Gdb-patches wrote:
> While looking at some parser-related changes for DAP, I found a few
> things that could be cleaned up independently.
> 
> Regression tested on x86-64 Fedora 36.

[Very] Nice cleanups. I've looked over the series, and it LGTM.

Keith

> ---
> Tom Tromey (3):
>        Remove op_name
>        Remove evaluate_expression
>        Remove evaluate_type
> 
>   gdb/ada-lang.c        |  4 ++--
>   gdb/arc-tdep.c        |  2 +-
>   gdb/breakpoint.c      |  2 +-
>   gdb/cli/cli-script.c  |  4 ++--
>   gdb/darwin-nat-info.c |  2 +-
>   gdb/dtrace-probe.c    |  4 ++--
>   gdb/eval.c            | 25 ++++---------------------
>   gdb/expprint.c        | 41 +++++++++++++++++------------------------
>   gdb/expression.h      | 12 +++++++-----
>   gdb/gnu-v3-abi.c      |  2 +-
>   gdb/linux-thread-db.c |  2 +-
>   gdb/mi/mi-main.c      |  6 +++---
>   gdb/objc-lang.c       |  4 +---
>   gdb/printcmd.c        | 12 ++++++------
>   gdb/stack.c           |  2 +-
>   gdb/stap-probe.c      |  2 +-
>   gdb/tracepoint.c      |  2 +-
>   gdb/typeprint.c       |  4 ++--
>   gdb/value.c           |  2 +-
>   gdb/value.h           | 11 -----------
>   gdb/varobj.c          | 14 +++++++-------
>   21 files changed, 62 insertions(+), 97 deletions(-)
> ---
> base-commit: 1f7f972f59470104734ec705c985ab319eb580db
> change-id: 20230428-minor-expr-cleanuups-77d8f8d15594
> 
> Best regards,

Re: [PATCH 0/3] Minor expression cleanups

[Tom Tromey]

>> While looking at some parser-related changes for DAP, I found a few
>> things that could be cleaned up independently.
>> Regression tested on x86-64 Fedora 36.

Keith> [Very] Nice cleanups. I've looked over the series, and it LGTM.

Thanks, I'm going to check this in.

Tom