RFA: PR 2409

[Tom Tromey <tromey at redhat dot com>]

This patch fixes PR 2409.

The bug is that gdb does not accept 'void (**) (void)' as a type.

To fix this I made the c-exp.y grammar look a bit more like C grammar
in GCC.  I took the name of the new production from g++.

While doing this I found it difficult to correctly build qualified
pointer types.  Previously we handled this by repeated calls to
follow_types, so we'd only see tp_const at the top-of-stack.  This
doesn't work with the new approach, since we can't call follow_types
from the abs_decl or ptr_operator productions.

To fix this I added some new APIs to insert pointers and qualifiers at
the bottom of the stack.  These don't affect any other language.  This
is a bit strange.

While doing this I noticed a few possible cleanups.  First, objc-exp.y
is an old copy of c-exp.y that has diverged over the years.  I think
it would be better to merge it into c-exp.y.  Second, f-exp.y has its
own variant of follow_types; this could be merged -- it is the only
reason that pop_type has to be non-static.  I haven't attempted either
of these.  Finally, I noticed a few missing and/or incorrect things; I
filed PRs for these.

Built and regtested on x86-64.  A few new test cases included.
Ok?

Tom

:ADDPATCH expressions:

2008-08-20  Tom Tromey  <tromey@redhat.com>

	PR gdb/2409:
	* parser-defs.h (insert_type, insert_type_address_space): Declare.
	(push_type_address_space): Remove.
	* parse.c (insert_into_type_stack): New function.
	(insert_type): Likewise.
	(insert_type_address_space): Rename from push_type_address_space.
	Insert tp_space_identifier.
	* c-exp.y (ptr_operator): New production.
	(abs_decl): Use ptr_operator.
	(space_identifier): Call insert_type_address_space.
	(ptype): Don't use const_or_volatile_or_space_identifier.
	(const_or_volatile_noopt): Call insert_type.

2008-08-20  Tom Tromey  <tromey@redhat.com>

	* gdb.base/whatis.exp: Add tests.

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index bd04dc2..79034c8 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -162,7 +162,7 @@ static int parse_number (char *, int, int, YYSTYPE *);
 /* %type <bval> block */
 
 /* Fancy type parsing.  */
-%type <voidval> func_mod direct_abs_decl abs_decl
+%type <voidval> func_mod direct_abs_decl abs_decl ptr_operator
 %type <tval> ptype
 %type <lval> array_mod
 
@@ -771,9 +771,7 @@ variable:	name_not_typename
 	;
 
 space_identifier : '@' NAME
-		{ push_type_address_space (copy_name ($2.stoken));
-		  push_type (tp_space_identifier);
-		}
+		{ insert_type_address_space (copy_name ($2.stoken)); }
 	;
 
 const_or_volatile: const_or_volatile_noopt
@@ -792,14 +790,23 @@ const_or_volatile_or_space_identifier:
 	|
 	;
 
-abs_decl:	'*'
-			{ push_type (tp_pointer); $$ = 0; }
-	|	'*' abs_decl
-			{ push_type (tp_pointer); $$ = $2; }
+ptr_operator:
+		ptr_operator '*'
+			{ insert_type (tp_pointer); }
+		const_or_volatile_or_space_identifier
+			{ $$ = 0; }
+	|	'*' 
+			{ insert_type (tp_pointer); }
+		const_or_volatile_or_space_identifier
+			{ $$ = 0; }
 	|	'&'
-			{ push_type (tp_reference); $$ = 0; }
-	|	'&' abs_decl
-			{ push_type (tp_reference); $$ = $2; }
+			{ insert_type (tp_reference); $$ = 0; }
+	|	'&' ptr_operator
+			{ insert_type (tp_reference); $$ = 0; }
+	;
+
+abs_decl:	ptr_operator direct_abs_decl
+	|	ptr_operator
 	|	direct_abs_decl
 	;
 
@@ -1044,7 +1051,7 @@ nonempty_typelist
 	;
 
 ptype	:	typebase
-	|	ptype const_or_volatile_or_space_identifier abs_decl const_or_volatile_or_space_identifier
+	|	ptype abs_decl
 		{ $$ = follow_types ($1); }
 	;
 
@@ -1053,13 +1060,13 @@ const_and_volatile: 	CONST_KEYWORD VOLATILE_KEYWORD
 	;
 
 const_or_volatile_noopt:  	const_and_volatile 
-			{ push_type (tp_const);
-			  push_type (tp_volatile); 
+			{ insert_type (tp_const);
+			  insert_type (tp_volatile); 
 			}
 	| 		CONST_KEYWORD
-			{ push_type (tp_const); }
+			{ insert_type (tp_const); }
 	| 		VOLATILE_KEYWORD
-			{ push_type (tp_volatile); }
+			{ insert_type (tp_volatile); }
 	;
 
 name	:	NAME { $$ = $1.stoken; }
diff --git a/gdb/parse.c b/gdb/parse.c
index b587ec3..c4bbf71 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -1153,6 +1153,49 @@ check_type_stack_depth (void)
     }
 }
 
