public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: Fix operator overload resolution for calendar types
@ 2020-08-25 19:47 Patrick Palka
  2020-08-25 19:51 ` Patrick Palka
  2020-08-27 16:24 ` Jonathan Wakely
  0 siblings, 2 replies; 3+ messages in thread
From: Patrick Palka @ 2020-08-25 19:47 UTC (permalink / raw)
  To: gcc-patches; +Cc: libstdc++, Patrick Palka

My original patch that implemented the calendar type operations failed
to enforce a constraint on some of the addition/subtraction operator
overloads that take a 'months' argument:

  Constraints: If the argument supplied by the caller for the months
  parameter is convertible to years, its implicit conversion sequence to
  years is worse than its implicit conversion sequence to months

This constraint is relevant when adding/subtracting a duration to/from
say a year_month when the given duration is convertible to both 'months'
and to 'years'.  The correct behavior here in light of this constraint
is to select the (more efficient) 'years'-taking overload, but we
currently emit an ambiguous overload error.

This patch follows the approach taken in the 'date' library and defines
the constrained 'months'-taking operator overloads as function
templates, so that we break such a implicit-conversion tie by selecting
the non-template 'years'-taking overload.

Tested on x86_64-pc-linux-gnu, does this look OK to commit?  (The below
diff is generated with --ignore-space-change for easier review.  In the
actual patch, the function templates are indented two extra spaces after
the template parameter list.)

libstdc++-v3/ChangeLog:

	* include/std/chrono (__detail::__unspecified_month_disambuator):
	Define.
	(year_month::operator+=): Turn the 'months'-taking overload
	into a function template, so that the 'years'-taking overload is
	selected in case of an equally-ranked implicit conversion
	sequence to both 'months' and 'years' from the supplied argument.
	(year_month::operator-=): Likewise.
	(year_month::operator+): Likewise.
	(year_month::operator-): Likewise.
	(year_month_day::operator+=): Likewise.
	(year_month_day::operator-=): Likewise.
	(year_month_day::operator+): Likewise.
	(year_month_day::operator-): Likewise.
	(year_month_day_last::operator+=): Likewise.
	(year_month_day_last::operator-=): Likewise.
	(year_month_day_last::operator+): Likewise
	(year_month_day_last::operator-): Likewise.
	(year_month_day_weekday::operator+=): Likewise
	(year_month_day_weekday::operator-=): Likewise.
	(year_month_day_weekday::operator+): Likewise.
	(year_month_day_weekday::operator-): Likewise.
	(year_month_day_weekday_last::operator+=): Likewise
	(year_month_day_weekday_last::operator-=): Likewise.
	(year_month_day_weekday_last::operator+): Likewise.
	(year_month_day_weekday_last::operator-): Likewise.
	(testsuite/std/time/year_month/2.cc): New test.
	(testsuite/std/time/year_month_day/2.cc): New test.
	(testsuite/std/time/year_month_day_last/2.cc): New test.
	(testsuite/std/time/year_month_weekday/2.cc): New test.
	(testsuite/std/time/year_month_weekday_last/2.cc): New test.
---
 libstdc++-v3/include/std/chrono               | 52 ++++++++++++++++++-
 .../testsuite/std/time/year_month/2.cc        | 40 ++++++++++++++
 .../testsuite/std/time/year_month_day/2.cc    | 40 ++++++++++++++
 .../std/time/year_month_day_last/2.cc         | 40 ++++++++++++++
 .../std/time/year_month_weekday/2.cc          | 40 ++++++++++++++
 .../std/time/year_month_weekday_last/2.cc     | 40 ++++++++++++++
 6 files changed, 250 insertions(+), 2 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/std/time/year_month/2.cc
 create mode 100644 libstdc++-v3/testsuite/std/time/year_month_day/2.cc
 create mode 100644 libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc
 create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc
 create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 3cc1438a7b6..0e272c3da58 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -2046,6 +2046,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     // YEAR_MONTH
 
