C++ support improvement patch

C++ support improvement patch

[Thomas Richter]

Hi folks,

please find a patch to gdb-6.4 (release version) attached. To apply
this patch, you need to get the current release (not CVS) version
of gdb, and apply it to the "gdb" subdirectory of the project.

This patch does the following:

o) GDB finds now members within the multiple-inheritance hierarchy of
a class. That is, if "this" points to a class "Foo" inherited from
"Bar" and "Baz", and "Baz" contains a member "me", then "print me"
will succeed now. gdb 6.4 and before just returned an "unknown symbol"
here, erraneously. This is because it was confused by a "class typedef"
g++ inserted here for subclasses in the dwarf-2 symbol table, which is
now cleverly avoided. The patch then, however, needs to be clever to
resolve expressions like this->Subclass::member, which - after a bit
of work - now works nicely as it should. It seems to me that it worked
more or less by accident before (Subclass was resolved as the constructor
name, not as the class name.)

o) For TAB-expansion, it removes the ":" as word-delimiter for
readline, thus allowing to find names in namespaces. The current
behaivour here is completely unusable for C++. The patch could
be better, though, it should read template argument delimiters < >
and function argument delimiters ( ) as "quotation characters",
though it doesn't. Though it's a huge improvement compared to
previous versions where TAB expansion was just not usable for C++.

o) GDB now avoids to set multiple breakpoints to the same location.
This goes in conjunction with another bug, namely that of being unable
to find constructors of multiply-inherited classes. Without the current
patch, setting a breakpoint for a constructor results in multiple breakpoint
locations found, all of them identical, and none of them actually 
beeing breaked on. With the patch applied, GDB at least finds only one
of the locations (good), but still doesn't break there (bad). This bug
seems partially caused by g++ not emmiting mangled names for constructors.

o) GDB does no longer print the character and the value for integer values
of size one. It only prints the character. The reason for this is that
with the ddd frontend, ddd parses the output of "info breakpoints" to
set or reset breakpoints (e.g. when moving them in the sources). Without
the patch, the syntax of the output is no longer valid, and is not parsable
by gdb again.

Finally, a comment: I have been unable to resolve the "breakpoint in
constructor" bug. This is a long-standing issue that remained open for
quite a while. Most likely, this is caused by two independent bugs:
i) gdb relies on the "MIPS linkage name" of members, though according to
the g++ folks it should not,
ii) g++ may create separate constructors depending on whehter the class is
constructed by itself, or as part of a subclass. gdb seems to be unaware
of this. I've no idea how to resolve this.

I would be thankful to receive commends on the patch,

thanks,
	Thomas

Now for the patch:

/* snip */

diff -Naur gdb_old/c-exp.y gdb/c-exp.y
--- gdb_old/c-exp.y	2005-09-20 10:55:55.000000000 +0200
+++ gdb/c-exp.y	2006-01-09 23:04:35.000000000 +0100
@@ -1752,6 +1752,11 @@
     /* Call lookup_symtab, not lookup_partial_symtab, in case there are
        no psymtabs (coff, xcoff, or some future change to blow away the
        psymtabs once once symbols are read).  */
+    /*
+    ** THOR : FIXME: lookup_symbol may also return LOC_BLOCK in case
+    ** we are really looking for a base-class of a class where in fact
+    ** we should then expect LOC_BLOCK.
+    */
     if (sym && SYMBOL_CLASS (sym) == LOC_BLOCK)
       {
 	yylval.ssym.sym = sym;
diff -Naur gdb_old/c-valprint.c gdb/c-valprint.c
--- gdb_old/c-valprint.c	2005-05-09 23:20:30.000000000 +0200
+++ gdb/c-valprint.c	2006-01-21 23:08:29.000000000 +0100
@@ -392,6 +392,11 @@
 	}
       else
 	{
+	  /*
+	  ** FIX THOR: Do not print *both*. This confuses ddd as the resulting
+	  ** output string is no longer parsable
+	  */
+	  if (TYPE_LENGTH(type) != 1)
 	  val_print_type_code_int (type, valaddr + embedded_offset, stream);
 	  /* C and C++ has no single byte int type, char is used instead.
 	     Since we don't know whether the value is really intended to
@@ -399,7 +404,7 @@
 	     equivalent as well. */
 	  if (TYPE_LENGTH (type) == 1)
 	    {
-	      fputs_filtered (" ", stream);
+	      /* fputs_filtered (" ", stream); also removed, THOR */
 	      LA_PRINT_CHAR ((unsigned char) unpack_long (type, valaddr + embedded_offset),
 			     stream);
 	    }
diff -Naur gdb_old/dwarf2read.c gdb/dwarf2read.c
--- gdb_old/dwarf2read.c	2005-11-04 03:58:31.000000000 +0100
+++ gdb/dwarf2read.c	2006-01-15 16:14:35.000000000 +0100
@@ -3524,7 +3524,7 @@
       if (strcmp (fip->fnfieldlists[i].name, fieldname) == 0)
 	break;
     }