+/* A helper function for insert_type and insert_type_address_space.
+   This does work of expanding the type stack and inserting the new
+   element, ELEMENT, into the stack at location SLOT.  */
+
+static void
+insert_into_type_stack (int slot, union type_stack_elt element)
+{
+  check_type_stack_depth ();
+
+  if (slot < type_stack_depth)
+    memmove (&type_stack[slot + 1], &type_stack[slot],
+	     (type_stack_depth - slot) * sizeof (union type_stack_elt));
+  type_stack[slot] = element;
+  ++type_stack_depth;
+}
+
+/* Insert a new type, TP, at the bottom of the type stack.  If TP is
+   tp_pointer or tp_reference, it is inserted at the bottom.  If TP is
+   a qualifier, it is inserted at slot 1 (just above a previous
+   tp_pointer) if there is anything on the stack, or simply pushed if
+   the stack is empty.  Other values for TP are invalid.  */
+
+void
+insert_type (enum type_pieces tp)
+{
+  union type_stack_elt element;
+  int slot;
+
+  gdb_assert (tp == tp_pointer || tp == tp_reference
+	      || tp == tp_const || tp == tp_volatile);
+
+  /* If there is anything on the stack (we know it will be a
+     tp_pointer), insert the qualifier above it.  Otherwise, simply
+     push this on the top of the stack.  */
+  if (type_stack_depth && (tp == tp_const || tp == tp_volatile))
+    slot = 1;
+  else
+    slot = 0;
+
+  element.piece = tp;
+  insert_into_type_stack (slot, element);
+}
+
 void
 push_type (enum type_pieces tp)
 {
@@ -1167,10 +1210,32 @@ push_type_int (int n)
   type_stack[type_stack_depth++].int_val = n;
 }
 
+/* Insert a tp_space_identifier and the corresponding address space
+   value into the stack.  STRING is the name of an address space, as
+   recognized by address_space_name_to_int.  If the stack is empty,
+   the new elements are simply pushed.  If the stack is not empty,
+   this function assumes that the first item on the stack is a
+   tp_pointer, and the new values are inserted above the first
+   item.  */
+
 void
-push_type_address_space (char *string)
+insert_type_address_space (char *string)
 {
-  push_type_int (address_space_name_to_int (string));
+  union type_stack_elt element;
+  int slot;
+
+  /* If there is anything on the stack (we know it will be a
+     tp_pointer), insert the address space qualifier above it.
+     Otherwise, simply push this on the top of the stack.  */
+  if (type_stack_depth)
+    slot = 1;
+  else
+    slot = 0;
+
+  element.piece = tp_space_identifier;
+  insert_into_type_stack (slot, element);
+  element.int_val = address_space_name_to_int (string);
+  insert_into_type_stack (slot, element);
 }
 
 enum type_pieces
diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h
index f49ff9e..b1380a3 100644
--- a/gdb/parser-defs.h
+++ b/gdb/parser-defs.h
@@ -148,11 +148,13 @@ extern int end_arglist (void);
 
 extern char *copy_name (struct stoken);
 
+extern void insert_type (enum type_pieces);
+
 extern void push_type (enum type_pieces);
 
 extern void push_type_int (int);
 
-extern void push_type_address_space (char *);
+extern void insert_type_address_space (char *);
 
 extern enum type_pieces pop_type (void);
 
diff --git a/gdb/testsuite/gdb.base/whatis.exp b/gdb/testsuite/gdb.base/whatis.exp
index d480995..ec75d3f 100644
--- a/gdb/testsuite/gdb.base/whatis.exp
+++ b/gdb/testsuite/gdb.base/whatis.exp
@@ -471,3 +471,17 @@ gdb_test "whatis char_addr" \
 gdb_test "whatis a_char_addr" \
     "type = char_addr" \
     "whatis applied to variable defined by typedef"
+
+# Regression tests for PR 2409.
+
+gdb_test "whatis void (**)()" \
+  "type = void \\(\\*\\*\\)\\(\\)" \
+  "whatis applied to pointer to pointer to function"
+
+gdb_test "whatis void (** const)()" \
+  "type = void \\(\\*\\*\\ const)\\(\\)" \
+  "whatis applied to const pointer to pointer to function"
+
+gdb_test "whatis void (* const *)()" \
+  "type = void \\(\\* const \\*\\)\\(\\)" \
+  "whatis applied to pointer to const pointer to function"