public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713]
@ 2020-10-28 15:04 Patrick Palka
  2020-10-28 15:21 ` Patrick Palka
  2020-10-28 16:00 ` Jonathan Wakely
  0 siblings, 2 replies; 5+ messages in thread
From: Patrick Palka @ 2020-10-28 15:04 UTC (permalink / raw)
  To: gcc-patches; +Cc: libstdc++, Patrick Palka

The conversion function year_month_weekday::operator sys_days computes
the number of days to offset from the first weekday of the month with:

 days{(index()-1)*7}
      ^~~~~~~~~~~~~  type 'unsigned'

We'd like the above to yield -7d when index() is 0u, but our 'days'
alias is based on long instead of int, so the conversion from unsigned
to long instead yields a large positive quantity.

This patch fixes this by casting the result of index() to int so that
the initializer is sign-extended in the conversion to long.  The added
testcase also verifies that we do the right thing when index() == 5.

Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

libstdc++-v3/ChangeLog:

	PR libstdc++/96713
	* include/std/chrono (year_month_weekday::operator sys_days):
	Cast the result of index() to int so that the initializer for
	days{} is sign-extended when it's converted to the underlying
	type.
	* testsuite/std/time/year_month_weekday/3.cc: New test.
---
 libstdc++-v3/include/std/chrono               |  3 +-
 .../std/time/year_month_weekday/3.cc          | 66 +++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 7539d7184ea..7c35b78fe59 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -2719,7 +2719,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator sys_days() const noexcept
       {
 	auto __d = sys_days{year() / month() / 1};
-	return __d + (weekday() - chrono::weekday(__d) + days{(index()-1)*7});
+	return __d + (weekday() - chrono::weekday(__d)
+		      + days{((int)index()-1)*7});
       }
 
       explicit constexpr
diff --git a/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc b/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
new file mode 100644
index 00000000000..86db85d04e2
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
@@ -0,0 +1,66 @@
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// PR libstdc++/97613
+// Test year_month_weekday to sys_days conversion for extreme values of index().
+
+#include <chrono>
+
+void
+test01()
+{
+  using namespace std::chrono;
+  using ymd = year_month_day;
+  using ymwd = year_month_weekday;
+
+  static_assert(ymd{sys_days{2020y/January/Sunday[0]}} == 2019y/December/29);
+  static_assert(ymd{sys_days{2020y/January/Monday[0]}} == 2019y/December/30);
+  static_assert(ymd{sys_days{2020y/January/Tuesday[0]}} == 2019y/December/31);
+  static_assert(ymd{sys_days{2020y/January/Wednesday[0]}} == 2019y/December/25);
+  static_assert(ymd{sys_days{2020y/January/Thursday[0]}} == 2019y/December/26);
+  static_assert(ymd{sys_days{2020y/January/Friday[0]}} == 2019y/December/27);
+  static_assert(ymd{sys_days{2020y/January/Saturday[0]}} == 2019y/December/28);
+
+  static_assert((2020y).is_leap());
+  static_assert(ymd{sys_days{2020y/March/Sunday[0]}} == 2020y/February/23);
+  static_assert(ymd{sys_days{2020y/March/Monday[0]}} == 2020y/February/24);
+  static_assert(ymd{sys_days{2020y/March/Tuesday[0]}} == 2020y/February/25);
+  static_assert(ymd{sys_days{2020y/March/Wednesday[0]}} == 2020y/February/26);
+  static_assert(ymd{sys_days{2020y/March/Thursday[0]}} == 2020y/February/27);
+  static_assert(ymd{sys_days{2020y/March/Friday[0]}} == 2020y/February/28);
+  static_assert(ymd{sys_days{2020y/March/Saturday[0]}} == 2020y/February/29);
+
+  static_assert(!(2019y).is_leap());
+  static_assert(ymd{sys_days{2019y/March/Sunday[0]}} == 2019y/February/24);
+  static_assert(ymd{sys_days{2019y/March/Monday[0]}} == 2019y/February/25);
+  static_assert(ymd{sys_days{2019y/March/Tuesday[0]}} == 2019y/February/26);
+  static_assert(ymd{sys_days{2019y/March/Wednesday[0]}} == 2019y/February/27);
+  static_assert(ymd{sys_days{2019y/March/Thursday[0]}} == 2019y/February/28);
+  static_assert(ymd{sys_days{2019y/March/Friday[0]}} == 2019y/February/22);
+  static_assert(ymd{sys_days{2019y/March/Saturday[0]}} == 2019y/February/23);
+
+  static_assert(ymd{sys_days{2020y/December/Sunday[5]}} == 2021y/January/3);
+  static_assert(ymd{sys_days{2020y/December/Monday[5]}} == 2021y/January/4);
+  static_assert(ymd{sys_days{2020y/December/Tuesday[5]}} == 2020y/December/29);
+  static_assert(ymd{sys_days{2020y/December/Wednesday[5]}} == 2020y/December/30);
+  static_assert(ymd{sys_days{2020y/December/Thursday[5]}} == 2020y/December/31);
+  static_assert(ymd{sys_days{2020y/December/Friday[5]}} == 2021y/January/1);
+  static_assert(ymd{sys_days{2020y/December/Saturday[5]}} == 2021y/January/2);
+}
-- 
2.29.0.rc0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713]
  2020-10-28 15:04 [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713] Patrick Palka
