public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Fix Ada qualification bug
@ 2021-07-16 19:17 Tom Tromey
  2021-07-16 19:17 ` [PATCH 1/5] Remove add_symbols_from_enclosing_procs Tom Tromey
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Tom Tromey @ 2021-07-16 19:17 UTC (permalink / raw)
  To: gdb-patches

This series fixes an Ada qualification bug, letting gdb automatically
disambiguate certain expressions without requiring manual
intervention.  This fix is the last patch; the rest are minor
refactorings that presented themselves along the way.

Regression tested on x86-64 Fedora 32.

Let me know what you think.

Tom



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] Remove add_symbols_from_enclosing_procs
  2021-07-16 19:17 [PATCH 0/5] Fix Ada qualification bug Tom Tromey
@ 2021-07-16 19:17 ` Tom Tromey
  2021-07-16 19:17 ` [PATCH 2/5] Refactor Ada resolution Tom Tromey
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-07-16 19:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I noticed that add_symbols_from_enclosing_procs is empty, and can be
removed.  The one caller, ada_add_local_symbols, can also be
simplified, removing some code that, I think, was an incorrect attempt
to handle nested functions.
---
 gdb/ada-lang.c | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index b098991612d..f45384a68cb 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4522,19 +4522,6 @@ ada_lookup_simple_minsym (const char *name)
   return result;
 }
 
-/* For all subprograms that statically enclose the subprogram of the
-   selected frame, add symbols matching identifier NAME in DOMAIN
-   and their blocks to the list of data in RESULT, as for
-   ada_add_block_symbols (q.v.).   If WILD_MATCH_P, treat as NAME
-   with a wildcard prefix.  */
-
-static void
-add_symbols_from_enclosing_procs (std::vector<struct block_symbol> &result,
-				  const lookup_name_info &lookup_name,
-				  domain_enum domain)
-{
-}
-
 /* True if TYPE is definitely an artificial type supplied to a symbol
    for which no debugging information was given in the symbol file.  */
 
@@ -4937,25 +4924,17 @@ remove_irrelevant_renamings (std::vector<struct block_symbol> *syms,
 }
 
 /* Add to RESULT all symbols from BLOCK (and its super-blocks)
-   whose name and domain match NAME and DOMAIN respectively.
-   If no match was found, then extend the search to "enclosing"
-   routines (in other words, if we're inside a nested function,
-   search the symbols defined inside the enclosing functions).
-   If WILD_MATCH_P is nonzero, perform the naming matching in
-   "wild" mode (see function "wild_match" for more info).
+   whose name and domain match LOOKUP_NAME and DOMAIN respectively.
 
-   Note: This function assumes that RESULT has 0 (zero) element in it.  */
+   Note: This function assumes that RESULT is empty.  */
 
 static void
 ada_add_local_symbols (std::vector<struct block_symbol> &result,
 		       const lookup_name_info &lookup_name,
 		       const struct block *block, domain_enum domain)
 {
-  int block_depth = 0;
-
   while (block != NULL)
     {
-      block_depth += 1;
       ada_add_block_symbols (result, block, lookup_name, domain, NULL);
 
       /* If we found a non-function match, assume that's the one.  */
@@ -4964,11 +4943,6 @@ ada_add_local_symbols (std::vector<struct block_symbol> &result,
 
       block = BLOCK_SUPERBLOCK (block);
     }
-
-  /* If no luck so far, try to find NAME as a local symbol in some lexically
-     enclosing subprogram.  */
-  if (result.empty () && block_depth > 2)
-    add_symbols_from_enclosing_procs (result, lookup_name, domain);
 }
 
 /* An object of this type is used as the callback argument when
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/5] Refactor Ada resolution
  2021-07-16 19:17 [PATCH 0/5] Fix Ada qualification bug Tom Tromey
  2021-07-16 19:17 ` [PATCH 1/5] Remove add_symbols_from_enclosing_procs Tom Tromey
@ 2021-07-16 19:17 ` Tom Tromey
  2021-07-16 19:17 ` [PATCH 3/5] Defer Ada character literal resolution Tom Tromey
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-07-16 19:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

In a subsequent patch, it will be convenient if an Ada expression
operation can supply its own replacement object.  This patch refactors
Ada expression resolution to make this possible.
---
 gdb/ada-exp.h  | 18 ++++++++++++++++++
 gdb/ada-exp.y  | 17 +++++++----------
 gdb/ada-lang.c | 17 +++++++++++++++++
 3 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/gdb/ada-exp.h b/gdb/ada-exp.h
