This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch c,c++,i386]:PR/15774 - Conflicting function decls not diagnosed
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Kai Tietz <ktietz70 at googlemail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>, Richard Henderson <rth at redhat dot com>
- Date: Wed, 22 Dec 2010 16:31:02 +0000 (UTC)
- Subject: Re: [patch c,c++,i386]:PR/15774 - Conflicting function decls not diagnosed
- References: <AANLkTimKfTGX1VZ-UO5bhknCubqqgESu5WrS6z4F1Anz@mail.gmail.com>
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