-
+  
   /* Create new list element if necessary.  */
   if (i < fip->nfnfields)
     flp = &fip->fnfieldlists[i];
diff -Naur gdb_old/language.c gdb/language.c
--- gdb_old/language.c	2005-10-03 23:21:20.000000000 +0200
+++ gdb/language.c	2006-01-21 22:48:12.000000000 +0100
@@ -1054,7 +1054,13 @@
 char *
 default_word_break_characters (void)
 {
+  /*
+  ** FIX THOR: No, do *NOT* use : as word-break character,
+  ** as this would disallow looking for namespace-scoped names
+  ** as in A::B::x
   return " \t\n!@#$%^&*()+=|~`}{[]\"';:?/>.<,-";
+  */
+  return " \t\n!@#$%^&*()+=|~`}{[]\"';?/>.<,-";
 }
 
 /* Print the index of array elements using the C99 syntax.  */
diff -Naur gdb_old/linespec.c gdb/linespec.c
--- gdb_old/linespec.c	2005-04-26 16:57:21.000000000 +0200
+++ gdb/linespec.c	2006-01-14 01:29:23.000000000 +0100
@@ -236,15 +236,16 @@
 		method_name = dem_opname;
 	    }
 
-	  if (strcmp_iw (name, method_name) == 0)
+	  if (strcmp_iw (name, method_name) == 0) {
 	    /* Find all the overloaded methods with that name.  */
 	    i1 += add_matching_methods (method_counter, t,
 					sym_arr + i1);
-	  else if (strncmp (class_name, name, name_len) == 0
-		   && (class_name[name_len] == '\0'
-		       || class_name[name_len] == '<'))
+	  } else if (strncmp (class_name, name, name_len) == 0
+	      && (class_name[name_len] == '\0'
+		  || class_name[name_len] == '<')) {
 	    i1 += add_constructors (method_counter, t,
 				    sym_arr + i1);
+	  }
 	}
     }
 
