Re: [rfa] struct dictionary

[David Carlton <carlton at math dot stanford dot edu>]

On Mon, 9 Jun 2003 18:45:21 -0400, Elena Zannoni <ezannoni@redhat.com> 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.

>> @@ -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.

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...

David Carlton
carlton@math.stanford.edu