This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Remove unique usage of the en_US locale in a test
ClosedPublic

Authored by ldionne on Sep 29 2022, 3:16 PM.

Details

Summary

Just like we guard other tests with REQUIRES: locale.en_US.UTF-8, it makes sense to
guard this test for its usage of the en_US locale.

Diff Detail

Event Timeline

ldionne created this revision.Sep 29 2022, 3:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2022, 3:16 PM
Herald added a subscriber: arichardson. · View Herald Transcript
ldionne requested review of this revision.Sep 29 2022, 3:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2022, 3:16 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I came across this in a different context, but please let me know if that doesn't make sense, I'm obviously not a locale expert.

mstorsjo accepted this revision.Sep 29 2022, 9:12 PM

Looks reasonable to me!

Mordante accepted this revision.Sep 29 2022, 10:52 PM

LGTM, but I would consider an alternate approach and not add this locale to features.py.

libcxx/test/std/localization/locale.categories/category.ctype/locale.codecvt.byname/ctor_char.pass.cpp
16

Looking at the test it doesn't really matter which locale is used. So I would suggest to use an existing locale in this patch instead, and update the commit title and message.

This revision is now accepted and ready to land.Sep 29 2022, 10:52 PM
ldionne updated this revision to Diff 464233.Sep 30 2022, 5:36 AM
ldionne retitled this revision from [libc++] Add missing requirement for en_US locale to test to [libc++] Remove unique usage of the en_US locale in a test.

Address comment.