Re: [C++ PATCH] Fix FUNCTION_TYPE hashing problem (PR c++/33506, take 2)

[Mark Mitchell <mark at codesourcery dot com>]

> 2007-09-21  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/33506
> 	* langhooks.h (struct lang_hooks_for_types): Add type_hash_eq
> 	field.
> 	* langhooks.c (lhd_type_hash_eq): New function.
> 	* langhooks-def.h (lhd_type_hash_eq): New prototype.
> 	(LANG_HOOKS_TYPE_HASH_EQ): Define.
> 	(LANG_HOOKS_FOR_TYPES_INITIALIZER): Add LANG_HOOKS_TYPE_HASH_EQ.
> 	* tree.c (type_hash_eq): For FUNCTION_TYPE use
> 	lang_hooks.type.type_hash_eq in addition to generic tests.
> 
> 	* cp-tree.h (cxx_type_hash_eq): New prototype.
> 	* cp-objcp-common.h (LANG_HOOKS_TYPE_HASH_EQ): Redefine.
> 	* tree.c (cxx_type_hash_eq): New function.
> 
> 	* g++.dg/ext/attrib29.C: New test.

This looks good.  Thank you for working on this.  Picky bits below.

> +  /* Return TRUE if TYPE1 and TYPE2 are identical for type hashing purposes.
> +     Called only after doing all language independent checks.  */
> +  bool (*type_hash_eq) (const_tree, const_tree);

Please add a comment here to indicate that the hook is only called for
FUNCTION_TYPEs.  Perhaps:

  At present, this function is only called when both TYPE1 and TYPE2 are
  FUNCTION_TYPEs.

> +  return lang_hooks.types.type_hash_eq (a->type, b->type);

I suppose that if you want to micro-optimize, it might be better to have
the default be NULL, and test for non-NULL before calling here?

> +bool
> +cxx_type_hash_eq (const_tree typea, const_tree typeb)
> +{
> +  if (TREE_CODE (typea) != FUNCTION_TYPE
> +      || TYPE_RAISES_EXCEPTIONS (typea) == TYPE_RAISES_EXCEPTIONS (typeb))
> +    return true;
> +
> +  return comp_except_specs (TYPE_RAISES_EXCEPTIONS (typea),
> +			    TYPE_RAISES_EXCEPTIONS (typeb), 1);
> +}

Here, I would just assert that TYPEA is a FUNCTION_TYPE; we shouldn't
need to pay for the test at present.  Also, let's not compare
TYPE_RAISES_EXCEPTIONS for equality here; comp_except_specs already does
that.  If we think that optimization is important, we should wrap
comp_except_specs in a macro that does the check first so that all
callers benefit.

OK with those changes.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713