Re: [C++ PATCH] Don't clear DECL_BUILT_IN_CLASS/DECL_FUNCTION_CODE when redeclaring a builtin if types match

[Mark Mitchell <mark@codesourcery.com>]

Jakub Jelinek wrote:

>>> The C FE always keeps DECL_BUILT_IN_CLASS when types match, even when
>>> newdecl defines a function.
>>> But the C++ FE doesn't do this:
>>>       if (new_defines_function)
>> The standard does prohibit redefining these functions.  But, in
>> practice, people would be surprised if they did this:
>>
>> /* Nobody should use memcpy, I think it's evil!  */
>> extern "C" void memcpy (void *, void *, size_t) { abort (); }
>>
>> void f() {
>>   memcpy (...);
>> }
> 
> If you do this, then you really want to use -fno-builtin-memcpy.
> Clearing DECL_BUILT_IN_CLASS in duplicate_decls will just mean
> that the behavior will be different in CUs where you define this
> non-conforming memcpy and other CUs linked into your program.

Yes, you may want to use -fno-builtin-memcpy in other translation units.
 But, it certainly seems likely to lead to user confusion if overriding
a standard library function (let's not call these "builtins" --
__builtin_memcpy is a "builtin", but "memcpy" is a standard library
function) is ignored by the compiler.

One of the things I'm concerned about is ability for users to bring code
to GCC from other compilers.  I'm certain that code exists that
overrides definitions of standard library functions.  Your suggestion
would make the compiler ignore the plain meaning of the user's code and
I think that's a bad thing.

> As I said in the mail, I'm ATM primarily not concerned about performance
> - glibc always uses always_inline gnu_inline functions for this stuff - but
> that adding an __asm ("...") redirect to the function will no longer result
> in the needed behavior - it will not have any effect on the __builtin_*
> functions.

We could argue about how GLIBC could best implement its checking code,
but that's GLIBC's business. :-)

I do agree that the call to __builtin_snprintf  in your builtin10.C
example should end up calling mysnprintf, if not expanded inline by the
compiler because the semantics of __builtin_x is that if it cannot
expand inline, then it calls the function whose *source* name is "x" --
and whose assembler name is whatever assembler name we would normally
associate with "x".  But, that doesn't mean that a user declaration of
"x" should be marked as builtin.  Instead, I think it means that the
code that expands builtins needs to be able to find the function with
the matching source name, and use that to find the corresponding
assembly name.  Perhaps when the user declares a replacement for a
standard library routine we need to update a table mapping builtins to
FUNCTION_DECLs.

I also agree that the C and C++ front-ends should be consistent in their
handling of this situation.  That's more important than whatever
semantic decision we end up making.  So, I think the C and C++
maintainers need to get together and see if we can get consensus on what
semantics we want.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713