This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Avoid pulling in xlocinfo.h in public headers
ClosedPublic

Authored by mstorsjo on Mar 23 2021, 1:34 PM.

Details

Summary

Including xlocinfo.h is a bit of a layering violation; locale.h is
the C library header we should use, while xlocinfo.h is essentially
part of the MS C++ library. Including xlocinfo.h brings in yvals.h,
which brings in yvals_core.h, which defines the MS STL's version
support macros, overriding what libc++'s <version> had defined.

Instead just include locale.h, and provide the few defines we need
for locale categories manually.

Diff Detail

Event Timeline

mstorsjo requested review of this revision.Mar 23 2021, 1:34 PM
mstorsjo created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2021, 1:34 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
curdeius accepted this revision as: curdeius.Mar 23 2021, 1:51 PM
curdeius added a subscriber: curdeius.

LGTM. I've been looking at those failing tests that were indeed pulling in MS STL's <yvals_core.h> but I didn't go further.

libcxx/include/__support/win32/locale_win32.h
26

Why not doing the same as https://github.com/microsoft/STL/blob/main/stl/inc/xlocinfo.h#L35? I.e. _X_MESSAGES + 1.
No strong feelings about it anyway.

amccarth accepted this revision.Mar 23 2021, 1:57 PM

LGTM.

LGTM. I've been looking at those failing tests that were indeed pulling in MS STL's <yvals_core.h> but I didn't go further.

Ooo, nice, that sound great if you're getting nerdsniped into looking into these ;-)

libcxx/include/__support/win32/locale_win32.h
26

Sure, I can do that too.

Technically, we don't need _X_ALL or _M_ALLat all, so we could just omit them too, but I thought it'd be best to provide the same full set as the corresponding MSVC headers do, for consistency.

Quuxplusone accepted this revision.Mar 24 2021, 12:44 PM

Assuming you trust @amccarth (and/or yourself) to be the voice of authority on whether this is correct re Win32, I'm happy to be the second libc++ approver here.

This revision is now accepted and ready to land.Mar 24 2021, 12:44 PM
This revision was automatically updated to reflect the committed changes.