[PATCH] Convert some is* macros to inline functions

[PATCH] Convert some is* macros to inline functions

[Sergey Dmitrouk]

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

Hello,

eight is* functions of C standard library are also declared in form of
macros in headers provided by musl.  Standard doesn't seem to allow them
to be implemented as macros.  Please find the attached patch that proposes
replacing these macros with inline functions.

Best regards,
Sergey

[-- Attachment #2: musl-is-macros-to-inline-funcs.patch --]
[-- Type: text/plain, Size: 6375 bytes --]

diff --git a/include/ctype.h b/include/ctype.h
index a6f44df..7f8d98d 100644
--- a/include/ctype.h
+++ b/include/ctype.h
@@ -8,16 +8,9 @@ extern "C" {
 #include <features.h>
 
 int   isalnum(int);
-int   isalpha(int);
 int   isblank(int);
 int   iscntrl(int);
-int   isdigit(int);
-int   isgraph(int);
-int   islower(int);
-int   isprint(int);
 int   ispunct(int);
-int   isspace(int);
-int   isupper(int);
 int   isxdigit(int);
 int   tolower(int);
 int   toupper(int);
@@ -27,14 +20,40 @@ static __inline int __isspace(int _c)
 	return _c == ' ' || (unsigned)_c-'\t' < 5;
 }
 
-#define isalpha(a) ((((unsigned)(a)|32)-'a') < 26)
-#define isdigit(a) (((unsigned)(a)-'0') < 10)
-#define islower(a) (((unsigned)(a)-'a') < 26)
-#define isupper(a) (((unsigned)(a)-'A') < 26)
-#define isprint(a) (((unsigned)(a)-0x20) < 0x5f)
-#define isgraph(a) (((unsigned)(a)-0x21) < 0x5e)
-#define isspace(a) __isspace(a)
+__inline int isalpha(int _c)
+{
+	return (((unsigned)_c | 32) - 'a') < 26;
+}
+
+__inline int isdigit(int _c)
+{
+	return ((unsigned)_c - '0') < 10;
+}
 
+__inline int islower(int _c)
+{
+	return ((unsigned)_c - 'a') < 26;
+}
+
+__inline int isupper(int _c)
+{
+	return ((unsigned)_c - 'A') < 26;
+}
+
+__inline int isprint(int _c)
+{
+	return ((unsigned)_c - 0x20) < 0x5f;
+}
+
+__inline int isgraph(int _c)
+{
+	return ((unsigned)_c - 0x21) < 0x5e;
+}
+
+__inline int isspace(int _c)
+{
+	return __isspace(_c);
+}
 
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
  || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) \
diff --git a/include/wchar.h b/include/wchar.h
index 9fd967c..3033df8 100644
--- a/include/wchar.h
+++ b/include/wchar.h
@@ -138,8 +138,6 @@ wint_t ungetwc (wint_t, FILE *);
 struct tm;
 size_t wcsftime (wchar_t *__restrict, size_t, const wchar_t *__restrict, const struct tm *__restrict);
 
-#undef iswdigit
-
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
  || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE)  || defined(_BSD_SOURCE)
 FILE *open_wmemstream(wchar_t **, size_t *);
@@ -164,7 +162,6 @@ int       iswalnum(wint_t);
 int       iswalpha(wint_t);
 int       iswblank(wint_t);
 int       iswcntrl(wint_t);
-int       iswdigit(wint_t);
 int       iswgraph(wint_t);
 int       iswlower(wint_t);
 int       iswprint(wint_t);
@@ -176,8 +173,15 @@ int       iswctype(wint_t, wctype_t);
 wint_t    towlower(wint_t);
 wint_t    towupper(wint_t);
 wctype_t  wctype(const char *);
-#undef iswdigit
-#define iswdigit(a) ((unsigned)(a)-'0' < 10)
+
+#ifndef __iswdigit_defined
+#define __iswdigit_defined
+__inline int iswdigit(wint_t _c)
+{
+	return ((unsigned)_c - L'0') < 10;
+}
+#endif
+
 #endif
 
 #ifdef __cplusplus
