Discussion:
[PATCH] wcwidth: check a macro version of wcwidth () as well
(too old to reply)
KO Myung-Hun
2017-10-06 15:36:49 UTC
Permalink
Raw Message
Check the functionality of a macro version wcwidth () as well as a
real function.

This is better than undefining wcwidth on OS/2 kLIBC without functional
check.

* lib/wchar.in.h: Revert commit caee51.
* m4/wcwidth.m4 (gl_cv_func_wcwidth_macro): Check if wcwidth () is a
macro.
---
lib/wchar.in.h | 7 +------
m4/wcwidth.m4 | 13 ++++++++++++-
2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/wchar.in.h b/lib/wchar.in.h
index c65cc6668..3307e235e 100644
--- a/lib/wchar.in.h
+++ b/lib/wchar.in.h
@@ -31,7 +31,7 @@
@PRAGMA_COLUMNS@

#if (((defined __need_mbstate_t || defined __need_wint_t) \
- && !defined __MINGW32__ && !defined __KLIBC__) \
+ && !defined __MINGW32__) \
|| (defined __hpux \
&& ((defined _INTTYPES_INCLUDED && !defined strtoimax) \
|| defined _GL_JUST_INCLUDE_SYSTEM_WCHAR_H)) \
@@ -452,11 +452,6 @@ _GL_CXXALIAS_RPL (wcwidth, int, (wchar_t));
# if !@HAVE_DECL_WCWIDTH@
/* wcwidth exists but is not declared. */
_GL_FUNCDECL_SYS (wcwidth, int, (wchar_t) _GL_ATTRIBUTE_PURE);
-# elif defined __KLIBC__
-/* On OS/2 kLIBC, wcwidth is a macro that expands to the name of a
- static inline function. The implementation of wcwidth in wcwidth.c
- causes a "conflicting types" error. */
-# undef wcwidth
# endif
_GL_CXXALIAS_SYS (wcwidth, int, (wchar_t));
# endif
diff --git a/m4/wcwidth.m4 b/m4/wcwidth.m4
index 58716416c..decceec8c 100644
--- a/m4/wcwidth.m4
+++ b/m4/wcwidth.m4
@@ -34,7 +34,18 @@ AC_DEFUN([gl_FUNC_WCWIDTH],
HAVE_DECL_WCWIDTH=0
fi

- if test $ac_cv_func_wcwidth = yes; then
+ AC_CACHE_CHECK([whether wcwidth is a macro],
+ gl_cv_func_wcwidth_macro,
+ [AC_EGREP_CPP([wchar_header_defines_wcwidth], [
+#include <wchar.h>
+#ifdef wcwidth
+ wchar_header_defines_wcwidth
+#endif],
+ gl_cv_func_wcwidth_macro=yes,
+ gl_cv_func_wcwidth_macro=no)])
+
+ if test $ac_cv_func_wcwidth = yes ||
+ test $gl_cv_func_wcwidth_macro = yes; then
HAVE_WCWIDTH=1
dnl On Mac OS X 10.3, wcwidth(0x0301) (COMBINING ACUTE ACCENT) returns 1.
dnl On OpenBSD 5.0, wcwidth(0x05B0) (HEBREW POINT SHEVA) returns 1.
--
2.13.3
Bruno Haible
2017-10-06 17:26:47 UTC
Permalink
Raw Message
Hi,
Post by KO Myung-Hun
Check the functionality of a macro version wcwidth () as well as a
real function.
This is better than undefining wcwidth on OS/2 kLIBC without functional
check.
With your patch, does the following command sequence pass its tests?

$ ./gnulib-tool --create-testdir --dir=testdir1 --single-configure --with-c++-tests wcwidth
$ cd testdir1
$ ./configure
$ make
$ make check
Post by KO Myung-Hun
+ gl_cv_func_wcwidth_macro,
...
+ gl_cv_func_wcwidth_macro=yes,
+ gl_cv_func_wcwidth_macro=no)])
In gnulib, for a long time, we use "quote all m4 macro arguments through brackets"
coding style. Just a matter of consistent coding style (that makes our Autoconf
macros halfway readable).

