public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [v3 PATCH] Implement C++17 GB50 resolution
@ 2017-02-14 21:39 Dinka Ranns
  2017-02-15  7:43 ` Ville Voutilainen
  2017-02-15  9:58 ` Jonathan Wakely
  0 siblings, 2 replies; 5+ messages in thread
From: Dinka Ranns @ 2017-02-14 21:39 UTC (permalink / raw)
  To: gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

Tested on Linux-x64

Implementation of resolution for C++17 GB50

2017-02-12 Dinka Ranns <dinka.ranns@googlemail.com>

        C++17 GB50 resolution
        * libstdc++-v3/include/std/chrono:
        (duration::operator++()): Add constexpr.
        (duration::operator++(int)): Likewise
        (duration::operator--()): Likewise
        (duration::operator--(int)): Likewise
        (duration::operator+=(const duration&)): Likewise
        (duration::operator-=(const duration&)): Likewise
        (duration::operator*=(const rep&)): Likewise
        (duration::operator/=(const rep&)): Likewise
        (duration::operator%=(const rep&)): Likewise
        (duration::operator%=(const duration&)): Likewise
        (time_point::operator+=(const duration&)): Likewise
        (time_point::operator-=(const duration&)): Likewise

        * libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc:
new tests
        * libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc: new

