pthread_equal

pthread_equal

[Jörg Krause]

Why does musl declares pthread_equal both as macro and as function?

Re: pthread_equal

[Rich Felker]

On Mon, Dec 08, 2014 at 03:42:25PM +0100, Jörg Krause wrote:
> Why does musl declares pthread_equal both as macro and as function?

C and POSIX allow any of their standard functions to be provided as
macros too, but the function definition must always be provided. The
reason I put the macro in musl is simply that it's easy to do and
gives better code (trivial inline comparison rather than spilling all
registers and making a function call) and it's not something where the
implementation could change or need to change.

Rich

Re: pthread_equal

[Jörg Krause]

On Mo, 2014-12-08 at 09:56 -0500, Rich Felker wrote:
> On Mon, Dec 08, 2014 at 03:42:25PM +0100, Jörg Krause wrote:
> > Why does musl declares pthread_equal both as macro and as function?
> 
> C and POSIX allow any of their standard functions to be provided as
> macros too, but the function definition must always be provided. The
> reason I put the macro in musl is simply that it's easy to do and
> gives better code (trivial inline comparison rather than spilling all
> registers and making a function call) and it's not something where the
> implementation could change or need to change.
> 
> Rich

I see! The problem was, that MPD (Music Player Daemon, implemented in C
++) for instance used ::pthread_equal(id, other_id) which did not build
with musl because of the macro expansion.

The maintainer removed the namespace operator to get it work with musl: 
http://git.musicpd.org/cgit/master/mpd.git/commit/?h=v0.18.x&id=d8fc2db910a11dbbba53ba7ecf96d0e32a081076

Jörg

Re: pthread_equal

[Rich Felker]

On Mon, Dec 08, 2014 at 05:18:41PM +0100, Jörg Krause wrote:
> On Mo, 2014-12-08 at 09:56 -0500, Rich Felker wrote:
> > On Mon, Dec 08, 2014 at 03:42:25PM +0100, Jörg Krause wrote:
> > > Why does musl declares pthread_equal both as macro and as function?
> > 
> > C and POSIX allow any of their standard functions to be provided as
> > macros too, but the function definition must always be provided. The
> > reason I put the macro in musl is simply that it's easy to do and
> > gives better code (trivial inline comparison rather than spilling all
> > registers and making a function call) and it's not something where the
> > implementation could change or need to change.
> 
> I see! The problem was, that MPD (Music Player Daemon, implemented in C
> ++) for instance used ::pthread_equal(id, other_id) which did not build
> with musl because of the macro expansion.
> 
> The maintainer removed the namespace operator to get it work with musl: 
> http://git.musicpd.org/cgit/master/mpd.git/commit/?h=v0.18.x&id=d8fc2db910a11dbbba53ba7ecf96d0e32a081076

I see.

For the standard C headers, the C++ versions are supposed to omit the
macros that the C versions might offer. However, there's no such rule
for POSIX headers since there's no formal spec for interaction of C++
and POSIX at all. Perhaps it would be useful to take the same approach
and suppress such macros if __cplusplus is defined, even in the POSIX
headers? But I think from an application portability perspective, they
should either use #undef or parens, i.e. (::pthread_equal)(id1,id2),
instead of assuming there is no macro.

Rich

Re: pthread_equal

[Jörg Krause]

On Mo, 2014-12-08 at 11:25 -0500, Rich Felker wrote:
> On Mon, Dec 08, 2014 at 05:18:41PM +0100, Jörg Krause wrote:
> > On Mo, 2014-12-08 at 09:56 -0500, Rich Felker wrote:
> > > On Mon, Dec 08, 2014 at 03:42:25PM +0100, Jörg Krause wrote:
> > > > Why does musl declares pthread_equal both as macro and as function?
> > > 
> > > C and POSIX allow any of their standard functions to be provided as
> > > macros too, but the function definition must always be provided. The
> > > reason I put the macro in musl is simply that it's easy to do and
> > > gives better code (trivial inline comparison rather than spilling all
> > > registers and making a function call) and it's not something where the
> > > implementation could change or need to change.
> > 
> > I see! The problem was, that MPD (Music Player Daemon, implemented in C
> > ++) for instance used ::pthread_equal(id, other_id) which did not build
> > with musl because of the macro expansion.
> > 
> > The maintainer removed the namespace operator to get it work with musl: 
> > http://git.musicpd.org/cgit/master/mpd.git/commit/?h=v0.18.x&id=d8fc2db910a11dbbba53ba7ecf96d0e32a081076
> 
> I see.
> 
> For the standard C headers, the C++ versions are supposed to omit the
> macros that the C versions might offer. However, there's no such rule
> for POSIX headers since there's no formal spec for interaction of C++
> and POSIX at all. Perhaps it would be useful to take the same approach
> and suppress such macros if __cplusplus is defined, even in the POSIX
> headers? But I think from an application portability perspective, they
> should either use #undef or parens, i.e. (::pthread_equal)(id1,id2),
> instead of assuming there is no macro.
> 
> Rich

