This is an archive of the discontinued LLVM Phabricator instance.

Differential D158199

[libc++] Fixes disabling wide character.
ClosedPublic

Authored by Mordante on Aug 17 2023, 9:56 AM.

Details

Reviewers
ldionne
philnik
Group Reviewers
Restricted Project
Commits
rG1eb90b9a4e8f: [libc++] Fixes disabling wide character.
Summary

While testing the std module with -DLIBCXX_ENABLE_WIDE_CHARACTERS=OFF
the build failed. These functions are not exported from the module since
they use wchar_t. Disable them in the headers too.

Diff Detail

Event Timeline

Mordante created this revision.Aug 17 2023, 9:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 17 2023, 9:56 AM
Mordante requested review of this revision.Aug 17 2023, 9:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 17 2023, 9:56 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
philnik requested changes to this revision.Aug 17 2023, 10:31 AM
philnik added a subscriber: philnik.

Maybe I'm missing something, but it looks to me like you are disabling too many symbols here.

libcxx/include/cstdlib
138–139

This doesn't seem to require wchar_t.

libcxx/include/iosfwd
124–126

This also doesn't use wchar_t.

This revision now requires changes to proceed.Aug 17 2023, 10:31 AM
Harbormaster completed remote builds in B253250: Diff 551179.Aug 17 2023, 1:16 PM
Mordante marked 2 inline comments as done.Aug 19 2023, 4:22 AM
Mordante added inline comments.
libcxx/include/cstdlib
138–139

Good catch!

libcxx/include/iosfwd
124–126

This is strictly true, but I still think this should be disabled. It's clearly intended for wchar_t. The same is done for char8_t below.

Mordante updated this revision to Diff 551740.Aug 19 2023, 4:23 AM
Mordante marked 2 inline comments as done.

Addresses review comments.

Harbormaster completed remote builds in B253644: Diff 551740.Aug 19 2023, 6:47 AM
philnik accepted this revision.Aug 19 2023, 8:13 AM
This revision is now accepted and ready to land.Aug 19 2023, 8:13 AM
Closed by commit rG1eb90b9a4e8f: [libc++] Fixes disabling wide character. (authored by Mordante). · Explain WhyAug 19 2023, 10:09 AM
This revision was automatically updated to reflect the committed changes.
Mordante added a commit: rG1eb90b9a4e8f: [libc++] Fixes disabling wide character..

Revision Contents

PathSize
libcxx/
include/
2 lines
2 lines

Diff 551769

libcxx/include/cstdlib

Show First 20 Lines • Show All 129 Lines • ▼ Show 20 Lines
using ::bsearch _LIBCPP_USING_IF_EXISTS; using ::bsearch _LIBCPP_USING_IF_EXISTS;
using ::qsort _LIBCPP_USING_IF_EXISTS; using ::qsort _LIBCPP_USING_IF_EXISTS;
using ::abs _LIBCPP_USING_IF_EXISTS; using ::abs _LIBCPP_USING_IF_EXISTS;
using ::labs _LIBCPP_USING_IF_EXISTS; using ::labs _LIBCPP_USING_IF_EXISTS;
using ::llabs _LIBCPP_USING_IF_EXISTS; using ::llabs _LIBCPP_USING_IF_EXISTS;
using ::div _LIBCPP_USING_IF_EXISTS; using ::div _LIBCPP_USING_IF_EXISTS;
using ::ldiv _LIBCPP_USING_IF_EXISTS; using ::ldiv _LIBCPP_USING_IF_EXISTS;
using ::lldiv _LIBCPP_USING_IF_EXISTS; using ::lldiv _LIBCPP_USING_IF_EXISTS;
using ::mblen _LIBCPP_USING_IF_EXISTS; using ::mblen _LIBCPP_USING_IF_EXISTS;
#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
philnikUnsubmitted
Done
ReplyInline Actions

This doesn't seem to require wchar_t.

philnik: This doesn't seem to require `wchar_t`.
MordanteAuthorUnsubmitted
Done
ReplyInline Actions

Good catch!

Mordante: Good catch!
using ::mbtowc _LIBCPP_USING_IF_EXISTS; using ::mbtowc _LIBCPP_USING_IF_EXISTS;
using ::wctomb _LIBCPP_USING_IF_EXISTS; using ::wctomb _LIBCPP_USING_IF_EXISTS;
using ::mbstowcs _LIBCPP_USING_IF_EXISTS; using ::mbstowcs _LIBCPP_USING_IF_EXISTS;
using ::wcstombs _LIBCPP_USING_IF_EXISTS; using ::wcstombs _LIBCPP_USING_IF_EXISTS;
#endif
#if !defined(_LIBCPP_CXX03_LANG) #if !defined(_LIBCPP_CXX03_LANG)
using ::at_quick_exit _LIBCPP_USING_IF_EXISTS; using ::at_quick_exit _LIBCPP_USING_IF_EXISTS;
using ::quick_exit _LIBCPP_USING_IF_EXISTS; using ::quick_exit _LIBCPP_USING_IF_EXISTS;
#endif #endif
#if _LIBCPP_STD_VER >= 17 #if _LIBCPP_STD_VER >= 17
using ::aligned_alloc _LIBCPP_USING_IF_EXISTS; using ::aligned_alloc _LIBCPP_USING_IF_EXISTS;
#endif #endif
_LIBCPP_END_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP_CSTDLIB #endif // _LIBCPP_CSTDLIB

libcxx/include/iosfwd

Show First 20 Lines • Show All 115 Lines • ▼ Show 20 Lines
template <class _CharT, class _Traits = char_traits<_CharT> > template <class _CharT, class _Traits = char_traits<_CharT> >
class _LIBCPP_TEMPLATE_VIS istreambuf_iterator; class _LIBCPP_TEMPLATE_VIS istreambuf_iterator;
template <class _CharT, class _Traits = char_traits<_CharT> > template <class _CharT, class _Traits = char_traits<_CharT> >
class _LIBCPP_TEMPLATE_VIS ostreambuf_iterator; class _LIBCPP_TEMPLATE_VIS ostreambuf_iterator;
template <class _State> class _LIBCPP_TEMPLATE_VIS fpos; template <class _State> class _LIBCPP_TEMPLATE_VIS fpos;
typedef fpos<mbstate_t> streampos; typedef fpos<mbstate_t> streampos;
#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
typedef fpos<mbstate_t> wstreampos; typedef fpos<mbstate_t> wstreampos;
#endif
philnikUnsubmitted
Done
ReplyInline Actions

This also doesn't use wchar_t.

philnik: This also doesn't use `wchar_t`.
MordanteAuthorUnsubmitted
Done
ReplyInline Actions

This is strictly true, but I still think this should be disabled. It's clearly intended for wchar_t. The same is done for char8_t below.

Mordante: This is strictly true, but I still think this should be disabled. It's clearly intended for…
#ifndef _LIBCPP_HAS_NO_CHAR8_T #ifndef _LIBCPP_HAS_NO_CHAR8_T
typedef fpos<mbstate_t> u8streampos; typedef fpos<mbstate_t> u8streampos;
#endif #endif
typedef fpos<mbstate_t> u16streampos; typedef fpos<mbstate_t> u16streampos;
typedef fpos<mbstate_t> u32streampos; typedef fpos<mbstate_t> u32streampos;
// Include other forward declarations here // Include other forward declarations here
template <class _Tp, class _Alloc = allocator<_Tp> > template <class _Tp, class _Alloc = allocator<_Tp> >
Show All 32 Lines