@@ -283,7 +284,7 @@
     {
       struct fn_field *f;
       char *phys_name;
-
+      
       f = TYPE_FN_FIELDLIST1 (t, method_counter);
 
       if (TYPE_FN_FIELD_STUB (f, field_counter))
@@ -305,13 +306,29 @@
       if (is_destructor_name (phys_name) != 0)
 	continue;
 
-      sym_arr[i1] = lookup_symbol (phys_name,
-				   NULL, VAR_DOMAIN,
-				   (int *) NULL,
-				   (struct symtab **) NULL);
-      if (sym_arr[i1])
-	i1++;
-      else
+      /* FIX THOR: Do not add duplicate symbols here. */
+      {
+	struct symbol *sym;
+	
+	sym = lookup_symbol (phys_name,
+			     NULL, VAR_DOMAIN,
+			     (int *) NULL,
+			     (struct symtab **) NULL);
+
+	if (sym) {
+	  int i2;
+	  for(i2 = 0;i2 < i1;i2++) {
+	    if (sym_arr[i2] == sym)
+	      break;
+	  }
+	  if (i2 == i1) {
+	    sym_arr[i1] = sym;
+	    i1++;
+	  }
+	}
+      }
+      /* The follownig is not used anyhow.
+	else */
 	{
 	  /* This error message gets printed, but the method
 	     still seems to be found
diff -Naur gdb_old/symtab.c gdb/symtab.c
--- gdb_old/symtab.c	2005-03-08 05:34:44.000000000 +0100
+++ gdb/symtab.c	2006-01-21 22:49:07.000000000 +0100
@@ -1075,7 +1075,7 @@
 
 static struct symbol *
 lookup_symbol_aux (const char *name, const char *linkage_name,
-		   const struct block *block, const domain_enum domain,
+		   const struct block *block, domain_enum domain,
 		   int *is_a_field_of_this, struct symtab **symtab)
 {
   struct symbol *sym;
@@ -1091,6 +1091,10 @@
   /* Search specified block and its superiors.  Don't search
      STATIC_BLOCK or GLOBAL_BLOCK.  */
 
+  /*
+  ** FIXME: Here provide a special search to lookup for 
+  ** TYPEs instead of constructor calls.
+  */
   sym = lookup_symbol_aux_local (name, linkage_name, block, domain,
 				 symtab);
   if (sym != NULL)
@@ -1106,10 +1110,36 @@
 
       if (v && check_field (v, name))
 	{
+	  /* THOR FIX: check_field no longer returns true in case name is
+	  ** a subclass (and not a field) of this, however this creates
+	  ** more problems. The lexer does not say (or even know) whether
+	  ** the type passed in is expected to be a member, or a type.
+	  ** And in case it is a type, this code would have found the
+	  ** constructor (mistakenly), identifying it as a member of *this.
+	  **
+	  ** However, now there's a second problem here, namely the code
+	  ** below then finds the constructor (and not the typedef) because
+	  ** that's what is first in the debug info, and then cannot deduce
+	  ** the right thing for the lexer: If A is a base of this, then
+	  ** this->A::x no longer works because A is found as method, not
+	  ** as type, and A::x no longer works, because A is not found
+	  ** as a field of this (correctly) but neither as a base of this.
+	  **
+	  ** What should happen here is to test whether A is a valid base
+	  ** of this, and then return the type, not the constructor,
+	  ** set is_a_field_of_this, and return a non_NULL symbol.
+	  */
 	  *is_a_field_of_this = 1;
 	  if (symtab != NULL)
 	    *symtab = NULL;
 	  return NULL;
+	} else if (v && check_base(v, name)) {
+	  /*
+	  ** Now all we have to archive is to enforce that lookup_block_symbol
+	  ** finds really a LOC_TYPEDEF, not a LOC_BLOCK. Yummie!
+	  */
+	  domain = STRUCT_DOMAIN;
+	  *is_a_field_of_this = 1;
 	}
     }
 
@@ -1284,7 +1314,6 @@
 	       looking in the statics even though the psymtab claimed
 	       the symbol was global, or vice-versa. It's possible
 	       that the psymtab gets it wrong in some cases.  */
-
 	    /* FIXME: carlton/2002-09-30: Should we really do that?
 	       If that happens, isn't it likely to be a GDB error, in
 	       which case we should fix the GDB error rather than
@@ -1295,10 +1324,16 @@
 				       block_index == GLOBAL_BLOCK ?
 				       STATIC_BLOCK : GLOBAL_BLOCK);
 	    sym = lookup_block_symbol (block, name, linkage_name, domain);
+	    /*
+	    ** THOR: I don't know why this fails, but it does. I should investigate
+	    ** later.
 	    if (!sym)
 	      error (_("Internal: %s symbol `%s' found in %s psymtab but not in symtab.\n%s may be an inlined function, or may be a template function\n(if a template, try specifying an instantiation: %s<type>)."),
 		     block_index == GLOBAL_BLOCK ? "global" : "static",
 		     name, ps->filename, name, name);
+	    */
+	    if (!sym)
+	      return NULL;
 	  }
 	if (symtab != NULL)
 	  *symtab = s;
@@ -1788,8 +1823,10 @@
 	{
 	  if (SYMBOL_DOMAIN (sym) == domain
 	      && (linkage_name != NULL
-		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1))
-	    return sym;
+		  ? strcmp (SYMBOL_LINKAGE_NAME (sym), linkage_name) == 0 : 1)) { /* FIX THOR */
+	    if (domain != VAR_DOMAIN || sym->aclass != LOC_TYPEDEF) /* Must *NOT* use type forwards here */
+	      return sym;
+	  } /* FIX THOR end */
 	}
       return NULL;
     }
@@ -3539,7 +3576,11 @@
 	   which are in symbols.  */
 	while (p > text)
 	  {
-	    if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == '\0')
+	    /*
+	    ** FIX THOR: Also allow : as character here to check for
+	    ** namespaced symbols as in A::B::x
+	    */
+	    if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == ':' || p[-1] == '\0')
 	      --p;
 	    else
 	      break;
