On 11/01/16 16:20, Jason Merrill wrote:
> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>> +@item -Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)}
>> +@opindex Wbuiltin-function-redefined
>> +@opindex Wno-builtin-function-redefined
>> +Do warn if built-in functions are redefined.  This option is only
>> +supported for C++ and Objective-C++.  It is implied by @option{-Wall},
>> +which can be disabled with @option{-Wno-builtin-function-redefined}.
>
> There's no redefinition here (only a redeclaration), so perhaps
> -Wbuiltin-redeclaration-mismatch?
>

Works for me.  Thanks.

> I'm not even sure we need a new warning.  Can we combine this warning
> with the block that currently follows?
>

After 20 years of not having a warning on that,
an implicitly enabled warning would at least break lots of bogus
test cases.  Of course in C we have an implicitly enabled warning,
so I would like to at least enable the warning on -Wall, thus
-Wshadow is too weak IMO.

>>           else if ((DECL_EXTERN_C_P (newdecl)
>>                     && DECL_EXTERN_C_P (olddecl))
>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>>             {
>>               /* A near match; override the builtin.  */
>>
>>               if (TREE_PUBLIC (newdecl))
>>                 warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>>                             "new declaration %q#D ambiguates built-in "
>>                             "declaration %q#D", newdecl, olddecl);
>>               else
>>                 warning (OPT_Wshadow,
>>                          DECL_BUILT_IN (olddecl)
>>                          ? G_("shadowing built-in function %q#D")
>>                          : G_("shadowing library function %q#D"),
>> olddecl);
>>             }
>
> It seems to me that if we have an extern "C" declaration that doesn't
> match the built-in, we should just warn.
>
> I looked at some of the testcases you mentioned in the bug report, and
> those declarations aren't extern "C", so we shouldn't be warning about
> them.  Does your current patch still warn about those?
>

No.  After looking at the false positives with the previous patch,
I changed my mind about that.

This started because I wanted to add builtin functions for some
special_function_p names.  And I wanted to warn the user if he uses a
name that is recognized by special_function_p, but the parameters
don't match.  Now I think we need to teach special_function_p to
distinguish "C" functions from "C++" functions, which it currently
cannot do, because that knowledge is only in the C++ FE.


Bernd.

Reply via email to