@ 2020-10-28 15:21 ` Patrick Palka
  2020-10-28 16:01   ` Jonathan Wakely
  2020-10-28 16:00 ` Jonathan Wakely
  1 sibling, 1 reply; 5+ messages in thread
From: Patrick Palka @ 2020-10-28 15:21 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches, libstdc++

On Wed, 28 Oct 2020, Patrick Palka wrote:

> The conversion function year_month_weekday::operator sys_days computes
> the number of days to offset from the first weekday of the month with:
> 
>  days{(index()-1)*7}
>       ^~~~~~~~~~~~~  type 'unsigned'
> 
> We'd like the above to yield -7d when index() is 0u, but our 'days'
> alias is based on long instead of int, so the conversion from unsigned
> to long instead yields a large positive quantity.
> 
> This patch fixes this by casting the result of index() to int so that
> the initializer is sign-extended in the conversion to long.  The added
> testcase also verifies that we do the right thing when index() == 5.
> 
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> 
> libstdc++-v3/ChangeLog:
> 
> 	PR libstdc++/96713
> 	* include/std/chrono (year_month_weekday::operator sys_days):
> 	Cast the result of index() to int so that the initializer for
> 	days{} is sign-extended when it's converted to the underlying
> 	type.
> 	* testsuite/std/time/year_month_weekday/3.cc: New test.
> ---
>  libstdc++-v3/include/std/chrono               |  3 +-
>  .../std/time/year_month_weekday/3.cc          | 66 +++++++++++++++++++
>  2 files changed, 68 insertions(+), 1 deletion(-)
>  create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
> 
> diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
> index 7539d7184ea..7c35b78fe59 100644
> --- a/libstdc++-v3/include/std/chrono
> +++ b/libstdc++-v3/include/std/chrono
> @@ -2719,7 +2719,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        operator sys_days() const noexcept
>        {
>  	auto __d = sys_days{year() / month() / 1};
> -	return __d + (weekday() - chrono::weekday(__d) + days{(index()-1)*7});
> +	return __d + (weekday() - chrono::weekday(__d)
> +		      + days{((int)index()-1)*7});

On second thought, for consistency with the rest of the header, I guess
we should use a functional cast instead of a C-style cast here:

-- >8 --

libstdc++-v3/ChangeLog:

	PR libstdc++/96713
	* include/std/chrono (year_month_weekday::operator sys_days):
	Cast the result of index() to int so that the initializer for
	days{} is sign-extended when it's converted to the underlying
	type.
	* testsuite/std/time/year_month_weekday/3.cc: New test.
---
 libstdc++-v3/include/std/chrono               |  3 +-
 .../std/time/year_month_weekday/3.cc          | 65 +++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 7539d7184ea..f947082c528 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -2719,7 +2719,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator sys_days() const noexcept
       {
 	auto __d = sys_days{year() / month() / 1};
-	return __d + (weekday() - chrono::weekday(__d) + days{(index()-1)*7});
+	return __d + (weekday() - chrono::weekday(__d)
+		      + days{(int(index())-1)*7});
       }
 
       explicit constexpr
diff --git a/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc b/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
new file mode 100644
index 00000000000..cccaccef211
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
@@ -0,0 +1,65 @@
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// PR libstdc++/97613
+// Test year_month_weekday to sys_days conversion for extreme values of index().
+
+#include <chrono>
+
+void
+test01()
+{
+  using namespace std::chrono;
+  using ymd = year_month_day;
+
+  static_assert(ymd{sys_days{2020y/January/Sunday[0]}} == 2019y/December/29);
+  static_assert(ymd{sys_days{2020y/January/Monday[0]}} == 2019y/December/30);
+  static_assert(ymd{sys_days{2020y/January/Tuesday[0]}} == 2019y/December/31);
+  static_assert(ymd{sys_days{2020y/January/Wednesday[0]}} == 2019y/December/25);
+  static_assert(ymd{sys_days{2020y/January/Thursday[0]}} == 2019y/December/26);
+  static_assert(ymd{sys_days{2020y/January/Friday[0]}} == 2019y/December/27);
+  static_assert(ymd{sys_days{2020y/January/Saturday[0]}} == 2019y/December/28);
+
+  static_assert((2020y).is_leap());
+  static_assert(ymd{sys_days{2020y/March/Sunday[0]}} == 2020y/February/23);
+  static_assert(ymd{sys_days{2020y/March/Monday[0]}} == 2020y/February/24);
+  static_assert(ymd{sys_days{2020y/March/Tuesday[0]}} == 2020y/February/25);
+  static_assert(ymd{sys_days{2020y/March/Wednesday[0]}} == 2020y/February/26);
+  static_assert(ymd{sys_days{2020y/March/Thursday[0]}} == 2020y/February/27);
+  static_assert(ymd{sys_days{2020y/March/Friday[0]}} == 2020y/February/28);
+  static_assert(ymd{sys_days{2020y/March/Saturday[0]}} == 2020y/February/29);
+
+  static_assert(!(2019y).is_leap());
+  static_assert(ymd{sys_days{2019y/March/Sunday[0]}} == 2019y/February/24);
+  static_assert(ymd{sys_days{2019y/March/Monday[0]}} == 2019y/February/25);
+  static_assert(ymd{sys_days{2019y/March/Tuesday[0]}} == 2019y/February/26);
+  static_assert(ymd{sys_days{2019y/March/Wednesday[0]}} == 2019y/February/27);
+  static_assert(ymd{sys_days{2019y/March/Thursday[0]}} == 2019y/February/28);
+  static_assert(ymd{sys_days{2019y/March/Friday[0]}} == 2019y/February/22);
+  static_assert(ymd{sys_days{2019y/March/Saturday[0]}} == 2019y/February/23);
+
+  static_assert(ymd{sys_days{2020y/December/Sunday[5]}} == 2021y/January/3);
+  static_assert(ymd{sys_days{2020y/December/Monday[5]}} == 2021y/January/4);
+  static_assert(ymd{sys_days{2020y/December/Tuesday[5]}} == 2020y/December/29);
+  static_assert(ymd{sys_days{2020y/December/Wednesday[5]}} == 2020y/December/30);
+  static_assert(ymd{sys_days{2020y/December/Thursday[5]}} == 2020y/December/31);
+  static_assert(ymd{sys_days{2020y/December/Friday[5]}} == 2021y/January/1);
+  static_assert(ymd{sys_days{2020y/December/Saturday[5]}} == 2021y/January/2);
+}
-- 
2.29.0.rc0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713]
  2020-10-28 15:04 [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713] Patrick Palka
  2020-10-28 15:21 ` Patrick Palka
