This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


2010/12/22 Joseph S. Myers <joseph@codesourcery.com>:
> On Wed, 22 Dec 2010, Kai Tietz wrote:
>
>> Well, a test is in PR, I can one explicit here, nevertheless we should
>> have for this already enough tests in testsuite.
>
> What existing test in the testsuite fails before the patch and passes
> after it, then?
>
> Among other things, the testsuite should verify that previous bugs do not
> reappear. ?That means that if you don't fix an existing failing test, you
> should add a new one, that fails without the patch applied but passes once
> it is applied. ?And there should be tests covering as much of the patch as
> possible; if you remove the new handling for any one of the attributes
> there, that should cause a test failure.
>
>> For the rest of your reply, I can see a relation to the subject of the
>> patch (and PR).
>
> Then please answer my points. ?How does your patch cause the results on a
> testcase to change from "no diagnostic" to "diagnostic"? ?(If it doesn't
> do so, please state the exact testcase, what the diagnostics are before
> the patch, what they are afterwards, and why the new version is superior.)
> Why do you need a new hook at all? ?Why are you duplicating code already
> present in the x86 back end that knows about what attributes are ABI
> attributes relevant to compatibility?
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>

As this patch fixes a bug (as describe in the PR) in diagnostic output
It doesn't change anything on the diagnostic logic of C, or C++.  As
we usually (and this is a good habit) not checking the type output in
testsuite, I expect here no regression. Nevertheless I can add one
explicit checking complete error/warning message for C.
ABI attributes changing calling-convention. This makes asignments of
function pointers declaring different calling conventions (like for
i386 regparm, stdcall, fastcall, thiscall, cdecl, sysv_abi, ms_abi)
incompatible. This is (or should be) diagnosed by FE, as it is
already. But the display of such a warning/error lacks those attribute
information, why check failed.

See following example:
t.c:
int foo(void);
int bar(int (__attribute__ ((regparm(3))) *arg)(void));
int doo(void)
{
  return bar(foo);
}

and simply compile it by gcc -c t.c

You will see that the warning message produced looks like that
(without the patch):
t.c: In function 'doo':
t.c:5:3: warning: passing argument 1 of 'bar' from incompatible
pointer type [enabled by default]
t.c:2:5: note: expected 'int (*)(void)' but argument is of type 'int (*)(void)'

The patch I sent simply makes displayed message understandable why
'int (*)(void)' isn't 'int (*)(void)' by displaying the callabi
attributes, which are actual the cause for the warning.

Regards,
Kai

-- 
|? (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]