From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19240 invoked by alias); 6 Jan 2003 21:42:26 -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 19233 invoked from network); 6 Jan 2003 21:42:25 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by 209.249.29.67 with SMTP; 6 Jan 2003 21:42:25 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h06LEeB03681 for ; Mon, 6 Jan 2003 16:14:40 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h06LgDa26943 for ; Mon, 6 Jan 2003 16:42:13 -0500 Received: from localhost.redhat.com (romulus-int.sfbay.redhat.com [172.16.27.46]) by pobox.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h06LgBn29469 for ; Mon, 6 Jan 2003 16:42:12 -0500 Received: by localhost.redhat.com (Postfix, from userid 469) id C0FACFF79; Mon, 6 Jan 2003 16:46:32 -0500 (EST) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15897.63800.544503.672211@localhost.redhat.com> Date: Mon, 06 Jan 2003 21:42:00 -0000 To: Adam Fedor Cc: GDB Patches Subject: Re: [RFA] Demangle ObjC symbols in symtab.c [4/5] In-Reply-To: <3E1616B7.1010001@doc.com> References: <3E1616B7.1010001@doc.com> X-SW-Source: 2003-01/txt/msg00233.txt.bz2 Adam Fedor writes: > 2003-01-03 Adam Fedor > > * symtab.c (symbol_init_demangled_name): Check for and demangle > ObjC symbols. > (make_symbol_completion_list): Look for ObjC symbols > > Index: symtab.c > =================================================================== > RCS file: /cvs/src/src/gdb/symtab.c,v > retrieving revision 1.84 > diff -u -p -r1.84 symtab.c > --- symtab.c 2 Jan 2003 14:27:26 -0000 1.84 > +++ symtab.c 3 Jan 2003 03:20:14 -0000 > @@ -440,8 +440,18 @@ void > symbol_init_demangled_name (struct general_symbol_info *gsymbol, > struct obstack *obstack) > { > - char *mangled = gsymbol->name; > - char *demangled = NULL; > - > if (gsymbol->language == language_unknown) > gsymbol->language = language_auto; > + > + if (gsymbol->language == language_objc > + || gsymbol->language == language_auto) > + { > + char *demangled = > + objc_demangle (gsymbol->name); > + if (demangled != NULL) > + { > + gsymbol->language = language_objc; > + gsymbol->language_specific.objc_specific.demangled_name = > + obsavestring (demangled, strlen (demangled), (obstack)); > + xfree (demangled); > + }+ ^^^^ ??? I assume it's a cut/paste typo. > > + else > + { > + gsymbol->language_specific.objc_specific.demangled_name = NULL; > + } > + } > + > if (gsymbol->language == language_cplus > || gsymbol->language == language_auto) > { > - demangled = > + char *demangled = > cplus_demangle (gsymbol->name, DMGL_PARAMS | DMGL_ANSI); > if (demangled != NULL) > { > @@ -462,9 +478,10 @@ symbol_init_demangled_name (struct gener > gsymbol->language_specific.cplus_specific.demangled_name = NULL; > } > } > + > if (gsymbol->language == language_java) > { > - demangled = > + char *demangled = > cplus_demangle (gsymbol->name, > DMGL_PARAMS | DMGL_ANSI | DMGL_JAVA); > if (demangled != NULL) > @@ -479,6 +496,9 @@ symbol_init_demangled_name (struct gener > gsymbol->language_specific.cplus_specific.demangled_name = NULL; > } > } > + > + if (gsymbol->language == language_auto) > + gsymbol->language = language_unknown; Why this? There was a (kind of a) rationale on setting the language to auto, to speed up some searches. There was a controversy about that too, and it never got resolved. Do the current C++ people have an opinion? See: http://sources.redhat.com/ml/gdb-patches/2000-09/msg00241.html and http://sources.redhat.com/ml/gdb-patches/2000-10/msg00251.html Also, why do you move the declaration of the variable 'demangle' inside each block? Ok on deleting the variable 'mangled'. > } > > /* Return the demangled name for a symbol based on the language for > @@ -831,7 +851,7 @@ lookup_symbol_aux (const char *name, con > } > #endif /* 0 */ > > - /* C++: If requested to do so by the caller, > + /* C++/Java/Objective-C: If requested to do so by the caller, > check to see if NAME is a field of `this'. */ > if (is_a_field_of_this) > { > @@ -1483,9 +1503,9 @@ find_main_psymtab (void) > for now we don't worry about the slight inefficiency of looking for > a match we'll never find, since it will go pretty quick. Once the > binary search terminates, we drop through and do a straight linear > - search on the symbols. Each symbol which is marked as being a C++ > - symbol (language_cplus set) has both the encoded and non-encoded names > - tested for a match. > + search on the symbols. Each symbol which is marked as being a ObjC/C++ > + symbol (language_cplus or language_objc set) has both the encoded and > + non-encoded names tested for a match. > > If MANGLED_NAME is non-NULL, verify that any symbol we find has this > particular mangled name. > @@ -3346,8 +3366,6 @@ make_symbol_completion_list (char *text, > } > } > > - sym_text_len = strlen (sym_text); > - > return_val_size = 100; > return_val_index = 0; > return_val = (char **) xmalloc ((return_val_size + 1) * sizeof (char *)); > @@ -3356,6 +3374,8 @@ make_symbol_completion_list (char *text, > /* Look through the partial symtabs for all symbols which begin > by matching SYM_TEXT. Add each one that you find to the list. */ > > + sym_text_len = strlen (sym_text); > + > ALL_PSYMTABS (objfile, ps) > { > /* If the psymtab's been read in we'll get it when we search > @@ -3388,11 +3408,72 @@ make_symbol_completion_list (char *text, > anything that isn't a text symbol (everything else will be > handled by the psymtab code above). */ > > - ALL_MSYMBOLS (objfile, msymbol) > - { > - QUIT; > - COMPLETION_LIST_ADD_SYMBOL (msymbol, sym_text, sym_text_len, text, word); > - } > + /* ObjC: In case we are completing on a selector, look thru the msymbols > + again and feed all the selectors into the mill. */ > + > + ALL_OBJFILES (objfile) > + { > + /*objfile_demangle_msymbols (objfile);*/ Do you need this line? If not, could you collapse the two for-loop macros into the ALL_MSYMBOLS one? you don't use the objfile variable in the code below, and it would minimize the diffs. > + ALL_OBJFILE_MSYMBOLS (objfile, msymbol) > + { > + static char *tmp = NULL; > + static unsigned int tmplen = 0; > + > + char *method, *category, *selector; > + char *tmp2 = NULL; > + > + QUIT; > + > + method = SYMBOL_DEMANGLED_NAME (msymbol); > + if (method == NULL) > + method = SYMBOL_NAME (msymbol); > + if (method == NULL) > + continue; > + > + /* add the minimal symbol no matter what */ > + completion_list_add_name (method, sym_text, sym_text_len, text, word); > + Why don't you use the (yes, awful) macro COMPLETION_LIST_ADD_SYMBOL ? > + /* is it a method? */ > + if ((method[0] != '-') && (method[0] != '+')) > + continue; > + if (sym_text[0] == '[') > + /* complete on shortened method method */ > + completion_list_add_name (method + 1, sym_text, sym_text_len, text, word); Do you add 2 methods for a symbol that starts with '['? > + > + while ((strlen (method) + 1) >= tmplen) > + { > + tmplen = (tmplen == 0) ? 1024 : tmplen * 2; Conditional expressions are generally frowned upon. > + tmp = xrealloc (tmp, tmplen); > + } > + selector = strchr (method, ' '); > + if (selector != NULL) > + selector++; > + > + category = strchr (method, '('); > + > + if ((category != NULL) && (selector != NULL)) > + { > + memcpy (tmp, method, (category - method)); > + tmp[category - method] = ' '; > + memcpy (tmp + (category - method) + 1, selector, strlen (selector) + 1); > + completion_list_add_name (tmp, sym_text, sym_text_len, text, word); > + if (sym_text[0] == '[') > + completion_list_add_name (tmp + 1, sym_text, sym_text_len, text, word); > + } > + > + if (selector != NULL) > + { > + /* complete on selector only */ > + strcpy (tmp, selector); > + tmp2 = strchr (tmp, ']'); > + if (tmp2 != NULL) > + *tmp2 = '\0'; > + > + completion_list_add_name (tmp, sym_text, sym_text_len, text, word); > + } I am not sure I understand: how many variations of the same name get added to the completion list? > + } > + } > + > > /* Search upwards from currently selected frame (so that we can > complete on local vars. */ > @@ -3409,6 +3490,7 @@ make_symbol_completion_list (char *text, > > ALL_BLOCK_SYMBOLS (b, i, sym) > { > + QUIT; > COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word); > if (SYMBOL_CLASS (sym) == LOC_TYPEDEF) > { > @@ -3514,25 +3596,58 @@ make_file_symbol_completion_list (char * > } > else > { > - /* It is not a quoted string. Break it based on the characters > - which are in symbols. */ > - while (p > text) > - { > - if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == '\0') > - --p; > - else > - break; > - } > + /* It is not a quoted string. Break it based on the characters > + which are in symbols. */ > + for (; p > text; --p) > + { > + if (isalnum (p[-1]) || p[-1] == '_' || p[-1] == '\0') > + continue; > + else > + { > + if ((current_language->la_language == language_objc)) > + { > + if (p[-1] == ':') /* might be part of a method name */ > + continue; > + else if (p[-1] == '[' && (p[-2] == '-' || p[-2] == '+')) > + p -= 2; /* beginning of a method name */ > + else if (p[-1] == ' ' || p[-1] == '(' || p[-1] == ')') > + { /* might be part of a method name */ > + char *t = p; > + > + /* Seeing a ' ' or a '(' is not conclusive evidence > + that we are in the middle of a method name. However, > + finding "-[" or "+[" should be pretty un-ambiguous. > + Unfortunately we have to find it now to decide. */ > + > + while (t > text) > + if (isalnum (t[-1]) || t[-1] == '_' || > + t[-1] == ' ' || t[-1] == ':' || > + t[-1] == '(' || t[-1] == ')') > + --t; > + else > + break; > + > + if (t[-1] == '[' && (t[-2] == '-' || t[-2] == '+')) > + p = t - 2; /* method name detected */ > + /* else we leave with p unchanged */ > + } > + } > + break; > + } > + } > sym_text = p; > } > } > > - sym_text_len = strlen (sym_text); > - > return_val_size = 10; > return_val_index = 0; > return_val = (char **) xmalloc ((return_val_size + 1) * sizeof (char *)); > return_val[0] = NULL; > + > + /* Look through the partial symtabs for all symbols which begin > + by matching SYM_TEXT. Add each one that you find to the list. */ > + > + sym_text_len = strlen (sym_text); > > /* Find the symtab for SRCFILE (this loads it if it was not yet read > in). */ As for the other patch, the comments should begin with a capital letter and end with a period.