index 598dc7072ad..e600ec224e5 100644
--- a/gdb/ada-exp.h
+++ b/gdb/ada-exp.h
@@ -95,6 +95,24 @@ struct ada_resolvable
 			bool parse_completion,
 			innermost_block_tracker *tracker,
 			struct type *context_type) = 0;
+
+  /* Possibly replace this object with some other expression object.
+     This is like 'resolve', but can return a replacement.
+
+     The default implementation calls 'resolve' and wraps this object
+     in a function call if that call returns true.  OWNER is a
+     reference to the unique pointer that owns the 'this'; it can be
+     'move'd from to construct the replacement.
+
+     This should either return a new object, or OWNER -- never
+     nullptr.  */
+
+  virtual operation_up replace (operation_up &&owner,
+				struct expression *exp,
+				bool deprocedure_p,
+				bool parse_completion,
+				innermost_block_tracker *tracker,
+				struct type *context_type);
 };
 
 /* In Ada, some generic operations must be wrapped with a handler that
diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index 5b6aca91153..afa085ec50f 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -119,16 +119,13 @@ resolve (operation_up &&op, bool deprocedure_p, struct type *context_type)
 {
   operation_up result = std::move (op);
   ada_resolvable *res = dynamic_cast<ada_resolvable *> (result.get ());
-  if (res != nullptr
-      && res->resolve (pstate->expout.get (),
-		       deprocedure_p,
-		       pstate->parse_completion,
-		       pstate->block_tracker,
-		       context_type))
-    result
-      = make_operation<ada_funcall_operation> (std::move (result),
-					       std::vector<operation_up> ());
-
+  if (res != nullptr)
+    return res->replace (std::move (result),
+			 pstate->expout.get (),
+			 deprocedure_p,
+			 pstate->parse_completion,
+			 pstate->block_tracker,
+			 context_type);
   return result;
 }
 
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index f45384a68cb..a435543861c 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10097,6 +10097,23 @@ ada_binop_exp (struct type *expect_type,
 namespace expr
 {
 
+/* See ada-exp.h.  */
+
+operation_up
+ada_resolvable::replace (operation_up &&owner,
+			 struct expression *exp,
+			 bool deprocedure_p,
+			 bool parse_completion,
+			 innermost_block_tracker *tracker,
+			 struct type *context_type)
+{
+  if (resolve (exp, deprocedure_p, parse_completion, tracker, context_type))
+    return (make_operation<ada_funcall_operation>
+	    (std::move (owner),
+	     std::vector<operation_up> ()));
+  return std::move (owner);
+}
+
 value *
 ada_wrapped_operation::evaluate (struct type *expect_type,
 				 struct expression *exp,
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/5] Defer Ada character literal resolution
  2021-07-16 19:17 [PATCH 0/5] Fix Ada qualification bug Tom Tromey
  2021-07-16 19:17 ` [PATCH 1/5] Remove add_symbols_from_enclosing_procs Tom Tromey
  2021-07-16 19:17 ` [PATCH 2/5] Refactor Ada resolution Tom Tromey
@ 2021-07-16 19:17 ` Tom Tromey
  2021-07-16 19:17 ` [PATCH 4/5] Remove the type_qualifier global Tom Tromey
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-07-16 19:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

In Ada, an enumeration type can use a character literal as one of the
enumerators.  The Ada expression parser handles the appropriate
conversion.

It turns out, though, that this conversion was handled incorrectly.
For an expression like TYPE'(EXP), the conversion would be done for
any such literal appearing in EXP -- but only the outermost such
expression should really be affected.

This patch defers the conversion until the resolution phase, fixing
the bug.
---
 gdb/ada-exp.h                                 | 29 +++++++++
 gdb/ada-exp.y                                 | 47 +--------------
 gdb/ada-lang.c                                | 60 +++++++++++++++++++
 gdb/testsuite/gdb.ada/char_enum_overload.exp  | 34 +++++++++++
 .../gdb.ada/char_enum_overload/foo.adb        | 22 +++++++
 .../gdb.ada/char_enum_overload/pck.adb        | 31 ++++++++++
 .../gdb.ada/char_enum_overload/pck.ads        | 25 ++++++++
 7 files changed, 204 insertions(+), 44 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/char_enum_overload.exp
 create mode 100644 gdb/testsuite/gdb.ada/char_enum_overload/foo.adb
 create mode 100644 gdb/testsuite/gdb.ada/char_enum_overload/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/char_enum_overload/pck.ads

