Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-15 Thread Shoaib Meenai via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL281641: [libc++] Avoid include in locale_win32.h (authored by smeenai). Changed prior to commit: https://reviews.llvm.org/D24374?vs=70899&id=71530#toc Repository: rL LLVM https://reviews.llvm.org/D

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-09 Thread Shoaib Meenai via cfe-commits
smeenai added a comment. In https://reviews.llvm.org/D24374#538534, @bcraig wrote: > In https://reviews.llvm.org/D24374#538521, @smeenai wrote: > > > @bcraig thanks for pointing me to that diff; there's a lot of nice cleanup > > going on there. Were you planning on updating and following up on i

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-09 Thread Shoaib Meenai via cfe-commits
smeenai updated this revision to Diff 70899. smeenai added a comment. Correcting support_win32.cpp https://reviews.llvm.org/D24374 Files: include/support/win32/locale_win32.h src/support/win32/locale_win32.cpp Index: src/support/win32/locale_win32.cpp ==

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-09 Thread Ben Craig via cfe-commits
bcraig added a comment. In https://reviews.llvm.org/D24374#538521, @smeenai wrote: > @bcraig thanks for pointing me to that diff; there's a lot of nice cleanup > going on there. Were you planning on updating and following up on it? > > I also realized I forgot to adjust `locale_win32.cpp` for th

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-09 Thread Shoaib Meenai via cfe-commits
smeenai planned changes to this revision. smeenai added a comment. @bcraig thanks for pointing me to that diff; there's a lot of nice cleanup going on there. Were you planning on updating and following up on it? I also realized I forgot to adjust `locale_win32.cpp` for this diff. Will re-upload

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-09 Thread Ben Craig via cfe-commits
bcraig added a subscriber: bcraig. bcraig added a comment. This seems related: https://reviews.llvm.org/D20596 In that (stale) review, I switch away from unique_ptr in general for locale related operations. https://reviews.llvm.org/D24374 ___ cfe-

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-08 Thread Shoaib Meenai via cfe-commits
smeenai added a comment. Cool. Any thoughts on this implementation vs. `___mb_cur_max_l_func`? In other words, do we have a general policy on using internal CRT functionality? FWIW, `support/win32/support.h` was using `xlocinfo.h` before my recent cleanup, which is also an internal header. ht

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-08 Thread Eric Fiselier via cfe-commits
EricWF added a comment. LGTM. I'm OK with things that are "technically" regressions in Windows support if it helps us get it working today. https://reviews.llvm.org/D24374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

Re: [PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-08 Thread Shoaib Meenai via cfe-commits
smeenai added a comment. I'm aware that my replacement code isn't entirely equivalent, since it won't restore the locale in case `MB_CUR_MAX` throws. However, I'm quite certain the `MB_CUR_MAX` implementation won't throw. I can make my own RAII wrapper if desired, however. Alternatively, MSDN

[PATCH] D24374: [libc++] Avoid include in locale_win32.h

2016-09-08 Thread Shoaib Meenai via cfe-commits
smeenai created this revision. smeenai added reviewers: compnerd, EricWF, mclow.lists. smeenai added a subscriber: cfe-commits. When `_LIBCPP_NO_EXCEPTIONS` is defined, we end up with compile errors when targeting MSVCRT: * Code includes `` * `` includes `` in order to get `abort` * `` includes `