[cfe-dev] [PATCH] Libc++ Windows fixes

Ruben Van Boxem vanboxem.ruben at gmail.com
Thu Sep 29 13:13:29 PDT 2011


2011/9/29 Ruben Van Boxem <vanboxem.ruben at gmail.com>

> 2011/9/29 Howard Hinnant <hhinnant at apple.com>
>
>> On Sep 29, 2011, at 7:15 AM, Ruben Van Boxem wrote:
>>
>> > 2011/9/29 Ruben Van Boxem <vanboxem.ruben at gmail.com>
>> > 2011/9/29 Ruben Van Boxem <vanboxem.ruben at gmail.com>
>> > 2011/9/29 Ruben Van Boxem <vanboxem.ruben at gmail.com>
>> > 2011/9/29 Howard Hinnant <hhinnant at apple.com>
>> >
>> > On Sep 28, 2011, at 6:17 PM, David Chisnall wrote:
>> >
>> > > On 28 Sep 2011, at 22:45, Howard Hinnant wrote:
>> > >
>> > >> I think FreeBSD should define _LLIBCPP_STABLE_APPLE_ABI too.  David?
>>  And maybe with David's latest patch to FreeBSD, should also #define
>> _LIBCPP_GET_C_LOCALE 0.
>> > >
>> > > FreeBSD should now be mirroring the Apple implementation for anything
>> xlocale related.  If there are any discrepancies, they are bugs and I'll fix
>> them in libc, rather than hack around them in libc++.
>> > >
>> > > I'm not entirely sure what defining _LLIBCPP_STABLE_APPLE_ABI means -
>> perhaps it could be replaced with a more descriptive name?
>> >
>> > See revision 140734, and let me know what needs fixing (hopefully
>> nothing).
>> >
>> > This improves the behavior on Windows (uses the C locale instead of
>> returning NULL like for Linux). Attached patch also removes unreachable code
>> (_LIBCPP_HAS_DEFAULTRUNELOCALE is defined for both __APPLE__ and
>> __FreeBSD__, so the additional checks in locale.cpp are never reached).
>> >
>> > I'm now also wondering about _LIBCPP_WCTYPE_IS_MASK. I've been looking
>> at MSDN, but can't figure out if it matches. I did find a warning that if a
>> plain "char" is passed to Windows is* functions, unexpected results are
>> obtained (
>> http://msdn.microsoft.com/en-us/library/4yc6feha%28v=vs.80%29.aspx). It
>> seems that char_type could be char in the common case, and this could yield
>> unreliable behavior?
>> >
>> > Ruben
>> >
>> > Never mind this patch. It's broken. Is there any reason not to call
>> plain "toupper" in do_toupper directly? Or at least "toupper_(c,__cloc())"?
>> >
>> > Ruben
>> >
>> > Okay, brace yourself...
>> >
>> > I've tried to remove locale.cpp's dependency on glibc's toupper and
>> tolower transformation tables, which are only available in glibc. I have
>> coded a fallback do_tolower and do_toupper using basic ASCII subtract and
>> add tricks if glibc or the DefaultRuneLocale is not available. I know this
>> is slower, but it works at least. It uses islower_l and isupper_l with
>> __cloc to check. The change in behavior should be:
>> > Mac: none
>> > FreeBSD: none
>> > Linux/glibc: none
>> > other platforms (including Windows): no __classic_upper/lower_table
>> functions declared, defined, or used.
>> > I also declared a symbol present in msvcrt.dll that represents the mask
>> table and added that case to classic_table.
>> >
>> > I would prefer having a (global) static "C" locale object that can just
>> be given through __cloc, although I don't know if this is safe, but as long
>> as __cloc is only used in _l functions (which I think it is), this approach
>> of a persistent (global, not function-local) C locale object should work (is
>> initialized before any threads could call __cloc at the same time) and fix
>> the FIXME in __cloc.
>> >
>> > I also tried to fix the ctype<wchar_t>::do_is to keep track of the
>> result, and now should work if multiple flags were requested.
>> >
>> > Comments welcome!
>> >
>> > Today is not my day. Right patch is attached, with missing bits included
>> this time. After this, I'm taking a break, before I spam this list with
>> broken patches :(
>> >
>> >
>> > Ruben
>> >
>> > <windows.patch>
>>
>> This works for Apple, thanks! Committed revision 140781.
>>
>
> Sorry about the mess, it seems the missing bits were... missing. A file was
> missed, and I had to rename the locale support header/source file or else
> the buildit script overwrites libc++'s locale.o with my
> support/win32/locale.o.
>
> Please apply, thanks,
>

Ugh, with patch.


>
> Ruben
>
>
>> Howard
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110929/015bcc65/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: windows.patch
Type: application/octet-stream
Size: 10814 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110929/015bcc65/attachment.obj>


More information about the cfe-dev mailing list