[patch c,c++,i386]:PR/15774 - Conflicting function decls not diagnosed

[Joseph S. Myers]

On Wed, 22 Dec 2010, Kai Tietz wrote:

> Hi,  this patch fixes the bug described at
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15774 in bugzilla.

Could you please explain more about how and why it fixes the bug?  The bug 
described there is:

    The C++ frontend doesn't always call targetm.compare_type_attributes
    when comparing function decls in decl.c: decls_match.  As a result
    conflicting decls are not diagnosed.

But your patch doesn't change the compatibility testing at all; it just 
changes how things are printed.  Is there code somewhere that generates 
strings for type names and compares those strings to determine 
compatibility?  If so, where?  If not, how does this patch fix the bug?  
In any case, the patch does not include testcases, and failure to diagnose 
invalid code, or bad diagnostics for such code, are exactly the sort of 
things that should be easy to test in the testsuite, so it's not OK 
without testcases or proper explanation of why they are hard, and should 
not have been approved without such testcases.

For compatibility testing, targetm.comp_type_attributes is the right thing 
to use; no new hook should be needed.  For printing, I don't see the need 
for a new hook either.  Why is it appropriate to print only attributes 
that pass the new hook?  Shouldn't you rather be printing all attributes, 
regardless of whether they affect the calling convention, since they are 
all part of the type in GNU terms?

If it is appropriate to condition printing on that condition, it would 
seem that you should share code between the compatibility testing 
(ix86_comp_type_attributes) and your new hook rather than duplicating 
lists of attributes with type compatibility issues.  Why does this patch 
not share code like that?

-- 
Joseph S. Myers
joseph@codesourcery.com