[PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/01/16 18:11, Jason Merrill wrote:
> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> On 11/01/16 16:20, Jason Merrill wrote:
>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>> 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.
>
> Would it, though?  Which test cases still break with the current patch?
>

Less than before, but there are still at least a few of them.

I can make a list and send it tomorrow.

>> 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.
>
> Right.  The -Wshadow warning is for file-local declarations, so that
> doesn't apply to your testcase; I was thinking that we should hit the
> first (currently unconditional) warning.
>
>>>>           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))))
>
> So I was thinking to drop the "else" and the compparms test.
>

Yes.  But then we must somehow avoid:

           else
             /* Discard the old built-in function.  */
             return NULL_TREE;

It maybe easier, just to copy the warning to the DECL_ANTICIPATED case?

>>>>             {
>>>>               /* 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);
>
> So we would hit this warning.  And change the message to remove
> "ambiguates", since we're removing the compparms.
>
>> 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.
>
> It could look at DECL_ASSEMBLER_NAME rather than DECL_NAME?
>

Yes.  I think previously special_function_p must have used the
DECL_ASSEMBLER_NAME, at least the comments still mention that.

Also for a "C" function there are target specific naming rules,
some prepend an underscore, some don't, and more, especially
W..DOS has special conventions IIRC.

Maybe a language callback would be good to have for this task.
So that special_function_p can easily check if something is a
C++ function, then it is immediately clear that it is not a
special function.



Bernd.