This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] For Adding clang check


* Florian Weimer:

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

I have pushed this.  Some libc++ people have been contacted and did not
object.

Thanks,
Florian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]