[PATCHv2,2/3] gdb: merge error handling from different expression parsers

Message ID 06889e172a22e80af44b30b1e70f1f06ecb3ba96.1704206350.git.aburgess@redhat.com
State New
Headers
Series Changes to error reporting from the expression parser |

Checks

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

Commit Message

Andrew Burgess Jan. 2, 2024, 2:43 p.m. UTC
  Many (all?) of the expression parsers implement yyerror to handle
parser errors, and all of these functions are basically identical.

This commit adds a new parser_state::parse_error() function, which
implements the common error handling code, this function can then be
called from all the different yyerror functions.

The benefit of this is that (in a future commit) I can improve the
error output, and all the expression parsers will benefit.

This commit is pure refactoring though, and so, there should be no
user visible changes after this commit.
---
 gdb/ada-exp.y     |  2 +-
 gdb/c-exp.y       |  5 +----
 gdb/d-exp.y       |  5 +----
 gdb/f-exp.y       |  5 +----
 gdb/go-exp.y      |  5 +----
 gdb/m2-exp.y      |  5 +----
 gdb/p-exp.y       |  5 +----
 gdb/parse.c       | 11 +++++++++++
 gdb/parser-defs.h |  5 +++++
 9 files changed, 23 insertions(+), 25 deletions(-)
  

Comments

Lancelot SIX Jan. 2, 2024, 3:01 p.m. UTC | #1
Hi Andrew,

Just a nit below

> diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
> index 93ebdf5c061..4d40245228b 100644
> --- a/gdb/parser-defs.h
> +++ b/gdb/parser-defs.h
> @@ -262,6 +262,11 @@ struct parser_state : public expr_builder
>      push (expr::make_operation<T> (std::move (lhs), std::move (rhs)));
>    }
>  
> +  /* Function called from various the various parsers yyerror functions to

I think the first "various" came from some previous wording and should
be dropped.

Best,
Lancelot.

> +     throw an error.  The error will include a message identifying the
> +     location of the error within the current expression.  */
> +  void parse_error (const char *msg);
> +
>    /* If this is nonzero, this block is used as the lexical context for
>       symbol names.  */
>  
> -- 
> 2.25.4
>
  
Andrew Burgess Jan. 3, 2024, 9:39 a.m. UTC | #2
Lancelot SIX <lsix@lancelotsix.com> writes:

> Hi Andrew,
>
> Just a nit below
>
>> diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
>> index 93ebdf5c061..4d40245228b 100644
>> --- a/gdb/parser-defs.h
>> +++ b/gdb/parser-defs.h
>> @@ -262,6 +262,11 @@ struct parser_state : public expr_builder
>>      push (expr::make_operation<T> (std::move (lhs), std::move (rhs)));
>>    }
>>  
>> +  /* Function called from various the various parsers yyerror functions to
>
> I think the first "various" came from some previous wording and should
> be dropped.

Fixed locally to:

  /* Function called from the various parsers' yyerror functions to throw
     an error.  The error will include a message identifying the location
     of the error within the current expression.  */
  void parse_error (const char *msg);

Thanks,
Andrew


>
> Best,
> Lancelot.
>
>> +     throw an error.  The error will include a message identifying the
>> +     location of the error within the current expression.  */
>> +  void parse_error (const char *msg);
>> +
>>    /* If this is nonzero, this block is used as the lexical context for
>>       symbol names.  */
>>  
>> -- 
>> 2.25.4
>>
  