Bruno
KO Myung-Hun
2017-10-07 04:09:26 UTC
Permalink
Raw Message
Hi/2.
Post by Bruno Haible
Hi,
Post by KO Myung-Hun
Check the functionality of a macro version wcwidth () as well as a
real function.
This is better than undefining wcwidth on OS/2 kLIBC without functional
check.
With your patch, does the following command sequence pass its tests?
$ ./gnulib-tool --create-testdir --dir=testdir1 --single-configure --with-c++-tests wcwidth
$ cd testdir1
$ ./configure
$ make
$ make check
I attach test-suite.log.
Post by Bruno Haible
Post by KO Myung-Hun
+ gl_cv_func_wcwidth_macro,
...
+ gl_cv_func_wcwidth_macro=yes,
+ gl_cv_func_wcwidth_macro=no)])
In gnulib, for a long time, we use "quote all m4 macro arguments through brackets"
coding style. Just a matter of consistent coding style (that makes our Autoconf
macros halfway readable).
Fixed. And I increased serial number by 1.
--
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/
Bruno Haible
2017-10-07 07:05:56 UTC
Permalink
Raw Message
Post by KO Myung-Hun
I attach test-suite.log.
Does the init.sh test failure go away with this patch?

diff --git a/tests/init.sh b/tests/init.sh
index 470605c..0821c57 100644
--- a/tests/init.sh
+++ b/tests/init.sh
@@ -250,7 +250,7 @@ test -n "$BASH_VERSION" && unalias -a
# That is part of the shell-selection test above. Why use aliases rather
# than functions? Because support for hyphen-containing aliases is more
# widespread than that for hyphen-containing function names.
-test -n "$EXEEXT" && shopt -s expand_aliases
+test -n "$EXEEXT" && test -n "$BASH_VERSION" && shopt -s expand_aliases

# Enable glibc's malloc-perturbing option.
# This is useful for exposing code that depends on the fact that
KO Myung-Hun
2017-10-07 08:04:54 UTC
Permalink
Raw Message
Hi/2.
Post by Bruno Haible
Post by KO Myung-Hun
I attach test-suite.log.
Does the init.sh test failure go away with this patch?
'shopt' related failure disappeared. However, 'invalid path dir' is
still. I looked into that. And ':' was a problem. Because ':' is not a
path separator on OS/2. Instead it's used as a separator of a drive
letter and directory parts.

I tried to use $PATH_SEPARATOR. However, it has nothing.

Any idea ?
--
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/
Bruno Haible
2017-10-07 09:57:46 UTC
Permalink
Raw Message
Post by KO Myung-Hun
Post by Bruno Haible
Does the init.sh test failure go away with this patch?
'shopt' related failure disappeared.
OK, I've pushed the patch.
Post by KO Myung-Hun
However, 'invalid path dir' is
still. I looked into that. And ':' was a problem. Because ':' is not a
path separator on OS/2. Instead it's used as a separator of a drive
letter and directory parts.
I tried to use $PATH_SEPARATOR. However, it has nothing.
Any idea ?
PATH_SEPARATOR is the right approach, yes. In m4/progtest.m4 and m4/lib-ld.m4
you find a code snippet that determines PATH_SEPARATOR. You can copy this to
init.sh.


2017-10-07 Bruno Haible <***@clisp.org>

test-framework-sh: Don't require bash on Windows and OS/2.
Reported by KO Myung-Hun.
* tests/test-init.sh: Use 'shopt' only when running in bash.

diff --git a/tests/init.sh b/tests/init.sh
index 470605c..0821c57 100644
--- a/tests/init.sh
+++ b/tests/init.sh
@@ -250,7 +250,7 @@ test -n "$BASH_VERSION" && unalias -a
# That is part of the shell-selection test above. Why use aliases rather
# than functions? Because support for hyphen-containing aliases is more
# widespread than that for hyphen-containing function names.
-test -n "$EXEEXT" && shopt -s expand_aliases
+test -n "$EXEEXT" && test -n "$BASH_VERSION" && shopt -s expand_aliases

