On 12/3/12, Diego Novillo <dnovi...@google.com> wrote:
> On Sat, Dec 1, 2012 at 8:47 PM, Lawrence Crowl <cr...@googlers.com> wrote:
>
>> +inline bool
>> +attribute_hasher::equal (const value_type *spec, const compare_type
>> *str)
>> +{
>> + return (!strncmp (spec->name, str->str, str->length)
>
> I have a slight preference for strncmp() == 0. It's easier to read (I
> realize that you are just cut-n-pasting old code here :)
Done.
>> @@ -76,13 +82,11 @@ bitmap_descriptor (const char *file, int
>> loc.function = function;
>> loc.line = line;
>>
>> - if (!bitmap_desc_hash)
>> - bitmap_desc_hash = htab_create (10, hash_descriptor, eq_descriptor,
>> NULL);
>> + if (!bitmap_desc_hash.is_created ())
>> + bitmap_desc_hash.create (10);
>>
>> - slot = (struct bitmap_descriptor **)
>> - htab_find_slot_with_hash (bitmap_desc_hash, &loc,
>> - htab_hash_pointer (file) + line,
>> - INSERT);
>> + slot = bitmap_desc_hash
>> + .find_slot_with_hash (&loc, htab_hash_pointer (file) + line,
>> INSERT);
>
> I'd rather split the argument list here.
Done, though it becomes three lines instead of two.
>
>> }
>>
>> -static int
>> -htab_cu_eq (const void *of1, const void *of2)
>> +inline bool
>> +cu_hash_table_entry_hasher::equal (const value_type *entry1,
>> + const compare_type *entry2)
>
> No static?
The in-class declaration has the static keyword. (It's still a
global symbol though, not local.)
>> -static hashval_t
>> -ttypes_filter_hash (const void *pentry)
>> +inline hashval_t
>> +ttypes_filter_hasher::hash (const value_type *entry)
>> {
>> - const struct ttypes_filter *entry = (const struct ttypes_filter *)
>> pentry;
>> return TREE_HASH (entry->t);
>
> The patch seems to have changed the types here. Is it value_type or
> ttypes_filter?
The hasher class defines value_type as a typedef to ttypes_filter.
There has been no change in actual type.
> Looks OK otherwise.
Okay, I'll put it in the commit queue.
--
Lawrence Crowl