diff -Naur gdb_old/valops.c gdb/valops.c
--- gdb_old/valops.c	2005-05-27 06:39:32.000000000 +0200
+++ gdb/valops.c	2006-01-10 00:26:12.000000000 +0100
@@ -91,6 +91,8 @@
 					   int static_offset);
 
 static int check_field_in (struct type *, const char *);
+static int check_base_in (struct type *type, const char *name);
+static int check_field_in_aux (struct type *type, const char *name,int cons_des);
 
 static struct value *value_struct_elt_for_reference (struct type *domain,
 						     int offset,
@@ -2226,10 +2228,21 @@
 /* Helper function for check_field: Given TYPE, a structure/union,
    return 1 if the component named NAME from the ultimate
    target structure/union is defined, otherwise, return 0. */
-
 static int
 check_field_in (struct type *type, const char *name)
 {
+  return check_field_in_aux(type,name,1);
+}
+
+/* FIX THOR: Helper function for check_field: Given TYPE, a structure/union,
+   return 1 if the component named NAME from the ultimate
+   target structure/union is defined, otherwise, return 0.
+   Unlike the above, this one can be told to ignore constructors
+   and destructors.
+*/
+static int
+check_field_in_aux (struct type *type, const char *name, int check_for_constructors)
+{
   int i;
 
   for (i = TYPE_NFIELDS (type) - 1; i >= TYPE_N_BASECLASSES (type); i--)
@@ -2245,24 +2258,107 @@
   /* Destructors are a special case.  */
   if (destructor_name_p (name, type))
     {
-      int m_index, f_index;
-
-      return get_destructor_fn_field (type, &m_index, &f_index);
+      if (check_for_constructors) {
+	int m_index, f_index;
+	
+	return get_destructor_fn_field (type, &m_index, &f_index);
+      }
+      return 0;
     }
 
   for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; --i)
     {
-      if (strcmp_iw (TYPE_FN_FIELDLIST_NAME (type, i), name) == 0)
-	return 1;
+      if (strcmp_iw (TYPE_FN_FIELDLIST_NAME (type, i), name) == 0) {
+	if (check_for_constructors) {
+	  return 1;
+	} else {
+	  /*
+	  ** Do not return constructors here.
+	  */
+	  if (TYPE_NAME (type) == NULL || strcmp_iw (TYPE_NAME(type), name))
+	    return 1;
+	}
+      }
     }
-
+  
+  /*
+  ** Old code
   for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
     if (check_field_in (TYPE_BASECLASS (type, i), name))
       return 1;
+  */
+  
+  /* FIX THOR : Must convert the base type to the
+  ** proper real base type with check_typedef.
+  */
+  for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--) {
+    struct type *t = check_typedef (TYPE_BASECLASS (type, i)); 
+    /* Be a bit more careful here: It might be that name is
+    ** found as a constructor of a sub-class. This is not
+    ** really desirable.
+    */
+    if (check_field_in_aux (t, name, 0)) {
+      return 1;
+    }
+  }
+  
+  return 0;
+}
+
+/* FIX THOR: Check whether name is a proper base class of type.
+*/
+int
+check_base (struct value *arg1, const char *name)
+{
+  struct type *t;
+
+  arg1 = coerce_array (arg1);
+
+  t = value_type (arg1);
 
+  /* Follow pointers until we get to a non-pointer.  */
+
+  for (;;)
+    {
+      CHECK_TYPEDEF (t);
+      if (TYPE_CODE (t) != TYPE_CODE_PTR && TYPE_CODE (t) != TYPE_CODE_REF)
+	break;
+      t = TYPE_TARGET_TYPE (t);
+    }
+
+  if (TYPE_CODE (t) == TYPE_CODE_MEMBER)
+    error (_("not implemented: member type in check_base"));
+
+  if (TYPE_CODE (t) != TYPE_CODE_STRUCT
+      && TYPE_CODE (t) != TYPE_CODE_UNION)
+    error (_("Internal error: `this' is not an aggregate"));
+
+  return check_base_in (t, name);
+}
+
+static int
+check_base_in (struct type *type, const char *name)
+{
+  int i;
+
+  for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; --i)
+    {
+      if (strcmp_iw (TYPE_FN_FIELDLIST_NAME (type, i), name) == 0) {
+	if (TYPE_NAME (type) && strcmp_iw (TYPE_NAME(type), name) == 0)
+	  return 1;
+      }
+    }
+  
+  for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--) {
+    struct type *t = check_typedef (TYPE_BASECLASS (type, i));
+    if (check_base_in(t,name))
+      return 1;
+  }
+  
   return 0;
 }
 
