[libcxx-commits] [PATCH] D70346: [libc++] [LWG3321] Mark "year_month_day_last::day() specification does not cover !ok() values" issue as "Nothing to do", but add assertion.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D70346#2082298 <https://reviews.llvm.org/D70346#2082298>, @howard.hinnant wrote:

> The spec does not allow `year_month_day_last::day()` to crash.  It must not assert.  It must not index an array out of bounds.  It must not cause undefined behavior.
>
> For example in my example implementation:  `(2020_y/month{13}/last).day() == 29_d`
>
> I recommend a check that `month().ok()` prior to using it as an index.


You are correct, according to http://eel.is/c++draft/time.cal.ymdlast#members-15:

> Returns: If `ok()` is `true`, returns a day representing the last day of the (year, month) pair represented by `*this`. Otherwise, the returned value is unspecified.

However, that doesn't strike me as a super useful behavior -- why was it chosen that way? It seems like it's a lot more useful from a user perspective to crash early when asking for the `day()` of a malformed date?

In the meantime, I'll revert this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70346/new/

https://reviews.llvm.org/D70346