From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6527 invoked by alias); 30 Aug 2002 04:07:39 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6520 invoked from network); 30 Aug 2002 04:07:38 -0000 Received: from unknown (HELO crack.them.org) (65.125.64.184) by sources.redhat.com with SMTP; 30 Aug 2002 04:07:38 -0000 Received: from nevyn.them.org ([66.93.61.169] ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 17ke0R-00076E-00; Fri, 30 Aug 2002 00:07:40 -0500 Received: from drow by nevyn.them.org with local (Exim 3.35 #1 (Debian)) id 17kd5K-000357-00; Fri, 30 Aug 2002 00:08:38 -0400 Date: Fri, 30 Aug 2002 00:14:00 -0000 From: Daniel Jacobowitz To: Jim Blandy Cc: gdb-patches@sources.redhat.com, ezannoni@redhat.com Subject: Re: RFA: Correct field names for class methods Message-ID: <20020830040838.GA11641@nevyn.them.org> Mail-Followup-To: Jim Blandy , gdb-patches@sources.redhat.com, ezannoni@redhat.com References: <20020827031346.GA16591@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.1i X-SW-Source: 2002-08/txt/msg01027.txt.bz2 On Thu, Aug 29, 2002 at 10:48:05PM -0500, Jim Blandy wrote: > > I understand the method code very little, so I have only superficial > comments. I'll try to do a more thorough reading next week, if Elena > doesn't beat me to it. Thanks. > > Daniel Jacobowitz writes: > > @@ -129,15 +128,11 @@ cp_print_class_method (char *valaddr, > > f = TYPE_FN_FIELDLIST1 (domain, i); > > len2 = TYPE_FN_FIELDLIST_LENGTH (domain, i); > > > > + check_stub_method_group (f, j); > > for (j = 0; j < len2; j++) > > { > > - QUIT; > > - if (TYPE_FN_FIELD_STUB (f, j)) > > - check_stub_method (domain, i, j); > > if (STREQ (SYMBOL_NAME (sym), TYPE_FN_FIELD_PHYSNAME (f, j))) > > - { > > - goto common; > > - } > > + goto common; > > } > > } > > } > > Not that it matters much, but why are you removing the QUIT from this > loop? Exactly... because I moved the check_stub_method outside of the loop, there is no longer any possible justification to QUIT; there. It's a loop of strcmp calls! If you want, I could add the QUIT in check_stub_method_group, but I don't think it's necessary. > > +/* Find the last component of the demangled C++ name NAME. > > + > > + This function return a pointer to the first colon before the > > + last component, or NULL if the name had only one component. */ > > Hmm, not every demangled name contains a closing paren. Shouldn't this > comment clarify that it only works on demangled method names? Yes, you're right. It used to not require the closing paren, and then I realized unpleasant things about the ways some local inner classes are mangled. > > @@ -3377,6 +3378,127 @@ read_member_functions (struct field_info > > } > > else > > { > > + int has_stub = 0; > > + int has_nondestructor = 0, has_destructor = 0; > > + int is_v3 = 0; > > + struct next_fnfield *tmp_sublist; > > + > > + /* Various versions of GCC emit various mostly-useless > > + strings in the name field for special member functions. > > + For some methods, we have a complete physname, so we > > + could fix this up now. For stub methods we will do this > > + later, in check_stub_method_group. For non-stub methods, > > + we have no clear way to know whether or not the physname > > + is correct; g++ 2.95.x only reliably emits full physnames > > + for operators which do not start with the method name > > + (constructors, destructors fall in this category; there > > + may be others?). But for other methods it may or may not > > + emit a full physname depending on the platform (if > > + CPLUS_MARKER can be `$' or `.', it will use minimal debug > > + information, but not otherwise). > > + > > + Rather than dealing with this, we take a different approach. > > + For v3 mangled names, we can use the full physname; for v2, > > + we use cplus_demangle_opname, because the only interesting > > + names are all operators. Skip if any method in the group > > + is a stub, to prevent our fouling up the workings of > > + gdb_mangle_name. > > + > > + Another thing that we need to clean up here: GCC 2.95.x > > + puts constructors and destructors in the same group. We > > + need to split this into two groups. */ > > + > > + tmp_sublist = sublist; > > + while (tmp_sublist != NULL) > > + { > > + if (tmp_sublist->fn_field.is_stub) > > + has_stub = 1; > > + if (tmp_sublist->fn_field.physname[0] == '_' > > + && tmp_sublist->fn_field.physname[1] == 'Z') > > + is_v3 = 1; > > + > > + if (is_destructor_name (tmp_sublist->fn_field.physname)) > > + has_destructor++; > > + else > > + has_nondestructor++; > > + > > + tmp_sublist = tmp_sublist->next; > > + } > > + > > + if (has_destructor && has_nondestructor) > > + { > > + struct next_fnfieldlist *destr_fnlist; > > + struct next_fnfield *last_sublist; > > + > > + /* Create a new fn_fieldlist for the destructors. */; > > + destr_fnlist = (struct next_fnfieldlist *) > > + xmalloc (sizeof (struct next_fnfieldlist)); > > + make_cleanup (xfree, destr_fnlist); > > + memset (destr_fnlist, 0, sizeof (struct next_fnfieldlist)); > > + destr_fnlist->fn_fieldlist.name > > + = obconcat (&objfile->type_obstack, "", "~", > > + new_fnlist->fn_fieldlist.name); > > + > > + destr_fnlist->fn_fieldlist.fn_fields = (struct fn_field *) > > + obstack_alloc (&objfile->type_obstack, > > + sizeof (struct fn_field) * has_destructor); > > + memset (destr_fnlist->fn_fieldlist.fn_fields, 0, > > + sizeof (struct fn_field) * has_destructor); > > + tmp_sublist = sublist; > > + last_sublist = NULL; > > + i = 0; > > + while (tmp_sublist != NULL) > > + { > > + if (!is_destructor_name (tmp_sublist->fn_field.physname)) > > + { > > + tmp_sublist = tmp_sublist->next; > > + continue; > > + } > > + > > + destr_fnlist->fn_fieldlist.fn_fields[i] > > + = tmp_sublist->fn_field; > > + if (last_sublist) > > + last_sublist->next = tmp_sublist->next; > > + else > > + sublist = tmp_sublist->next; > > + last_sublist = tmp_sublist; > > + tmp_sublist = tmp_sublist->next; > > + } > > + > > + destr_fnlist->fn_fieldlist.length = has_destructor; > > + destr_fnlist->next = fip->fnlist; > > + fip->fnlist = destr_fnlist; > > + nfn_fields++; > > + total_length += has_destructor; > > + length -= has_destructor; > > + } > > + else if (is_v3) > > + { > > + /* v3 mangling prevents the use of abbreviated physnames, > > + so we can do this here. There are stubbed methods in v3 > > + only: > > + - in -gstabs instead of -gstabs+ > > + - or for static methods, who are output as a function type > > + instead of a method type. */ > > + > > + update_method_name_from_physname (&new_fnlist->fn_fieldlist.name, > > + sublist->fn_field.physname); > > + } > > + else if (!has_stub) > > + { > > + char dem_opname[256]; > > + int ret; > > + ret = cplus_demangle_opname (new_fnlist->fn_fieldlist.name, > > + dem_opname, DMGL_ANSI); > > + if (!ret) > > + ret = cplus_demangle_opname (new_fnlist->fn_fieldlist.name, > > + dem_opname, 0); > > + if (ret) > > + new_fnlist->fn_fieldlist.name > > + = obsavestring (dem_opname, strlen (dem_opname), > > + &objfile->type_obstack); > > + } > > + > > Is there any way we could pull this out into a separate function or > functions? read_member_functions is bad enough already. Hmm... I'm not sure. In particular, when splitting constructors from destructors it tramples on the function state a bit: fip, nfn_fields, total_length, length, and new_fnlist. My usual criterion for breaking out a function is whether its arguments would make any sense on their own (outside of the calling function's control flow), and these wouldn't. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer