Re: [v3 PATCH] Implement C++17 GB50 resolution
On 18/02/17 20:48 +, Dinka Ranns wrote: Comments addressed. Please find the new diff attached to this e-mail. Changelog after review comments : 2017-02-18 Dinka RannsGNU 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 enable_if::value, + _GLIBCXX17_CONSTEXPR typename enable_if::value, duration&>::type operator%=(const rep& __rhs) { @@ -410,7 +410,7 @@ _GLIBCXX_END_NAMESPACE_VERSION } template - typename enable_if::value, + _GLIBCXX17_CONSTEXPR typename enable_if::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++! commit eac82ba48c414d10806453a68785225dc4f86971 Author: Jonathan Wakely Date: Sun Feb 19 14:19:17 2017 + C++17 GB50 resolution (P0505R0) 2017-02-19 Dinka Ranns 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 + _GLIBCXX17_CONSTEXPR typename enable_if::value, duration&>::type operator%=(const rep& __rhs) @@ -410,6 +411,7 @@ _GLIBCXX_END_NAMESPACE_VERSION } template + _GLIBCXX17_CONSTEXPR typename enable_if::value, duration&>::type operator%=(const duration& __d) @@ -631,14 +633,14 @@ _GLIBCXX_END_NAMESPACE_VERSION { return __d; } // arithmetic - time_point& + _GLIBCXX17_CONSTEXPR time_point&
Re: [v3 PATCH] Implement C++17 GB50 resolution
Comments addressed. Please find the new diff attached to this e-mail. Changelog after review comments : 2017-02-18 Dinka RannsC++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 wrote: > Hi, Dinka, thanks for the patch. > > On 14/02/17 21:22 +, 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 >> #include > > > 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 > > 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 000..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 } } > 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& +
Re: [v3 PATCH] Implement C++17 GB50 resolution
Hi, Dinka, thanks for the patch. On 14/02/17 21:22 +, 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 #include 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 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 000..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 } }
Re: [v3 PATCH] Implement C++17 GB50 resolution
On 14 February 2017 at 23:22, Dinka Rannswrote: > 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.
[v3 PATCH] Implement C++17 GB50 resolution
Tested on Linux-x64 Implementation of resolution for C++17 GB50 2017-02-12 Dinka RannsC++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 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 enable_if::value, + constexpr typename enable_if::value, duration&>::type operator%=(const rep& __rhs) { @@ -410,7 +410,7 @@ _GLIBCXX_END_NAMESPACE_VERSION } template - typename enable_if::value, + constexpr typename enable_if::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 #include +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 000..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