On 27/08/20 12:37 -0400, Patrick Palka wrote:
On Thu, 27 Aug 2020, Jonathan Wakely wrote:
On 27/08/20 11:29 -0400, Patrick Palka via Libstdc++ wrote:
> This fixes the months-based addition for year_month when the
> year_month's month component is zero.
>
> Successfully tested on x86_64-pc-linux-gnu, on the 'date' library's
> calendar and (now) on libcxx's calendar tests. Does this look OK to
> commit?
>
> libstdc++-v3/ChangeLog:
>
> * include/std/chrono (year_month::operator+): Properly handle a
> month value of 0 by casting the month value to int before
> subtracting 1 from it so that the difference is signed in the
> subsequent addition.
> * testsuite/std/time/year_month/1.cc: Test addition of months to
> a year_month whose month value is below and above the normalized
> range of [1,12].
> ---
> libstdc++-v3/include/std/chrono | 2 +-
> libstdc++-v3/testsuite/std/time/year_month/1.cc | 12 ++++++++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/std/chrono
> b/libstdc++-v3/include/std/chrono
> index 417954d103b..398008c8f31 100644
> --- a/libstdc++-v3/include/std/chrono
> +++ b/libstdc++-v3/include/std/chrono
> @@ -2133,7 +2133,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> {
> // TODO: Optimize?
> auto __m = __ym.month() + __dm;
> - auto __i = unsigned{__ym.month()} - 1 + __dm.count();
> + auto __i = int(unsigned{__ym.month()}) - 1 + __dm.count();
The mix of paren-init and braced-init makes me do a double-take.
It makes me stop and wonder if there is some reason? Maybe to check
for narrowing? But it can't narrow since it just calls
year_month::operator unsigned().
[time.cal] seems to mostly use braced-init for converting a calendar
type to the underlying type, and I mindlessly followed suit :) e.g. in
[time.cal.year.nonmembers] and [time.cal.day.nonmembers].
Ah, but there's [time.cal.month.nonmembers]/7 which uses a static_cast
instead.
But it's really just because int{unsigned{__ym}} /would/ give a
narrowing warning, right?
Hmm yes, looks like it.
Would either int(unsigned(__my)) or static_cast<int>(__ym) make sense
instead, do avoid people wondering about it in future?
The first form works for me. The second form is rejected it seems.
Does the following look OK to commit?
OK, thanks.
-- >8 --
Subject: [PATCH] libstdc++: Fix arithmetic bug in
chrono::year_month::operator+
This fixes the months-based addition for year_month when the
year_month's month component is zero.
libstdc++-v3/ChangeLog:
* include/std/chrono (year_month::operator+): Properly handle a
month value of 0 by casting the month value to int before
subtracting 1 from it so that the difference is sign-extended in
the subsequent addition.
* testsuite/std/time/year_month/1.cc: Test adding months to a
year_month whose month component is below or above the
normalized range of [1,12].
---
libstdc++-v3/include/std/chrono | 2 +-
libstdc++-v3/testsuite/std/time/year_month/1.cc | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 417954d103b..f0fa66c7a2b 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -2133,7 +2133,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
// TODO: Optimize?
auto __m = __ym.month() + __dm;
- auto __i = unsigned{__ym.month()} - 1 + __dm.count();
+ auto __i = int(unsigned(__ym.month())) - 1 + __dm.count();
auto __y = (__i < 0
? __ym.year() + years{(__i - 11) / 12}
: __ym.year() + years{__i / 12});
diff --git a/libstdc++-v3/testsuite/std/time/year_month/1.cc
b/libstdc++-v3/testsuite/std/time/year_month/1.cc
index 007cfeb2f72..4c331dcdb50 100644
--- a/libstdc++-v3/testsuite/std/time/year_month/1.cc
+++ b/libstdc++-v3/testsuite/std/time/year_month/1.cc
@@ -83,4 +83,16 @@ constexpr_year_month()
static_assert(2017y/33 + months{0} == 2019y/9);
static_assert(2010y/January + months{-12} == 2009y/January);
+
+ static_assert(2010y/month{0} + months{-1} == 2009y/November);
+ static_assert(2010y/month{0} + months{0} == 2009y/December);
+ static_assert(2010y/month{0} + months{1} == 2010y/January);
+ static_assert(2010y/month{0} + months{2} == 2010y/February);
+ static_assert(2010y/month{0} + months{11} == 2010y/November);
+ static_assert(2010y/month{0} + months{12} == 2010y/December);
+ static_assert(2010y/month{0} + months{13} == 2011y/January);
+
+ static_assert(months{-1} + 2010y/month{37} == 2012y/December);
+ static_assert(months{0} + 2010y/month{37} == 2013y/January);
+ static_assert(months{1} + 2010y/month{37} == 2013y/February);
}
--
2.28.0.337.ge9b77c84a0