+    namespace __detail
+    {
+      // [time.cal.ym], [time.cal.ymd], etc constrain the 'months'-taking
+      // addition/subtraction operator overloads like so:
+      //
+      //   Constraints: if the argument supplied by the caller for the months
+      //   parameter is convertible to years, its implicit conversion sequence
+      //   to years is worse than its implicit conversion sequence to months.
+      //
+      // We realize this constraint by defining the 'months'-taking overloads as
+      // function templates (with a dummy defaulted template parameter), so that
+      // overload resolution doesn't select the 'months'-taking overload unless
+      // the implicit conversion sequence to 'months' is better than that to
+      // 'years'.
+      using __months_years_conversion_disambiguator = void;
+    }
+
     class year_month
     {
     private:
@@ -2068,6 +2085,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       month() const noexcept
       { return _M_m; }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month&
 	operator+=(const months& __dm) noexcept
 	{
@@ -2075,6 +2093,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return *this;
 	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month&
 	operator-=(const months& __dm) noexcept
 	{
@@ -2108,6 +2127,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator<=>(const year_month& __x, const year_month& __y) noexcept
 	= default;
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month
 	operator+(const year_month& __ym, const months& __dm) noexcept
 	{
@@ -2120,10 +2140,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return __y / __m;
 	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month
 	operator+(const months& __dm, const year_month& __ym) noexcept
 	{ return __ym + __dm; }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month
 	operator-(const year_month& __ym, const months& __dm) noexcept
 	{ return __ym + -__dm; }
@@ -2200,6 +2222,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : year_month_day(sys_days{__dp.time_since_epoch()})
       { }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_day&
 	operator+=(const months& __m) noexcept
 	{
@@ -2207,6 +2230,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return *this;
 	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_day&
 	operator-=(const months& __m) noexcept
 	{
@@ -2262,10 +2286,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator<=>(const year_month_day& __x, const year_month_day& __y) noexcept
 	= default;
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_day
 	operator+(const year_month_day& __ymd, const months& __dm) noexcept
 	{ return (__ymd.year() / __ymd.month() + __dm) / __ymd.day(); }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_day
 	operator+(const months& __dm, const year_month_day& __ymd) noexcept
 	{ return __ymd + __dm; }
@@ -2278,6 +2304,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator+(const years& __dy, const year_month_day& __ymd) noexcept
       { return __ymd + __dy; }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_day
 	operator-(const year_month_day& __ymd, const months& __dm) noexcept
 	{ return __ymd + -__dm; }
@@ -2364,6 +2391,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : _M_y{__y}, _M_mdl{__mdl}
       { }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_day_last&
 	operator+=(const months& __m) noexcept
 	{
@@ -2371,6 +2399,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return *this;
 	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_day_last&
 	operator-=(const months& __m) noexcept
 	{
@@ -2438,16 +2467,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		  const year_month_day_last& __y) noexcept
 	= default;
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_day_last
 	operator+(const year_month_day_last& __ymdl,
 		  const months& __dm) noexcept
 	{ return (__ymdl.year() / __ymdl.month() + __dm) / last; }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_day_last
 	operator+(const months& __dm,
 		  const year_month_day_last& __ymdl) noexcept
 	{ return __ymdl + __dm; }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_day_last
 	operator-(const year_month_day_last& __ymdl,
 		  const months& __dm) noexcept
@@ -2544,6 +2576,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : year_month_weekday{sys_days{__dp.time_since_epoch()}}
       { }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_weekday&
 	operator+=(const months& __m) noexcept
 	{
@@ -2551,6 +2584,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return *this;
 	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_weekday&
 	operator-=(const months& __m) noexcept
 	{
@@ -2626,10 +2660,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  && __x.weekday() == __y.weekday();
       }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_weekday
 	operator+(const year_month_weekday& __ymwd, const months& __dm) noexcept
-      { return (__ymwd.year() / __ymwd.month() + __dm) / __ymwd.weekday_indexed(); }
+	{
+	  return ((__ymwd.year() / __ymwd.month() + __dm)
+		  / __ymwd.weekday_indexed());
+	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_weekday
 	operator+(const months& __dm, const year_month_weekday& __ymwd) noexcept
 	{ return __ymwd + __dm; }
@@ -2642,6 +2681,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator+(const years& __dy, const year_month_weekday& __ymwd) noexcept
       { return __ymwd + __dy; }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_weekday
 	operator-(const year_month_weekday& __ymwd, const months& __dm) noexcept
 	{ return __ymwd + -__dm; }
@@ -2690,6 +2730,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       : _M_y{__y}, _M_m{__m}, _M_wdl{__wdl}
       { }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_weekday_last&
 	operator+=(const months& __m) noexcept
 	{
@@ -2697,6 +2738,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return *this;
 	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	constexpr year_month_weekday_last&
 	operator-=(const months& __m) noexcept
 	{
@@ -2759,11 +2801,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  && __x.weekday_last() == __y.weekday_last();
       }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_weekday_last
 	operator+(const year_month_weekday_last& __ymwdl,
 		  const months& __dm) noexcept
-      { return (__ymwdl.year() / __ymwdl.month() + __dm) / __ymwdl.weekday_last(); }
+	{
+	  return ((__ymwdl.year() / __ymwdl.month() + __dm)
+		  / __ymwdl.weekday_last());
+	}
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_weekday_last
 	operator+(const months& __dm,
 		  const year_month_weekday_last& __ymwdl) noexcept
@@ -2779,6 +2826,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		const year_month_weekday_last& __ymwdl) noexcept
       { return __ymwdl + __dy; }
 
+      template<typename = __detail::__months_years_conversion_disambiguator>
 	friend constexpr year_month_weekday_last
 	operator-(const year_month_weekday_last& __ymwdl,
 		  const months& __dm) noexcept
diff --git a/libstdc++-v3/testsuite/std/time/year_month/2.cc b/libstdc++-v3/testsuite/std/time/year_month/2.cc
new file mode 100644
index 00000000000..36e14667547
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/year_month/2.cc
@@ -0,0 +1,40 @@
+// { 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/>.
+
+// Class template year_month [time.cal.year_month]
+
+#include <chrono>
+
+constexpr void
+constexpr_year_month_op_overload_disambiguation()
+{
+  using namespace std::chrono;
+  using decades = duration<long long, std::ratio<31556952 * 10>>;
+  static_assert(std::convertible_to<decades, months>
+		&& std::convertible_to<decades, years>);
+  using ym = year_month;
+
+  constexpr ym ym1 = 2015y/June;
+  static_assert(ym1 + decades{1} == 2025y/June);
+  static_assert(ym1 - decades{1} == 2005y/June);
+  static_assert(decades{1} + ym1 == 2025y/June);
+  static_assert((ym{ym1} += decades{1}) == 2025y/June);
+  static_assert((ym{ym1} -= decades{1}) == 2005y/June);
+}
diff --git a/libstdc++-v3/testsuite/std/time/year_month_day/2.cc b/libstdc++-v3/testsuite/std/time/year_month_day/2.cc
new file mode 100644
index 00000000000..80d1f033c1d
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/year_month_day/2.cc
@@ -0,0 +1,40 @@
+// { 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/>.
+
+// Class template year_month_day [time.cal.year_month_day]
+
+#include <chrono>
+
+constexpr void
+constexpr_year_month_day_op_overload_disambiguation()
+{
+  using namespace std::chrono;
+  using decades = duration<long long, std::ratio<31556952 * 10>>;
+  static_assert(std::convertible_to<decades, months>
+		&& std::convertible_to<decades, years>);
+  using ymd = year_month_day;
+
+  constexpr ymd ymd1 = 2015y/June/15d;
+  static_assert(ymd1 + decades{1} == 2025y/June/15d);
+  static_assert(ymd1 - decades{1} == 2005y/June/15d);
+  static_assert(decades{1} + ymd1 == 2025y/June/15d);
+  static_assert((ymd{ymd1} += decades{1}) == 2025y/June/15d);
+  static_assert((ymd{ymd1} -= decades{1}) == 2005y/June/15d);
+}
diff --git a/libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc b/libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc
new file mode 100644
index 00000000000..dadbd3c38b5
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc
@@ -0,0 +1,40 @@
+// { 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/>.
+
+// Class template year_month_day_last [time.cal.year_month_day_last]
+
+#include <chrono>
+
+constexpr void
+constexpr_year_month_day_last_op_overload_disambiguation()
+{
+  using namespace std::chrono;
+  using decades = duration<long long, std::ratio<31556952 * 10>>;
+  static_assert(std::convertible_to<decades, months>
+		&& std::convertible_to<decades, years>);
+  using ymdl = year_month_day_last;
+
+  constexpr ymdl ymdl1 = 2015y/June/last;
+  static_assert(ymdl1 + decades{1} == 2025y/June/last);
+  static_assert(ymdl1 - decades{1} == 2005y/June/last);
+  static_assert(decades{1} + ymdl1 == 2025y/June/last);
+  static_assert((ymdl{ymdl1} += decades{1}) == 2025y/June/last);
+  static_assert((ymdl{ymdl1} -= decades{1}) == 2005y/June/last);
+}
diff --git a/libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc b/libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc
new file mode 100644
index 00000000000..6ddfb15b283
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc
@@ -0,0 +1,40 @@
+// { 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/>.
+
+// Class template year_month_weekday [time.cal.year_month_weekday]
+
+#include <chrono>
+
+constexpr void
+constexpr_year_month_weekday_op_overload_disambiguation()
+{
+  using namespace std::chrono;
+  using decades = duration<long long, std::ratio<31556952 * 10>>;
+  static_assert(std::convertible_to<decades, months>
+		&& std::convertible_to<decades, years>);
+  using ymwd = year_month_weekday;
+
+  constexpr ymwd ymwd1 = 2015y/June/Monday[3];
+  static_assert(ymwd1 + decades{1} == 2025y/June/Monday[3]);
+  static_assert(ymwd1 - decades{1} == 2005y/June/Monday[3]);
+  static_assert(decades{1} + ymwd1 == 2025y/June/Monday[3]);
+  static_assert((ymwd{ymwd1} += decades{1}) == 2025y/June/Monday[3]);
+  static_assert((ymwd{ymwd1} -= decades{1}) == 2005y/June/Monday[3]);
+}
diff --git a/libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc b/libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc
new file mode 100644
index 00000000000..170b5a45ad6
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc
@@ -0,0 +1,40 @@
+// { 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/>.
+
+// Class template year_month_weekday_last [time.cal.year_month_weekday_last]
+
+#include <chrono>
+
+constexpr void
+constexpr_year_month_weekday_last_op_overload_disambiguation()
+{
+  using namespace std::chrono;
+  using decades = duration<long long, std::ratio<31556952 * 10>>;
+  static_assert(std::convertible_to<decades, months>
+		&& std::convertible_to<decades, years>);
+  using ymwdl = year_month_weekday_last;
+
+  constexpr ymwdl ymwdl1 = 2015y/June/Monday[last];
+  static_assert(ymwdl1 + decades{1} == 2025y/June/Monday[last]);
+  static_assert(ymwdl1 - decades{1} == 2005y/June/Monday[last]);
+  static_assert(decades{1} + ymwdl1 == 2025y/June/Monday[last]);
+  static_assert((ymwdl{ymwdl1} += decades{1}) == 2025y/June/Monday[last]);
+  static_assert((ymwdl{ymwdl1} -= decades{1}) == 2005y/June/Monday[last]);
+}
-- 
2.28.0.337.ge9b77c84a0


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

* Re: [PATCH] libstdc++: Fix operator overload resolution for calendar types
  2020-08-25 19:47 [PATCH] libstdc++: Fix operator overload resolution for calendar types Patrick Palka
@ 2020-08-25 19:51 ` Patrick Palka
  2020-08-27 16:24 ` Jonathan Wakely
  1 sibling, 0 replies; 3+ messages in thread
From: Patrick Palka @ 2020-08-25 19:51 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches, libstdc++

On Tue, 25 Aug 2020, Patrick Palka wrote:

> My original patch that implemented the calendar type operations failed
> to enforce a constraint on some of the addition/subtraction operator
> overloads that take a 'months' argument:
> 
>   Constraints: If the argument supplied by the caller for the months
>   parameter is convertible to years, its implicit conversion sequence to
>   years is worse than its implicit conversion sequence to months
> 
> This constraint is relevant when adding/subtracting a duration to/from
> say a year_month when the given duration is convertible to both 'months'
> and to 'years'.  The correct behavior here in light of this constraint
> is to select the (more efficient) 'years'-taking overload, but we
> currently emit an ambiguous overload error.
> 
> This patch follows the approach taken in the 'date' library and defines
> the constrained 'months'-taking operator overloads as function
> templates, so that we break such a implicit-conversion tie by selecting
> the non-template 'years'-taking overload.
> 
> Tested on x86_64-pc-linux-gnu, does this look OK to commit?  (The below
> diff is generated with --ignore-space-change for easier review.  In the
> actual patch, the function templates are indented two extra spaces after
> the template parameter list.)
> 
> libstdc++-v3/ChangeLog:
> 
> 	* include/std/chrono (__detail::__unspecified_month_disambuator):
> 	Define.

Whoops, this ChangeLog line should say
__detail::__months_years_conversion_disambiguator.

> 	(year_month::operator+=): Turn the 'months'-taking overload
> 	into a function template, so that the 'years'-taking overload is
> 	selected in case of an equally-ranked implicit conversion
> 	sequence to both 'months' and 'years' from the supplied argument.
> 	(year_month::operator-=): Likewise.
> 	(year_month::operator+): Likewise.
> 	(year_month::operator-): Likewise.
> 	(year_month_day::operator+=): Likewise.
> 	(year_month_day::operator-=): Likewise.
> 	(year_month_day::operator+): Likewise.
> 	(year_month_day::operator-): Likewise.
> 	(year_month_day_last::operator+=): Likewise.
> 	(year_month_day_last::operator-=): Likewise.
> 	(year_month_day_last::operator+): Likewise
> 	(year_month_day_last::operator-): Likewise.
> 	(year_month_day_weekday::operator+=): Likewise
> 	(year_month_day_weekday::operator-=): Likewise.
> 	(year_month_day_weekday::operator+): Likewise.
> 	(year_month_day_weekday::operator-): Likewise.
> 	(year_month_day_weekday_last::operator+=): Likewise
> 	(year_month_day_weekday_last::operator-=): Likewise.
> 	(year_month_day_weekday_last::operator+): Likewise.
> 	(year_month_day_weekday_last::operator-): Likewise.
> 	(testsuite/std/time/year_month/2.cc): New test.
> 	(testsuite/std/time/year_month_day/2.cc): New test.
> 	(testsuite/std/time/year_month_day_last/2.cc): New test.
> 	(testsuite/std/time/year_month_weekday/2.cc): New test.
> 	(testsuite/std/time/year_month_weekday_last/2.cc): New test.
> ---
>  libstdc++-v3/include/std/chrono               | 52 ++++++++++++++++++-
>  .../testsuite/std/time/year_month/2.cc        | 40 ++++++++++++++
>  .../testsuite/std/time/year_month_day/2.cc    | 40 ++++++++++++++
>  .../std/time/year_month_day_last/2.cc         | 40 ++++++++++++++
>  .../std/time/year_month_weekday/2.cc          | 40 ++++++++++++++
>  .../std/time/year_month_weekday_last/2.cc     | 40 ++++++++++++++
>  6 files changed, 250 insertions(+), 2 deletions(-)
>  create mode 100644 libstdc++-v3/testsuite/std/time/year_month/2.cc
>  create mode 100644 libstdc++-v3/testsuite/std/time/year_month_day/2.cc
>  create mode 100644 libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc
>  create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc
>  create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc
> 
> diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
> index 3cc1438a7b6..0e272c3da58 100644
> --- a/libstdc++-v3/include/std/chrono
> +++ b/libstdc++-v3/include/std/chrono
> @@ -2046,6 +2046,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  
>      // YEAR_MONTH
>  
> +    namespace __detail
> +    {
> +      // [time.cal.ym], [time.cal.ymd], etc constrain the 'months'-taking
> +      // addition/subtraction operator overloads like so:
> +      //
> +      //   Constraints: if the argument supplied by the caller for the months
> +      //   parameter is convertible to years, its implicit conversion sequence
> +      //   to years is worse than its implicit conversion sequence to months.
> +      //
> +      // We realize this constraint by defining the 'months'-taking overloads as
> +      // function templates (with a dummy defaulted template parameter), so that
> +      // overload resolution doesn't select the 'months'-taking overload unless
> +      // the implicit conversion sequence to 'months' is better than that to
> +      // 'years'.
> +      using __months_years_conversion_disambiguator = void;
> +    }
> +
>      class year_month
>      {
>      private:
> @@ -2068,6 +2085,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        month() const noexcept
>        { return _M_m; }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month&
>  	operator+=(const months& __dm) noexcept
>  	{
> @@ -2075,6 +2093,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  return *this;
>  	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month&
>  	operator-=(const months& __dm) noexcept
>  	{
> @@ -2108,6 +2127,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        operator<=>(const year_month& __x, const year_month& __y) noexcept
>  	= default;
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month
>  	operator+(const year_month& __ym, const months& __dm) noexcept
>  	{
> @@ -2120,10 +2140,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  return __y / __m;
>  	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month
>  	operator+(const months& __dm, const year_month& __ym) noexcept
>  	{ return __ym + __dm; }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month
>  	operator-(const year_month& __ym, const months& __dm) noexcept
>  	{ return __ym + -__dm; }
> @@ -2200,6 +2222,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        : year_month_day(sys_days{__dp.time_since_epoch()})
>        { }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_day&
>  	operator+=(const months& __m) noexcept
>  	{
> @@ -2207,6 +2230,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  return *this;
>  	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_day&
>  	operator-=(const months& __m) noexcept
>  	{
> @@ -2262,10 +2286,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        operator<=>(const year_month_day& __x, const year_month_day& __y) noexcept
>  	= default;
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_day
>  	operator+(const year_month_day& __ymd, const months& __dm) noexcept
>  	{ return (__ymd.year() / __ymd.month() + __dm) / __ymd.day(); }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_day
>  	operator+(const months& __dm, const year_month_day& __ymd) noexcept
>  	{ return __ymd + __dm; }
> @@ -2278,6 +2304,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        operator+(const years& __dy, const year_month_day& __ymd) noexcept
>        { return __ymd + __dy; }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_day
>  	operator-(const year_month_day& __ymd, const months& __dm) noexcept
>  	{ return __ymd + -__dm; }
> @@ -2364,6 +2391,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        : _M_y{__y}, _M_mdl{__mdl}
>        { }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_day_last&
>  	operator+=(const months& __m) noexcept
>  	{
> @@ -2371,6 +2399,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  return *this;
>  	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_day_last&
>  	operator-=(const months& __m) noexcept
>  	{
> @@ -2438,16 +2467,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  		  const year_month_day_last& __y) noexcept
>  	= default;
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_day_last
>  	operator+(const year_month_day_last& __ymdl,
>  		  const months& __dm) noexcept
>  	{ return (__ymdl.year() / __ymdl.month() + __dm) / last; }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_day_last
>  	operator+(const months& __dm,
>  		  const year_month_day_last& __ymdl) noexcept
>  	{ return __ymdl + __dm; }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_day_last
>  	operator-(const year_month_day_last& __ymdl,
>  		  const months& __dm) noexcept
> @@ -2544,6 +2576,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        : year_month_weekday{sys_days{__dp.time_since_epoch()}}
>        { }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_weekday&
>  	operator+=(const months& __m) noexcept
>  	{
> @@ -2551,6 +2584,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  return *this;
>  	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_weekday&
>  	operator-=(const months& __m) noexcept
>  	{
> @@ -2626,10 +2660,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  && __x.weekday() == __y.weekday();
>        }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_weekday
>  	operator+(const year_month_weekday& __ymwd, const months& __dm) noexcept
> -      { return (__ymwd.year() / __ymwd.month() + __dm) / __ymwd.weekday_indexed(); }
> +	{
> +	  return ((__ymwd.year() / __ymwd.month() + __dm)
> +		  / __ymwd.weekday_indexed());
> +	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_weekday
>  	operator+(const months& __dm, const year_month_weekday& __ymwd) noexcept
>  	{ return __ymwd + __dm; }
> @@ -2642,6 +2681,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        operator+(const years& __dy, const year_month_weekday& __ymwd) noexcept
>        { return __ymwd + __dy; }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_weekday
>  	operator-(const year_month_weekday& __ymwd, const months& __dm) noexcept
>  	{ return __ymwd + -__dm; }
> @@ -2690,6 +2730,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        : _M_y{__y}, _M_m{__m}, _M_wdl{__wdl}
>        { }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_weekday_last&
>  	operator+=(const months& __m) noexcept
>  	{
> @@ -2697,6 +2738,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  return *this;
>  	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	constexpr year_month_weekday_last&
>  	operator-=(const months& __m) noexcept
>  	{
> @@ -2759,11 +2801,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  && __x.weekday_last() == __y.weekday_last();
>        }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_weekday_last
>  	operator+(const year_month_weekday_last& __ymwdl,
>  		  const months& __dm) noexcept
> -      { return (__ymwdl.year() / __ymwdl.month() + __dm) / __ymwdl.weekday_last(); }
> +	{
> +	  return ((__ymwdl.year() / __ymwdl.month() + __dm)
> +		  / __ymwdl.weekday_last());
> +	}
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_weekday_last
>  	operator+(const months& __dm,
>  		  const year_month_weekday_last& __ymwdl) noexcept
> @@ -2779,6 +2826,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  		const year_month_weekday_last& __ymwdl) noexcept
>        { return __ymwdl + __dy; }
>  
> +      template<typename = __detail::__months_years_conversion_disambiguator>
>  	friend constexpr year_month_weekday_last
>  	operator-(const year_month_weekday_last& __ymwdl,
>  		  const months& __dm) noexcept
> diff --git a/libstdc++-v3/testsuite/std/time/year_month/2.cc b/libstdc++-v3/testsuite/std/time/year_month/2.cc
> new file mode 100644
> index 00000000000..36e14667547
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/time/year_month/2.cc
> @@ -0,0 +1,40 @@
> +// { 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/>.
> +
> +// Class template year_month [time.cal.year_month]
> +
> +#include <chrono>
> +
> +constexpr void
> +constexpr_year_month_op_overload_disambiguation()
> +{
> +  using namespace std::chrono;
> +  using decades = duration<long long, std::ratio<31556952 * 10>>;
> +  static_assert(std::convertible_to<decades, months>
> +		&& std::convertible_to<decades, years>);
> +  using ym = year_month;
> +
> +  constexpr ym ym1 = 2015y/June;
> +  static_assert(ym1 + decades{1} == 2025y/June);
> +  static_assert(ym1 - decades{1} == 2005y/June);
> +  static_assert(decades{1} + ym1 == 2025y/June);
> +  static_assert((ym{ym1} += decades{1}) == 2025y/June);
> +  static_assert((ym{ym1} -= decades{1}) == 2005y/June);
> +}
> diff --git a/libstdc++-v3/testsuite/std/time/year_month_day/2.cc b/libstdc++-v3/testsuite/std/time/year_month_day/2.cc
> new file mode 100644
> index 00000000000..80d1f033c1d
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/time/year_month_day/2.cc
> @@ -0,0 +1,40 @@
> +// { 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/>.
> +
> +// Class template year_month_day [time.cal.year_month_day]
> +
> +#include <chrono>
> +
> +constexpr void
> +constexpr_year_month_day_op_overload_disambiguation()
> +{
> +  using namespace std::chrono;
> +  using decades = duration<long long, std::ratio<31556952 * 10>>;
> +  static_assert(std::convertible_to<decades, months>
> +		&& std::convertible_to<decades, years>);
> +  using ymd = year_month_day;
> +
> +  constexpr ymd ymd1 = 2015y/June/15d;
> +  static_assert(ymd1 + decades{1} == 2025y/June/15d);
> +  static_assert(ymd1 - decades{1} == 2005y/June/15d);
> +  static_assert(decades{1} + ymd1 == 2025y/June/15d);
> +  static_assert((ymd{ymd1} += decades{1}) == 2025y/June/15d);
> +  static_assert((ymd{ymd1} -= decades{1}) == 2005y/June/15d);
> +}
> diff --git a/libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc b/libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc
> new file mode 100644
> index 00000000000..dadbd3c38b5
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc
> @@ -0,0 +1,40 @@
> +// { 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/>.
> +
> +// Class template year_month_day_last [time.cal.year_month_day_last]
> +
> +#include <chrono>
> +
> +constexpr void
> +constexpr_year_month_day_last_op_overload_disambiguation()
> +{
> +  using namespace std::chrono;
> +  using decades = duration<long long, std::ratio<31556952 * 10>>;
> +  static_assert(std::convertible_to<decades, months>
> +		&& std::convertible_to<decades, years>);
> +  using ymdl = year_month_day_last;
> +
> +  constexpr ymdl ymdl1 = 2015y/June/last;
> +  static_assert(ymdl1 + decades{1} == 2025y/June/last);
> +  static_assert(ymdl1 - decades{1} == 2005y/June/last);
> +  static_assert(decades{1} + ymdl1 == 2025y/June/last);
> +  static_assert((ymdl{ymdl1} += decades{1}) == 2025y/June/last);
> +  static_assert((ymdl{ymdl1} -= decades{1}) == 2005y/June/last);
> +}
> diff --git a/libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc b/libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc
> new file mode 100644
> index 00000000000..6ddfb15b283
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc
> @@ -0,0 +1,40 @@
> +// { 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/>.
> +
> +// Class template year_month_weekday [time.cal.year_month_weekday]
> +
> +#include <chrono>
> +
> +constexpr void
> +constexpr_year_month_weekday_op_overload_disambiguation()
> +{
> +  using namespace std::chrono;
> +  using decades = duration<long long, std::ratio<31556952 * 10>>;
> +  static_assert(std::convertible_to<decades, months>
> +		&& std::convertible_to<decades, years>);
> +  using ymwd = year_month_weekday;
> +
> +  constexpr ymwd ymwd1 = 2015y/June/Monday[3];
> +  static_assert(ymwd1 + decades{1} == 2025y/June/Monday[3]);
> +  static_assert(ymwd1 - decades{1} == 2005y/June/Monday[3]);
> +  static_assert(decades{1} + ymwd1 == 2025y/June/Monday[3]);
> +  static_assert((ymwd{ymwd1} += decades{1}) == 2025y/June/Monday[3]);
> +  static_assert((ymwd{ymwd1} -= decades{1}) == 2005y/June/Monday[3]);
> +}
> diff --git a/libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc b/libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc
> new file mode 100644
> index 00000000000..170b5a45ad6
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc
> @@ -0,0 +1,40 @@
> +// { 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/>.
> +
> +// Class template year_month_weekday_last [time.cal.year_month_weekday_last]
> +
> +#include <chrono>
> +
> +constexpr void
> +constexpr_year_month_weekday_last_op_overload_disambiguation()
> +{
> +  using namespace std::chrono;
> +  using decades = duration<long long, std::ratio<31556952 * 10>>;
> +  static_assert(std::convertible_to<decades, months>
> +		&& std::convertible_to<decades, years>);
> +  using ymwdl = year_month_weekday_last;
> +
> +  constexpr ymwdl ymwdl1 = 2015y/June/Monday[last];
> +  static_assert(ymwdl1 + decades{1} == 2025y/June/Monday[last]);
> +  static_assert(ymwdl1 - decades{1} == 2005y/June/Monday[last]);
> +  static_assert(decades{1} + ymwdl1 == 2025y/June/Monday[last]);
> +  static_assert((ymwdl{ymwdl1} += decades{1}) == 2025y/June/Monday[last]);
> +  static_assert((ymwdl{ymwdl1} -= decades{1}) == 2005y/June/Monday[last]);
> +}
> -- 
> 2.28.0.337.ge9b77c84a0
> 
> 


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

* Re: [PATCH] libstdc++: Fix operator overload resolution for calendar types
  2020-08-25 19:47 [PATCH] libstdc++: Fix operator overload resolution for calendar types Patrick Palka
  2020-08-25 19:51 ` Patrick Palka
@ 2020-08-27 16:24 ` Jonathan Wakely
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2020-08-27 16:24 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches, libstdc++

On 25/08/20 15:47 -0400, Patrick Palka via Libstdc++ wrote:
>My original patch that implemented the calendar type operations failed
>to enforce a constraint on some of the addition/subtraction operator
>overloads that take a 'months' argument:
>
>  Constraints: If the argument supplied by the caller for the months
>  parameter is convertible to years, its implicit conversion sequence to
>  years is worse than its implicit conversion sequence to months
>
>This constraint is relevant when adding/subtracting a duration to/from
>say a year_month when the given duration is convertible to both 'months'
>and to 'years'.  The correct behavior here in light of this constraint
>is to select the (more efficient) 'years'-taking overload, but we
>currently emit an ambiguous overload error.
>
>This patch follows the approach taken in the 'date' library and defines
>the constrained 'months'-taking operator overloads as function
>templates, so that we break such a implicit-conversion tie by selecting
>the non-template 'years'-taking overload.
>
>Tested on x86_64-pc-linux-gnu, does this look OK to commit?  (The below
>diff is generated with --ignore-space-change for easier review.  In the
>actual patch, the function templates are indented two extra spaces after
>the template parameter list.)
>
>libstdc++-v3/ChangeLog:
>
>	* include/std/chrono (__detail::__unspecified_month_disambuator):
>	Define.
>	(year_month::operator+=): Turn the 'months'-taking overload
>	into a function template, so that the 'years'-taking overload is
>	selected in case of an equally-ranked implicit conversion
>	sequence to both 'months' and 'years' from the supplied argument.
>	(year_month::operator-=): Likewise.
>	(year_month::operator+): Likewise.
>	(year_month::operator-): Likewise.
>	(year_month_day::operator+=): Likewise.
>	(year_month_day::operator-=): Likewise.
>	(year_month_day::operator+): Likewise.
>	(year_month_day::operator-): Likewise.
>	(year_month_day_last::operator+=): Likewise.
>	(year_month_day_last::operator-=): Likewise.
>	(year_month_day_last::operator+): Likewise
>	(year_month_day_last::operator-): Likewise.
>	(year_month_day_weekday::operator+=): Likewise
>	(year_month_day_weekday::operator-=): Likewise.
>	(year_month_day_weekday::operator+): Likewise.
>	(year_month_day_weekday::operator-): Likewise.
>	(year_month_day_weekday_last::operator+=): Likewise
>	(year_month_day_weekday_last::operator-=): Likewise.
>	(year_month_day_weekday_last::operator+): Likewise.
>	(year_month_day_weekday_last::operator-): Likewise.
>	(testsuite/std/time/year_month/2.cc): New test.
>	(testsuite/std/time/year_month_day/2.cc): New test.
>	(testsuite/std/time/year_month_day_last/2.cc): New test.
>	(testsuite/std/time/year_month_weekday/2.cc): New test.
>	(testsuite/std/time/year_month_weekday_last/2.cc): New test.
>---
> libstdc++-v3/include/std/chrono               | 52 ++++++++++++++++++-
> .../testsuite/std/time/year_month/2.cc        | 40 ++++++++++++++
> .../testsuite/std/time/year_month_day/2.cc    | 40 ++++++++++++++
> .../std/time/year_month_day_last/2.cc         | 40 ++++++++++++++
> .../std/time/year_month_weekday/2.cc          | 40 ++++++++++++++
> .../std/time/year_month_weekday_last/2.cc     | 40 ++++++++++++++
> 6 files changed, 250 insertions(+), 2 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/std/time/year_month/2.cc
> create mode 100644 libstdc++-v3/testsuite/std/time/year_month_day/2.cc
> create mode 100644 libstdc++-v3/testsuite/std/time/year_month_day_last/2.cc
> create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday/2.cc
> create mode 100644 libstdc++-v3/testsuite/std/time/year_month_weekday_last/2.cc
>
>diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
>index 3cc1438a7b6..0e272c3da58 100644
>--- a/libstdc++-v3/include/std/chrono
>+++ b/libstdc++-v3/include/std/chrono
>@@ -2046,6 +2046,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>     // YEAR_MONTH
>
>+    namespace __detail
>+    {
>+      // [time.cal.ym], [time.cal.ymd], etc constrain the 'months'-taking
>+      // addition/subtraction operator overloads like so:
>+      //
>+      //   Constraints: if the argument supplied by the caller for the months
>+      //   parameter is convertible to years, its implicit conversion sequence
>+      //   to years is worse than its implicit conversion sequence to months.
>+      //
>+      // We realize this constraint by defining the 'months'-taking overloads as
>+      // function templates (with a dummy defaulted template parameter), so that
>+      // overload resolution doesn't select the 'months'-taking overload unless
>+      // the implicit conversion sequence to 'months' is better than that to
>+      // 'years'.
>+      using __months_years_conversion_disambiguator = void;
>+    }
>+
>     class year_month
>     {
>     private:
>@@ -2068,6 +2085,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       month() const noexcept
>       { return _M_m; }
>
>+      template<typename = __detail::__months_years_conversion_disambiguator>
> 	constexpr year_month&
> 	operator+=(const months& __dm) noexcept
> 	{

OK for trunk.

I don't really like this solution. It seems like a "clever" hack
rather than a direct expression of the requirement (hence the need to
give it a long descriptive name to say what's going on). But it's
probably better than the alternatives, all things considered.

I considered defining a new type that is implicitly constructible from
both year and month, but does this disambiguation in one place, on its
constructors. Then use a single function taking that type instead of
every pair of overloaded functions taking year and month.

Or adding this in addition to the existing overloads:

   template<typename T>
     requires (convertible_to<T, year> || convertible_to<T, month>)
     constexpr year_month&
     operator+=(const T& t) noexcept
     {
       if constexpr (convertible_to<T, year>)
         return operator+=(static_cast<year>(t));
       else
         return operator+=(static_cast<month>(t));
     }

Or replacing the overload for month with:

   template<convertible_to<month> T>
     requires (!convertible_to<T, year>)
     constexpr year_month&
     operator+=(const T& t) noexcept

But those all probably compile much slower than your solution, and the
new type would have runtime overhead too (it would need to store a
flag to say which type it was constructed with).



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

end of thread, other threads:[~2020-08-27 16:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-25 19:47 [PATCH] libstdc++: Fix operator overload resolution for calendar types Patrick Palka
2020-08-25 19:51 ` Patrick Palka
2020-08-27 16:24 ` 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).