+/* FIX THOR end */
 
 /* C++: Given ARG1, a value of type (pointer to a)* structure/union,
    return 1 if the component named NAME from the ultimate
diff -Naur gdb_old/value.h gdb/value.h
--- gdb_old/value.h	2005-05-27 06:39:33.000000000 +0200
+++ gdb/value.h	2006-01-10 00:25:03.000000000 +0100
@@ -496,6 +496,7 @@
 				  struct ui_file *stream);
 
 extern int check_field (struct value *, const char *);
+extern int check_base (struct value *, const char *name);
 
 extern void typedef_print (struct type *type, struct symbol *news,
 			   struct ui_file *stream);

/* snip */

Re: C++ support improvement patch

[Daniel Jacobowitz]

Thanks for the patch.  There's a lot of bits that just change formatting
or comments; it's making it a little hard for me to work out what
changes do what.

On Mon, Jan 23, 2006 at 10:50:40AM +0100, Thomas Richter wrote:
> o) GDB finds now members within the multiple-inheritance hierarchy of
> a class. That is, if "this" points to a class "Foo" inherited from
> "Bar" and "Baz", and "Baz" contains a member "me", then "print me"
> will succeed now. gdb 6.4 and before just returned an "unknown symbol"
> here, erraneously. This is because it was confused by a "class typedef"
> g++ inserted here for subclasses in the dwarf-2 symbol table, which is
> now cleverly avoided. The patch then, however, needs to be clever to
> resolve expressions like this->Subclass::member, which - after a bit
> of work - now works nicely as it should. It seems to me that it worked
> more or less by accident before (Subclass was resolved as the constructor
> name, not as the class name.)

Can you give a testcase for this problem?  It should already work,
and if it doesn't I want to know why!

Also, what version of GCC were you testing against?

> o) For TAB-expansion, it removes the ":" as word-delimiter for
> readline, thus allowing to find names in namespaces. The current
> behaivour here is completely unusable for C++. The patch could
> be better, though, it should read template argument delimiters < >
> and function argument delimiters ( ) as "quotation characters",
> though it doesn't. Though it's a huge improvement compared to
> previous versions where TAB expansion was just not usable for C++.

This isn't the right fix, though.  Someone posted (Pierre Muller, a
long time ago - April 2002!) an initial patch to make tab completion
context sensitive (for Pascal) - tab complete to the colon, type the
colon, and then tab complete structure members after the colon.

It isn't suitable as-is and no one ever had time to go back and improve
it but that's what ought to be done.

> o) GDB now avoids to set multiple breakpoints to the same location.
> This goes in conjunction with another bug, namely that of being unable
> to find constructors of multiply-inherited classes. Without the current
> patch, setting a breakpoint for a constructor results in multiple breakpoint
> locations found, all of them identical, and none of them actually 
> beeing breaked on. With the patch applied, GDB at least finds only one
> of the locations (good), but still doesn't break there (bad). This bug
> seems partially caused by g++ not emmiting mangled names for constructors.

Would be better not to return duplicates, instead of not putting
duplicates on the list... we know exactly why this happens.  It's
all part of the multiple constructors issue that you mentioned below,
and you can find more information about this in the gdb@ list archives.

> o) GDB does no longer print the character and the value for integer values
> of size one. It only prints the character. The reason for this is that
> with the ddd frontend, ddd parses the output of "info breakpoints" to
> set or reset breakpoints (e.g. when moving them in the sources). Without
> the patch, the syntax of the output is no longer valid, and is not parsable
> by gdb again.

This is what GDB has always done.  I admit it's strange in expressions.
What I'd like to see would be only the value in expression printouts,
without changing the (useful) output of "print 'c'".

> i) gdb relies on the "MIPS linkage name" of members, though according to
> the g++ folks it should not,

I have patches to remove this dependence that I will be revisiting in
2006.