@ 2020-10-28 16:00 ` Jonathan Wakely
  1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2020-10-28 16:00 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches, libstdc++

On 28/10/20 11:04 -0400, Patrick Palka via Libstdc++ wrote:
>The conversion function year_month_weekday::operator sys_days computes
>the number of days to offset from the first weekday of the month with:
>
> days{(index()-1)*7}
>      ^~~~~~~~~~~~~  type 'unsigned'
>
>We'd like the above to yield -7d when index() is 0u, but our 'days'
>alias is based on long instead of int, so the conversion from unsigned
>to long instead yields a large positive quantity.
>
>This patch fixes this by casting the result of index() to int so that
>the initializer is sign-extended in the conversion to long.  The added
>testcase also verifies that we do the right thing when index() == 5.
>
>Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

Yes, thanks.


>libstdc++-v3/ChangeLog:
>
>	PR libstdc++/96713
>	* include/std/chrono (year_month_weekday::operator sys_days):
>	Cast the result of index() to int so that the initializer for
>	days{} is sign-extended when it's converted to the underlying
>	type.
>	* testsuite/std/time/year_month_weekday/3.cc: New test.
>---
> libstdc++-v3/include/std/chrono               |  3 +-
> .../std/time/year_month_weekday/3.cc          | 66 +++++++++++++++++++
> 2 files changed, 68 insertions(+), 1 deletion(-)
> create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
>
>diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
>index 7539d7184ea..7c35b78fe59 100644
>--- a/libstdc++-v3/include/std/chrono
>+++ b/libstdc++-v3/include/std/chrono
>@@ -2719,7 +2719,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       operator sys_days() const noexcept
>       {
> 	auto __d = sys_days{year() / month() / 1};
>-	return __d + (weekday() - chrono::weekday(__d) + days{(index()-1)*7});
>+	return __d + (weekday() - chrono::weekday(__d)
>+		      + days{((int)index()-1)*7});
>       }
>
>       explicit constexpr
>diff --git a/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc b/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
>new file mode 100644
>index 00000000000..86db85d04e2
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
>@@ -0,0 +1,66 @@
>+// { dg-options "-std=gnu++2a" }
>+// { dg-do compile { target c++2a } }
>+
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library.  This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+// GNU General Public License for more details.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// PR libstdc++/97613
>+// Test year_month_weekday to sys_days conversion for extreme values of index().
>+
>+#include <chrono>
>+
>+void
>+test01()
>+{
>+  using namespace std::chrono;
>+  using ymd = year_month_day;
>+  using ymwd = year_month_weekday;
>+
>+  static_assert(ymd{sys_days{2020y/January/Sunday[0]}} == 2019y/December/29);
>+  static_assert(ymd{sys_days{2020y/January/Monday[0]}} == 2019y/December/30);
>+  static_assert(ymd{sys_days{2020y/January/Tuesday[0]}} == 2019y/December/31);
>+  static_assert(ymd{sys_days{2020y/January/Wednesday[0]}} == 2019y/December/25);
>+  static_assert(ymd{sys_days{2020y/January/Thursday[0]}} == 2019y/December/26);
>+  static_assert(ymd{sys_days{2020y/January/Friday[0]}} == 2019y/December/27);
>+  static_assert(ymd{sys_days{2020y/January/Saturday[0]}} == 2019y/December/28);
>+
>+  static_assert((2020y).is_leap());
>+  static_assert(ymd{sys_days{2020y/March/Sunday[0]}} == 2020y/February/23);
>+  static_assert(ymd{sys_days{2020y/March/Monday[0]}} == 2020y/February/24);
>+  static_assert(ymd{sys_days{2020y/March/Tuesday[0]}} == 2020y/February/25);
>+  static_assert(ymd{sys_days{2020y/March/Wednesday[0]}} == 2020y/February/26);
>+  static_assert(ymd{sys_days{2020y/March/Thursday[0]}} == 2020y/February/27);
>+  static_assert(ymd{sys_days{2020y/March/Friday[0]}} == 2020y/February/28);
>+  static_assert(ymd{sys_days{2020y/March/Saturday[0]}} == 2020y/February/29);
>+
>+  static_assert(!(2019y).is_leap());
>+  static_assert(ymd{sys_days{2019y/March/Sunday[0]}} == 2019y/February/24);
>+  static_assert(ymd{sys_days{2019y/March/Monday[0]}} == 2019y/February/25);
>+  static_assert(ymd{sys_days{2019y/March/Tuesday[0]}} == 2019y/February/26);
>+  static_assert(ymd{sys_days{2019y/March/Wednesday[0]}} == 2019y/February/27);
>+  static_assert(ymd{sys_days{2019y/March/Thursday[0]}} == 2019y/February/28);
>+  static_assert(ymd{sys_days{2019y/March/Friday[0]}} == 2019y/February/22);
>+  static_assert(ymd{sys_days{2019y/March/Saturday[0]}} == 2019y/February/23);
>+
>+  static_assert(ymd{sys_days{2020y/December/Sunday[5]}} == 2021y/January/3);
>+  static_assert(ymd{sys_days{2020y/December/Monday[5]}} == 2021y/January/4);
>+  static_assert(ymd{sys_days{2020y/December/Tuesday[5]}} == 2020y/December/29);
>+  static_assert(ymd{sys_days{2020y/December/Wednesday[5]}} == 2020y/December/30);
>+  static_assert(ymd{sys_days{2020y/December/Thursday[5]}} == 2020y/December/31);
>+  static_assert(ymd{sys_days{2020y/December/Friday[5]}} == 2021y/January/1);
>+  static_assert(ymd{sys_days{2020y/December/Saturday[5]}} == 2021y/January/2);
>+}
>-- 
>2.29.0.rc0
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713]
  2020-10-28 15:21 ` Patrick Palka
