Re: [PATCH] For Adding clang check

[Jonathan Wakely <jwakely@redhat.com>]

On 29/11/19 09:25 +0100, Florian Weimer wrote:
>* kamlesh kumar:
>
>> It fixes this.
>> https://bugs.llvm.org/show_bug.cgi?id=44169
>
>What's the rationale for the condition?  What's special about Clang
>3.5?
>
>My understanding is that a compiler needs support for asm aliases
>*and* the C++ library headers need to be compatible.  Is there a way
>to determine if libc++ is compatible?

Libc++ already checks for this macro in its <string.h> wrapper:
https://github.com/llvm/llvm-project/blob/master/libcxx/include/string.h#L62

If the __CORRECT_ISO_CPP_STRING_H_PROTO macro is *not* defined after
doing #include_next <string.h> (to get the libc header) then libc++
makes use of a Clang extension to declare new overloads:
https://github.com/llvm/llvm-project/blob/master/libcxx/include/string.h#L70

The _LIBCPP_PREFERRED_OVERLOAD macro is defined as:
#    define _LIBCPP_PREFERRED_OVERLOAD __attribute__ ((__enable_if__(true, "")))

That Clang-only attribute means the compiler will use the new
overloads in preference to the libc strchr. A non-standard extension
is needed because according to the C++ rules the new overloads should
be ambiguous with the one that was declared by libc's <string.h>.


>For libstdc++ with GCC, the
>compiler version check covers libstdc++ implicity, but that does not
>apply to Clang, or libc++ with either compiler.

Libstdc++ with GCC already works.

Libstdc++ with Clang needs this patch.

If I'm reading the libc++ code right ...

Libc++ with GCC should already work, because the __GNUC_PREREQ will
pass and libc++ is already aware of the existence and effects of the
__CORRECT_ISO_CPP_STRING_H_PROTO macro. (I doubt libc++ works with
ancient GCC versions, but if it does they'll get the wrong signatures
... well tough luck, use a newer GCC).

Libc++ with Clang doesn't need this patch, because it uses the Clang
extension, but after this patch it would no longer need to use the
extension. The right signatures would be declared by glibc.

So of the four combinations, two already work and are unaffected by
this patch. One already works and is affected, but not in a way users
will notice (the correct signatures are already there, the patch just
changes whether they come from glibc or libc++). And one doesn't
currently work but is fixed by the patch.

I think the patch is right.