Introduce and use operation::type_p

Message ID 20240918163528.1263244-1-tromey@adacore.com
State New
Headers
Series Introduce and use operation::type_p |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Test failed

Commit Message

Tom Tromey Sept. 18, 2024, 4:35 p.m. UTC
  There's currently code in gdb that checks if an expression evaluates
to a type.  In some spots this is done by comparing the opcode against
OP_TYPE, but other spots more correctly also compare with OP_TYPEOF
and OP_DECLTYPE.

This patch cleans up this area, replacing opcode-checking with a new
method on 'operation'.

Generally, checking the opcode should be considered deprecated,
although it's unfortunately difficult to get rid of opcodes entirely.

I also took advantage of this change to turn eval_op_type into a
method, removing a bit of indirection.
---
 gdb/ada-lang.c     |  2 +-
 gdb/dtrace-probe.c |  2 +-
 gdb/eval.c         | 11 +++++++----
 gdb/expop.h        | 20 +++++++++++---------
 gdb/expression.h   | 10 ++++++++++
 gdb/typeprint.c    |  2 +-
 gdb/varobj.c       |  5 +----
 7 files changed, 32 insertions(+), 20 deletions(-)
  

Comments

Tom Tromey Oct. 1, 2024, 7:04 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> There's currently code in gdb that checks if an expression evaluates
Tom> to a type.  In some spots this is done by comparing the opcode against
Tom> OP_TYPE, but other spots more correctly also compare with OP_TYPEOF
Tom> and OP_DECLTYPE.

Tom> This patch cleans up this area, replacing opcode-checking with a new
Tom> method on 'operation'.

Tom> Generally, checking the opcode should be considered deprecated,
Tom> although it's unfortunately difficult to get rid of opcodes entirely.

Tom> I also took advantage of this change to turn eval_op_type into a
Tom> method, removing a bit of indirection.

I'm checking this in now.

Tom
  
Keith Seitz Oct. 1, 2024, 7:09 p.m. UTC | #2
On 10/1/24 12:04 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
> 
> Tom> There's currently code in gdb that checks if an expression evaluates
> Tom> to a type.  In some spots this is done by comparing the opcode against
> Tom> OP_TYPE, but other spots more correctly also compare with OP_TYPEOF
> Tom> and OP_DECLTYPE.
> 
> Tom> This patch cleans up this area, replacing opcode-checking with a new
> Tom> method on 'operation'.
> 
> Tom> Generally, checking the opcode should be considered deprecated,
> Tom> although it's unfortunately difficult to get rid of opcodes entirely.
> 
> Tom> I also took advantage of this change to turn eval_op_type into a
> Tom> method, removing a bit of indirection.
> 
> I'm checking this in now.

Coincidentally, I was just looking at this. :-)

And I think you should check it in!

FWIW,
Keith
  
Tom Tromey Oct. 1, 2024, 7:15 p.m. UTC | #3
Keith> Coincidentally, I was just looking at this. :-)

Keith> And I think you should check it in!

Thanks.  I'm re-testing it first.