@ 2020-10-28 16:01   ` Jonathan Wakely
  2020-10-28 16:52     ` Patrick Palka
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2020-10-28 16:01 UTC (permalink / raw)
  To: Patrick Palka; +Cc: libstdc++, gcc-patches

On 28/10/20 11:21 -0400, Patrick Palka via Libstdc++ wrote:
>On Wed, 28 Oct 2020, Patrick Palka wrote:
>
>> The conversion function year_month_weekday::operator sys_days computes
>> the number of days to offset from the first weekday of the month with:
>>
>>  days{(index()-1)*7}
>>       ^~~~~~~~~~~~~  type 'unsigned'
>>
>> We'd like the above to yield -7d when index() is 0u, but our 'days'
>> alias is based on long instead of int, so the conversion from unsigned
>> to long instead yields a large positive quantity.
>>
>> This patch fixes this by casting the result of index() to int so that
>> the initializer is sign-extended in the conversion to long.  The added
>> testcase also verifies that we do the right thing when index() == 5.
>>
>> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>>
>> libstdc++-v3/ChangeLog:
>>
>> 	PR libstdc++/96713
>> 	* include/std/chrono (year_month_weekday::operator sys_days):
>> 	Cast the result of index() to int so that the initializer for
>> 	days{} is sign-extended when it's converted to the underlying
>> 	type.
>> 	* testsuite/std/time/year_month_weekday/3.cc: New test.
>> ---
>>  libstdc++-v3/include/std/chrono               |  3 +-
>>  .../std/time/year_month_weekday/3.cc          | 66 +++++++++++++++++++
>>  2 files changed, 68 insertions(+), 1 deletion(-)
>>  create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
>>
>> diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
>> index 7539d7184ea..7c35b78fe59 100644
>> --- a/libstdc++-v3/include/std/chrono
>> +++ b/libstdc++-v3/include/std/chrono
>> @@ -2719,7 +2719,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>        operator sys_days() const noexcept
>>        {
>>  	auto __d = sys_days{year() / month() / 1};
>> -	return __d + (weekday() - chrono::weekday(__d) + days{(index()-1)*7});
>> +	return __d + (weekday() - chrono::weekday(__d)
>> +		      + days{((int)index()-1)*7});
>
>On second thought, for consistency with the rest of the header, I guess
>we should use a functional cast instead of a C-style cast here:

Or static_cast<int>(index()) :-)