diff --git a/gdb/ada-exp.h b/gdb/ada-exp.h
index e600ec224e5..594abe067e8 100644
--- a/gdb/ada-exp.h
+++ b/gdb/ada-exp.h
@@ -742,6 +742,35 @@ class ada_name_association : public ada_association
   operation_up m_val;
 };
 
+/* A character constant expression.  This is a separate operation so
+   that it can participate in resolution, so that TYPE'(CST) can
+   work correctly for enums with character enumerators.  */
+class ada_char_operation : public long_const_operation,
+			   public ada_resolvable
+{
+public:
+
+  using long_const_operation::long_const_operation;
+
+  bool resolve (struct expression *exp,
+		bool deprocedure_p,
+		bool parse_completion,
+		innermost_block_tracker *tracker,
+		struct type *context_type) override
+  {
+    /* This should never be called, because this class also implements
+       'replace'.  */
+    gdb_assert_not_reached ("unexpected call");
+  }
+
+  operation_up replace (operation_up &&owner,
+			struct expression *exp,
+			bool deprocedure_p,
+			bool parse_completion,
+			innermost_block_tracker *tracker,
+			struct type *context_type) override;
+};
+
 } /* namespace expr */
 
 #endif /* ADA_EXP_H */
diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index afa085ec50f..a0b8b7df8ce 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -91,8 +91,6 @@ static void write_name_assoc (struct parser_state *, struct stoken);
 
 static const struct block *block_lookup (const struct block *, const char *);
 
-static LONGEST convert_char_literal (struct type *, LONGEST);
-
 static void write_ambiguous_var (struct parser_state *,
 				 const struct block *, const char *, int);
 
@@ -869,11 +867,9 @@ primary	:	INT
 	;
 
 primary	:	CHARLIT
-		  { write_int (pstate,
-			       convert_char_literal (type_qualifier, $1.val),
-			       (type_qualifier == NULL) 
-			       ? $1.type : type_qualifier);
-		  }
+			{
+			  pstate->push_new<ada_char_operation> ($1.type, $1.val);
+			}
 	;
 
 primary	:	FLOAT
@@ -1718,43 +1714,6 @@ write_name_assoc (struct parser_state *par_state, struct stoken name)
   push_association<ada_name_association> (ada_pop ());
 }
 