diff --git a/include/wctype.h b/include/wctype.h
index 3ac24f1..3747c87 100644
--- a/include/wctype.h
+++ b/include/wctype.h
@@ -22,13 +22,10 @@ typedef const int * wctrans_t;
 #undef WEOF
 #define WEOF 0xffffffffU
 
-#undef iswdigit
-
 int       iswalnum(wint_t);
 int       iswalpha(wint_t);
 int       iswblank(wint_t);
 int       iswcntrl(wint_t);
-int       iswdigit(wint_t);
 int       iswgraph(wint_t);
 int       iswlower(wint_t);
 int       iswprint(wint_t);
@@ -43,8 +40,13 @@ wint_t    towupper(wint_t);
 wctrans_t wctrans(const char *);
 wctype_t  wctype(const char *);
 
-#undef iswdigit
-#define iswdigit(a) (((unsigned)(a)-L'0') < 10)
+#ifndef __iswdigit_defined
+#define __iswdigit_defined
+__inline int iswdigit(wint_t _c)
+{
+	return ((unsigned)_c - L'0') < 10;
+}
+#endif
 
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
  || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
diff --git a/src/ctype/isalpha.c b/src/ctype/isalpha.c
index f155d3a..b5403d6 100644
--- a/src/ctype/isalpha.c
+++ b/src/ctype/isalpha.c
@@ -1,11 +1,7 @@
 #include <ctype.h>
 #include "libc.h"
-#undef isalpha
 
