For Adding clang check

  ChangeLog :

2019-11-28  Kamlesh Kumar  <kamleshbhalui@gmail.com>

       * string/string.h (__glibc_clang_prereq): Used.
---
 string/string.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

* Kamlesh Kumar:

> ChangeLog :
>
> 2019-11-28  Kamlesh Kumar  <kamleshbhalui@gmail.com>
>
>        * string/string.h (__glibc_clang_prereq): Used.
> ---
>  string/string.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/string/string.h b/string/string.h
> index 73c22a535a..06a8977165 100644
> --- a/string/string.h
> +++ b/string/string.h
> @@ -33,7 +33,7 @@ __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

Sorry, what's the purpose of this change?  Thanks.

If it fixes a user-visible problem, it should reference a bug in
Bugzilla.
  

It fixes this.
https://bugs.llvm.org/show_bug.cgi?id=44169


On Fri, Nov 29, 2019 at 12:22 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * Kamlesh Kumar:
>
> > ChangeLog :
> >
> > 2019-11-28  Kamlesh Kumar  <kamleshbhalui@gmail.com>
> >
> >        * string/string.h (__glibc_clang_prereq): Used.
> > ---
> >  string/string.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/string/string.h b/string/string.h
> > index 73c22a535a..06a8977165 100644
> > --- a/string/string.h
> > +++ b/string/string.h
> > @@ -33,7 +33,7 @@ __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
>
> Sorry, what's the purpose of this change?  Thanks.
>
> If it fixes a user-visible problem, it should reference a bug in
> Bugzilla.
  

* 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?  For libstdc++ with GCC, the
compiler version check covers libstdc++ implicity, but that does not
apply to Clang, or libc++ with either compiler.

> On Fri, Nov 29, 2019 at 12:22 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> * Kamlesh Kumar:
>>
>> > ChangeLog :
>> >
>> > 2019-11-28  Kamlesh Kumar  <kamleshbhalui@gmail.com>
>> >
>> >        * string/string.h (__glibc_clang_prereq): Used.
>> > ---
>> >  string/string.h | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/string/string.h b/string/string.h
>> > index 73c22a535a..06a8977165 100644
>> > --- a/string/string.h
>> > +++ b/string/string.h
>> > @@ -33,7 +33,7 @@ __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
>>
>> Sorry, what's the purpose of this change?  Thanks.
>>
>> If it fixes a user-visible problem, it should reference a bug in
>> Bugzilla.
  

Hi Florian,
While compiling this (https://godbolt.org/z/uV2Pjp) c++ program with
clang (it uses libstdc++ by default)
static_assert fails.
that is because
this macro  __CORRECT_ISO_CPP_STRING_H_PROTO does not get defined when
clang is used, because it is like this in string.h

#if defined __cplusplus && __GNUC_PREREQ (4, 4)
# define __CORRECT_ISO_CPP_STRING_H_PROTO
#endif


And return type of strchr  in this case is char *, while we expect it
to be const char *.
That does not happen because latter declration is hidden by this macro
__CORRECT_ISO_CPP_STRING_H_PROTO.

I have used clang 3.5  here in check, because this is minimum version
which worked with libc++ and libstdc++.

,kamlesh

On Fri, Nov 29, 2019 at 1:55 PM Florian Weimer <fw@deneb.enyo.de> 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?  For libstdc++ with GCC, the
> compiler version check covers libstdc++ implicity, but that does not
> apply to Clang, or libc++ with either compiler.
>
> > On Fri, Nov 29, 2019 at 12:22 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>
> >> * Kamlesh Kumar:
> >>
> >> > ChangeLog :
> >> >
> >> > 2019-11-28  Kamlesh Kumar  <kamleshbhalui@gmail.com>
> >> >
> >> >        * string/string.h (__glibc_clang_prereq): Used.
> >> > ---
> >> >  string/string.h | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/string/string.h b/string/string.h
> >> > index 73c22a535a..06a8977165 100644
> >> > --- a/string/string.h
> >> > +++ b/string/string.h
> >> > @@ -33,7 +33,7 @@ __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
> >>
> >> Sorry, what's the purpose of this change?  Thanks.
> >>
> >> If it fixes a user-visible problem, it should reference a bug in
> >> Bugzilla.
  

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.