# [libc++] Fix compilation error on platforms that don't implement std::tmClosedPublicActions

Authored by ldionne on Sep 8 2022, 7:00 AM.

# Details

Reviewers
 jloser
Group Reviewers
 Restricted Project
Commits
rGd529e8110bdf: [libc++] Fix compilation error on platforms that don't implement std::tm
Summary

Instead of mentioning tm directly in the definition of __convert_to_tm,
take it as a template argument. As a fly-by also fix incorrect Lit feature
(should have been no-localization instead of libcpp-has-no-localization).

# Diff Detail

### Event Timeline

ldionne created this revision.Sep 8 2022, 7:00 AM
Herald added a project: Restricted Project. Sep 8 2022, 7:00 AM
ldionne requested review of this revision.Sep 8 2022, 7:00 AM
Herald added a project: Restricted Project. Sep 8 2022, 7:00 AM
Herald added a reviewer: Restricted Project.
jloser added a subscriber: jloser.Sep 8 2022, 7:54 AM
libcxx/include/__chrono/convert_to_tm.h
15

Since we're taking the tm type as a template argument, we never explicitly mention std::tm now. Therefore, we should be able to remove this include, right?

ldionne marked an inline comment as done.Sep 8 2022, 7:57 AM
libcxx/include/__chrono/convert_to_tm.h
15

Yes, that sounds right.

ldionne updated this revision to Diff 458742.Sep 8 2022, 7:57 AM
ldionne marked an inline comment as done.

jloser accepted this revision.Sep 8 2022, 8:43 AM

LGTM!

Which platforms don't have tm that we support? Can we add a CI for them?

libcxx/include/__chrono/convert_to_tm.h
32

Why is this change? As mentioned during our review it looks a bit odd now. However this function will get support for all chrono types that are formattable. An example of the "final" version can be seen in the function __make_tm
in https://reviews.llvm.org/D126592#change-qGrpldbZrZTa .

libcxx/test/std/time/time.syn/formatter.day.pass.cpp
9

Thanks! Nice catch.

ldionne marked an inline comment as done.Sep 8 2022, 12:34 PM

Which platforms don't have tm that we support? Can we add a CI for them?

Apple's DriverKit is an example of that. Adding CI for that is on my todo list, but there's some stuff that needs to happen before that can be done.

libcxx/include/__chrono/convert_to_tm.h
32

Oh, yes, you're right. I'll undo that part.

ldionne updated this revision to Diff 458828.Sep 8 2022, 12:34 PM
ldionne marked an inline comment as done.

ldionne accepted this revision as: Restricted Project.Sep 8 2022, 3:10 PM
This revision is now accepted and ready to land.Sep 8 2022, 3:10 PM
This revision was automatically updated to reflect the committed changes.

Which platforms don't have tm that we support? Can we add a CI for them?

Apple's DriverKit is an example of that. Adding CI for that is on my todo list, but there's some stuff that needs to happen before that can be done.

Thanks for the info.

LGTM!

libcxx/include/__chrono/convert_to_tm.h
27

I'm not thrilled by this name, but when I have a better name I'll incorporate that in a next patch.