Patch

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index fcb5aa4379b..2a1cff50887 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -1212,7 +1212,7 @@  ada_parse (struct parser_state *par_state)
 static void
 yyerror (const char *msg)
 {
-  error (_("Error in expression, near `%s'."), pstate->lexptr);
+  pstate->parse_error (msg);
 }
 
 /* Emit expression to access an instance of SYM, in block BLOCK (if
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 2b4c21850d3..6697b3b2278 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -3482,8 +3482,5 @@  c_print_token (FILE *file, int type, YYSTYPE value)
 static void
 yyerror (const char *msg)
 {
-  if (pstate->prev_lexptr)
-    pstate->lexptr = pstate->prev_lexptr;
-
-  error (_("A %s in expression, near `%s'."), msg, pstate->lexptr);
+  pstate->parse_error (msg);
 }
diff --git a/gdb/d-exp.y b/gdb/d-exp.y
index e2507982d50..627c681d895 100644
--- a/gdb/d-exp.y
+++ b/gdb/d-exp.y
@@ -1631,9 +1631,6 @@  d_parse (struct parser_state *par_state)
 static void
 yyerror (const char *msg)
 {
-  if (pstate->prev_lexptr)
-    pstate->lexptr = pstate->prev_lexptr;
-
-  error (_("A %s in expression, near `%s'."), msg, pstate->lexptr);
+  pstate->parse_error (msg);
 }
 
diff --git a/gdb/f-exp.y b/gdb/f-exp.y
index e4e2171d641..88a95bccb11 100644
--- a/gdb/f-exp.y
+++ b/gdb/f-exp.y
@@ -1736,8 +1736,5 @@  f_language::parser (struct parser_state *par_state) const
 static void
 yyerror (const char *msg)
 {
-  if (pstate->prev_lexptr)
-    pstate->lexptr = pstate->prev_lexptr;
-
-  error (_("A %s in expression, near `%s'."), msg, pstate->lexptr);
+  pstate->parse_error (msg);
 }
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index c9b9c0b1ab7..561a3bef1b0 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -1545,8 +1545,5 @@  go_language::parser (struct parser_state *par_state) const
 static void
 yyerror (const char *msg)
 {
-  if (pstate->prev_lexptr)
-    pstate->lexptr = pstate->prev_lexptr;
-
-  error (_("A %s in expression, near `%s'."), msg, pstate->lexptr);
+  pstate->parse_error (msg);
 }
diff --git a/gdb/m2-exp.y b/gdb/m2-exp.y
index 092a8be248d..9a8767f5ac7 100644
--- a/gdb/m2-exp.y
+++ b/gdb/m2-exp.y
@@ -1006,8 +1006,5 @@  m2_language::parser (struct parser_state *par_state) const
 static void
 yyerror (const char *msg)
 {
-  if (pstate->prev_lexptr)
-    pstate->lexptr = pstate->prev_lexptr;
-
-  error (_("A %s in expression, near `%s'."), msg, pstate->lexptr);
+  pstate->parse_error (msg);
 }
diff --git a/gdb/p-exp.y b/gdb/p-exp.y
index b0f334897ad..9dfa8c5fd4f 100644
--- a/gdb/p-exp.y
+++ b/gdb/p-exp.y
@@ -1660,8 +1660,5 @@  pascal_language::parser (struct parser_state *par_state) const
 static void
 yyerror (const char *msg)
 {
-  if (pstate->prev_lexptr)
-    pstate->lexptr = pstate->prev_lexptr;
-
-  error (_("A %s in expression, near `%s'."), msg, pstate->lexptr);
+  pstate->parse_error (msg);
 }
diff --git a/gdb/parse.c b/gdb/parse.c
index b57d112fafd..efac0dee1af 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -244,6 +244,17 @@  parser_state::push_dollar (struct stoken str)
     (create_internalvar (copy.c_str () + 1));
 }
 
+/* See parser-defs.h.  */
+
+void
+parser_state::parse_error (const char *msg)
+{
+  if (this->prev_lexptr)
+    this->lexptr = this->prev_lexptr;
+
+  error (_("A %s in expression, near `%s'."), msg, this->lexptr);
+}
+
 
 
 const char *
diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
index 93ebdf5c061..4d40245228b 100644
--- a/gdb/parser-defs.h
+++ b/gdb/parser-defs.h
@@ -262,6 +262,11 @@  struct parser_state : public expr_builder
     push (expr::make_operation<T> (std::move (lhs), std::move (rhs)));
   }
 
+  /* Function called from various the various parsers yyerror functions to
+     throw an error.  The error will include a message identifying the
+     location of the error within the current expression.  */
+  void parse_error (const char *msg);
+
   /* If this is nonzero, this block is used as the lexical context for
      symbol names.  */