Tom
  

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 0f7100dfc94..8fbe6500950 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10772,7 +10772,7 @@  ada_unop_atr_operation::evaluate (struct type *expect_type,
   struct type *type_arg = nullptr;
   value *val = nullptr;
 
-  if (std::get<0> (m_storage)->opcode () == OP_TYPE)
+  if (std::get<0> (m_storage)->type_p ())
     {
       value *tem = std::get<0> (m_storage)->evaluate (nullptr, exp,
 						      EVAL_AVOID_SIDE_EFFECTS);
diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index 0f4e1643483..ac1b1c5ef13 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -493,7 +493,7 @@  dtrace_process_dof_probe (struct objfile *objfile,
 	    {
 	    }
 
-	  if (expr != NULL && expr->first_opcode () == OP_TYPE)
+	  if (expr != NULL && expr->type_p ())
 	    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 bbf4375f1d3..3929282da76 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1872,18 +1872,21 @@  eval_op_postdec (struct type *expect_type, struct expression *exp,
     }
 }
 
-/* A helper function for OP_TYPE.  */
+namespace expr
+{
 
 struct value *
-eval_op_type (struct type *expect_type, struct expression *exp,
-	      enum noside noside, struct type *type)
+type_operation::evaluate (struct type *expect_type, struct expression *exp,
+			  enum noside noside)
 {
   if (noside == EVAL_AVOID_SIDE_EFFECTS)
-    return value::allocate (type);
+    return value::allocate (std::get<0> (m_storage));
   else
     error (_("Attempt to use a type name as an expression"));
 }
 
+}
+
 /* A helper function for BINOP_ASSIGN_MODIFY.  */
 
 struct value *
diff --git a/gdb/expop.h b/gdb/expop.h
index 2d46a9dad6a..af031f53133 100644
--- a/gdb/expop.h
+++ b/gdb/expop.h
@@ -172,9 +172,6 @@  extern struct value *eval_op_ind (struct type *expect_type,
 				  struct expression *exp,
 				  enum noside noside,
 				  struct value *arg1);
-extern struct value *eval_op_type (struct type *expect_type,
-				   struct expression *exp,
-				   enum noside noside, struct type *type);
 extern struct value *eval_op_alignof (struct type *expect_type,
 				      struct expression *exp,
 				      enum noside noside,
@@ -1560,16 +1557,16 @@  class type_operation
 
   value *evaluate (struct type *expect_type,
 		   struct expression *exp,
-		   enum noside noside) override
-  {
-    return eval_op_type (expect_type, exp, noside, std::get<0> (m_storage));
-  }
+		   enum noside noside) override;
 
   enum exp_opcode opcode () const override
   { return OP_TYPE; }
 
   bool constant_p () const override
   { return true; }
+
+  bool type_p () const override
+  { return true; }
 };
 
 /* Implement the "typeof" operation.  */
@@ -1593,6 +1590,9 @@  class typeof_operation
 
   enum exp_opcode opcode () const override
   { return OP_TYPEOF; }
+
+  bool type_p () const override
+  { return true; }
 };
 
 /* Implement 'decltype'.  */
@@ -1638,6 +1638,9 @@  class decltype_operation
 
   enum exp_opcode opcode () const override
   { return OP_DECLTYPE; }
+
+  bool type_p () const override
+  { return true; }
 };
 
 /* Implement 'typeid'.  */
@@ -1652,9 +1655,8 @@  class typeid_operation
 		   struct expression *exp,
 		   enum noside noside) override
   {
-    enum exp_opcode sub_op = std::get<0> (m_storage)->opcode ();
     enum noside sub_noside
-      = ((sub_op == OP_TYPE || sub_op == OP_DECLTYPE || sub_op == OP_TYPEOF)
+      = (std::get<0> (m_storage)->type_p ()
 	 ? EVAL_AVOID_SIDE_EFFECTS
 	 : noside);
 
diff --git a/gdb/expression.h b/gdb/expression.h
index 5bfc74c973f..2eb866f455b 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -147,6 +147,11 @@  class operation
   virtual bool uses_objfile (struct objfile *objfile) const
   { return false; }
 
+  /* Some expression nodes represent a type, not a value.  This method
+     should be overridden to return 'true' in these situations.  */
+  virtual bool type_p () const
+  { return false; }
+
   /* Generate agent expression bytecodes for this operation.  */
   void generate_ax (struct expression *exp, struct agent_expr *ax,
 		    struct axs_value *value,
@@ -215,6 +220,11 @@  struct expression
     op->dump (stream, 0);
   }
 
+  /* Call the type_p method on the outermost sub-expression of this
+     expression, and return the result.  */
+  bool type_p () const
+  { return op->type_p (); }
+
   /* Return true if this expression uses OBJFILE (and will become
      dangling when OBJFILE is unloaded), otherwise return false.
      OBJFILE must not be a separate debug info file.  */
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index 2e1c5ea81e7..274f6029a7e 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -513,7 +513,7 @@  whatis_exp (const char *exp, int show)
       val = expr->evaluate_type ();
       type = val->type ();
 
-      if (show == -1 && expr->first_opcode () == OP_TYPE)
+      if (show == -1 && expr->type_p ())
 	{
 	  /* The user expression names a type directly.  */
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 0cd0bd03b6e..04f1fc879d2 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -323,10 +323,7 @@  varobj_create (const char *objname,
 	}
 
       /* Don't allow variables to be created for types.  */
-      enum exp_opcode opcode = var->root->exp->first_opcode ();
-      if (opcode == OP_TYPE
-	  || opcode == OP_TYPEOF
-	  || opcode == OP_DECLTYPE)
+      if (var->root->exp->type_p ())
 	{
 	  gdb_printf (gdb_stderr, "Attempt to use a type name"
 		      " as an expression.\n");