[rfa] fix for PR c++/1553

[David Carlton <carlton at kealia dot com>]

Here's a fix for PR c++/1553.  It cleans up a botch I made in
dwarf2read.c:read_structure_scope: I had all this conditional logic
for determining the name, including a case where the code basically
says "well, it would be a little bit of work to get the name right
now, so let's remember that we got it wrong and try to fix the
situation later".  And, of course, not only is this fragile and a
maintenance problem, it proved to be buggy - I got abstract
declarations of nested classes wrong, basically.

So here's a patch: it splits the logic for figuring out the class name
into a separate function, and that separate function should, I hope,
actually be correct. :-) The submitter has reported that it fixes his
problems.

I've included some tests for the problems in question; they're a
little bit compiler dependent, but I did verify that they failed with
GCC 3.2 before the patch and succeed with the patch.  I also verified
that they succeed with the patch with a GCC 3.4 snapshot and with GCC
2.95.3 -gdwarf-2.

This patch also points out an issue with local.exp - that test does
'ptype NestedInnerLocal' in a situation where NestedInnerLocal
shouldn't be visible.  I'm open to suggestions as to what to do here -
the patch that I'm submitting allows the new behavior as a pass
without changing what we do with the old behavior, but we could also
fail the old behavior or delete the test entirely.  Anybody else have
an opinion on the issue?

And, before Michael asks, I didn't add a copyright to pr-1553.cc
because some of the code is derived from the bug report.

OK to commit?  (It should go in 6.1, too, if you don't get around to
reviewing it before the branch.)

David Carlton
carlton@kealia.com

2004-02-24  David Carlton  <carlton@kealia.com>

	* dwarf2read.c (read_structure_scope): Determine type name by
	calling determine_class_name.
	(determine_class_name): New.

2004-02-24  David Carlton  <carlton@kealia.com>

	* gdb.cp/pr-1553.exp: New.  Tests for PR c++/1553.
	* gdb.cp/pr-1553.cc: Ditto.
	* gdb.cp/local.exp (ptype NestedInnerLocal): Add comment, third
	pass branch.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.135
diff -u -p -c -r1.135 dwarf2read.c
*** dwarf2read.c	21 Feb 2004 02:13:35 -0000	1.135
--- dwarf2read.c	24 Feb 2004 17:14:02 -0000
*************** static void dwarf2_attach_fn_fields_to_t
*** 806,811 ****
--- 806,813 ----
  
  static void read_structure_scope (struct die_info *, struct dwarf2_cu *);
  
+ static char *determine_class_name (struct die_info *die, struct dwarf2_cu *cu);
+ 
  static void read_common_block (struct die_info *, struct dwarf2_cu *);
  
  static void read_namespace (struct die_info *die, struct dwarf2_cu *);
