[libcxx-commits] [PATCH] D124175: [libcxx] Reject month 0 in get_date/__get_month
Mark de Wever via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Apr 21 10:25:48 PDT 2022
Mordante added a comment.
Thanks for working on this!
In D124175#3464773 <https://reviews.llvm.org/D124175#3464773>, @DavidSpickett wrote:
> For https://github.com/llvm/llvm-project/issues/47663.
>
> From https://en.cppreference.com/w/cpp/locale/time_get/get_date (not sure if I should be getting the actual standard document for this) this seems like it could be ok.
Nothing wrong with cppreference, but for the Standard wording we usually use http://eel.is/c++draft/locale.time.get.virtuals#lib:do_get_date,time_get. (Not sure whether you know this resource.)
> I read in a draft standard for `do_get_date` specifically "Returns: An iterator pointing immediately beyond the last character recognized as possibly part of a valid date.". So "possibly" seems like it would allow for both behaviours.
I think libstdc++'s behaviour is the correct behaviour. But fixing that should be a separate review.
================
Comment at: libcxx/test/std/localization/locale.categories/category.time/locale.time.get.byname/get_date.pass.cpp:18
// GLIBC fails on the zh_CN test.
// XFAIL: linux
----------------
:-( This is not your change, but it basically means these changes aren't tested in most of our CI. I'll create a fix.
================
Comment at: libcxx/test/std/localization/locale.categories/category.time/locale.time.get.byname/get_date.pass.cpp:114
+ // We consume the first two chars checking the month.
+ assert(base(i) == in+2);
+ // tm is not modified.
----------------
This should be guarded with
```
#if _LIBCPP_VERSION
assert(base(i) == in+2);
#else
assert(base(i) == in+2);
#endif
```
Our tests should be portable and MSVC STL and GCC use them.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124175/new/
https://reviews.llvm.org/D124175
More information about the libcxx-commits
mailing list