-int isalpha(int c)
-{
-	return ((unsigned)c|32)-'a' < 26;
-}
+extern inline int isalpha(int);
 
 int __isalpha_l(int c, locale_t l)
 {
diff --git a/src/ctype/isdigit.c b/src/ctype/isdigit.c
index 4d8a103..b4e58d2 100644
--- a/src/ctype/isdigit.c
+++ b/src/ctype/isdigit.c
@@ -1,11 +1,7 @@
 #include <ctype.h>
 #include "libc.h"
-#undef isdigit
 
-int isdigit(int c)
-{
-	return (unsigned)c-'0' < 10;
-}
+extern inline int isdigit(int);
 
 int __isdigit_l(int c, locale_t l)
 {
diff --git a/src/ctype/isgraph.c b/src/ctype/isgraph.c
index a0aae08..b831164 100644
--- a/src/ctype/isgraph.c
+++ b/src/ctype/isgraph.c
@@ -1,11 +1,7 @@
 #include <ctype.h>
 #include "libc.h"
-#undef isgraph
 
-int isgraph(int c)
-{
-	return (unsigned)c-0x21 < 0x5e;
-}
+extern inline int isgraph(int);
 
 int __isgraph_l(int c, locale_t l)
 {
diff --git a/src/ctype/islower.c b/src/ctype/islower.c
index 0264021..3e3dde1 100644
--- a/src/ctype/islower.c
+++ b/src/ctype/islower.c
@@ -1,11 +1,7 @@
 #include <ctype.h>
 #include "libc.h"
-#undef islower
 
-int islower(int c)
-{
-	return (unsigned)c-'a' < 26;
-}
+extern inline int islower(int);
 
 int __islower_l(int c, locale_t l)
 {
diff --git a/src/ctype/isprint.c b/src/ctype/isprint.c
index 067275f..25dd9fc 100644
--- a/src/ctype/isprint.c
+++ b/src/ctype/isprint.c
@@ -1,11 +1,7 @@
 #include <ctype.h>
 #include "libc.h"
-#undef isprint
 
-int isprint(int c)
-{
-	return (unsigned)c-0x20 < 0x5f;
-}
+extern inline int isprint(int);
 
 int __isprint_l(int c, locale_t l)
 {
diff --git a/src/ctype/isspace.c b/src/ctype/isspace.c
index 231e907..86614a3 100644
--- a/src/ctype/isspace.c
+++ b/src/ctype/isspace.c
@@ -1,11 +1,7 @@
 #include <ctype.h>
 #include "libc.h"
-#undef isspace
 
-int isspace(int c)
-{
-	return c == ' ' || (unsigned)c-'\t' < 5;
-}
+extern inline int isspace(int);
 
 int __isspace_l(int c, locale_t l)
 {
diff --git a/src/ctype/isupper.c b/src/ctype/isupper.c
index 68c36f4..ef808e4 100644
--- a/src/ctype/isupper.c
+++ b/src/ctype/isupper.c
@@ -1,11 +1,7 @@
 #include <ctype.h>
 #include "libc.h"
-#undef isupper
 
-int isupper(int c)
-{
-	return (unsigned)c-'A' < 26;
-}
+extern inline int isupper(int);
 
 int __isupper_l(int c, locale_t l)
 {
diff --git a/src/ctype/iswdigit.c b/src/ctype/iswdigit.c
index ed9a88e..550d6e1 100644
--- a/src/ctype/iswdigit.c
+++ b/src/ctype/iswdigit.c
@@ -1,12 +1,7 @@
 #include <wctype.h>
 #include "libc.h"
 
-#undef iswdigit
-
-int iswdigit(wint_t wc)
-{
-	return (unsigned)wc-'0' < 10;
-}
+extern inline int iswdigit(wint_t);
 
 int __iswdigit_l(wint_t c, locale_t l)
 {

Re: [PATCH] Convert some is* macros to inline functions

[Jens Gustedt]

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

Am Montag, den 13.10.2014, 17:20 +0300 schrieb Sergey Dmitrouk:
> Hello,
> 
> eight is* functions of C standard library are also declared in form of
> macros in headers provided by musl.  Standard doesn't seem to allow them
> to be implemented as macros.

why would that be?

In section 7.1.4 the C standard explicitly says:

  > Any function declared in a header may be additionally implemented
  > as a function-like macro defined in the header, ...


> Please find the attached patch that proposes replacing these macros
> with inline functions.

I don't think that this is necessary.

They only advantage of inline functions, here would be that the
conversion of the arguments would be done with implicit conversions
instead of casts. (For the macros this could be achieved by using
compound literals instead of casts, but well...)

Jens


-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Convert some is* macros to inline functions

[Sergey Dmitrouk]

On Mon, Oct 13, 2014 at 07:35:14AM -0700, Jens Gustedt wrote:
> Am Montag, den 13.10.2014, 17:20 +0300 schrieb Sergey Dmitrouk:
> In section 7.1.4 the C standard explicitly says:
>
>   > Any function declared in a header may be additionally implemented
>   > as a function-like macro defined in the header, ...

Didn't look there, you're right.  I was checking description of headers
instead.

> > Please find the attached patch that proposes replacing these macros
> > with inline functions.
>
> I don't think that this is necessary.
>
> They only advantage of inline functions, here would be that the
> conversion of the arguments would be done with implicit conversions
> instead of casts. (For the macros this could be achieved by using
> compound literals instead of casts, but well...)

It's not necessary for C (as I know now), but it's required by С++
standard in 17.6.1.2:

 > Names that are defined as functions in C shall be defined as functions
 > in the C++ standard library. 175)

 > 175) This disallows the practice, allowed in C, of providing a masking
 > macro in addition to the function prototype. The only way to achieve
 > equivalent inline behavior in C++ is to provide a definition as an
 > extern inline function.

Current headers do not conform to C++ when included as <header.h>, and
that's what I'm trying to fix.

Would you consider a version that uses inline functions only when
__cplusplus is defined?  There is already 'extern "C"', so I guess it
makes sense.

Regards,
Sergey

Re: [PATCH] Convert some is* macros to inline functions

[Rich Felker]

On Mon, Oct 13, 2014 at 09:00:56PM +0300, Sergey Dmitrouk wrote:
> On Mon, Oct 13, 2014 at 07:35:14AM -0700, Jens Gustedt wrote:
> > Am Montag, den 13.10.2014, 17:20 +0300 schrieb Sergey Dmitrouk:
> > In section 7.1.4 the C standard explicitly says:
> >
> >   > Any function declared in a header may be additionally implemented
> >   > as a function-like macro defined in the header, ...
> 
> Didn't look there, you're right.  I was checking description of headers
> instead.
> 
> > > Please find the attached patch that proposes replacing these macros
> > > with inline functions.
> >
> > I don't think that this is necessary.
> >
> > They only advantage of inline functions, here would be that the
> > conversion of the arguments would be done with implicit conversions
> > instead of casts. (For the macros this could be achieved by using
> > compound literals instead of casts, but well...)
> 
> It's not necessary for C (as I know now), but it's required by С++
> standard in 17.6.1.2:
> 
>  > Names that are defined as functions in C shall be defined as functions
>  > in the C++ standard library. 175)
> 
>  > 175) This disallows the practice, allowed in C, of providing a masking
>  > macro in addition to the function prototype. The only way to achieve
>  > equivalent inline behavior in C++ is to provide a definition as an
>  > extern inline function.
> 
> Current headers do not conform to C++ when included as <header.h>, and
> that's what I'm trying to fix.
> 
> Would you consider a version that uses inline functions only when
> __cplusplus is defined?  There is already 'extern "C"', so I guess it
> makes sense.

Suppressing the macros for C++ would be acceptable if this is required
(it's not clear to me; my understanding was that it's only required
for the <c_____> headers, not the <______.h> ones, and that the
wrappers for the former already take care of removing the macros). The
external inline functions are probably not acceptable; there are all
sorts of ugly issues with extern inline function support that I don't
want to deal with.

Rich

Re: [PATCH] Convert some is* macros to inline functions

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2014-10-13 14:06:22 -0400]:
> On Mon, Oct 13, 2014 at 09:00:56PM +0300, Sergey Dmitrouk wrote:
> > Current headers do not conform to C++ when included as <header.h>, and
> > that's what I'm trying to fix.
> > 
> > Would you consider a version that uses inline functions only when
> > __cplusplus is defined?  There is already 'extern "C"', so I guess it
> > makes sense.
> 
> Suppressing the macros for C++ would be acceptable if this is required
> (it's not clear to me; my understanding was that it's only required
> for the <c_____> headers, not the <______.h> ones, and that the
> wrappers for the former already take care of removing the macros). The
> external inline functions are probably not acceptable; there are all
> sorts of ugly issues with extern inline function support that I don't
> want to deal with.

only providig the macros #ifndef __cplusplus makes sense

(but only do it in the 26 c headers the c++ standard talks about
posix allows macro definitions for all functions in posix headers)

the c++ standard could be more explicit about this incompatibility
with c: it states the requirement in a note for the <cname> headers
and then defines the semantics for the <name.h> headers in terms of
the <cname> one


c++14 17.6.1.2 [headers]

4   Except as noted in Clauses 18 through 30 and Annex D, the contents
    of each header cname shall be the same as that of the corresponding
    header name.h, as specified in the C standard library (1.2) or the
    C Unicode TR, as appropriate, as if by inclusion. In the C++
    standard library, however, the declarations (except for names which
    are defined as macros in C) are within namespace scope (3.3.6) of
    the namespace std. It is unspecified whether these names are first
    declared within the global namespace scope and are then injected
    into namespace std by explicit using-declarations (7.3.3).
5   Names which are defined as macros in C shall be defined as macros
    in the C++ standard library, even if C grants license for
    implementation as functions. [ Note: The names defined as macros in
    C include the following: assert, offsetof, setjmp, va_arg, va_end,
    and va_start. -- end note ]
6   Names that are defined as functions in C shall be defined as
    functions in the C++ standard library.^175
7   Identifiers that are keywords or operators in C++ shall not be
    defined as macros in C++ standard library headers.^176
8   D.5, C standard library headers, describes the effects of using the
    name.h (C header) form in a C++ program.^177

    175) This disallows the practice, allowed in C, of providing a
    masking macro in addition to the function prototype. The only way
    to achieve equivalent inline behavior in C++ is to provide a
    definition as an extern inline function.
    176) In particular, including the standard header <iso646.h> or
    <ciso646> has no effect.
    177) The ".h" headers dump all their names into the global
    namespace, whereas the newer forms keep their names in namespace
    std. Therefore, the newer forms are the preferred forms for all
    uses except for C++ programs which are intended to be strictly
    compatible with C.