> ii) g++ may create separate constructors depending on whehter the class is
> constructed by itself, or as part of a subclass. gdb seems to be unaware
> of this. I've no idea how to resolve this.

As I mentioned above, please see the GDB list archives.  It was just
discussed again recently and I have some hope that 2006 will see a
solution.

-- 
Daniel Jacobowitz
CodeSourcery

Re: C++ support improvement patch

[David Ayers]

Daniel Jacobowitz schrieb:

>>o) For TAB-expansion, it removes the ":" as word-delimiter for
>>readline, thus allowing to find names in namespaces. The current
>>behaivour here is completely unusable for C++. The patch could
>>be better, though, it should read template argument delimiters < >
>>and function argument delimiters ( ) as "quotation characters",
>>though it doesn't. Though it's a huge improvement compared to
>>previous versions where TAB expansion was just not usable for C++.
> 
> 
> This isn't the right fix, though.  Someone posted (Pierre Muller, a
> long time ago - April 2002!) an initial patch to make tab completion
> context sensitive (for Pascal) - tab complete to the colon, type the
> colon, and then tab complete structure members after the colon.
> 
> It isn't suitable as-is and no one ever had time to go back and improve
> it but that's what ought to be done.
> 

I also would be very interested in this part of an acceptable patch as
to possibly take a shot at fixing tab completion for Objective-C methods.

/me searches the archives
I think you mean:
http://sources.redhat.com/ml/gdb-patches/2002-04/msg00774.html

It would also be interesting to know if Apple has an acceptable proposal
for this especially in light of Objective-C++.

Cheers,
David

Re: C++ support improvement patch

[Thomas Richter]

Hi Daniel,

> Thanks for the patch.  There's a lot of bits that just change formatting
> or comments; it's making it a little hard for me to work out what
> changes do what.

I think parts of the comments are useful to understand what's going
on here. The most sensitive part is in symtab.c.

> > o) GDB finds now members within the multiple-inheritance hierarchy of
> > a class. That is, if "this" points to a class "Foo" inherited from
> > "Bar" and "Baz", and "Baz" contains a member "me", then "print me"
> > will succeed now. gdb 6.4 and before just returned an "unknown symbol"
> > here, erraneously. This is because it was confused by a "class typedef"
> > g++ inserted here for subclasses in the dwarf-2 symbol table, which is
> > now cleverly avoided. The patch then, however, needs to be clever to
> > resolve expressions like this->Subclass::member, which - after a bit
> > of work - now works nicely as it should. It seems to me that it worked
> > more or less by accident before (Subclass was resolved as the constructor
> > name, not as the class name.)
> 
> Can you give a testcase for this problem?  It should already work,
> and if it doesn't I want to know why!

It doesn't. Sorry, the code where it doesn't is part of a larger code
project I wouldn't really be able to send (and you wouldn't be willing
to check, either ;-), but I'll see whether I'll be able to setup a minimal
testcase.

> Also, what version of GCC were you testing against?

That's g++ 3.4.4, the g++ in Debian stable.

> > o) For TAB-expansion, it removes the ":" as word-delimiter for
> > readline, thus allowing to find names in namespaces. The current
> > behaivour here is completely unusable for C++. The patch could
> > be better, though, it should read template argument delimiters < >
> > and function argument delimiters ( ) as "quotation characters",
> > though it doesn't. Though it's a huge improvement compared to
> > previous versions where TAB expansion was just not usable for C++.
> 
> This isn't the right fix, though.  Someone posted (Pierre Muller, a
> long time ago - April 2002!) an initial patch to make tab completion
> context sensitive (for Pascal) - tab complete to the colon, type the
> colon, and then tab complete structure members after the colon.
> 
> It isn't suitable as-is and no one ever had time to go back and improve
> it but that's what ought to be done.

Thanks for letting me know that this is being worked on. I agree that
the given patch is pretty much a "hands on" approach. If there's a better
way to do it, please do. Otherwise, the patch is "better than nothing".

