This is an archive of the discontinued LLVM Phabricator instance.

First part of the <chrono> calendar stuff
ClosedPublic

Authored by mclow.lists on Sep 6 2018, 4:58 PM.

Details

Summary

Before we get to actually doing calendar stuff, or time zone stuff, there's a whole bunch of support classes that need to be implemented. Things like day, week, month, year, day_month, etc.

This patch implements the first chunk of those.
Apologies for the size of the patch, but (like the <span> stuff), it's 90+% tests.

Diff Detail

Event Timeline

mclow.lists created this revision.Sep 6 2018, 4:58 PM

What's with all the XFAIL's?

test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp
10

What?

41

What's this test doing?

test/std/utilities/time/time.cal/time.cal.ymwdlast/time.cal.ymwdlast.nonmembers/streaming.pass.cpp
10

What?

There also seem to be some non-ASCII characters in this changeset. Could you hunt them out and destroy them?

EricWF requested changes to this revision.Oct 11 2018, 10:23 AM

Have you run git clang-format over this change set?

The blocking issues I see are:

  • The literal's need to be guarded against older clang dialects. So do their tests.
  • There are a bunch of tests with meaningless XFAIL directives in them. They need to be removed.
include/chrono
2667

Including this file with Clang 6.0 in C++2a mode causes a compile error because of "-Wreserved-user-defined-literal". We need to wrap this block with:

#if defined(__clang__)
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wreserved-user-defined-literal"
#endif
    [...]
#if defined(__clang__)
#pragma clang diagnostic pop
#endif
This revision now requires changes to proceed.Oct 11 2018, 10:23 AM

Also I'm not sure we want to commit to this ABI where the representation of types like day, month, and year are smaller than the integer types they can be constructed from.

This is the first of part of this functionality. Some of these bits are not here yet.

test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp
10

The sys_days type is not implemented yet.
That will be in the next patch.

test/std/utilities/time/time.cal/time.cal.ymwdlast/time.cal.ymwdlast.nonmembers/streaming.pass.cpp
10

Again; the streaming bits are not here in this patch.
Next patch.

EricWF added inline comments.Oct 12 2018, 10:50 PM
test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp
10

Right. But this XFAIL does nothing. The test suite only accepts XFAIL: <condition> for syntax.

If the XFAIL was actually considered, this test would fail as "unexpectedly passing"

mclow.lists accepted this revision.Oct 15 2018, 9:09 AM

After discussions with @EricWF, I landed a modified version as revision 344529.

NoQ added a subscriber: NoQ.Oct 15 2018, 5:14 PM

There seem to be more buildbots failing even after rL344535, eg. http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/50318/console

Failing Tests (3):
    libc++ :: std/utilities/time/time.cal/time.cal.day/time.cal.day.nonmembers/literals.fail.cpp
    libc++ :: std/utilities/time/time.cal/time.cal.year/time.cal.year.nonmembers/literals.fail.cpp
    libc++ :: std/utilities/time/time.duration/time.duration.literals/literals.pass.cpp
NoQ added a comment.Oct 15 2018, 7:44 PM

Had to revert. Sorry! rL344580.

This failure was masked by another error, so i guess it was missed.

In D51762#1266086, @NoQ wrote:

Had to revert. Sorry! rL344580.

This failure was masked by another error, so i guess it was missed.

This was supposed to be fixed by commit r344535.

NoQ added a comment.Oct 15 2018, 8:32 PM

Well, i guess something went wrong, because the job behind the link is in fact the first job on this buildbot that included r344535.

NoQ added a comment.Oct 15 2018, 8:37 PM

Also i should not have reverted r344546, it was completely unrelated >_<

Can this be closed? My understanding is that this has been committed now.

NoQ accepted this revision.Nov 13 2018, 10:22 AM

Whoops right sry again!

NoQ added a comment.Nov 13 2018, 10:23 AM

Ugh i cannot close it :/

mclow.lists marked 2 inline comments as done.Jan 14 2019, 10:07 AM
mclow.lists added inline comments.
include/chrono
2667

I did this by defining _LIBCPP_HAS_NO_CXX20_CHRONO_LITERALS instead

ldionne accepted this revision.Mar 9 2020, 6:19 AM

Committed in 5b08c1742a536f54bd5e270b00ff851cbc7314ef

EricWF accepted this revision.Mar 9 2020, 7:32 AM
This revision is now accepted and ready to land.Mar 9 2020, 7:32 AM
ldionne closed this revision.Mar 9 2020, 10:40 AM

Like I said, this was committed in 5b08c1742a536f54bd5e270b00ff851cbc7314ef

ruoka added a subscriber: ruoka.Apr 14 2020, 8:48 PM