From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6089 invoked by alias); 10 Jun 2003 18:37:19 -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 5102 invoked from network); 10 Jun 2003 18:36:55 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 10 Jun 2003 18:36:55 -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 h5AIasH05708 for ; Tue, 10 Jun 2003 14:36:54 -0400 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 h5AIasI04300 for ; Tue, 10 Jun 2003 14:36:54 -0400 Received: from localhost.redhat.com (IDENT:3t1urYBI5+EtceBfWPnccIuxncjqZyfk@tooth.toronto.redhat.com [172.16.14.29]) by pobox.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h5AIapE08799; Tue, 10 Jun 2003 14:36:51 -0400 Received: by localhost.redhat.com (Postfix, from userid 469) id 53AAD2C986; Tue, 10 Jun 2003 14:43:11 -0400 (EDT) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <16102.9919.32103.991732@localhost.redhat.com> Date: Tue, 10 Jun 2003 18:37:00 -0000 To: David Carlton Cc: Elena Zannoni , gdb-patches@sources.redhat.com, Jim Blandy Subject: Re: [rfa] struct dictionary In-Reply-To: References: <16101.3585.739945.993550@localhost.redhat.com> X-SW-Source: 2003-06/txt/msg00333.txt.bz2 David Carlton writes: > On Mon, 9 Jun 2003 18:45:21 -0400, Elena Zannoni said: > > David Carlton writes: > > > As usual, some comments. Could you check in the removal of the > > sorting as one patch, and the dictionary as another? > > Will do. > > > See below for some things, but in general it's ok. > > Great! > > > I don't feel too comfortable with the Java stuff, are there any > > tests that use that particular code? It sure looks safe to me > > though. > > The Java testsuite might exercise it a little bit, but it might not. > Once this goes in, I'll e-mail Tom Tromey to ask him to pound on it a > bit. > > >> +/* Various vtbls that we'll actually use. */ > >> + > >> +static const struct dict_vtbl dict_hashed_vtbl = > >> + { > >> + DICT_HASHED, free_obstack, add_symbol_nonexpandable, > >> + iterator_first_hashed, iterator_next_hashed, > >> + iter_name_first_hashed, iter_name_next_hashed, > >> + }; > >> + > > > Could you do these one per line and put a comment with the field > > name next to it (for easy grepping)? I am not too fond of the use > > of vtable terminology, could you use 'vector' or something else? > > Sure. > > >> + $(infcall_h) $(dictionary.h) > > > dictionary_h > > Whoops, good eye! > > >> @@ -348,9 +309,11 @@ finish_block (struct symbol *symbol, str > >> TYPE_FIELDS (ftype) = (struct field *) > >> TYPE_ALLOC (ftype, nparams * sizeof (struct field)); > >> > >> - for (i = iparams = 0; iparams < nparams; i++) > >> + for (sym = dict_iterator_first (BLOCK_DICT (block), &iter), > >> + iparams = 0; > >> + iparams < nparams; > >> + sym = dict_iterator_next (&iter)) > > > could you use the macro ALL_BLOCK_SYMBOLS here? and move the check for > > iparams inside the loop body? > > Sure, that makes sense. I was just going with the way it had been > written before, but now that I look at it, it was only written that > way for reasons that are no longer relevant: you want me to do > > ALL_BLOCK_SYMBOLS (block, iter, sym) > { > if (iparams == nparams) > break; > > .... > } > > And before this change, that wouldn't have worked, because > ALL_BLOCK_SYMBOLS was secretly two nested loops, so you can't break > out of them, but after my change it's just a single loop, so the break > works just fine. Excellent. > > >> -static struct block *new_block (int); > >> +static struct block *new_block (int function); > > > can this take an enum parameter with some meaningful names? > > Well, I'm just using it as a boolean, so it seems to me that doing an > enum would be a little funny. How about I rename the argument to > function_p, or is_function, or something like that? But if you'd > prefer an enum, I could do something like > > enum block_type { FUNCTION, NON_FUNCTION }; > > instead. For me the real problem is to understand the code that does the call. I don't know what 1 or 0 means at that point w/o going looking for the function definition. And I lose my train of thoughts. Don't really care too much because this is only in mdebugread.c though :-) > > >> @@ -1236,13 +1231,16 @@ parse_symbol (SYMR *sh, union aux_ext *a > >> > >> if (nparams > 0) > >> { > >> + struct dict_iterator iter; > >> TYPE_NFIELDS (ftype) = nparams; > >> TYPE_FIELDS (ftype) = (struct field *) > >> TYPE_ALLOC (ftype, nparams * sizeof (struct field)); > >> > >> - for (i = iparams = 0; iparams < nparams; i++) > >> + for (sym = dict_iterator_first (BLOCK_DICT (b), &iter), > >> + iparams = 0; > >> + iparams < nparams; > >> + sym = dict_iterator_next (&iter)) > > > could you still use the macro here ALL_BLOCK_SYMBOLS and add an if > > (iparams == nparams) break; inside the loop? > > Yes, presumably I can handle this like the buildsym case above. > > >> - register int i, j; > >> + struct dict_iterator iter; > >> + register int j; > > > no need for register. > > Right, sorry. > > >> @@ -493,6 +495,11 @@ dump_symtab (struct objfile *objfile, st > >> fprintf_filtered (outfile, " under "); > >> gdb_print_host_address (BLOCK_SUPERBLOCK (b), outfile); > >> } > >> +#if 0 > >> + /* NOTE: carlton/2003-04-28: If we really want to be able to > >> + print out something here, we'll need to add an extra > >> + dictionary method just for that purpose. */ > >> + > > > Hmmm, I think we should. We don't want to change gdb's behavior. > > Well, it's for a maint command, so we can really do whatever we want > here. But if you think that's an important part of the info, I'll add > the extra method. > thanks yes. > I'll make those changes and commit it tomorrow afternoon (unless > exam grading takes longer than I expect), if nobody complains; and > I'll be spending all day Thursday doing GDB stuff as well, so I'll be > available if I accidentally break anything... > ok, great. BTW, i have started setting up the same test framework that Michael Chastain uses for his tests. I have done a preliminary run, but I screwed up the combinations of gcc and binutils to test with, so I'll get back to that tomorrow, and I should be able to help with testing for gdb6. elena > David Carlton > carlton@math.stanford.edu