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).
