This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Make bsd_locale_fallbacks.h modular and move it into __locale/locale_base_api/
ClosedPublic

Authored by philnik on Mar 19 2023, 3:45 PM.

Details

Summary

This is a first step towards granularizing <locale>.

Diff Detail

Event Timeline

philnik created this revision.Mar 19 2023, 3:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 19 2023, 3:45 PM
philnik requested review of this revision.Mar 19 2023, 3:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 19 2023, 3:45 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik updated this revision to Diff 507471.Mar 22 2023, 12:23 PM

Try to fix CI

philnik updated this revision to Diff 507496.Mar 22 2023, 2:09 PM

Try to fix CI

ldionne added inline comments.Mar 30 2023, 8:46 AM
libcxx/include/CMakeLists.txt
217

Granularization patches are usually pretty mechanical because the organization of our headers already makes sense. However, the organization in <locale> makes little sense (at least to me). There's clearly some design trying to emerge, but it seems to have grown in weird ways with time. So I think it is worth designing how we want the organization to look like. I suggest:

__locale_dir/
    locale_base_api/
        bsd_locale_defaults.h   // 
        bsd_locale_fallbacks.h  // These headers are meant to be included by the per-platform
        locale_guard.h          // shims to provide the BSD-like API that we expect
 
        android.h
        solaris.h
        [...]
        freebsd.h
    locale_base_api.h           // This header would contain an #if-else chain that includes the
                                // appropriate shims for the current platform. It would also document 
                                // what base API a new platform has to support for libc++ to support 
                                // <locale>, which is kind of missing right now.
    scan_keyword.h
    [...]

Then, in <locale> we would include <__locale_dir/shims.h> and we would be guaranteed to have a BSD-like API available.

This patch doesn't have to go all the way, but once we agree on the end organization, I would move stuff around in a way that's consistent with where we want to end up.

philnik updated this revision to Diff 509709.Mar 30 2023, 9:56 AM
philnik marked an inline comment as done.

Address comments

philnik updated this revision to Diff 509716.Mar 30 2023, 10:18 AM

Try to fix CI

philnik updated this revision to Diff 509720.Mar 30 2023, 10:25 AM

Try to fix CI

philnik updated this revision to Diff 509772.Mar 30 2023, 12:13 PM

Fix lint_modulemap.sh.py

philnik retitled this revision from [libc++] Make bsd_locale_fallbacks.h modular and move it into __support/xlocale to [libc++] Make bsd_locale_fallbacks.h modular and move it into __locale/locale_base_api/.Mar 31 2023, 2:39 AM
philnik updated this revision to Diff 509951.Mar 31 2023, 2:43 AM

Try to fix CI

philnik updated this revision to Diff 510339.Apr 2 2023, 5:27 AM

Try to fix CI

philnik updated this revision to Diff 510573.Apr 3 2023, 11:28 AM

Try to fix CI

philnik updated this revision to Diff 510602.Apr 3 2023, 2:04 PM

Try to fix CI

philnik updated this revision to Diff 510735.Apr 4 2023, 3:28 AM

Another try

philnik updated this revision to Diff 511900.Apr 8 2023, 9:36 AM

Try to fix CI

philnik updated this revision to Diff 511915.Apr 8 2023, 11:45 AM

Try to fix CI

philnik updated this revision to Diff 511940.Apr 8 2023, 6:16 PM

Try to fix CI

philnik updated this revision to Diff 511995.Apr 9 2023, 5:58 AM

Try to fix CI

philnik updated this revision to Diff 512008.Apr 9 2023, 7:54 AM

Next try

ldionne accepted this revision.Apr 20 2023, 8:12 AM
ldionne added inline comments.
libcxx/include/__bsd_locale_defaults.h
14

This doesn't seem right anymore. Please check the other ones as well.

libcxx/utils/generate_iwyu_mapping.py
33 ↗(On Diff #512008)

I thought __tuple_dir had been renamed to __tuple already, maybe you need to rebase?

This revision is now accepted and ready to land.Apr 20 2023, 8:12 AM
philnik updated this revision to Diff 515483.Apr 20 2023, 2:22 PM
philnik marked 2 inline comments as done.

Address comments

This revision was landed with ongoing or failed builds.Apr 20 2023, 8:36 PM
This revision was automatically updated to reflect the committed changes.