[RFA] "Sort" C++ fieldlists

[Keith Seitz <keiths@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

Hi,

My method overload resolution patch was committed with the following 
comment (from value_struct_elt_for_reference):

   /* This assumes, of course, that all artificial methods appear
      BEFORE any concrete methods.  */

Turns out that isn't a really great assumption. In particular, it fails 
on GCC 3.4.6. So I'm requesting that the attached patch be approved. The 
patch inserts artificial methods into the type's fieldlist starting at 
index 0 and concrete methods starting at the end index.

This shows no regressions (and no advantages) using GCC on linux. 
However, if you run cpexprs.exp, this patch will fix 17 failures with 
GCC 3.4.6. At the worst, consider this a defensive patch against 
breaking the value_struct_elt_for_reference assumption.

Keith

ChangeLog
2009-12-03  Keith Seitz  <keiths@redhat.com>

	* dwarf2read.c (dwarf2_attach_fn_fields_to_type): Ensure
	that artificial methods are inserted into the fn fieldlist
	before concrete methods.
	* valops.c (value_struct_elt_for_reference): Update comments
	concerning above assumption.

[-- Attachment #2: sort-methods.patch --]
[-- Type: text/plain, Size: 1935 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.342
diff -u -p -r1.342 dwarf2read.c
--- dwarf2read.c	2 Dec 2009 11:44:35 -0000	1.342
+++ dwarf2read.c	4 Dec 2009 01:55:11 -0000
@@ -4845,14 +4845,24 @@ dwarf2_attach_fn_fields_to_type (struct 
     {
       struct nextfnfield *nfp = flp->head;
       struct fn_fieldlist *fn_flp = &TYPE_FN_FIELDLIST (type, i);
-      int k;
+      int k, start, end;
 
       TYPE_FN_FIELDLIST_NAME (type, i) = flp->name;
       TYPE_FN_FIELDLIST_LENGTH (type, i) = flp->length;
       fn_flp->fn_fields = (struct fn_field *)
 	TYPE_ALLOC (type, sizeof (struct fn_field) * flp->length);
+
+      /* To ease symbol searching, we must require that all artificial
+	 methods appear before any concrete methods in the fieldlist.
+	 See value_struct_elt_for_reference for where this requirement
+	 is needed/used.  */
+      start = 0;
+      end = flp->length - 1;
       for (k = flp->length; (k--, nfp); nfp = nfp->next)
-	fn_flp->fn_fields[k] = nfp->fnfield;
+	{
+	  int idx = (nfp->fnfield.is_artificial) ? start++ : end--;
+	  fn_flp->fn_fields[idx] = nfp->fnfield;
+	}
 
       total_length += flp->length;
     }
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.228
diff -u -p -r1.228 valops.c
--- valops.c	2 Dec 2009 19:29:42 -0000	1.228
+++ valops.c	4 Dec 2009 01:55:12 -0000
@@ -2721,7 +2721,8 @@ value_struct_elt_for_reference (struct t
 		error (_("non-unique member `%s' requires type instantiation"), name);
 
 	      /* This assumes, of course, that all artificial methods appear
-		 BEFORE any concrete methods.  */
+		 BEFORE any concrete methods.  This requirement is enforced
+		 for DWARF by dwarf2_attach_fn_fields_to_type.  */
 	      j = TYPE_FN_FIELDLIST_LENGTH (t, i) - 1;
 	    }