...

c++14 D.5 [depr.c.headers]

2   Every C header, each of which has a name of the form name.h,
    behaves as if each name placed in the standard library namespace by
    the corresponding cname header is placed within the global
    namespace scope. It is unspecified whether these names are first
    declared or defined within namespace scope (3.3.6) of the namespace
    std and are then injected into the global namespace scope by
    explicit using-declarations (7.3.3).

Re: [PATCH] Convert some is* macros to inline functions

[Sergey Dmitrouk]

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

On Mon, Oct 13, 2014 at 11:49:56AM -0700, Szabolcs Nagy wrote:
> the c++ standard could be more explicit about this incompatibility
> with c: it states the requirement in a note for the <cname> headers
> and then defines the semantics for the <name.h> headers in terms of
> the <cname> one

Sad, but true.  I wish it was stated better.

 > The only way to achieve equivalent inline behavior in C++ is to
 > provide a definition as an extern inline function.

Seems to be true, so newlib and glibc have checks for __cplusplus define.
Alternative would be to provide 26 fake headers containing #include_next
directive, but it's not very portable solution.

Attached is the patch, which simply adds #ifndef __cplusplus around
macros.  At the end, it seems to be the right thing to do, although the
way it's defined is rather confusing.

Thanks,
Sergey

[-- Attachment #2: musl-hide-is-macros-for-cxx.patch --]
[-- Type: text/plain, Size: 1523 bytes --]

diff --git a/include/ctype.h b/include/ctype.h
index a6f44df..eef574b 100644
--- a/include/ctype.h
+++ b/include/ctype.h
@@ -27,6 +27,7 @@ static __inline int __isspace(int _c)
 	return _c == ' ' || (unsigned)_c-'\t' < 5;
 }
 