*************** read_structure_scope (struct die_info *d
*** 3000,3012 ****
    struct objfile *objfile = cu->objfile;
    struct type *type;
    struct attribute *attr;
-   const char *name = NULL;
    const char *previous_prefix = processing_current_prefix;
    struct cleanup *back_to = NULL;
-   /* This says whether or not we want to try to update the structure's
-      name to include enclosing namespace/class information, if
-      any.  */
-   int need_to_update_name = 0;
  
    type = alloc_type (objfile);
  
--- 3002,3009 ----
*************** read_structure_scope (struct die_info *d
*** 3014,3053 ****
    attr = dwarf2_attr (die, DW_AT_name, cu);
    if (attr && DW_STRING (attr))
      {
-       name = DW_STRING (attr);
- 
        if (cu->language == language_cplus)
  	{
! 	  struct die_info *spec_die = die_specification (die, cu);
! 
! 	  if (spec_die != NULL)
! 	    {
! 	      char *specification_prefix = determine_prefix (spec_die, cu);
! 	      processing_current_prefix = specification_prefix;
! 	      back_to = make_cleanup (xfree, specification_prefix);
! 	    }
! 	}
! 
!       if (processing_has_namespace_info)
! 	{
! 	  /* FIXME: carlton/2003-11-10: This variable exists only for
! 	     const-correctness reasons.  When I tried to change
! 	     TYPE_TAG_NAME to be a const char *, I ran into a cascade
! 	     of changes which would have forced decode_line_1 to take
! 	     a const char **.  */
! 	  char *new_prefix = obconcat (&objfile->objfile_obstack,
! 				       processing_current_prefix,
! 				       processing_current_prefix[0] == '\0'
! 				       ? "" : "::",
! 				       name);
! 	  TYPE_TAG_NAME (type) = new_prefix;
  	  processing_current_prefix = new_prefix;
  	}
        else
  	{
  	  TYPE_TAG_NAME (type) = obsavestring (name, strlen (name),
  					       &objfile->objfile_obstack);
- 	  need_to_update_name = (cu->language == language_cplus);
  	}
      }
  
--- 3011,3031 ----
    attr = dwarf2_attr (die, DW_AT_name, cu);
    if (attr && DW_STRING (attr))
      {
        if (cu->language == language_cplus)
  	{
! 	  char *new_prefix = determine_class_name (die, cu);
! 	  TYPE_TAG_NAME (type) = obsavestring (new_prefix,
! 					       strlen (new_prefix),
! 					       &objfile->objfile_obstack);
! 	  back_to = make_cleanup (xfree, new_prefix);
  	  processing_current_prefix = new_prefix;
  	}
        else
  	{
+ 	  const char *name = DW_STRING (attr);
+ 
  	  TYPE_TAG_NAME (type) = obsavestring (name, strlen (name),
  					       &objfile->objfile_obstack);
  	}
      }
  
*************** read_structure_scope (struct die_info *d
*** 3108,3148 ****
  	      /* C++ member function. */
  	      process_die (child_die, cu);
  	      dwarf2_add_member_fn (&fi, child_die, type, cu);
- 	      if (need_to_update_name)
- 		{
- 		  /* The demangled names of member functions contain
- 		     information about enclosing namespaces/classes,
- 		     if any.  */
- 
- 		  /* FIXME: carlton/2003-11-10: The excessive
- 		     demangling here is a bit wasteful, as is the
- 		     memory usage for names.  */
- 
- 		  /* NOTE: carlton/2003-11-10: As commented in
- 		     add_partial_structure, the demangler sometimes
- 		     prints the type info in a different form from the
- 		     debug info.  We could solve this by using the
- 		     demangled name to get the prefix; if doing so,
- 		     however, we'd need to be careful when reading a
- 		     class that's nested inside a template class.
- 		     That would also cause problems when trying to
- 		     determine RTTI information, since we use the
- 		     demangler to determine the appropriate class
- 		     name.  */
- 		  char *actual_class_name
- 		    = class_name_from_physname (dwarf2_linkage_name
- 						(child_die, cu));
- 		  if (actual_class_name != NULL
- 		      && strcmp (actual_class_name, name) != 0)
- 		    {
- 		      TYPE_TAG_NAME (type)
- 			= obsavestring (actual_class_name,
- 					strlen (actual_class_name),
- 					&objfile->objfile_obstack);
- 		    }
- 		  xfree (actual_class_name);
- 		  need_to_update_name = 0;
- 		}
  	    }
  	  else if (child_die->tag == DW_TAG_inheritance)
  	    {
--- 3086,3091 ----
*************** read_structure_scope (struct die_info *d
*** 3222,3227 ****
--- 3165,3223 ----
    processing_current_prefix = previous_prefix;
    if (back_to != NULL)
      do_cleanups (back_to);
+ }
+ 
+ /* Determine the name of the type represented by DIE, which should be
+    a named C++ compound type.  Return the name in question; the caller
+    is responsible for xfree()'ing it.  */
+ 
+ static char *
+ determine_class_name (struct die_info *die, struct dwarf2_cu *cu)
+ {
+   struct cleanup *back_to = NULL;
+   struct die_info *spec_die = die_specification (die, cu);
+   char *new_prefix = NULL;
+ 
+   /* If this is the definition of a class that is declared by another
+      die, then processing_current_prefix may not be accurate; see
+      read_func_scope for a similar example.  */
+   if (spec_die != NULL)
+     {
+       char *specification_prefix = determine_prefix (spec_die, cu);
+       processing_current_prefix = specification_prefix;
+       back_to = make_cleanup (xfree, specification_prefix);
+     }
+ 
+   /* If we don't have namespace debug info, guess the name by trying
+      to demangle the names of members, just like we did in
+      add_partial_structure.  */
+   if (!processing_has_namespace_info)
+     {
+       struct die_info *child;
+ 
+       for (child = die->child;
+ 	   child != NULL && child->tag != 0;
+ 	   child = sibling_die (child))
+ 	{
+ 	  if (child->tag == DW_TAG_subprogram)
+ 	    {
+ 	      new_prefix = class_name_from_physname (dwarf2_linkage_name
+ 						     (child, cu));
+ 
+ 	      if (new_prefix != NULL)
+ 		break;
+ 	    }
+ 	}
+     }
+ 
+   if (new_prefix == NULL)
+     new_prefix = typename_concat (processing_current_prefix,
+ 				  dwarf2_name (die, cu));
+ 
+   if (back_to != NULL)
+     do_cleanups (back_to);
+ 
+   return new_prefix;
  }
  
  /* Given a pointer to a die which begins an enumeration, process all
--- /dev/null	2002-08-30 16:31:37.000000000 -0700
+++ pr-1553.cc	2004-02-24 09:48:02.000000000 -0800
@@ -0,0 +1,53 @@
+class A {
+public:
+  class B;
+  class C;
+};
+
+class A::B {
+  int a_b;
+
+public:
+  C* get_c(int i);
+};
+
+class A::C
+{
+  int a_c;
+};
+
+class E {
+public:
+  class F;
+};
+ 
+class E::F {
+public:
+  int e_f;
+ 
+  F& operator=(const F &other);
+};
+
+void refer_to (E::F *f) {
+  // Do nothing.
+}
+
+void refer_to (A::C **ref) {
+  // Do nothing.  But, while we're at it, force out debug info for
+  // A::B and E::F.
+
+  A::B b;
+  E::F f;
+
+  refer_to (&f);
+}
+
+int main () {
+  A::C* c_var;
+  A::B* b_var;
+  E *e_var;
+
+  // Keep around a reference so that GCC 3.4 doesn't optimize the variable
+  // away.
+  refer_to (&c_var);
+}
--- /dev/null	2002-08-30 16:31:37.000000000 -0700
+++ pr-1553.exp	2004-02-24 10:00:13.000000000 -0800
@@ -0,0 +1,62 @@
+# Copyright 2004 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# Test for PR gdb/1553.
+
+# This file is part of the gdb testsuite.
+
+set ws "\[\r\n\t \]+"
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+if { [skip_cplus_tests] } { continue }
+
+#
+# test running programs
+#
+set prms_id 0
+set bug_id 0
+
+set testfile "pr-1553"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+    gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
+}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+gdb_test "ptype c_var" "type = class A::C \{${ws}private:${ws}int a_c;${ws}\} \\*"
+
+gdb_test "ptype E::F" "type = class E::F \{${ws}public:${ws}int e_f;${ws}E::F & operator=\\(E::F const ?&\\);${ws}\}"
+
+gdb_exit
+return 0
Index: local.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/local.exp,v
retrieving revision 1.5
diff -u -p -r1.5 local.exp
--- local.exp	11 Feb 2004 14:01:25 -0000	1.5
+++ local.exp	24 Feb 2004 17:10:00 -0000
@@ -263,10 +263,20 @@ gdb_expect {
 #
 # chastain 2002-04-08
 
+# NOTE (2004-02-24, carlton): This test really is invalid -
+# 'NestedInnerLocal' shouldn't be visible, so only the third
+# expression should count as a pass.  I'm leaving in the earlier
+# passes, however, given the current problematic state of our local
+# class support, but once we fix PR gdb/482, we should delete this
+# test.
+
 send_gdb "ptype NestedInnerLocal\n"
 gdb_expect {
   -re "type = class NestedInnerLocal \{\[\r\n\t \]*public:\[\r\n\t \]*int nil;\[\r\n\t \]*int nil_foo\\(int\\);\[\r\n\t \]*\}.*$gdb_prompt $" { pass "ptype NestedInnerLocal" }
   -re "type = class NestedInnerLocal \{\[\r\n\t \]*public:\[\r\n\t \]*int nil;\[\r\n\t \]*NestedInnerLocal *& *operator *= *\\((main${sep}::|)InnerLocal::NestedInnerLocal const *&\\);\[\r\n\t \]*NestedInnerLocal\\((main${sep}::|)InnerLocal::NestedInnerLocal const *&\\);\[\r\n\t \]*NestedInnerLocal\\((void|)\\);\[\r\n\t \]*int nil_foo\\(int\\);\[\r\n\t \]*\}.*$gdb_prompt $" { pass "ptype NestedInnerLocal" }
+  -re "No symbol \"NestedInnerLocal\" in current context\.\r\n$gdb_prompt $" {
+    pass "ptype NestedInnerLocal"
+  }
   -re ".*$gdb_prompt $"   {  fail "ptype NestedInnerLocal" }
   timeout             { fail "(timeout) ptype NestedInnerLocal" }
 }