-/* Convert the character literal whose ASCII value would be VAL to the
-   appropriate value of type TYPE, if there is a translation.
-   Otherwise return VAL.  Hence, in an enumeration type ('A', 'B'),
-   the literal 'A' (VAL == 65), returns 0.  */
-
-static LONGEST
-convert_char_literal (struct type *type, LONGEST val)
-{
-  char name[7];
-  int f;
-
-  if (type == NULL)
-    return val;
-  type = check_typedef (type);
-  if (type->code () != TYPE_CODE_ENUM)
-    return val;
-
-  if ((val >= 'a' && val <= 'z') || (val >= '0' && val <= '9'))
-    xsnprintf (name, sizeof (name), "Q%c", (int) val);
-  else
-    xsnprintf (name, sizeof (name), "QU%02x", (int) val);
-  size_t len = strlen (name);
-  for (f = 0; f < type->num_fields (); f += 1)
-    {
-      /* Check the suffix because an enum constant in a package will
-	 have a name like "pkg__QUxx".  This is safe enough because we
-	 already have the correct type, and because mangling means
-	 there can't be clashes.  */
-      const char *ename = TYPE_FIELD_NAME (type, f);
-      size_t elen = strlen (ename);
-
-      if (elen >= len && strcmp (name, ename + elen - len) == 0)
-	return TYPE_FIELD_ENUMVAL (type, f);
-    }
-  return val;
-}
-
 static struct type *
 type_int (struct parser_state *par_state)
 {
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index a435543861c..1038ccbb316 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10114,6 +10114,66 @@ ada_resolvable::replace (operation_up &&owner,
   return std::move (owner);
 }
 
+/* Convert the character literal whose ASCII value would be VAL to the
+   appropriate value of type TYPE, if there is a translation.
+   Otherwise return VAL.  Hence, in an enumeration type ('A', 'B'),
+   the literal 'A' (VAL == 65), returns 0.  */
+
+static LONGEST
+convert_char_literal (struct type *type, LONGEST val)
+{
+  char name[7];
+  int f;
+
+  if (type == NULL)
+    return val;
+  type = check_typedef (type);
+  if (type->code () != TYPE_CODE_ENUM)
+    return val;
+
+  if ((val >= 'a' && val <= 'z') || (val >= '0' && val <= '9'))
+    xsnprintf (name, sizeof (name), "Q%c", (int) val);
+  else
+    xsnprintf (name, sizeof (name), "QU%02x", (int) val);
+  size_t len = strlen (name);
+  for (f = 0; f < type->num_fields (); f += 1)
+    {
+      /* Check the suffix because an enum constant in a package will
+	 have a name like "pkg__QUxx".  This is safe enough because we
+	 already have the correct type, and because mangling means
+	 there can't be clashes.  */
+      const char *ename = TYPE_FIELD_NAME (type, f);
+      size_t elen = strlen (ename);
+
+      if (elen >= len && strcmp (name, ename + elen - len) == 0)
+	return TYPE_FIELD_ENUMVAL (type, f);
+    }
+  return val;
+}
+
+/* See ada-exp.h.  */
+
+operation_up
+ada_char_operation::replace (operation_up &&owner,
+			     struct expression *exp,
+			     bool deprocedure_p,
+			     bool parse_completion,
+			     innermost_block_tracker *tracker,
+			     struct type *context_type)
+{
+  operation_up result = std::move (owner);
+
+  if (context_type != nullptr && context_type->code () == TYPE_CODE_ENUM)
+    {
+      gdb_assert (result.get () == this);
+      std::get<0> (m_storage) = context_type;
+      std::get<1> (m_storage)
+	= convert_char_literal (context_type, std::get<1> (m_storage));
+    }
+
+  return make_operation<ada_wrapped_operation> (std::move (result));
+}
+
 value *
 ada_wrapped_operation::evaluate (struct type *expect_type,
 				 struct expression *exp,
diff --git a/gdb/testsuite/gdb.ada/char_enum_overload.exp b/gdb/testsuite/gdb.ada/char_enum_overload.exp
new file mode 100644
index 00000000000..2cd93f82854
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/char_enum_overload.exp
@@ -0,0 +1,34 @@
+# Copyright 2021 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile foo
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+gdb_test "print pck.Global_Enum_Type'(Overloaded('+'))" "= 1 'Y'" \
+    "call correct overload"
+gdb_test "print pck.Global_Enum_Type'('+')" " = 2 '\\+'" \
+    "use enum constant"
diff --git a/gdb/testsuite/gdb.ada/char_enum_overload/foo.adb b/gdb/testsuite/gdb.ada/char_enum_overload/foo.adb
new file mode 100644
index 00000000000..ee0a0aa4f0e
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/char_enum_overload/foo.adb
@@ -0,0 +1,22 @@
+--  Copyright 2021 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   Gchar : Global_Enum_Type := Global_Enum_Type'(Overloaded('+'));
+begin
+   Do_Nothing (Gchar'Address);  -- STOP
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/char_enum_overload/pck.adb b/gdb/testsuite/gdb.ada/char_enum_overload/pck.adb
new file mode 100644
index 00000000000..6aba048fd8f
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/char_enum_overload/pck.adb
@@ -0,0 +1,31 @@
+--  Copyright 2021 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+   procedure Overloaded (Value : Global_Enum_Type) is
+   begin
+      null;
+   end Overloaded;
+
+   function Overloaded (Value : Character) return Global_Enum_Type is
+   begin
+      return 'Y';
+   end Overloaded;
+
+   procedure Do_Nothing (A : System.Address) is
+   begin
+      null;
+   end Do_Nothing;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/char_enum_overload/pck.ads b/gdb/testsuite/gdb.ada/char_enum_overload/pck.ads
new file mode 100644
index 00000000000..7dc478f774b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/char_enum_overload/pck.ads
@@ -0,0 +1,25 @@
+--  Copyright 2021 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with System;
+
+package Pck is
+   type Global_Enum_Type is ('x', 'Y', '+');
+
+   procedure Overloaded (Value : Global_Enum_Type);
+   function Overloaded (Value : Character) return Global_Enum_Type;
+
+   procedure Do_Nothing (A : System.Address);
+end Pck;
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/5] Remove the type_qualifier global
  2021-07-16 19:17 [PATCH 0/5] Fix Ada qualification bug Tom Tromey
                   ` (2 preceding siblings ...)
  2021-07-16 19:17 ` [PATCH 3/5] Defer Ada character literal resolution Tom Tromey
@ 2021-07-16 19:17 ` Tom Tromey
  2021-07-16 19:17 ` [PATCH 5/5] Handle type qualifier for enumeration name Tom Tromey
  2021-08-02 16:22 ` [PATCH 0/5] Fix Ada qualification bug Tom Tromey
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-07-16 19:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The type_qualifier global is no longer needed in the Ada expression
parser, so this removes it.
---
 gdb/ada-exp.y | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index a0b8b7df8ce..66d58b06cb5 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -68,10 +68,6 @@ struct name_info {
 
 static struct parser_state *pstate = NULL;
 
-/* If expression is in the context of TYPE'(...), then TYPE, else
- * NULL.  */
-static struct type *type_qualifier;
-
 int yyparse (void);
 
 static int yylex (void);
@@ -428,8 +424,6 @@ pop_associations (int n)
 %type <bval> block
 %type <lval> arglist tick_arglist
 
-%type <tval> save_qualifier
-
 %token DOT_ALL
 
 /* Special type cases, put in to allow the parser to distinguish different
@@ -516,8 +510,7 @@ primary :	primary '(' arglist ')'
 			}
 	;
 
-primary :	var_or_type '\'' save_qualifier { type_qualifier = $1; } 
-		   '(' exp ')'
+primary :	var_or_type '\'' '(' exp ')'
 			{
 			  if ($1 == NULL)
 			    error (_("Type required for qualification"));
@@ -525,13 +518,9 @@ primary :	var_or_type '\'' save_qualifier { type_qualifier = $1; }
 						      check_typedef ($1));
 			  pstate->push_new<ada_qual_operation>
 			    (std::move (arg), $1);
-			  type_qualifier = $3;
 			}
 	;
 
-save_qualifier : 	{ $$ = type_qualifier; }
-	;
-
 primary :
 		primary '(' simple_exp DOTDOT simple_exp ')'
 			{ ada_wrap3<ada_ternop_slice_operation> (); }
@@ -1093,7 +1082,6 @@ ada_parse (struct parser_state *par_state)
   pstate = par_state;
 
   lexer_init (yyin);		/* (Re-)initialize lexer.  */
-  type_qualifier = NULL;
   obstack_free (&temp_parse_space, NULL);
   obstack_init (&temp_parse_space);
   components.clear ();
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 5/5] Handle type qualifier for enumeration name
  2021-07-16 19:17 [PATCH 0/5] Fix Ada qualification bug Tom Tromey
                   ` (3 preceding siblings ...)
  2021-07-16 19:17 ` [PATCH 4/5] Remove the type_qualifier global Tom Tromey
@ 2021-07-16 19:17 ` Tom Tromey
  2021-08-02 16:22 ` [PATCH 0/5] Fix Ada qualification bug Tom Tromey
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-07-16 19:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Pierre-Marie noticed that the Ada expression "TYPE'(NAME)" resolved
incorrectly when "TYPE" was an enumeration type.  Here, "NAME" should
be unambiguous.

This patch fixes this problem.  Note that the patch is not perfect --
it does not give an error if TYPE is an enumeration type but NAME is
not an enumerator but does have some other meaning in scope.  Fixing
this proved difficult, and so I've left it out.
---
 gdb/ada-lang.c                            | 33 +++++++++++++++++++++--
 gdb/testsuite/gdb.ada/enum_qual.exp       | 32 ++++++++++++++++++++++
 gdb/testsuite/gdb.ada/enum_qual/gener.ads | 22 +++++++++++++++
 gdb/testsuite/gdb.ada/enum_qual/qual.adb  | 27 +++++++++++++++++++
 4 files changed, 112 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/enum_qual.exp
 create mode 100644 gdb/testsuite/gdb.ada/enum_qual/gener.ads
 create mode 100644 gdb/testsuite/gdb.ada/enum_qual/qual.adb

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 1038ccbb316..caf8780fe7c 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -3425,6 +3425,29 @@ ada_resolve_funcall (struct symbol *sym, const struct block *block,
   return candidates[i];
 }
 
+/* Resolve a mention of a name where the context type is an
+   enumeration type.  */
+
+static int
+ada_resolve_enum (std::vector<struct block_symbol> &syms,
+		  const char *name, struct type *context_type,
+		  bool parse_completion)
+{
+  gdb_assert (context_type->code () == TYPE_CODE_ENUM);
+  context_type = ada_check_typedef (context_type);
+
+  for (int i = 0; i < syms.size (); ++i)
+    {
+      /* We already know the name matches, so we're just looking for
+	 an element of the correct enum type.  */
+      if (ada_check_typedef (SYMBOL_TYPE (syms[i].symbol)) == context_type)
+	return i;
+    }
+
+  error (_("No name '%s' in enumeration type '%s'"), name,
+	 ada_type_name (context_type));
+}
+
 /* See ada-lang.h.  */
 
 block_symbol
@@ -3474,6 +3497,10 @@ ada_resolve_variable (struct symbol *sym, const struct block *block,
     error (_("No definition found for %s"), sym->print_name ());
   else if (candidates.size () == 1)
     i = 0;
+  else if (context_type != nullptr
+	   && context_type->code () == TYPE_CODE_ENUM)
+    i = ada_resolve_enum (candidates, sym->linkage_name (), context_type,
+			  parse_completion);
   else if (deprocedure_p && !is_nonfunction (candidates))
     {
       i = ada_resolve_function
@@ -4937,8 +4964,10 @@ ada_add_local_symbols (std::vector<struct block_symbol> &result,
     {
       ada_add_block_symbols (result, block, lookup_name, domain, NULL);
 
-      /* If we found a non-function match, assume that's the one.  */
-      if (is_nonfunction (result))
+      /* If we found a non-function match, assume that's the one.  We
+	 only check this when finding a function boundary, so that we
+	 can accumulate all results from intervening blocks first.  */
+      if (BLOCK_FUNCTION (block) != nullptr && is_nonfunction (result))
 	return;
 
       block = BLOCK_SUPERBLOCK (block);
diff --git a/gdb/testsuite/gdb.ada/enum_qual.exp b/gdb/testsuite/gdb.ada/enum_qual.exp
new file mode 100644
index 00000000000..783574a6dcf
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/enum_qual.exp
@@ -0,0 +1,32 @@
+# Copyright 2021 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+standard_ada_testfile qual
+
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable debug] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/qual.adb]
+runto "qual.adb:$bp_location"
+
+gdb_test "print kind'(no_element)" " = no_element" \
+    "print qualified no_element"
diff --git a/gdb/testsuite/gdb.ada/enum_qual/gener.ads b/gdb/testsuite/gdb.ada/enum_qual/gener.ads
new file mode 100644
index 00000000000..882185a3fa8
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/enum_qual/gener.ads
@@ -0,0 +1,22 @@
+--  Copyright 2021 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+generic
+  type Component_T is private;
+package Gener is
+
+  No_Element : Component_T;
+
+end Gener;
diff --git a/gdb/testsuite/gdb.ada/enum_qual/qual.adb b/gdb/testsuite/gdb.ada/enum_qual/qual.adb
new file mode 100644
index 00000000000..a6eff245e40
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/enum_qual/qual.adb
@@ -0,0 +1,27 @@
+--  Copyright 2021 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Gener;
+
+procedure Qual is
+
+   package P is new Gener (Integer);
+
+   type Kind is (Present, No_Element);
+   K : Kind := No_Element;
+
+begin
+   null;  -- STOP
+end Qual;
-- 
2.26.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/5] Fix Ada qualification bug
  2021-07-16 19:17 [PATCH 0/5] Fix Ada qualification bug Tom Tromey
                   ` (4 preceding siblings ...)
  2021-07-16 19:17 ` [PATCH 5/5] Handle type qualifier for enumeration name Tom Tromey
@ 2021-08-02 16:22 ` Tom Tromey
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-08-02 16:22 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> This series fixes an Ada qualification bug, letting gdb automatically
Tom> disambiguate certain expressions without requiring manual
Tom> intervention.  This fix is the last patch; the rest are minor
Tom> refactorings that presented themselves along the way.

Tom> Regression tested on x86-64 Fedora 32.

I'm checking these in.

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-02 16:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-16 19:17 [PATCH 0/5] Fix Ada qualification bug Tom Tromey
2021-07-16 19:17 ` [PATCH 1/5] Remove add_symbols_from_enclosing_procs Tom Tromey
2021-07-16 19:17 ` [PATCH 2/5] Refactor Ada resolution Tom Tromey
2021-07-16 19:17 ` [PATCH 3/5] Defer Ada character literal resolution Tom Tromey
2021-07-16 19:17 ` [PATCH 4/5] Remove the type_qualifier global Tom Tromey
2021-07-16 19:17 ` [PATCH 5/5] Handle type qualifier for enumeration name Tom Tromey
2021-08-02 16:22 ` [PATCH 0/5] Fix Ada qualification bug Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).