Re: [PATCH] For Adding clang check

[Florian Weimer <fweimer at redhat dot com>]

* Jonathan Wakely:

> 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.

Thanks.  I filed: <https://sourceware.org/bugzilla/show_bug.cgi?id=25232>

I tried to distill the discussion into the patch below.

Florian

8<------------------------------------------------------------------8<
From: Kamlesh Kumar <kamleshbhalui@gmail.com>
Subject: <string.h>: Define __CORRECT_ISO_CPP_STRING_H_PROTO for Clang [BZ #25232]

Without the asm redirects, strchr et al. are not const-correct.

libc++ has a wrapper header that works with and without
__CORRECT_ISO_CPP_STRING_H_PROTO (using a Clang extension).  But when
Clang is used with libstdc++ or just C headers, the overloaded functions
with the correct types are not declared.

This change does not impact current GCC (with libstdc++ or libc++).

-----
 string/string.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/string/string.h b/string/string.h
index 73c22a535a..faf997b972 100644
--- a/string/string.h
+++ b/string/string.h
@@ -33,7 +33,8 @@ __BEGIN_DECLS
 #include <stddef.h>
 
 /* Tell the caller that we provide correct C++ prototypes.  */
-#if defined __cplusplus && __GNUC_PREREQ (4, 4)
+#if defined __cplusplus && (__GNUC_PREREQ (4, 4) \
+			    || __glibc_clang_prereq (3, 5))
 # define __CORRECT_ISO_CPP_STRING_H_PROTO
 #endif