From an application developer point of view I would look at the POSIX
specification which says pthread_equal is a function defined as: 
int pthread_equal(pthread_t t1, pthread_t t2)

I also proposed the solution with the parentesis. But in my opinion it
is a little bit confusing for an application developer to assume a
function specified by POSIX may be implemented as a macro.

Re: pthread_equal

[Rich Felker]

On Mon, Dec 08, 2014 at 10:11:49PM +0100, Jörg Krause wrote:
> On Mo, 2014-12-08 at 11:25 -0500, Rich Felker wrote:
> > On Mon, Dec 08, 2014 at 05:18:41PM +0100, Jörg Krause wrote:
> > > On Mo, 2014-12-08 at 09:56 -0500, Rich Felker wrote:
> > > > On Mon, Dec 08, 2014 at 03:42:25PM +0100, Jörg Krause wrote:
> > > > > Why does musl declares pthread_equal both as macro and as function?
> > > > 
> > > > C and POSIX allow any of their standard functions to be provided as
> > > > macros too, but the function definition must always be provided. The
> > > > reason I put the macro in musl is simply that it's easy to do and
> > > > gives better code (trivial inline comparison rather than spilling all
> > > > registers and making a function call) and it's not something where the
> > > > implementation could change or need to change.
> > > 
> > > I see! The problem was, that MPD (Music Player Daemon, implemented in C
> > > ++) for instance used ::pthread_equal(id, other_id) which did not build
> > > with musl because of the macro expansion.
> > > 
> > > The maintainer removed the namespace operator to get it work with musl: 
> > > http://git.musicpd.org/cgit/master/mpd.git/commit/?h=v0.18.x&id=d8fc2db910a11dbbba53ba7ecf96d0e32a081076
> > 
> > I see.
> > 
> > For the standard C headers, the C++ versions are supposed to omit the
> > macros that the C versions might offer. However, there's no such rule
> > for POSIX headers since there's no formal spec for interaction of C++
> > and POSIX at all. Perhaps it would be useful to take the same approach
> > and suppress such macros if __cplusplus is defined, even in the POSIX
> > headers? But I think from an application portability perspective, they
> > should either use #undef or parens, i.e. (::pthread_equal)(id1,id2),
> > instead of assuming there is no macro.
> 
> From an application developer point of view I would look at the POSIX
> specification which says pthread_equal is a function defined as: 
> int pthread_equal(pthread_t t1, pthread_t t2)

It's not specified on a per-header/per-function basis but globally, in
XSH 2.1.1 Use and Implementation of Functions, item 2:

    Any function declared in a header may also be implemented as a
    macro defined in the header, so a function should not be declared
    explicitly if its header is included. Any macro definition of a
    function can be suppressed locally by enclosing the name of the
    function in parentheses, because the name is then not followed by
    the <left-parenthesis> that indicates expansion of a macro
    function name. For the same syntactic reason, it is permitted to
    take the address of a function even if it is also defined as a
    macro. The use of the C-language #undef construct to remove any
    such macro definition shall also ensure that an actual function is
    referred to.

Source:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_01_01

This is analogous to what the C standard specifies for the standard C
functions.

> I also proposed the solution with the parentesis. But in my opinion it
> is a little bit confusing for an application developer to assume a
> function specified by POSIX may be implemented as a macro.

I agree however that this is counter-intuitive for C++ programmers, so
we should probably go through and suppress the macros for C++. There
probably aren't many to do anyway. We can probably get that changed in
this release cycle, especially if anyone is willing to help make a
list of them or a proposed patch to add #ifndef __cplusplus around
them.

Rich

[PATCH] don't shadow functions with macros in C++

[Bobby Bingham]

