Page MenuHomePhabricator

[libcxx] Add appropriate 'REQUIRE' directives to tests that require en_US.UTF-8.
ClosedPublic

Authored by dsanders on Jan 21 2016, 6:32 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

dsanders updated this revision to Diff 45527.Jan 21 2016, 6:32 AM
dsanders retitled this revision from to [libcxx] Add appropriate 'REQUIRE' directives to tests that require en_US.UTF-8..
dsanders updated this object.
dsanders added reviewers: mclow.lists, hans.
dsanders added a subscriber: cfe-commits.

Hi,

I'd like to merge this to the 3.8 branch once it has been accepted.

bcraig added a subscriber: bcraig.Jan 21 2016, 7:11 AM

LGTM, but that doesn't mean much.

I suspect that there are some tests that have snuck past this sweep, but I haven't done the work to figure out which ones. I base this statement off of these search results...
LOCALE_en_US_UTF_8 found in 78 files underneath libcxx/test (prior to this change)
locale.en_US.UTF-8 found in 46 files underneath libcxx/test (prior to this change)
23 new instances of locale.en_US.UTF-8 in this change.

That suggests that there are at least 78 - (46+23) = 9 files that mention LOCALE_en_US_UTF_8 that don't have the REQUIRES clause. Maybe some other REQUIRES clause is making it so your test runs work. In my opinion, it is fine to leave that work for someone else.

dsanders updated this revision to Diff 45531.Jan 21 2016, 7:35 AM

Added one more. It was also guarded by a check for ru_RU.UTF-8 so it was missed on the first sweep.

Thanks. I've added one more which came up after enabling all the missing locales except for en_US.UTF-8.

I'll commit this and find the other 8 with grep afterwards.

This revision was automatically updated to reflect the committed changes.