[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
        assert(base(i) == in+2);
        assert(base(i) == in+2);
Our tests should be portable and MSVC STL and GCC use them.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list