+#ifndef __cplusplus
 #define isalpha(a) ((((unsigned)(a)|32)-'a') < 26)
 #define isdigit(a) (((unsigned)(a)-'0') < 10)
 #define islower(a) (((unsigned)(a)-'a') < 26)
@@ -34,6 +35,7 @@ static __inline int __isspace(int _c)
 #define isprint(a) (((unsigned)(a)-0x20) < 0x5f)
 #define isgraph(a) (((unsigned)(a)-0x21) < 0x5e)
 #define isspace(a) __isspace(a)
+#endif
 
 
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
diff --git a/include/wchar.h b/include/wchar.h
index 9fd967c..8cfb584 100644
--- a/include/wchar.h
+++ b/include/wchar.h
@@ -176,9 +176,12 @@ int       iswctype(wint_t, wctype_t);
 wint_t    towlower(wint_t);
 wint_t    towupper(wint_t);
 wctype_t  wctype(const char *);
+
+#ifndef __cplusplus
 #undef iswdigit
 #define iswdigit(a) ((unsigned)(a)-'0' < 10)
 #endif
+#endif
 
 #ifdef __cplusplus
 }
diff --git a/include/wctype.h b/include/wctype.h
index 3ac24f1..3da1219 100644
--- a/include/wctype.h
+++ b/include/wctype.h
@@ -43,8 +43,10 @@ wint_t    towupper(wint_t);
 wctrans_t wctrans(const char *);
 wctype_t  wctype(const char *);
 
+#ifndef __cplusplus
 #undef iswdigit
 #define iswdigit(a) (((unsigned)(a)-L'0') < 10)
+#endif
 
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
  || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)

Re: [PATCH] Convert some is* macros to inline functions

[Rich Felker]

On Tue, Oct 14, 2014 at 12:01:08PM +0300, Sergey Dmitrouk wrote:
> On Mon, Oct 13, 2014 at 11:49:56AM -0700, Szabolcs Nagy wrote:
> > the c++ standard could be more explicit about this incompatibility
> > with c: it states the requirement in a note for the <cname> headers
> > and then defines the semantics for the <name.h> headers in terms of
> > the <cname> one
> 
> Sad, but true.  I wish it was stated better.
> 
>  > The only way to achieve equivalent inline behavior in C++ is to
>  > provide a definition as an extern inline function.
> 
> Seems to be true, so newlib and glibc have checks for __cplusplus define.
> Alternative would be to provide 26 fake headers containing #include_next
> directive, but it's not very portable solution.
> 
> Attached is the patch, which simply adds #ifndef __cplusplus around
> macros.  At the end, it seems to be the right thing to do, although the
> way it's defined is rather confusing.

Thanks. Committed with minor change (moved the inline function
__isspace, which is unused without the macro, inside of the #ifndef).

Rich