Any of those options is OK.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713]
  2020-10-28 16:01   ` Jonathan Wakely
@ 2020-10-28 16:52     ` Patrick Palka
  0 siblings, 0 replies; 5+ messages in thread
From: Patrick Palka @ 2020-10-28 16:52 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Patrick Palka, libstdc++, gcc-patches

On Wed, 28 Oct 2020, Jonathan Wakely wrote:

> On 28/10/20 11:21 -0400, Patrick Palka via Libstdc++ wrote:
> > On Wed, 28 Oct 2020, Patrick Palka wrote:
> > 
> > > The conversion function year_month_weekday::operator sys_days computes
> > > the number of days to offset from the first weekday of the month with:
> > > 
> > >  days{(index()-1)*7}
> > >       ^~~~~~~~~~~~~  type 'unsigned'
> > > 
> > > We'd like the above to yield -7d when index() is 0u, but our 'days'
> > > alias is based on long instead of int, so the conversion from unsigned
> > > to long instead yields a large positive quantity.
> > > 
> > > This patch fixes this by casting the result of index() to int so that
> > > the initializer is sign-extended in the conversion to long.  The added
> > > testcase also verifies that we do the right thing when index() == 5.
> > > 
> > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> > > 
> > > libstdc++-v3/ChangeLog:
> > > 
> > > 	PR libstdc++/96713
> > > 	* include/std/chrono (year_month_weekday::operator sys_days):
> > > 	Cast the result of index() to int so that the initializer for
> > > 	days{} is sign-extended when it's converted to the underlying
> > > 	type.
> > > 	* testsuite/std/time/year_month_weekday/3.cc: New test.
> > > ---
> > >  libstdc++-v3/include/std/chrono               |  3 +-
> > >  .../std/time/year_month_weekday/3.cc          | 66 +++++++++++++++++++
> > >  2 files changed, 68 insertions(+), 1 deletion(-)
> > >  create mode 100644
> > > libstdc++-v3/testsuite/std/time/year_month_weekday/3.cc
> > > 
> > > diff --git a/libstdc++-v3/include/std/chrono
> > > b/libstdc++-v3/include/std/chrono
> > > index 7539d7184ea..7c35b78fe59 100644
> > > --- a/libstdc++-v3/include/std/chrono
> > > +++ b/libstdc++-v3/include/std/chrono
> > > @@ -2719,7 +2719,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >        operator sys_days() const noexcept
> > >        {
> > >  	auto __d = sys_days{year() / month() / 1};
> > > -	return __d + (weekday() - chrono::weekday(__d) + days{(index()-1)*7});
> > > +	return __d + (weekday() - chrono::weekday(__d)
> > > +		      + days{((int)index()-1)*7});
> > 
> > On second thought, for consistency with the rest of the header, I guess
> > we should use a functional cast instead of a C-style cast here:
> 
> Or static_cast<int>(index()) :-)
> 
> Any of those options is OK.

Thanks for the review.  I committed the patch just now, albeit with
reference to the unrelated PR96713 instead of PR97613 :( I'll adjust the
ChangeLog entry when it's generated tomorrow.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-28 16:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-28 15:04 [PATCH] libstdc++: Fix arithmetic bug in year_month_weekday conversion [PR96713] Patrick Palka
2020-10-28 15:21 ` Patrick Palka
2020-10-28 16:01   ` Jonathan Wakely
2020-10-28 16:52     ` Patrick Palka
2020-10-28 16:00 ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).