[-- Attachment #1: Type: text/plain, Size: 1947 bytes --]

C++ programmers typically expect something like "::function(x,y)" to work
and may be surprised to find that "(::function)(x,y)" is actually required
due to the headers declaring a macro version of some standard functions.

We already omit function-like macros for C++ in most cases where there is
a real function available. This commit extends this to the remaining
function-like macros which have a real function version.
---
 include/complex.h | 2 ++
 include/pthread.h | 2 ++
 include/threads.h | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/include/complex.h b/include/complex.h
index 13a45c5..e1af0d5 100644
--- a/include/complex.h
+++ b/include/complex.h
@@ -101,6 +101,7 @@ double creal(double complex);
 float crealf(float complex);
 long double creall(long double complex);

+#ifndef __cplusplus
 #define __CIMAG(x, t) \
 	((union { _Complex t __z; t __xy[2]; }){(_Complex t)(x)}.__xy[1])

@@ -111,6 +112,7 @@ long double creall(long double complex);
 #define cimag(x) __CIMAG(x, double)
 #define cimagf(x) __CIMAG(x, float)
 #define cimagl(x) __CIMAG(x, long double)
+#endif

 #define __CMPLX(x, y, t) \
 	((union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)
diff --git a/include/pthread.h b/include/pthread.h
index f7c9568..2697e8b 100644
--- a/include/pthread.h
+++ b/include/pthread.h
@@ -84,7 +84,9 @@ __attribute__((const))
 pthread_t pthread_self(void);

 int pthread_equal(pthread_t, pthread_t);
+#ifndef __cplusplus
 #define pthread_equal(x,y) ((x)==(y))
+#endif

 int pthread_setcancelstate(int, int *);
 int pthread_setcanceltype(int, int *);
diff --git a/include/threads.h b/include/threads.h
index 0e5836c..0179482 100644
--- a/include/threads.h
+++ b/include/threads.h
@@ -51,7 +51,9 @@ void thrd_yield(void);

 thrd_t thrd_current(void);
 int thrd_equal(thrd_t, thrd_t);
+#ifndef __cplusplus
 #define thrd_equal(A, B) ((A) == (B))
+#endif

 void call_once(once_flag *, void (*)(void));

--
Bobby Bingham

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] don't shadow functions with macros in C++

[Jörg Krause]

On Mo, 2014-12-08 at 20:18 -0600, Bobby Bingham wrote:
> C++ programmers typically expect something like "::function(x,y)" to work
> and may be surprised to find that "(::function)(x,y)" is actually required
> due to the headers declaring a macro version of some standard functions.
> 
> We already omit function-like macros for C++ in most cases where there is
> a real function available. This commit extends this to the remaining
> function-like macros which have a real function version.
> ---
>  include/complex.h | 2 ++
>  include/pthread.h | 2 ++
>  include/threads.h | 2 ++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/include/complex.h b/include/complex.h
> index 13a45c5..e1af0d5 100644
> --- a/include/complex.h
> +++ b/include/complex.h
> @@ -101,6 +101,7 @@ double creal(double complex);
>  float crealf(float complex);
>  long double creall(long double complex);
> 
> +#ifndef __cplusplus
>  #define __CIMAG(x, t) \
>  	((union { _Complex t __z; t __xy[2]; }){(_Complex t)(x)}.__xy[1])
> 
> @@ -111,6 +112,7 @@ long double creall(long double complex);
>  #define cimag(x) __CIMAG(x, double)
>  #define cimagf(x) __CIMAG(x, float)
>  #define cimagl(x) __CIMAG(x, long double)
> +#endif
> 
>  #define __CMPLX(x, y, t) \
>  	((union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)
> diff --git a/include/pthread.h b/include/pthread.h
> index f7c9568..2697e8b 100644
> --- a/include/pthread.h
> +++ b/include/pthread.h
> @@ -84,7 +84,9 @@ __attribute__((const))
>  pthread_t pthread_self(void);
> 
>  int pthread_equal(pthread_t, pthread_t);
> +#ifndef __cplusplus
>  #define pthread_equal(x,y) ((x)==(y))
> +#endif
> 
>  int pthread_setcancelstate(int, int *);
>  int pthread_setcanceltype(int, int *);
> diff --git a/include/threads.h b/include/threads.h
> index 0e5836c..0179482 100644
> --- a/include/threads.h
> +++ b/include/threads.h
> @@ -51,7 +51,9 @@ void thrd_yield(void);
> 
>  thrd_t thrd_current(void);
>  int thrd_equal(thrd_t, thrd_t);
> +#ifndef __cplusplus
>  #define thrd_equal(A, B) ((A) == (B))
> +#endif
> 
>  void call_once(once_flag *, void (*)(void));
> 
> --
> Bobby Bingham

Many thanks for the patch!

Reviewed-by: Jörg Krause <jkrause@posteo.de>

Re: [PATCH] don't shadow functions with macros in C++

[Rich Felker]

On Mon, Dec 08, 2014 at 08:18:12PM -0600, Bobby Bingham wrote:
> C++ programmers typically expect something like "::function(x,y)" to work
> and may be surprised to find that "(::function)(x,y)" is actually required
> due to the headers declaring a macro version of some standard functions.
> 
> We already omit function-like macros for C++ in most cases where there is
> a real function available. This commit extends this to the remaining
> function-like macros which have a real function version.
> ---

Thanks! Committed.

Rich