libstdc++: Fix chrono::__detail::ceil to work with C++11 (was Re: [PATCH v5 6/8] libstdc++ atomic_futex: Avoid rounding errors in std::future::wait_* [PR91486])

On Friday 11 September 2020 at 19:59:36 +0100, Jonathan Wakely wrote:
> commit 53ad6b1979f4bd7121e977c4a44151b14d8a0147
> Author: Jonathan Wakely <jwakely@redhat.com>
> Date:   Fri Sep 11 19:59:11 2020
> 
>     libstdc++: Fix chrono::__detail::ceil to work with C++11
>     
>     In C++11 constexpr functions can only have a return statement, so we
>     need to fix __detail::ceil to make it valid in C++11. This can be done
>     by moving the comparison and increment into a new function, __ceil_impl,
>     and calling that with the result of the duration_cast.
>     
>     This would mean the standard C++17 std::chrono::ceil function would make
>     two further calls, which would add too much overhead when not inlined.
>     For C++17 and later use a using-declaration to add chrono::ceil to
>     namespace __detail. For C++11 and C++14 define chrono::__detail::__ceil
>     as a C++11-compatible constexpr function template.
>
>
>     libstdc++-v3/ChangeLog:
>     
>             * include/std/chrono [C++17] (chrono::__detail::ceil): Add
>             using declaration to make chrono::ceil available for internal
>             use with a consistent name.
>             (chrono::__detail::__ceil_impl): New function template.
>             (chrono::__detail::ceil): Use __ceil_impl to compare and
>             increment the value. Remove SFINAE constraint.

This change introduces a new implementation of ceil that, as far as I can
tell, has no tests. A patch is attached to add the equivalent of the
existing chrono::ceil tests for chrono::__detail::ceil. The tests fail to
compile if I run them without 53ad6b1979f4bd7121e977c4a44151b14d8a0147 as
expected due to the previous non-C++11-compliant implementation.

Mike.

On 19/09/20 11:37 +0100, Mike Crowe wrote:
>On Friday 11 September 2020 at 19:59:36 +0100, Jonathan Wakely wrote:
>> commit 53ad6b1979f4bd7121e977c4a44151b14d8a0147
>> Author: Jonathan Wakely <jwakely@redhat.com>
>> Date:   Fri Sep 11 19:59:11 2020
>>
>>     libstdc++: Fix chrono::__detail::ceil to work with C++11
>>
>>     In C++11 constexpr functions can only have a return statement, so we
>>     need to fix __detail::ceil to make it valid in C++11. This can be done
>>     by moving the comparison and increment into a new function, __ceil_impl,
>>     and calling that with the result of the duration_cast.
>>
>>     This would mean the standard C++17 std::chrono::ceil function would make
>>     two further calls, which would add too much overhead when not inlined.
>>     For C++17 and later use a using-declaration to add chrono::ceil to
>>     namespace __detail. For C++11 and C++14 define chrono::__detail::__ceil
>>     as a C++11-compatible constexpr function template.
>>
>>
>>     libstdc++-v3/ChangeLog:
>>
>>             * include/std/chrono [C++17] (chrono::__detail::ceil): Add
>>             using declaration to make chrono::ceil available for internal
>>             use with a consistent name.
>>             (chrono::__detail::__ceil_impl): New function template.
>>             (chrono::__detail::ceil): Use __ceil_impl to compare and
>>             increment the value. Remove SFINAE constraint.
>
>This change introduces a new implementation of ceil that, as far as I can
>tell, has no tests. A patch is attached to add the equivalent of the
>existing chrono::ceil tests for chrono::__detail::ceil. The tests fail to
>compile if I run them without 53ad6b1979f4bd7121e977c4a44151b14d8a0147 as
>expected due to the previous non-C++11-compliant implementation.

Pushed to master, thanks.

>From b9dffbf4f1bc05a887269ea95a3b86d5a611e720 Mon Sep 17 00:00:00 2001
>From: Mike Crowe <mac@mcrowe.com>
>Date: Wed, 16 Sep 2020 15:31:28 +0100
>Subject: [PATCH 1/2] libstdc++: Test C++11 implementation of
> std::chrono::__detail::ceil
>
>Commit 53ad6b1979f4bd7121e977c4a44151b14d8a0147 split the implementation
>of std::chrono::__detail::ceil so that when compiling for C++17 and
>later std::chrono::ceil is used but when compiling for earlier versions
>a separate implementation is used to comply with C++11's limited
>constexpr rules. Let's run the equivalent of the existing
>std::chrono::ceil test cases on std::chrono::__detail::ceil too to make
>sure that it doesn't get broken.
>
>libstdc++-v3/ChangeLog:
>
>	* testsuite/20_util/duration_cast/rounding_c++11.cc: Copy
>        rounding.cc and alter to support compilation for C++11 and to
>        test std::chrono::__detail::ceil.
>---
> .../20_util/duration_cast/rounding_c++11.cc   | 43 +++++++++++++++++++
> 1 file changed, 43 insertions(+)
> create mode 100644 libstdc++-v3/testsuite/20_util/duration_cast/rounding_c++11.cc
>
>diff --git a/libstdc++-v3/testsuite/20_util/duration_cast/rounding_c++11.cc b/libstdc++-v3/testsuite/20_util/duration_cast/rounding_c++11.cc
>new file mode 100644
>index 00000000000..f10d27fd082
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/duration_cast/rounding_c++11.cc
>@@ -0,0 +1,43 @@
>+// Copyright (C) 2016-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/>.
>+
>+// { dg-options "-std=gnu++11" }
>+// { dg-do compile { target c++11 } }
>+
>+#include <chrono>
>+
>+using std::chrono::seconds;
>+using std::chrono::milliseconds;
>+
>+using fp_seconds = std::chrono::duration<float>;
>+
>+static_assert( std::chrono::__detail::ceil<seconds>(milliseconds(1000))
>+	       == seconds(1) );
>+static_assert( std::chrono::__detail::ceil<seconds>(milliseconds(1001))
>+	       == seconds(2) );
>+static_assert( std::chrono::__detail::ceil<seconds>(milliseconds(1500))
>+	       == seconds(2) );
>+static_assert( std::chrono::__detail::ceil<seconds>(milliseconds(1999))
>+	       == seconds(2) );
>+static_assert( std::chrono::__detail::ceil<seconds>(milliseconds(2000))
>+	       == seconds(2) );
>+static_assert( std::chrono::__detail::ceil<seconds>(milliseconds(2001))
>+	       == seconds(3) );
>+static_assert( std::chrono::__detail::ceil<seconds>(milliseconds(2500))
>+	       == seconds(3) );
>+static_assert( std::chrono::__detail::ceil<fp_seconds>(milliseconds(500))
>+	       == fp_seconds{0.5f} );
>-- 
>2.28.0
>