[PATCH v3 1/4] include: Fix undefined char16_t in C11.
In c++11 char16_t is a distinct fundamental type, but in c11 it is merely a typedef in <uchar.h>. Explicitly mention char16_t only in c++11 (where it is built-in), otherwise define WCHAR as unsigned short (without naming char16_t) and just hope that on C11 this matches u"..." Remove WINE_UNICODE_CHAR16; it is now the default when supported. Signed-off-by: Kevin Puetz <PuetzKevinA(a)JohnDeere.com> --- Per https://www.winehq.org/pipermail/wine-devel/2020-July/170502.html Jacek wants to avoid including <uchar.h>; without that C11 has to make some assumptions the standard doesn't guarantee. But that's far from wine's first foray into implementation-defined behavior... c11 defines char16_t as a typedef to uint_least16_t, which is likely (though not guaranteed) the same as unsigned short (e.g. it could be int if that is a 16-bit type). I do see that basetsd has a comment saying only ILP32, LP64, or P64 typemodels are supported, though, so probably wine would already not work on a platform with 16-bit int. GCC and clang provide a macro __CHAR16_TYPE__, but basetsd.h needs I32 and CHAR8_BIT!=8 is far too exotic, so it's unecessary in practice. It will be `#define __CHAR_TYPE__ unsigned short` anyway; the other C11-compliant possibilities would already break wine. That makes both the pro and con arguments moot, so it's now omitted. --- Removing WINE_UNICODE_CHAR1 changes little for C or C++98/C++03, or anything using -fshort-wchar/WINE_UNICODE_NATIVE, but it has some ABI implications for wineg++ -std=c++11 -fno-short-wchar. On the plus side: * TEXT(), OLESTR() macros just work in UNICODE for c++11 and (usually) c11 * We revert an #ifdef with ABI implications, now there's just -f(no-)short-wchar and -std=c++*, which already mattered * in C++11, WCHAR overloads are no longer ambiguous vs integral types and WCHAR is recognizable as text rather than promoted to int. This is much more like the situation on MSVC where WCHAR == wchar_t; char16_t is also distinct from uint16_t, unsigned short, etc. * We avoid the situation where TEXT("...") -> u"..." is accepted by the compiler and produces char16_t[], but that isn't compatible with TCHAR[] or LPCTSTR. Silently getting the wrong type is obnoxious in C++, since templates can move the type-mismatch compile error far away from the offending TEXT("...") macro, or even compile but use an unexpected overload. Now u"..." either doesn't compile, or it's correct, so we can drop https://www.winehq.org/pipermail/wine-devel/2020-July/170227.html In C11 there's no templates or overloading; it's non-portable UB (strict-aliasing) if an unsigned short * points to unsigned int[], but not a practical issue on any platform basetsd.h supports. On the minus side: * any libraries built with wineg++ -std=c++11 -fno-short-wchar change their mangled names for functions with WCHAR/LPCWSTR parameters because char16_t is a distinct fundamental type. This doesn't affect compatibility of wine itself (which always exports things under their MSVC-ish mangling as-if using wchar_t), but wineg++ could fail to link to a .dll.so built with wine 5.0.x headers. I don't know to what extent wineg++ tries to promise that this works, but IMO we are headed into a major version 6, and binaries are already not fully interchangeable due to winecrt0/__wine_spec_init changes. So I think think it seems preferable to just have good defaults that are MSVC-like, and fewer possible mistakes. But it's certainly possible to keep the WINE_UNICODE_CHAR16 opt-in, (or add a WINE_NO_UNICODE_CHAR16 opt-out), if we must. Or we could follow gcc's lead on the upcoming c++20 -f(no-)char8_t and winegcc could have -f(no-)char16_t, or -fchar16_t-wchar (synthesizing the macro like it does with WINE_UNICODE_NATIVE). But I like the simplicity of "just works if the compiler has support" unless someone objects. --- include/sqltypes.h | 2 +- include/tchar.h | 2 +- include/winnt.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/sqltypes.h b/include/sqltypes.h index 0923f6b362..4c45de7e98 100644 --- a/include/sqltypes.h +++ b/include/sqltypes.h @@ -30,7 +30,7 @@ extern "C" { typedef unsigned char SQLCHAR; #if defined(WINE_UNICODE_NATIVE) typedef wchar_t SQLWCHAR; -#elif defined(WINE_UNICODE_CHAR16) +#elif __cpp_unicode_literals >= 200710 typedef char16_t SQLWCHAR; #else typedef unsigned short SQLWCHAR; diff --git a/include/tchar.h b/include/tchar.h index 9fc4c72099..a8e64b4bf6 100644 --- a/include/tchar.h +++ b/include/tchar.h @@ -240,7 +240,7 @@ typedef unsigned short wctype_t; #ifndef __TCHAR_DEFINED #if defined(WINE_UNICODE_NATIVE) typedef wchar_t _TCHAR; -#elif defined(WINE_UNICODE_CHAR16) +#elif __cpp_unicode_literals >= 200710 typedef char16_t _TCHAR; #else typedef unsigned short _TCHAR; diff --git a/include/winnt.h b/include/winnt.h index 1eb82876f1..b0736a036d 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -462,7 +462,7 @@ typedef int LONG, *PLONG; /* Some systems might have wchar_t, but we really need 16 bit characters */ #if defined(WINE_UNICODE_NATIVE) typedef wchar_t WCHAR; -#elif defined(WINE_UNICODE_CHAR16) +#elif __cpp_unicode_literals >= 200710 typedef char16_t WCHAR; #else typedef unsigned short WCHAR;
When winegcc is using an underlying POSIX libc (rather than -mno-cygwin) it will only have `char` and `wchar_t` functions. If _TCHAR is neither of these there may be no suitable function to alias _tcs* to. Signed-off-by: Kevin Puetz <PuetzKevinA(a)JohnDeere.com> --- wine-5.2 (c12089039637dec5e598ed1c41e707f057494242) allowed <tchar.h> to be used without MSVCRT. _TEXT(...) and _TCHAR typedef are useful, but the _tcs* macros may still be unusable. Omitting them will be a clearer compile error when to not exist than mapping them to e.g. wcs* functions which do not accept a windows WCHAR (!= wchar_t), --- include/tchar.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/tchar.h b/include/tchar.h index a8e64b4bf6..0d6829ce1b 100644 --- a/include/tchar.h +++ b/include/tchar.h @@ -37,9 +37,15 @@ extern "C" { #define _strninc(str,n) (((char*)(str))+(n)) #define _strspnp(s1,s2) (*((s1)+=strspn((s1),(s2))) ? (s1) : NULL) +#if defined(__MSVCRT__) || defined(_MSC_VER) || (defined(WINE_UNICODE_NATIVE) && defined(_UNICODE)) || !(defined(_UNICODE) || defined(_MBCS)) /***************************************************************************** * tchar mappings + * + * These can only be defined when libc in use will have functions accepting _TCHAR, i.e. + * -mno-cygwin / __MSVCRT__ or __MSC_VER + * -fshort-wchar / WINE_UNICODE_NATIVE and _UNICODE (_TCHAR == WCHAR == wchar_t, so the libc wcs* functions are UTF-16) + * _TCHAR == `char` without _MBCS */ #ifndef _UNICODE # ifndef _MBCS @@ -223,6 +229,8 @@ extern "C" { #define _vtprintf WINE_tchar_routine(vprintf, vprintf, vwprintf) #define _TEOF WINE_tchar_routine(EOF, EOF, WEOF) +#endif /* tchar mappings */ + #define __T(x) __TEXT(x) #define _T(x) __T(x) #define _TEXT(x) __T(x)
Hi Kevin, On 24/09/2020 17:12, Kevin Puetz wrote:
--- a/include/tchar.h +++ b/include/tchar.h @@ -37,9 +37,15 @@ extern "C" { #define _strninc(str,n) (((char*)(str))+(n)) #define _strspnp(s1,s2) (*((s1)+=strspn((s1),(s2))) ? (s1) : NULL)
+#if defined(__MSVCRT__) || defined(_MSC_VER) || (defined(WINE_UNICODE_NATIVE) && defined(_UNICODE)) || !(defined(_UNICODE) || defined(_MBCS))
This seems to be more complicated than it needs to be. Since guarded macros really belong to crt, maybe we should have a guard bellow includes of string.h, wchar.h, etc. (to give corecrt.h a chance to be included) and simply use: #ifdef __WINE_USE_MSVCRT Thanks, Jacek
-----Original Message----- From: Jacek Caban <jacek(a)codeweavers.com> Sent: Wednesday, September 30, 2020 4:08 PM
Hi Kevin,
On 24/09/2020 17:12, Kevin Puetz wrote:
--- a/include/tchar.h +++ b/include/tchar.h @@ -37,9 +37,15 @@ extern "C" { #define _strninc(str,n) (((char*)(str))+(n)) #define _strspnp(s1,s2) (*((s1)+=strspn((s1),(s2))) ? (s1) : NULL)
+#if defined(__MSVCRT__) || defined(_MSC_VER) || (defined(WINE_UNICODE_NATIVE) && defined(_UNICODE)) || !(defined(_UNICODE) || defined(_MBCS))
This seems to be more complicated than it needs to be. Since guarded macros really belong to crt, maybe we should have a guard bellow includes of string.h, wchar.h, etc. (to give corecrt.h a chance to be included) and simply use:
#ifdef __WINE_USE_MSVCRT
I guess I was (again) aiming for as much portability as I could. Wine 5.0.x allowed the use of tchar.h for 2 cases: 1. __MSVCRT__ (i.e. -mno-cygwin) 2. "plain" 8-bit text (no _UNICODE or _MBCS) Anything else got a #error. But any settings allowed to #include <tchar.h> at all got the _tcs* functions. My original patches (which c1208903 applied partially) removed that #error to allow using tchar.h with char16_t. They kept the _tcs macros guarded since a standard libc won't have utf16 functions, but allowed a couple more cases: 3. _UNICODE && WINE_UNICODE_NATIVE (i.e. -fshort-wchar) This was just by analogy: if char str functions were OK for 8-bit _TCHAR, the wchar_t wcs functions should be equally OK when _TCHAR==wchar_t 4. MSVC I don't really have a specific use for these; I was really submitting this clean-up just because I felt c1208903 had left things inconsistent (_tcs macros defined that won't work, or maybe even compile). Since that started with (part of) my patch I felt responsible to fix it 😊. I don't have any objection to simplifying this further, and just ruling that _tcs* functions are MSVCRT-only. But I didn't propose that because it would be remove some functionality that 5.0.x had. If you want to go that way, though (make a breaking change from 5.0.x), I think it would make more sense to follow-through all the way and match the Windows SDK/ucrt split, i.e. move tchar.h into include/msvcrt, and leave it with no #ifdef. TCHAR and TEXT() are already in winnt.h, so it seems defensible to give glibc just those, keeping _TCHAR, _TEXT, and _tcs*() functions for MSVCRT (where they always have usable definitions). That would break a few things we're compiling with wineg++, but not in ways that would be hard to fix.
On 01.10.2020 00:30, Puetz Kevin A wrote:
-----Original Message----- From: Jacek Caban <jacek(a)codeweavers.com> Sent: Wednesday, September 30, 2020 4:08 PM
Hi Kevin,
On 24/09/2020 17:12, Kevin Puetz wrote:
--- a/include/tchar.h +++ b/include/tchar.h @@ -37,9 +37,15 @@ extern "C" { #define _strninc(str,n) (((char*)(str))+(n)) #define _strspnp(s1,s2) (*((s1)+=strspn((s1),(s2))) ? (s1) : NULL)
+#if defined(__MSVCRT__) || defined(_MSC_VER) || (defined(WINE_UNICODE_NATIVE) && defined(_UNICODE)) || !(defined(_UNICODE) || defined(_MBCS))
This seems to be more complicated than it needs to be. Since guarded macros really belong to crt, maybe we should have a guard bellow includes of string.h, wchar.h, etc. (to give corecrt.h a chance to be included) and simply use:
#ifdef __WINE_USE_MSVCRT I guess I was (again) aiming for as much portability as I could.
Wine 5.0.x allowed the use of tchar.h for 2 cases: 1. __MSVCRT__ (i.e. -mno-cygwin) 2. "plain" 8-bit text (no _UNICODE or _MBCS)
Anything else got a #error. But any settings allowed to #include <tchar.h> at all got the _tcs* functions.
My original patches (which c1208903 applied partially) removed that #error to allow using tchar.h with char16_t. They kept the _tcs macros guarded since a standard libc won't have utf16 functions, but allowed a couple more cases:
3. _UNICODE && WINE_UNICODE_NATIVE (i.e. -fshort-wchar) This was just by analogy: if char str functions were OK for 8-bit _TCHAR, the wchar_t wcs functions should be equally OK when _TCHAR==wchar_t 4. MSVC
I don't really have a specific use for these; I was really submitting this clean-up just because I felt c1208903 had left things inconsistent (_tcs macros defined that won't work, or maybe even compile). Since that started with (part of) my patch I felt responsible to fix it 😊.
I don't have any objection to simplifying this further, and just ruling that _tcs* functions are MSVCRT-only. But I didn't propose that because it would be remove some functionality that 5.0.x had.
If you want to go that way, though (make a breaking change from 5.0.x), I think it would make more sense to follow-through all the way and match the Windows SDK/ucrt split, i.e. move tchar.h into include/msvcrt, and leave it with no #ifdef.
TCHAR and TEXT() are already in winnt.h, so it seems defensible to give glibc just those, keeping _TCHAR, _TEXT, and _tcs*() functions for MSVCRT (where they always have usable definitions).
That would break a few things we're compiling with wineg++, but not in ways that would be hard to fix.
I'm not concerned too much about corner case 5.0 compatibility in this particular case. I think that realistically you're probably the only user of this feature so far, so we have some flexibility for the solution. I also think that current behaviour is not too bad: it works when it can, otherwise the compiler will issue a warning or an error on an attempt to use a problematic macro macro. But if you want to guard better, I don't mind, but let's keep it simple so that it's easier to read and less likely to require future maintenance. Thanks, Jacek
This is, at least pedanticly, independent of the similar macro UNICODE which controls <windef.h>, e.g. TEXT(), TCHAR, WINELIB_NAME_AW. So _T(...) and _TEXT(...) can't reuse the implementation of TEXT(...). Signed-off-by: Kevin Puetz <PuetzKevinA(a)JohnDeere.com> --- https://devblogs.microsoft.com/oldnewthing/20040212-00/?p=40643 --- include/tchar.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/include/tchar.h b/include/tchar.h index 0d6829ce1b..b933b9dd15 100644 --- a/include/tchar.h +++ b/include/tchar.h @@ -231,10 +231,6 @@ extern "C" { #endif /* tchar mappings */ -#define __T(x) __TEXT(x) -#define _T(x) __T(x) -#define _TEXT(x) __T(x) - #ifdef _UNICODE #if !defined(_WINT_T_DEFINED) && !defined(__WINT_TYPE__) typedef unsigned short wint_t; @@ -247,12 +243,16 @@ typedef unsigned short wctype_t; #ifndef __TCHAR_DEFINED #if defined(WINE_UNICODE_NATIVE) +#define __T(x) L##x typedef wchar_t _TCHAR; -#elif __cpp_unicode_literals >= 200710 +#else +#define __T(x) u##x +#if __cpp_unicode_literals >= 200710 typedef char16_t _TCHAR; #else typedef unsigned short _TCHAR; #endif +#endif typedef _TCHAR _TUCHAR; typedef _TCHAR _TSCHAR; typedef _TCHAR _TXCHAR; @@ -261,6 +261,7 @@ typedef _TCHAR _TINT; #endif #else /* _UNICODE */ +#define __T(x) x #ifndef __TCHAR_DEFINED typedef char _TCHAR; typedef unsigned char _TUCHAR; @@ -275,6 +276,9 @@ typedef unsigned int _TINT; #endif #endif +#define _T(x) __T(x) +#define _TEXT(x) __T(x) + #ifndef _TCHAR_DEFINED typedef _TCHAR TCHAR, *PTCHAR; #define _TCHAR_DEFINED
When <basetyps.h> defines wchar_t, it must obey -f(no-)short-wchar by referring to __WCHAR_TYPE__ which may not be `unsigned short`. Similarly, windef.h defining NULL may need to use GCC's __null. Otherwise one-definition-rule problems arise depending on whether wine or libc headers are included first. tchar.h wint_t and wctype_t are defined by <wchar.h>, which was already being included. Implementing these by deferring to the C89-specified libc headers, (or msvcrt's versions with -mno-cygwin) ensures matching definitions. Signed-off-by: Kevin Puetz <PuetzKevinA(a)JohnDeere.com> --- <basetyps.h> uses gcc's __need_* macros to document/filter that it only intends to define wchar_t (and not the rest, e.g. size_t/ptrdiff_t) to match the names defined by the MSVC version as closely as possible. That's not portable, but if not supported it'll just get the rest too, which is unlikely to really hurt anything in practice. <windef.h> always ended up with all of <stddef.h> later (via <winnt.h>), and the real SDK header does to, so there's no reason to be subtle there. --- include/basetyps.h | 8 +++++--- include/tchar.h | 9 --------- include/windef.h | 9 +++++---- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/include/basetyps.h b/include/basetyps.h index b0dd5d77f4..a2c5ed49f2 100644 --- a/include/basetyps.h +++ b/include/basetyps.h @@ -91,9 +91,11 @@ typedef unsigned long error_status_t; #endif #ifndef _WCHAR_T_DEFINED -#ifndef __cplusplus -typedef unsigned short wchar_t; -#endif +# ifndef __cplusplus +# define __need_wchar_t /* tells GCC's stddef.h we only need wchar_t */ +# include <stddef.h> +# undef __need_wchar_t +# endif #define _WCHAR_T_DEFINED #endif diff --git a/include/tchar.h b/include/tchar.h index b933b9dd15..6d785c0f51 100644 --- a/include/tchar.h +++ b/include/tchar.h @@ -232,15 +232,6 @@ extern "C" { #endif /* tchar mappings */ #ifdef _UNICODE -#if !defined(_WINT_T_DEFINED) && !defined(__WINT_TYPE__) -typedef unsigned short wint_t; -#endif - -#ifndef _WCTYPE_T_DEFINED -typedef unsigned short wctype_t; -#define _WCTYPE_T_DEFINED -#endif - #ifndef __TCHAR_DEFINED #if defined(WINE_UNICODE_NATIVE) #define __T(x) L##x diff --git a/include/windef.h b/include/windef.h index 521c3ab451..c0323d4275 100644 --- a/include/windef.h +++ b/include/windef.h @@ -206,11 +206,12 @@ extern "C" { /* Misc. constants. */ -#undef NULL -#ifdef __cplusplus -#define NULL 0 -#else +#ifndef NULL +#ifdef RC_INVOKED #define NULL ((void*)0) +#else +#include <stddef.h> +#endif #endif #ifdef FALSE
Since Jacek and I have now been back and forth with several different versons, here's a little higher-level summary to save Alexandre some mental energy: wine-5.2 added WINE_UNICUDE_CHAR16 (c2679945dd1ffddbc8e8a43dc7263be7738d4a4e) but only took only some parts of my original C++11 char16_t patch. When we finally got 5.0.x working well, I started trying to keep our patches rebased on master and submit more things upstream. The two issues whose fix has been controversial are: 1. in C++11 you get can get overloading surprises if the TEXT macro uses u"" (producing char16_t[]) but LPCWSTR/LPCTSTR are some other type. 2. defining WINE_UNICODE_CHAR16 in C or pre-C++11 would use char16_t without the #include that defines it. v1: https://www.winehq.org/pipermail/wine-devel/2020-July/170227.html This was just rebasing the original solutions we had been using in 4.x/5.0.x 1. force a compile error (don't use u"") in TEXT without WINE_UNICODE_CHAR16 2. get the C11 definition of char16_t from <uchar.h> v2: https://www.winehq.org/pipermail/wine-devel/2020-September/173888.html 1. use char16_t by default in c++11 (so u"" is then always right) Then there's no need for forcing a compile error. 2. use __CHAR16_TYPE__ to define WCHAR (and maybe TCHAR) This avoids using <uchar.h> or char16_t, but will be a compatible typedef if the user's code does so elsewhere. v3: https://www.winehq.org/pipermail/wine-devel/2020-September/174356.html 1. Same as v2 2. Just keep `unsigned short`, yeilding to Jacek that this is what __CHAR16_TYPE__ would always be anyway. Accept that wine already makes a lot of assumptions about size of integer types, and doesn't benefit from trying hard to be pedantically portable.
participants (3)
-
Jacek Caban -
Kevin Puetz -
Puetz Kevin A