# Enable glibc's malloc-perturbing option.
# This is useful for exposing code that depends on the fact that
KO Myung-Hun
2017-10-07 10:39:00 UTC
Permalink
Raw Message
Post by Bruno Haible
Post by KO Myung-Hun
Post by Bruno Haible
Does the init.sh test failure go away with this patch?
'shopt' related failure disappeared.
OK, I've pushed the patch.
Post by KO Myung-Hun
However, 'invalid path dir' is
still. I looked into that. And ':' was a problem. Because ':' is not a
path separator on OS/2. Instead it's used as a separator of a drive
letter and directory parts.
I tried to use $PATH_SEPARATOR. However, it has nothing.
Any idea ?
PATH_SEPARATOR is the right approach, yes. In m4/progtest.m4 and m4/lib-ld.m4
you find a code snippet that determines PATH_SEPARATOR. You can copy this to
init.sh.
Ok. Here is the patch.
--
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/
Bruno Haible
2017-10-07 12:31:37 UTC
Permalink
Raw Message
Post by KO Myung-Hun
Post by Bruno Haible
PATH_SEPARATOR is the right approach, yes. In m4/progtest.m4 and m4/lib-ld.m4
you find a code snippet that determines PATH_SEPARATOR. You can copy this to
init.sh.
Ok. Here is the patch.
Thanks. Looks perfect. Applied.

Bruno
Bruno Haible
2017-10-07 07:23:46 UTC
Permalink
Raw Message
Hi,
Post by KO Myung-Hun
Post by Bruno Haible
With your patch, does the following command sequence pass its tests?
$ ./gnulib-tool --create-testdir --dir=testdir1 --single-configure --with-c++-tests wcwidth
$ cd testdir1
$ ./configure
$ make
$ make check
I attach test-suite.log.
Thanks. I'm glad to see that it passes the test-wchar-c++ test.
Post by KO Myung-Hun
Post by Bruno Haible
Post by KO Myung-Hun
+ gl_cv_func_wcwidth_macro,
...
+ gl_cv_func_wcwidth_macro=yes,
+ gl_cv_func_wcwidth_macro=no)])
In gnulib, for a long time, we use "quote all m4 macro arguments through brackets"
coding style. Just a matter of consistent coding style (that makes our Autoconf
macros halfway readable).
Fixed. And I increased serial number by 1.
Thanks, I applied your patch, with the additional optimizations (below):

- Don't perform the "checking whether wcwidth is a macro" test when wcwidth
is already known to be a function (which is the case on the vast majority
of platforms).
- Simplify 'test EXPR1 || test EXPR2' to 'test EXPR1 -o EXPR2'.

Hope I didn't introduce any bugs through this.

diff --git a/ChangeLog b/ChangeLog
index f4fa964..470012f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2017-10-06 KO Myung-Hun <***@chollian.net>
+
+ wcwidth: check a macro version of wcwidth () as well
+ * lib/wchar.in.h: Revert commit from 2016-01-14.
+ * m4/wcwidth.m4 (gl_FUNC_WCWIDTH): Test if wcwidth is a macro.
+
2017-10-06 Bruno Haible <***@clisp.org>

