diff mbox series

wcwidth: adjust manpage input (Re: [PATCH] fix wcwidth to work with gcc 16 sign extension)

Message ID f8a83c65-1971-44ab-a51c-14d09d89c8ee@towo.net
State New
Headers
Series wcwidth: adjust manpage input (Re: [PATCH] fix wcwidth to work with gcc 16 sign extension) |

Commit Message

Thomas Wolff June 6, 2026, 8:40 a.m. UTC 
  amending my code patch... sorry I didn't include this right away

Am 04.06.2026 um 17:33 schrieb Jeff Johnston:
> Patch applied.  Thanks.
>
> -- Jeff J.
>
> On Tue, Jun 2, 2026 at 8:43 PM Thomas Wolff <towo@towo.net> wrote:
>
>     As discussed in the cygwin thread, I withdraw my previous patch and
>     provide the attached one to fix wcwidth for gcc 16.
>     Thomas
>
>     Am 01.06.2026 um 18:02 schrieb Thomas Wolff:
>     >
>     > Am 31.05.2026 um 13:57 schrieb Takashi Yano:
>     >> Hi Thomas,
>     >>
>     >> On Sun, 31 May 2026 10:06:12 +0200
>     >> Thomas Wolff wrote:
>     >>> Hi Brian,
>     >>>
>     >>> Am 31.05.2026 um 05:50 schrieb Brian Inglis via Cygwin:
>     >>>> On 2026-05-28 22:58, Thomas Wolff wrote:
>     >>>>> to make it compliant with newlib and the manual page;
>     >>>>> fixes cases of wrong width calculation:
>     >>>>> https://cygwin.com/pipermail/cygwin/2026-April/259597.html
>     >>>>> as mentioned in
>     >>>>> https://cygwin.com/pipermail/cygwin/2026-May/259734.html
>     >>>>> as described in
>     >>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125451#c14
>     >>>>> attachment:
>     >>>> 0001-wchar.h-tweak-wcwidth-prototype-parameter-wchar_t-wi.patch
>     >>>>
>     >>>> The existing wcwidth declaration in
>     newlib/libc/include/wchar.h agrees
>     >>>> with
>     >>>> POSIX 8 SUS V5.
>     >>>>
>     >>>> It is the man doc, definition, and implementation in
>     >>>> newlib/libc/string/wcwidth.c which need changed to match the
>     >>>> specification and return codes in:
>     >>>>
>     >>>>
>     https://pubs.opengroup.org/onlinepubs/9799919799/functions/wcwidth.html
>
>     >>>>
>     >>> Your argument overlooks one significant deviation: in POSIX,
>     wchar_t
>     >>> has
>     >>> 32 bits, in cygwin only 16.
>     >>> So to make wcwidth work for *all* Unicode character code
>     points, the 32
>     >>> bit version must be used.
>     >>> I tested positively that this fixes the broken test case with
>     gcc 16 I
>     >>> had reported to the cygwin list.
>     >> However, newlib is not used only by Cygwin, so I think newlib
>     itself
>     >> should
>     >> follow POSIX. Shouldn't we have our own wcwidth()
>     implementation for
>     >> Cygwin?
>     >>
>     >> On second thought, since a 16‑bit wchar_t needs to be converted
>     to a
>     >> 32‑bit
>     >> Unicode code point especially for surrogate pair, we cannot use
>     >> wcwidth in
>     >> the same way as Linux does. I wonder what the correct approach
>     would be.
>     > I don't there is a "correct" approach as POSIX probably did not
>     > consider this problem.
>     > But I just responded to a cute idea on the cygwin mailing list,
>     which
>     > was unfeasible but I modified it with a proposal to return width
>     1 for
>     > a high surrogate, remember it, and then return 1 or 0 for the low
>     > surrogate, respectively.
>
From b082f9a3108b5c7628a359aa7a636960457e3366 Mon Sep 17 00:00:00 2001
From: Thomas Wolff <towo@towo.net>
Date: Sat, 6 Jun 2026 00:00:00 +0000
Subject: [PATCH] wcwidth: adjust manpage source to parameter width patch

---
 newlib/libc/string/wcwidth.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

Comments

Jeff Johnston June 8, 2026, 9:26 p.m. UTC | #1 
Patch applied.

-- Jeff J.

On Sat, Jun 6, 2026 at 4:40 AM Thomas Wolff <towo@towo.net> wrote:

> amending my code patch... sorry I didn't include this right away
>
> Am 04.06.2026 um 17:33 schrieb Jeff Johnston:
>
> Patch applied.  Thanks.
>
> -- Jeff J.
>
> On Tue, Jun 2, 2026 at 8:43 PM Thomas Wolff <towo@towo.net> wrote:
>
>> As discussed in the cygwin thread, I withdraw my previous patch and
>> provide the attached one to fix wcwidth for gcc 16.
>> Thomas
>>
>> Am 01.06.2026 um 18:02 schrieb Thomas Wolff:
>> >
>> > Am 31.05.2026 um 13:57 schrieb Takashi Yano:
>> >> Hi Thomas,
>> >>
>> >> On Sun, 31 May 2026 10:06:12 +0200
>> >> Thomas Wolff wrote:
>> >>> Hi Brian,
>> >>>
>> >>> Am 31.05.2026 um 05:50 schrieb Brian Inglis via Cygwin:
>> >>>> On 2026-05-28 22:58, Thomas Wolff wrote:
>> >>>>> to make it compliant with newlib and the manual page;
>> >>>>> fixes cases of wrong width calculation:
>> >>>>> https://cygwin.com/pipermail/cygwin/2026-April/259597.html
>> >>>>> as mentioned in
>> >>>>> https://cygwin.com/pipermail/cygwin/2026-May/259734.html
>> >>>>> as described in
>> >>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125451#c14
>> >>>>> attachment:
>> >>>> 0001-wchar.h-tweak-wcwidth-prototype-parameter-wchar_t-wi.patch
>> >>>>
>> >>>> The existing wcwidth declaration in newlib/libc/include/wchar.h
>> agrees
>> >>>> with
>> >>>> POSIX 8 SUS V5.
>> >>>>
>> >>>> It is the man doc, definition, and implementation in
>> >>>> newlib/libc/string/wcwidth.c which need changed to match the
>> >>>> specification and return codes in:
>> >>>>
>> >>>>
>> https://pubs.opengroup.org/onlinepubs/9799919799/functions/wcwidth.html
>> >>>>
>> >>> Your argument overlooks one significant deviation: in POSIX, wchar_t
>> >>> has
>> >>> 32 bits, in cygwin only 16.
>> >>> So to make wcwidth work for *all* Unicode character code points, the
>> 32
>> >>> bit version must be used.
>> >>> I tested positively that this fixes the broken test case with gcc 16 I
>> >>> had reported to the cygwin list.
>> >> However, newlib is not used only by Cygwin, so I think newlib itself
>> >> should
>> >> follow POSIX. Shouldn't we have our own wcwidth() implementation for
>> >> Cygwin?
>> >>
>> >> On second thought, since a 16‑bit wchar_t needs to be converted to a
>> >> 32‑bit
>> >> Unicode code point especially for surrogate pair, we cannot use
>> >> wcwidth in
>> >> the same way as Linux does. I wonder what the correct approach would
>> be.
>> > I don't there is a "correct" approach as POSIX probably did not
>> > consider this problem.
>> > But I just responded to a cute idea on the cygwin mailing list, which
>> > was unfeasible but I modified it with a proposal to return width 1 for
>> > a high surrogate, remember it, and then return 1 or 0 for the low
>> > surrogate, respectively.
>>
>
>
Jeff Johnston June 10, 2026, 4:44 p.m. UTC | #2 
Patch applied.  Thanks.

-- Jeff J.

On Wed, Jun 10, 2026 at 10:16 AM Jon Turney <jon.turney@dronecode.org.uk>
wrote:

> On 08/06/2026 22:26, Jeff Johnston wrote:
> > Patch applied.
>   'make info' chokes on this.
>
> >   MAKEINFO ../../../src/newlib/libc/libc.info
> > wcwidth.def:34: misplaced {
> > wcwidth.def:36: misplaced }
> > wcwidth.def:38: misplaced {
> > wcwidth.def:40: misplaced }
> > wcwidth.def:44: misplaced {
> > wcwidth.def:48: misplaced {
> > wcwidth.def:48: misplaced }
> > wcwidth.def:49: misplaced }
>
> I think this is because makedoc does not correctly escape '{' or '}' for
> texinfo, outside of a fixed-width font section (which makedoc expects to
> be marked-up with '.' or '|'at the start of lines -- see courierize in
> makedoc.c)
>
> That's definitely a shortcoming in makedoc, but since this *is* a code
> example, courierize-ing it seems appropriate.
>
> Patch attached.
>
Jon Turney June 19, 2026, 1:54 p.m. UTC | #3 
On 10/06/2026 17:44, Jeff Johnston wrote:
> Patch applied.  Thanks.
> 
> -- Jeff J.
> 
> On Wed, Jun 10, 2026 at 10:16 AM Jon Turney <jon.turney@dronecode.org.uk>
> wrote:
> 
>> On 08/06/2026 22:26, Jeff Johnston wrote:
>>> Patch applied.
>>    'make info' chokes on this.
>>
>>>    MAKEINFO ../../../src/newlib/libc/libc.info
>>> wcwidth.def:34: misplaced {
>>> wcwidth.def:36: misplaced }
>>> wcwidth.def:38: misplaced {
>>> wcwidth.def:40: misplaced }
>>> wcwidth.def:44: misplaced {
>>> wcwidth.def:48: misplaced {
>>> wcwidth.def:48: misplaced }
>>> wcwidth.def:49: misplaced }
>>
>> I think this is because makedoc does not correctly escape '{' or '}' for
>> texinfo, outside of a fixed-width font section (which makedoc expects to
>> be marked-up with '.' or '|'at the start of lines -- see courierize in
>> makedoc.c)
>>
>> That's definitely a shortcoming in makedoc, but since this *is* a code
>> example, courierize-ing it seems appropriate.
>>
>> Patch attached.
>>

Huh, even now I don't think this is quite what the original patch intended.

> +EXAMPLE
> +	An application function to determine the width of a 21-bit
> +	Unicode character may look like this:

"EXAMPLE" is defined as a command in doc.str, but there are no existing 
uses of it.

It's different to e.g. DESCRIPTION, RETURNS, PORTABILITY etc. in that it 
doesn't emit a subheading "EXAMPLE", which I imagine is what was 
expected here.

(If you look at the currently generated info page for wcwidth, this 
example is part of the "RETURNS" subsection at the moment, which doesn't 
seem right.)

I'll send another patch.
diff mbox series

Patch

diff --git a/newlib/libc/string/wcwidth.c b/newlib/libc/string/wcwidth.c
index dfcaa6c21..93cc3e829 100644
--- a/newlib/libc/string/wcwidth.c
+++ b/newlib/libc/string/wcwidth.c
@@ -7,15 +7,17 @@  INDEX
 
 SYNOPSIS
 	#include <wchar.h>
-	int wcwidth(const wint_t <[wc]>);
+	int wcwidth(const wchar_t <[wc]>);
 
 DESCRIPTION
 	The <<wcwidth>> function shall determine the number of column
 	positions required for the wide character <[wc]>. The application
 	shall ensure that the value of <[wc]> is a character representable
-	as a wint_t (combining Unicode surrogate pairs into single 21-bit
-	Unicode code points), and is a wide-character code corresponding to a
+	as a wchar_t, and is a wide-character code corresponding to a
 	valid character in the current locale.
+	Note that for a Unicode character outside the 16-bit range, 
+	the application must split it into Unicode surrogates 
+	and use the <<wcswidth>> function instead.
 
 RETURNS
 	The <<wcwidth>> function shall either return 0 (if <[wc]> is a null
@@ -23,6 +25,30 @@  RETURNS
 	be occupied by the wide-character code <[wc]>, or return -1 (if <[wc]>
 	does not correspond to a printable wide-character code).
 
+EXAMPLE
+	An application function to determine the width of a 21-bit 
+	Unicode character may look like this:
+
+		typedef unsigned int uchar_t;
+		// determine high and low surrogates of Unicode character
+		wchar_t hisurr(uchar_t xc)
+		{
+		  return 0xD800 | (((xc - 0x10000) >> 10) & 0x3FF);
+		}
+		wchar_t losurr(uchar_t xc)
+		{
+		  return 0xDC00 | (xc & 0x3FF);
+		}
+
+		// determine width of 21-bit Unicode character
+		int ucwidth(uchar_t uc)
+		{
+		  if (uc < 0x10000)
+		    return wcwidth(uc);
+		  else
+		    return wcswidth((wchar_t[]){hisurr(uc), losurr(uc)}, 2);
+		}
+
 PORTABILITY
 <<wcwidth>> has been introduced in the Single UNIX Specification Volume 2.
 <<wcwidth>> has been marked as an extension in the Single UNIX Specification Volume 3.
@@ -165,6 +191,8 @@  bisearch(wint_t ucs, const struct interval *table, int max)
 
 int
 __wcwidth (const wint_t ucs)
+// unlike wcwidth, the parameter type of __wcwidth must be 32 bits wide
+// in order to support wcswidth
 {
 #ifdef _MB_CAPABLE
   /* sorted list of non-overlapping intervals of East Asian Ambiguous chars */