[-- Attachment #2: gb50-changes.diff --]
[-- Type: text/plain, Size: 4160 bytes --]

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index ceae7f8..6a6995c 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -349,50 +349,50 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	operator-() const
 	{ return duration(-__r); }
 
-	duration&
+	constexpr duration&
 	operator++()
 	{
 	  ++__r;
 	  return *this;
 	}
 
-	duration
+	constexpr duration
 	operator++(int)
 	{ return duration(__r++); }
 
-	duration&
+	constexpr duration&
 	operator--()
 	{
 	  --__r;
 	  return *this;
 	}
 
-	duration
+	constexpr duration
 	operator--(int)
 	{ return duration(__r--); }
 
-	duration&
+	constexpr duration&
 	operator+=(const duration& __d)
 	{
 	  __r += __d.count();
 	  return *this;
 	}
 
-	duration&
+	constexpr duration&
 	operator-=(const duration& __d)
 	{
 	  __r -= __d.count();
 	  return *this;
 	}
 
-	duration&
+	constexpr duration&
 	operator*=(const rep& __rhs)
 	{
 	  __r *= __rhs;
 	  return *this;
 	}
 
-	duration&
+	constexpr duration&
 	operator/=(const rep& __rhs)
 	{
 	  __r /= __rhs;
@@ -401,7 +401,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
 
 	// DR 934.
 	template<typename _Rep2 = rep>
-	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
+	  constexpr typename enable_if<!treat_as_floating_point<_Rep2>::value,
 			     duration&>::type
 	  operator%=(const rep& __rhs)
 	  {
@@ -410,7 +410,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	  }
 
 	template<typename _Rep2 = rep>
-	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
+	  constexpr typename enable_if<!treat_as_floating_point<_Rep2>::value,
 			     duration&>::type
 	  operator%=(const duration& __d)
 	  {
@@ -631,14 +631,14 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	{ return __d; }
 
 	// arithmetic
-	time_point&
+	constexpr time_point&
 	operator+=(const duration& __dur)
 	{
 	  __d += __dur;
 	  return *this;
 	}
 
-	time_point&
+	constexpr time_point&
 	operator-=(const duration& __dur)
 	{
 	  __d -= __dur;
diff --git a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
index 285f941..1128a52 100644
--- a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
+++ b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
@@ -19,11 +19,31 @@
 
 #include <chrono>
 #include <testsuite_common_types.h>
+constexpr auto test_operators()
+{
+  std::chrono::nanoseconds d1 { };
+  d1++;
+  ++d1;
+  d1--;
+  --d1;
+
+  auto d2(d1);
+
+  d1+=d2;
+  d1-=d2;
 
+  d1*=1;
+  d1/=1;
+  d1%=1;
+  d1%=d2;
+
+  return d1;
+}
 int main()
 {
   constexpr std::chrono::nanoseconds d1 { };
   constexpr auto d2(+d1);
   constexpr auto d3(-d2);
+  constexpr auto d4 = test_operators();
   return 0;
 }
diff --git a/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
new file mode 100644
index 0000000..e87a226
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
@@ -0,0 +1,39 @@
+// { dg-do compile { target c++11 } }
+
+// Copyright (C) 2011-2016 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/>.
+
+#include <chrono>
+#include <testsuite_common_types.h>
+constexpr auto test_operators()
+{
+  using namespace std::chrono;
+  nanoseconds d1 { };
+  time_point<system_clock> c1 { };
+
+  c1+=d1;
+  c1-=d1;
+
+
+  return 11;
+}
+int main()
+{
+  constexpr auto a = test_operators();
+
+  return 0;
+}

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

* Re: [v3 PATCH] Implement C++17 GB50 resolution
  2017-02-14 21:39 [v3 PATCH] Implement C++17 GB50 resolution Dinka Ranns
@ 2017-02-15  7:43 ` Ville Voutilainen
  2017-02-15  9:58 ` Jonathan Wakely
  1 sibling, 0 replies; 5+ messages in thread
From: Ville Voutilainen @ 2017-02-15  7:43 UTC (permalink / raw)
  To: Dinka Ranns; +Cc: gcc-patches, libstdc++

On 14 February 2017 at 23:22, Dinka Ranns <dinka.ranns@googlemail.com> wrote:
>         C++17 GB50 resolution
>         * libstdc++-v3/include/std/chrono:

Pardon me for not noticing this while looking at the earlier versions
of this patch, but these should
not include the libstdc++-v3 prefix, so it should be

* include/std/chrono:

>         * libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc:

And this should be

* testsuite/20_util/duration/arithmetic/constexpr.cc:

>         * libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc: new

and likewise here,

* testsuite/20_util/time_point/arithmetic/constexpr.cc: new

That's minor and can be fixed by a maintainer committing the patch,
but for future reference.

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

* Re: [v3 PATCH] Implement C++17 GB50 resolution
  2017-02-14 21:39 [v3 PATCH] Implement C++17 GB50 resolution Dinka Ranns
  2017-02-15  7:43 ` Ville Voutilainen
@ 2017-02-15  9:58 ` Jonathan Wakely
  2017-02-18 21:16   ` Dinka Ranns
  1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2017-02-15  9:58 UTC (permalink / raw)
  To: Dinka Ranns; +Cc: gcc-patches, libstdc++

Hi, Dinka, thanks for the patch.

On 14/02/17 21:22 +0000, Dinka Ranns wrote:
>diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
>index ceae7f8..6a6995c 100644
>--- a/libstdc++-v3/include/std/chrono
>+++ b/libstdc++-v3/include/std/chrono
>@@ -349,50 +349,50 @@ _GLIBCXX_END_NAMESPACE_VERSION
> 	operator-() const
> 	{ return duration(-__r); }
>
>-	duration&
>+	constexpr duration&

This needs to use _GLIBCXX17_CONSTEXPR instead of 'constexpr'

These functions aren't constexpr in C++11 and C++14, and the standard
(annoyingly) forbids us from adding constexpr anywhere it isn't
present in the standard.

The macro _GLIBCXX17_CONSTEXPR expands to 'constexpr' if __cplusplus >
201402L, and expands to nothing otherwise.

Each new 'constexpr' you've added needs to use that macro.

>diff --git a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
>index 285f941..1128a52 100644
>--- a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
>+++ b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
>@@ -19,11 +19,31 @@
>
> #include <chrono>
> #include <testsuite_common_types.h>

There should be a blank line before and after this function, however
...

>+constexpr auto test_operators()
>+{
>+  std::chrono::nanoseconds d1 { };
>+  d1++;

This new function uses C++14 return type deduction, so will fail if
the test is run in C++11 mode (the default is C++14, but it can be
overridden on the command-line).

>+  ++d1;
>+  d1--;
>+  --d1;

Also once you change the new 'constexpr' specifiers to use the
_GLIBCXX17_CONSTEXPR macro this test will fail in C++14 mode. I think
this new function needs to be moved to a new test file, such as
testsuite/20_util/duration/arithmetic/constexpr_c++17.cc

That should contain just your new test_operators() function, because
the rest of the class will be tested by the existing constexpr.cc test
file. So something like:

// { dg-options "-std=gnu++17" }
// { dg-do compile { target c++1z } }

// Copyright etc. etc.
// ...

#include <chrono>

constexpr auto test_operators()
{
  // ...
}

constexpr auto d4 = test_operators();


Note that the "dg-do compile" line should use the c++1z target instead
of c++11, and needs to override the default dialect with a dg-options
line.

This test doesn't need a "main" function because it's a "dg-do
compile" test, so isn't linked. (The existing test that you modified
didn't need one either, but it doesn't do any harm leaving it there).


>diff --git a/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
>new file mode 100644
>index 0000000..e87a226
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
>@@ -0,0 +1,39 @@
>+// { dg-do compile { target c++11 } }

Since the time_point member functions will only be constexpr in C++17
this test also needs to use c++1z instead of c++11, and needs to
override the default C++14 dialect, i.e.

// { dg-options "-std=gnu++17" }
// { dg-do compile { target c++1z } }

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

* Re: [v3 PATCH] Implement C++17 GB50 resolution
  2017-02-15  9:58 ` Jonathan Wakely
@ 2017-02-18 21:16   ` Dinka Ranns
  2017-02-19 16:09     ` Jonathan Wakely
  0 siblings, 1 reply; 5+ messages in thread
From: Dinka Ranns @ 2017-02-18 21:16 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 4457 bytes --]

Comments addressed. Please find the new diff attached to this e-mail.

Changelog after review comments :

2017-02-18 Dinka Ranns <dinka.ranns@googlemail.com>

        C++17 GB50 resolution
        * include/std/chrono:
        (duration::operator++()): Add _GLIBCXX17_CONSTEXPR.
        (duration::operator++(int)): Likewise
        (duration::operator--()): Likewise
        (duration::operator--(int)): Likewise
        (duration::operator+=(const duration&)): Likewise
        (duration::operator-=(const duration&)): Likewise
        (duration::operator*=(const rep&)): Likewise
        (duration::operator/=(const rep&)): Likewise
        (duration::operator%=(const rep&)): Likewise
        (duration::operator%=(const duration&)): Likewise
        (time_point::operator+=(const duration&)): Likewise
        (time_point::operator-=(const duration&)): Likewise

        * testsuite/20_util/duration/arithmetic/constexpr_c++17.cc: new tests
        * testsuite/20_util/time_point/arithmetic/constexpr.cc: new tests




On 15 February 2017 at 09:35, Jonathan Wakely <jwakely@redhat.com> wrote:
> Hi, Dinka, thanks for the patch.
>
> On 14/02/17 21:22 +0000, Dinka Ranns wrote:
>>
>> diff --git a/libstdc++-v3/include/std/chrono
>> b/libstdc++-v3/include/std/chrono
>> index ceae7f8..6a6995c 100644
>> --- a/libstdc++-v3/include/std/chrono
>> +++ b/libstdc++-v3/include/std/chrono
>> @@ -349,50 +349,50 @@ _GLIBCXX_END_NAMESPACE_VERSION
>>         operator-() const
>>         { return duration(-__r); }
>>
>> -       duration&
>> +       constexpr duration&
>
>
> This needs to use _GLIBCXX17_CONSTEXPR instead of 'constexpr'
>
> These functions aren't constexpr in C++11 and C++14, and the standard
> (annoyingly) forbids us from adding constexpr anywhere it isn't
> present in the standard.
>
> The macro _GLIBCXX17_CONSTEXPR expands to 'constexpr' if __cplusplus >
> 201402L, and expands to nothing otherwise.
>
> Each new 'constexpr' you've added needs to use that macro.
>
>> diff --git
>> a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
>> b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
>> index 285f941..1128a52 100644
>> --- a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
>> +++ b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr.cc
>> @@ -19,11 +19,31 @@
>>
>> #include <chrono>
>> #include <testsuite_common_types.h>
>
>
> There should be a blank line before and after this function, however
> ...
>
>> +constexpr auto test_operators()
>> +{
>> +  std::chrono::nanoseconds d1 { };
>> +  d1++;
>
>
> This new function uses C++14 return type deduction, so will fail if
> the test is run in C++11 mode (the default is C++14, but it can be
> overridden on the command-line).
>
>> +  ++d1;
>> +  d1--;
>> +  --d1;
>
>
> Also once you change the new 'constexpr' specifiers to use the
> _GLIBCXX17_CONSTEXPR macro this test will fail in C++14 mode. I think
> this new function needs to be moved to a new test file, such as
> testsuite/20_util/duration/arithmetic/constexpr_c++17.cc
>
> That should contain just your new test_operators() function, because
> the rest of the class will be tested by the existing constexpr.cc test
> file. So something like:
>
> // { dg-options "-std=gnu++17" }
> // { dg-do compile { target c++1z } }
>
> // Copyright etc. etc.
> // ...
>
> #include <chrono>
>
> constexpr auto test_operators()
> {
>  // ...
> }
>
> constexpr auto d4 = test_operators();
>
>
> Note that the "dg-do compile" line should use the c++1z target instead
> of c++11, and needs to override the default dialect with a dg-options
> line.
>
> This test doesn't need a "main" function because it's a "dg-do
> compile" test, so isn't linked. (The existing test that you modified
> didn't need one either, but it doesn't do any harm leaving it there).
>
>
>> diff --git
>> a/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
>> b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
>> new file mode 100644
>> index 0000000..e87a226
>> --- /dev/null
>> +++ b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
>> @@ -0,0 +1,39 @@
>> +// { dg-do compile { target c++11 } }
>
>
> Since the time_point member functions will only be constexpr in C++17
> this test also needs to use c++1z instead of c++11, and needs to
> override the default C++14 dialect, i.e.
>
> // { dg-options "-std=gnu++17" }
> // { dg-do compile { target c++1z } }
>

[-- Attachment #2: gb50_changes_v2.diff --]
[-- Type: text/plain, Size: 4993 bytes --]

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 2c33be0..d05eaaf 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -349,50 +349,50 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	operator-() const
 	{ return duration(-__r); }
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator++()
 	{
 	  ++__r;
 	  return *this;
 	}
 
-	duration
+	_GLIBCXX17_CONSTEXPR duration
 	operator++(int)
 	{ return duration(__r++); }
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator--()
 	{
 	  --__r;
 	  return *this;
 	}
 
-	duration
+	_GLIBCXX17_CONSTEXPR duration
 	operator--(int)
 	{ return duration(__r--); }
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator+=(const duration& __d)
 	{
 	  __r += __d.count();
 	  return *this;
 	}
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator-=(const duration& __d)
 	{
 	  __r -= __d.count();
 	  return *this;
 	}
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator*=(const rep& __rhs)
 	{
 	  __r *= __rhs;
 	  return *this;
 	}
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator/=(const rep& __rhs)
 	{
 	  __r /= __rhs;
@@ -401,7 +401,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
 
 	// DR 934.
 	template<typename _Rep2 = rep>
-	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
+	  _GLIBCXX17_CONSTEXPR typename enable_if<!treat_as_floating_point<_Rep2>::value,
 			     duration&>::type
 	  operator%=(const rep& __rhs)
 	  {
@@ -410,7 +410,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	  }
 
 	template<typename _Rep2 = rep>
-	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
+	  _GLIBCXX17_CONSTEXPR typename enable_if<!treat_as_floating_point<_Rep2>::value,
 			     duration&>::type
 	  operator%=(const duration& __d)
 	  {
@@ -631,14 +631,14 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	{ return __d; }
 
 	// arithmetic
-	time_point&
+	_GLIBCXX17_CONSTEXPR time_point&
 	operator+=(const duration& __dur)
 	{
 	  __d += __dur;
 	  return *this;
 	}
 
-	time_point&
+	_GLIBCXX17_CONSTEXPR time_point&
 	operator-=(const duration& __dur)
 	{
 	  __d -= __dur;
diff --git a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr_c++17.cc b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr_c++17.cc
new file mode 100644
index 0000000..2721765
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr_c++17.cc
@@ -0,0 +1,45 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++1z } }
+
+// Copyright (C) 2011-2017 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/>.
+
+#include <chrono>
+#include <testsuite_common_types.h>
+constexpr auto test_operators()
+{
+  std::chrono::nanoseconds d1 { 1 };
+  d1++;
+  ++d1;
+  d1--;
+  --d1;
+
+  auto d2(d1);
+
+  d1+=d2;
+  d1-=d2;
+
+  d1*=1;
+  d1/=1;
+  d1%=1;
+  d1%=d2;
+
+  return d1;
+}
+
+constexpr auto d4 = test_operators();
+
diff --git a/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
new file mode 100644
index 0000000..2f2aceb
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
@@ -0,0 +1,36 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++1z } }
+
+// Copyright (C) 2011-2016 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/>.
+
+#include <chrono>
+#include <testsuite_common_types.h>
+constexpr auto test_operators()
+{
+  using namespace std::chrono;
+  nanoseconds d1 { };
+  time_point<system_clock> c1 { };
+
+  c1+=d1;
+  c1-=d1;
+
+
+  return 11;
+}
+
+constexpr auto a = test_operators();

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

* Re: [v3 PATCH] Implement C++17 GB50 resolution
  2017-02-18 21:16   ` Dinka Ranns
@ 2017-02-19 16:09     ` Jonathan Wakely
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Wakely @ 2017-02-19 16:09 UTC (permalink / raw)
  To: Dinka Ranns; +Cc: gcc-patches, libstdc++

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

On 18/02/17 20:48 +0000, Dinka Ranns wrote:
>Comments addressed. Please find the new diff attached to this e-mail.
>
>Changelog after review comments :
>
>2017-02-18 Dinka Ranns <dinka.ranns@googlemail.com>

GNU ChangeLog rules say two spaces after the date and after your name.

>        C++17 GB50 resolution
>        * include/std/chrono:
>        (duration::operator++()): Add _GLIBCXX17_CONSTEXPR.
>        (duration::operator++(int)): Likewise
>        (duration::operator--()): Likewise
>        (duration::operator--(int)): Likewise
>        (duration::operator+=(const duration&)): Likewise
>        (duration::operator-=(const duration&)): Likewise
>        (duration::operator*=(const rep&)): Likewise
>        (duration::operator/=(const rep&)): Likewise
>        (duration::operator%=(const rep&)): Likewise
>        (duration::operator%=(const duration&)): Likewise
>        (time_point::operator+=(const duration&)): Likewise
>        (time_point::operator-=(const duration&)): Likewise
>
>        * testsuite/20_util/duration/arithmetic/constexpr_c++17.cc: new tests
>        * testsuite/20_util/time_point/arithmetic/constexpr.cc: new tests

I also tweaked the ChangeLog entry to add some full stops.

>@@ -401,7 +401,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
>
> 	// DR 934.
> 	template<typename _Rep2 = rep>
>-	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
>+	  _GLIBCXX17_CONSTEXPR typename enable_if<!treat_as_floating_point<_Rep2>::value,
> 			     duration&>::type
> 	  operator%=(const rep& __rhs)
> 	  {
>@@ -410,7 +410,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
> 	  }
>
> 	template<typename _Rep2 = rep>
>-	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
>+	  _GLIBCXX17_CONSTEXPR typename enable_if<!treat_as_floating_point<_Rep2>::value,
> 			     duration&>::type
> 	  operator%=(const duration& __d)
> 	  {

I added line breaks to these two signatures to keep the line shorter
than 80 columns, and to keep the "duration&" lined up with the first
argument to enable_if. That meant 20_util/duration/literals/range.cc
started to FAIL because the expected error was on a different line, so
I adjusted that test. 

The attached patch includes those tweaks and is what I've tested and
committed. Thanks for your first contribution to libstdc++!



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 6615 bytes --]

commit eac82ba48c414d10806453a68785225dc4f86971
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sun Feb 19 14:19:17 2017 +0000

    C++17 GB50 resolution (P0505R0)
    
    2017-02-19  Dinka Ranns  <dinka.ranns@googlemail.com>
    
    	C++17 GB50 resolution
    	* include/std/chrono (duration::operator++()): Add
    	_GLIBCXX17_CONSTEXPR.
    	(duration::operator++(int)): Likewise.
    	(duration::operator--()): Likewise.
    	(duration::operator--(int)): Likewise.
    	(duration::operator+=(const duration&)): Likewise.
    	(duration::operator-=(const duration&)): Likewise.
    	(duration::operator*=(const rep&)): Likewise.
    	(duration::operator/=(const rep&)): Likewise.
    	(duration::operator%=(const rep&)): Likewise.
    	(duration::operator%=(const duration&)): Likewise.
    	(time_point::operator+=(const duration&)): Likewise.
    	(time_point::operator-=(const duration&)): Likewise.
    	* testsuite/20_util/duration/arithmetic/constexpr_c++17.cc: New test.
    	* testsuite/20_util/duration/literals/range.cc: Adjust dg-error.
    	* testsuite/20_util/time_point/arithmetic/constexpr.cc: New test.

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 2c33be0..b3dc430 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -349,50 +349,50 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	operator-() const
 	{ return duration(-__r); }
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator++()
 	{
 	  ++__r;
 	  return *this;
 	}
 
-	duration
+	_GLIBCXX17_CONSTEXPR duration
 	operator++(int)
 	{ return duration(__r++); }
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator--()
 	{
 	  --__r;
 	  return *this;
 	}
 
-	duration
+	_GLIBCXX17_CONSTEXPR duration
 	operator--(int)
 	{ return duration(__r--); }
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator+=(const duration& __d)
 	{
 	  __r += __d.count();
 	  return *this;
 	}
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator-=(const duration& __d)
 	{
 	  __r -= __d.count();
 	  return *this;
 	}
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator*=(const rep& __rhs)
 	{
 	  __r *= __rhs;
 	  return *this;
 	}
 
-	duration&
+	_GLIBCXX17_CONSTEXPR duration&
 	operator/=(const rep& __rhs)
 	{
 	  __r /= __rhs;
@@ -401,6 +401,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
 
 	// DR 934.
 	template<typename _Rep2 = rep>
+	  _GLIBCXX17_CONSTEXPR
 	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
 			     duration&>::type
 	  operator%=(const rep& __rhs)
@@ -410,6 +411,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	  }
 
 	template<typename _Rep2 = rep>
+	  _GLIBCXX17_CONSTEXPR
 	  typename enable_if<!treat_as_floating_point<_Rep2>::value,
 			     duration&>::type
 	  operator%=(const duration& __d)
@@ -631,14 +633,14 @@ _GLIBCXX_END_NAMESPACE_VERSION
 	{ return __d; }
 
 	// arithmetic
-	time_point&
+	_GLIBCXX17_CONSTEXPR time_point&
 	operator+=(const duration& __dur)
 	{
 	  __d += __dur;
 	  return *this;
 	}
 
-	time_point&
+	_GLIBCXX17_CONSTEXPR time_point&
 	operator-=(const duration& __dur)
 	{
 	  __d -= __dur;
diff --git a/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr_c++17.cc b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr_c++17.cc
new file mode 100644
index 0000000..2721765
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/duration/arithmetic/constexpr_c++17.cc
@@ -0,0 +1,45 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++1z } }
+
+// Copyright (C) 2011-2017 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/>.
+
+#include <chrono>
+#include <testsuite_common_types.h>
+constexpr auto test_operators()
+{
+  std::chrono::nanoseconds d1 { 1 };
+  d1++;
+  ++d1;
+  d1--;
+  --d1;
+
+  auto d2(d1);
+
+  d1+=d2;
+  d1-=d2;
+
+  d1*=1;
+  d1/=1;
+  d1%=1;
+  d1%=d2;
+
+  return d1;
+}
+
+constexpr auto d4 = test_operators();
+
diff --git a/libstdc++-v3/testsuite/20_util/duration/literals/range.cc b/libstdc++-v3/testsuite/20_util/duration/literals/range.cc
index eafc806..c0d1a6e 100644
--- a/libstdc++-v3/testsuite/20_util/duration/literals/range.cc
+++ b/libstdc++-v3/testsuite/20_util/duration/literals/range.cc
@@ -26,6 +26,6 @@ test01()
 
   // std::numeric_limits<int64_t>::max() == 9223372036854775807;
   auto h = 9223372036854775808h;
-  // { dg-error "cannot be represented" "" { target *-*-* } 890 }
+  // { dg-error "cannot be represented" "" { target *-*-* } 892 }
 }
 // { dg-prune-output "in constexpr expansion" } // needed for -O0
diff --git a/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
new file mode 100644
index 0000000..2f2aceb
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/time_point/arithmetic/constexpr.cc
@@ -0,0 +1,36 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++1z } }
+
+// Copyright (C) 2011-2016 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/>.
+
+#include <chrono>
+#include <testsuite_common_types.h>
+constexpr auto test_operators()
+{
+  using namespace std::chrono;
+  nanoseconds d1 { };
+  time_point<system_clock> c1 { };
+
+  c1+=d1;
+  c1-=d1;
+
+
+  return 11;
+}
+
+constexpr auto a = test_operators();

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

end of thread, other threads:[~2017-02-19 16:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-14 21:39 [v3 PATCH] Implement C++17 GB50 resolution Dinka Ranns
2017-02-15  7:43 ` Ville Voutilainen
2017-02-15  9:58 ` Jonathan Wakely
2017-02-18 21:16   ` Dinka Ranns
2017-02-19 16:09     ` 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).