getopt-posix: Clarify copyright header.
diff --git a/m4/wcwidth.m4 b/m4/wcwidth.m4
index 6f6a719..b44a9ed 100644
--- a/m4/wcwidth.m4
+++ b/m4/wcwidth.m4
@@ -34,18 +34,20 @@ AC_DEFUN([gl_FUNC_WCWIDTH],
HAVE_DECL_WCWIDTH=0
fi

- AC_CACHE_CHECK([whether wcwidth is a macro],
- [gl_cv_func_wcwidth_macro],
- [AC_EGREP_CPP([wchar_header_defines_wcwidth], [
+ if test $ac_cv_func_wcwidth != yes; then
+ AC_CACHE_CHECK([whether wcwidth is a macro],
+ [gl_cv_func_wcwidth_macro],
+ [AC_EGREP_CPP([wchar_header_defines_wcwidth], [
#include <wchar.h>
#ifdef wcwidth
wchar_header_defines_wcwidth
#endif],
- [gl_cv_func_wcwidth_macro=yes],
- [gl_cv_func_wcwidth_macro=no])])
+ [gl_cv_func_wcwidth_macro=yes],
+ [gl_cv_func_wcwidth_macro=no])
+ ])
+ fi

- if test $ac_cv_func_wcwidth = yes ||
- test $gl_cv_func_wcwidth_macro = yes; then
+ if test $ac_cv_func_wcwidth = yes -o "$gl_cv_func_wcwidth_macro" = yes; then
HAVE_WCWIDTH=1
dnl On Mac OS X 10.3, wcwidth(0x0301) (COMBINING ACUTE ACCENT) returns 1.
dnl On OpenBSD 5.0, wcwidth(0x05B0) (HEBREW POINT SHEVA) returns 1.
KO Myung-Hun
2017-10-07 11:09:29 UTC
Permalink
Raw Message
Hi/2.
Post by Bruno Haible
Hi,
Post by KO Myung-Hun
Post by Bruno Haible
With your patch, does the following command sequence pass its tests?
$ ./gnulib-tool --create-testdir --dir=testdir1 --single-configure --with-c++-tests wcwidth
$ cd testdir1
$ ./configure
$ make
$ make check
I attach test-suite.log.
Thanks. I'm glad to see that it passes the test-wchar-c++ test.
Post by KO Myung-Hun
Post by Bruno Haible
Post by KO Myung-Hun
+ gl_cv_func_wcwidth_macro,
...
+ gl_cv_func_wcwidth_macro=yes,
+ gl_cv_func_wcwidth_macro=no)])
In gnulib, for a long time, we use "quote all m4 macro arguments through brackets"
coding style. Just a matter of consistent coding style (that makes our Autoconf
macros halfway readable).
Fixed. And I increased serial number by 1.
- Don't perform the "checking whether wcwidth is a macro" test when wcwidth
is already known to be a function (which is the case on the vast majority
of platforms).
- Simplify 'test EXPR1 || test EXPR2' to 'test EXPR1 -o EXPR2'.
Hope I didn't introduce any bugs through this.
It works well. Thanks.
--
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/
Eric Blake
2017-10-09 19:29:10 UTC
Permalink
Raw Message
Post by Bruno Haible
- Simplify 'test EXPR1 || test EXPR2' to 'test EXPR1 -o EXPR2'.
No, that is a pessimization. POSIX explicitly states that use of -a or
-o with test is prone to obscure breakage, and should not be treated as
portable.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Bruno Haible
2017-10-09 20:07:36 UTC
Permalink
Raw Message
Post by Eric Blake
Post by Bruno Haible
- Simplify 'test EXPR1 || test EXPR2' to 'test EXPR1 -o EXPR2'.
No, that is a pessimization. POSIX explicitly states that use of -a or
-o with test is prone to obscure breakage, and should not be treated as
portable.
I could live with that, as I haven't seen test '-a' or '-o' breakage in
the last 15 years. But it's worse than that: POSIX
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html
now marks '-a' and '-o' as "obsolescent", which means that new implementations
of 'test' need not support it.


2017-10-09 Bruno Haible <***@clisp.org>

wcwidth: Don't use obsolete syntax of 'test'.
Reported by Eric Blake.
* m4/wcwidth.m4 (gl_FUNC_WCWIDTH): Don't optimize two 'test'
invocations into one, as POSIX marks '-a' and '-o' as "obsolescent".

diff --git a/m4/wcwidth.m4 b/m4/wcwidth.m4
index b44a9ed..1243606 100644
--- a/m4/wcwidth.m4
+++ b/m4/wcwidth.m4
@@ -1,4 +1,4 @@
-# wcwidth.m4 serial 24
+# wcwidth.m4 serial 25
dnl Copyright (C) 2006-2017 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -47,7 +47,7 @@ AC_DEFUN([gl_FUNC_WCWIDTH],
])
fi

- if test $ac_cv_func_wcwidth = yes -o "$gl_cv_func_wcwidth_macro" = yes; then
+ if test $ac_cv_func_wcwidth = yes || test $gl_cv_func_wcwidth_macro = yes; then
HAVE_WCWIDTH=1
dnl On Mac OS X 10.3, wcwidth(0x0301) (COMBINING ACUTE ACCENT) returns 1.
dnl On OpenBSD 5.0, wcwidth(0x05B0) (HEBREW POINT SHEVA) returns 1.
Loading...