This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] For Adding clang check
- From: Florian Weimer <fweimer at redhat dot com>
- To: Jonathan Wakely <jwakely at redhat dot com>
- Cc: kamlesh kumar <kamleshbhalui at gmail dot com>, libc-alpha at sourceware dot org, Andreas Schwab <schwab at suse dot de>, Brooks Moses <bmoses at google dot com>
- Date: Thu, 05 Dec 2019 16:51:00 +0100
- Subject: Re: [PATCH] For Adding clang check
- References: <20191128184733.27377-1-kamleshbhalui@gmail.com> <87tv6n7s0t.fsf@mid.deneb.enyo.de> <CABKRkgj80BU+yuJRrvb0WTPYexKT4NDBj38dwTZqc_Xm27v+_Q@mail.gmail.com> <87a78fay23.fsf@mid.deneb.enyo.de> <20191129105759.GX11522@redhat.com> <87fti63obg.fsf@oldenburg2.str.redhat.com>
* 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