[PATCH] D51762: First part of the calendar stuff
ldionne closed this revision. ldionne added a comment. Like I said, this was committed in 5b08c1742a536f54bd5e270b00ff851cbc7314ef CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51762/new/ https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
ldionne accepted this revision. ldionne added a comment. Herald added subscribers: Charusso, dexonsmith. Committed in 5b08c1742a536f54bd5e270b00ff851cbc7314ef CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51762/new/ https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
mclow.lists marked 2 inline comments as done.
mclow.lists added inline comments.
Comment at: include/chrono:2667
+#if _LIBCPP_STD_VER > 17
+constexpr chrono::day operator ""d(unsigned long long __d) noexcept
+{
EricWF wrote:
> 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
> ```
I did this by defining `_LIBCPP_HAS_NO_CXX20_CHRONO_LITERALS` instead
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D51762/new/
https://reviews.llvm.org/D51762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
NoQ added a comment. Ugh i cannot close it :/ https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
NoQ accepted this revision. NoQ added a comment. Whoops right sry again! https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
ldionne added a comment. Can this be closed? My understanding is that this has been committed now. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
NoQ added a comment. Re-applied https://reviews.llvm.org/rL344546 as https://reviews.llvm.org/rL344582. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
NoQ added a comment. Also i should not have reverted r344546, it was completely unrelated >_< https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
NoQ added a comment. Well, i guess something went wrong, because the job behind the link is in fact the first job on this buildbot that included r344535. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
mclow.lists added a comment. In https://reviews.llvm.org/D51762#1266086, @NoQ wrote: > Had to revert. Sorry! https://reviews.llvm.org/rL344580. > > This failure was masked by another error, so i guess it was missed. This was supposed to be fixed by commit r344535. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
NoQ added a comment. Had to revert. Sorry! https://reviews.llvm.org/rL344580. This failure was masked by another error, so i guess it was missed. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
NoQ added a comment. There seem to be more buildbots failing even after https://reviews.llvm.org/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 https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
mclow.lists accepted this revision. mclow.lists added a comment. After discussions with @EricWF, I landed a modified version as revision 344529. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
EricWF added inline comments. Comment at: test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp:10 +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// XFAIL + mclow.lists wrote: > EricWF wrote: > > What? > The `sys_days` type is not implemented yet. > That will be in the next patch. > Right. But this `XFAIL` does nothing. The test suite only accepts `XFAIL: ` for syntax. If the `XFAIL` was actually considered, this test would fail as "unexpectedly passing" https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
mclow.lists added a comment. This is the first of part of this functionality. Some of these bits are not here yet. Comment at: test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp:10 +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// XFAIL + EricWF wrote: > What? The `sys_days` type is not implemented yet. That will be in the next patch. Comment at: test/std/utilities/time/time.cal/time.cal.ymwdlast/time.cal.ymwdlast.nonmembers/streaming.pass.cpp:10 +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// XFAIL + EricWF wrote: > What? Again; the streaming bits are not here in this patch. Next patch. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
EricWF added a comment. 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. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
EricWF requested changes to this revision.
EricWF added a comment.
This revision now requires changes to proceed.
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.
Comment at: include/chrono:2667
+#if _LIBCPP_STD_VER > 17
+constexpr chrono::day operator ""d(unsigned long long __d) noexcept
+{
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
```
https://reviews.llvm.org/D51762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
EricWF added a comment. There also seem to be some non-ASCII characters in this changeset. Could you hunt them out and destroy them? https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51762: First part of the calendar stuff
EricWF added a comment.
Herald added a subscriber: arphaman.
What's with all the XFAIL's?
Comment at:
test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp:10
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// XFAIL
+
What?
Comment at:
test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp:41
+
+// ASSERT_NOEXCEPT(year_month_weekday{std::declval()});
+
What's this test doing?
Comment at:
test/std/utilities/time/time.cal/time.cal.ymwdlast/time.cal.ymwdlast.nonmembers/streaming.pass.cpp:10
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// XFAIL
+
What?
https://reviews.llvm.org/D51762
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