> > o) GDB now avoids to set multiple breakpoints to the same location.
> > This goes in conjunction with another bug, namely that of being unable
> > to find constructors of multiply-inherited classes. Without the current
> > patch, setting a breakpoint for a constructor results in multiple breakpoint
> > locations found, all of them identical, and none of them actually 
> > beeing breaked on. With the patch applied, GDB at least finds only one
> > of the locations (good), but still doesn't break there (bad). This bug
> > seems partially caused by g++ not emmiting mangled names for constructors.
> 
> Would be better not to return duplicates, instead of not putting
> duplicates on the list... we know exactly why this happens.  It's
> all part of the multiple constructors issue that you mentioned below,
> and you can find more information about this in the gdb@ list archives.

Could you provide a reference, please? Currently, the found symbols are
identically (in fact, it's even the same structure using the same pointer),
so I don't quite see what's the benefit as there's no chance of telling
them apart later. I get two breakpoints at the same location (then, breaking
nothing). In the long run, though, I agree and the problem should be fixed,
not the symptom.

> > o) GDB does no longer print the character and the value for integer values
> > of size one. It only prints the character. The reason for this is that
> > with the ddd frontend, ddd parses the output of "info breakpoints" to
> > set or reset breakpoints (e.g. when moving them in the sources). Without
> > the patch, the syntax of the output is no longer valid, and is not parsable
> > by gdb again.
> 
> This is what GDB has always done.  I admit it's strange in expressions.
> What I'd like to see would be only the value in expression printouts,
> without changing the (useful) output of "print 'c'".

Thus, a similar patch applied higher in the calling chain, within
"info"?

> > i) gdb relies on the "MIPS linkage name" of members, though according to
> > the g++ folks it should not,
> 
> I have patches to remove this dependence that I will be revisiting in
> 2006.

I personally wouldn't care unless I would be able to set breakpoints. (-;

> > ii) g++ may create separate constructors depending on whehter the class is
> > constructed by itself, or as part of a subclass. gdb seems to be unaware
> > of this. I've no idea how to resolve this.
> 
> As I mentioned above, please see the GDB list archives.  It was just
> discussed again recently and I have some hope that 2006 will see a
> solution.

That would be great!

So long, and thanks for the comments.

Thomas Richter

Re: C++ support improvement patch

[Daniel Jacobowitz]

On Mon, Jan 23, 2006 at 03:59:50PM +0100, Thomas Richter wrote:
> Thanks for letting me know that this is being worked on. I agree that
> the given patch is pretty much a "hands on" approach. If there's a better
> way to do it, please do. Otherwise, the patch is "better than nothing".

Maybe, maybe not.  The readline completion is fairly finicky.  I'm
pretty sure colon was included for a reason, although I don't know
offhand what it was; but this is exactly the same problem as with
var.<tab> and it shouldn't be solved differently.

> > > o) GDB now avoids to set multiple breakpoints to the same location.
> > > This goes in conjunction with another bug, namely that of being unable
> > > to find constructors of multiply-inherited classes. Without the current
> > > patch, setting a breakpoint for a constructor results in multiple breakpoint
> > > locations found, all of them identical, and none of them actually 
> > > beeing breaked on. With the patch applied, GDB at least finds only one
> > > of the locations (good), but still doesn't break there (bad). This bug
> > > seems partially caused by g++ not emmiting mangled names for constructors.
> > 
> > Would be better not to return duplicates, instead of not putting
> > duplicates on the list... we know exactly why this happens.  It's
> > all part of the multiple constructors issue that you mentioned below,
> > and you can find more information about this in the gdb@ list archives.
> 
> Could you provide a reference, please? Currently, the found symbols are
> identically (in fact, it's even the same structure using the same pointer),
> so I don't quite see what's the benefit as there's no chance of telling
> them apart later. I get two breakpoints at the same location (then, breaking
> nothing). In the long run, though, I agree and the problem should be fixed,
> not the symptom.

For instance, this and followups:
  http://sourceware.org/ml/gdb/2006-01/msg00102.html

Although that doesn't address this particular sub-problem.  The actual
cause of the two duplicate breakpoints is GDB's bogus dependence on
DW_AT_MIPS_linkage_name, which the abstract constructors do not have.
Both your breakpoints are probably being placed at the not-in-charge
constructor; if you constructed a derived class which virtually
inherited from this one, you would probably hit that breakpoint.

> Thus, a similar patch applied higher in the calling chain, within
> "info"?

Probably a new interface.  I haven't thought about where it would go.
But breaking the current behavior for 'print' just isn't acceptable;
too many people expect it.


-- 
Daniel Jacobowitz
CodeSourcery