Message ID | 5cad3347-a5eb-9cdf-75ec-2e258665efe2@gmail.com |
---|---|
State | New |
Headers | show |
Series | Fix _GLIBCXX_DEBUG tests | expand |
On 13/12/20 15:52 +0100, François Dumont via Libstdc++ wrote: >Some tests are XPASS because array assertions have been disabled for a >good reason in C++11. > >I wonder if the respective non-constexpr _GLIBCXX_ASSERTION checks >shouldn't target C++14 too. At the moment they are failing as expected >but because of an Undefined Behavior no ? Hmm, maybe my "fix" for the bug was too hasty, and I should have done this instead: --- a/libstdc++-v3/include/bits/c++config +++ b/libstdc++-v3/include/bits/c++config @@ -684,7 +684,7 @@ namespace std #undef _GLIBCXX_HAS_BUILTIN -#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED +#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED && __cplusplus >= 201402L # define __glibcxx_assert_1(_Condition) \ if (__builtin_is_constant_evaluated()) \ { \ That would allow us to keep the std::array runtime assertions for C++11, and only disable them in constexpr contexts. >The other test is failing because of some cleanup in headers which >makes <memory> include necessary. > >   libstdc++: Fix several _GLIBCXX_DEBUG tests > >   libstdc++-v3/ChangeLog: > >           * testsuite/23_containers/array/debug/back2_neg.cc: target >c++14 because assertion >           for constexpr is disabled in C++11. >           * testsuite/23_containers/array/debug/front2_neg.cc: Likewise. >           * >testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc: >Likewise. >           * >testsuite/23_containers/vector/debug/multithreaded_swap.cc: Include ><memory> >           for shared_ptr. > >Ok to commit ? Yes, thanks.
On 13/12/20 11:17 pm, Jonathan Wakely wrote: > On 13/12/20 15:52 +0100, François Dumont via Libstdc++ wrote: >> Some tests are XPASS because array assertions have been disabled for >> a good reason in C++11. >> >> I wonder if the respective non-constexpr _GLIBCXX_ASSERTION checks >> shouldn't target C++14 too. At the moment they are failing as >> expected but because of an Undefined Behavior no ? > > Hmm, maybe my "fix" for the bug was too hasty, and I should have done > this instead: > > --- a/libstdc++-v3/include/bits/c++config > +++ b/libstdc++-v3/include/bits/c++config > @@ -684,7 +684,7 @@ namespace std > > #undef _GLIBCXX_HAS_BUILTIN > > -#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED > +#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED && __cplusplus >= > 201402L > # define __glibcxx_assert_1(_Condition) \ > if (__builtin_is_constant_evaluated()) \ > { \ > > That would allow us to keep the std::array runtime assertions for > C++11, and only disable them in constexpr contexts. I already tried to restore this check in C++11 runtime without success but I didn't try this approach. I'll have a try but C++11 forces constexpr to be just a return statement so I fear that it won't appreciate the additional assertion. > > >> The other test is failing because of some cleanup in headers which >> makes <memory> include necessary. >> >>    libstdc++: Fix several _GLIBCXX_DEBUG tests >> >>    libstdc++-v3/ChangeLog: >> >>            * >> testsuite/23_containers/array/debug/back2_neg.cc: target c++14 >> because assertion >>            for constexpr is disabled in C++11. >>            * >> testsuite/23_containers/array/debug/front2_neg.cc: Likewise. >>            * >> testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc: >> Likewise. >>            * >> testsuite/23_containers/vector/debug/multithreaded_swap.cc: Include >> <memory> >>            for shared_ptr. >> >> Ok to commit ? > > Yes, thanks. > >
On Mon, 14 Dec 2020, 06:51 François Dumont via Libstdc++, < libstdc++@gcc.gnu.org> wrote: > On 13/12/20 11:17 pm, Jonathan Wakely wrote: > > On 13/12/20 15:52 +0100, François Dumont via Libstdc++ wrote: > >> Some tests are XPASS because array assertions have been disabled for > >> a good reason in C++11. > >> > >> I wonder if the respective non-constexpr _GLIBCXX_ASSERTION checks > >> shouldn't target C++14 too. At the moment they are failing as > >> expected but because of an Undefined Behavior no ? > > > > Hmm, maybe my "fix" for the bug was too hasty, and I should have done > > this instead: > > > > --- a/libstdc++-v3/include/bits/c++config > > +++ b/libstdc++-v3/include/bits/c++config > > @@ -684,7 +684,7 @@ namespace std > > > > #undef _GLIBCXX_HAS_BUILTIN > > > > -#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED > > +#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED && __cplusplus >= > > 201402L > > # define __glibcxx_assert_1(_Condition) \ > > if (__builtin_is_constant_evaluated()) \ > > { \ > > > > That would allow us to keep the std::array runtime assertions for > > C++11, and only disable them in constexpr contexts. > > I already tried to restore this check in C++11 runtime without success > but I didn't try this approach. > > I'll have a try but C++11 forces constexpr to be just a return statement > so I fear that it won't appreciate the additional assertion. > Ah yes, we'd need something like Daniel suggested, and it's not worth it just for C++11. Just limiting the tests to c++14 is fine. > > > > > >> The other test is failing because of some cleanup in headers which > >> makes <memory> include necessary. > >> > >>    libstdc++: Fix several _GLIBCXX_DEBUG tests > >> > >>    libstdc++-v3/ChangeLog: > >> > >>            * > >> testsuite/23_containers/array/debug/back2_neg.cc: target c++14 > >> because assertion > >>            for constexpr is disabled in C++11. > >>            * > >> testsuite/23_containers/array/debug/front2_neg.cc: Likewise. > >>            * > >> testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc: > >> Likewise. > >>            * > >> testsuite/23_containers/vector/debug/multithreaded_swap.cc: Include > >> <memory> > >>            for shared_ptr. > >> > >> Ok to commit ? > > > > Yes, thanks. > > > > > >
On 14/12/20 11:08 am, Jonathan Wakely wrote: > > > On Mon, 14 Dec 2020, 06:51 François Dumont via Libstdc++, > <libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote: > > On 13/12/20 11:17 pm, Jonathan Wakely wrote: > > On 13/12/20 15:52 +0100, François Dumont via Libstdc++ wrote: > >> Some tests are XPASS because array assertions have been > disabled for > >> a good reason in C++11. > >> > >> I wonder if the respective non-constexpr _GLIBCXX_ASSERTION checks > >> shouldn't target C++14 too. At the moment they are failing as > >> expected but because of an Undefined Behavior no ? > > > > Hmm, maybe my "fix" for the bug was too hasty, and I should have > done > > this instead: > > > > --- a/libstdc++-v3/include/bits/c++config > > +++ b/libstdc++-v3/include/bits/c++config > > @@ -684,7 +684,7 @@ namespace std > > > > #undef _GLIBCXX_HAS_BUILTIN > > > > -#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED > > +#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED && __cplusplus >= > > 201402L > > # define __glibcxx_assert_1(_Condition) \ > > if (__builtin_is_constant_evaluated()) \ > > { \ > > > > That would allow us to keep the std::array runtime assertions for > > C++11, and only disable them in constexpr contexts. > > I already tried to restore this check in C++11 runtime without > success > but I didn't try this approach. > > I'll have a try but C++11 forces constexpr to be just a return > statement > so I fear that it won't appreciate the additional assertion. > > > > Ah yes, we'd need something like Daniel suggested, and it's not worth > it just for C++11. > > Just limiting the tests to c++14 is fine. > > Attached patch committed then. François
On 14/12/20 22:36 +0100, François Dumont wrote: >On 14/12/20 11:08 am, Jonathan Wakely wrote: >> >> >>On Mon, 14 Dec 2020, 06:51 François Dumont via Libstdc++, >><libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote: >> >> On 13/12/20 11:17 pm, Jonathan Wakely wrote: >> > On 13/12/20 15:52 +0100, François Dumont via Libstdc++ wrote: >> >> Some tests are XPASS because array assertions have been >> disabled for >> >> a good reason in C++11. >> >> >> >> I wonder if the respective non-constexpr _GLIBCXX_ASSERTION checks >> >> shouldn't target C++14 too. At the moment they are failing as >> >> expected but because of an Undefined Behavior no ? >> > >> > Hmm, maybe my "fix" for the bug was too hasty, and I should have >> done >> > this instead: >> > >> > --- a/libstdc++-v3/include/bits/c++config >> > +++ b/libstdc++-v3/include/bits/c++config >> > @@ -684,7 +684,7 @@ namespace std >> > >> > #undef _GLIBCXX_HAS_BUILTIN >> > >> > -#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED >> > +#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED && __cplusplus >= >> > 201402L >> > # define __glibcxx_assert_1(_Condition) \ >> > if (__builtin_is_constant_evaluated()) \ >> > { \ >> > >> > That would allow us to keep the std::array runtime assertions for >> > C++11, and only disable them in constexpr contexts. >> >> I already tried to restore this check in C++11 runtime without >> success >> but I didn't try this approach. >> >> I'll have a try but C++11 forces constexpr to be just a return >> statement >> so I fear that it won't appreciate the additional assertion. >> >> >> >>Ah yes, we'd need something like Daniel suggested, and it's not >>worth it just for C++11. >> >>Just limiting the tests to c++14 is fine. >> >> >Attached patch committed then. Thanks.
On 15/12/20 15:20 +0000, Jonathan Wakely wrote: >On 14/12/20 22:36 +0100, François Dumont wrote: >>On 14/12/20 11:08 am, Jonathan Wakely wrote: >>> >>> >>>On Mon, 14 Dec 2020, 06:51 François Dumont via Libstdc++, >>><libstdc++@gcc.gnu.org <mailto:libstdc%2B%2B@gcc.gnu.org>> wrote: >>> >>> On 13/12/20 11:17 pm, Jonathan Wakely wrote: >>> > On 13/12/20 15:52 +0100, François Dumont via Libstdc++ wrote: >>> >> Some tests are XPASS because array assertions have been >>> disabled for >>> >> a good reason in C++11. >>> >> >>> >> I wonder if the respective non-constexpr _GLIBCXX_ASSERTION checks >>> >> shouldn't target C++14 too. At the moment they are failing as >>> >> expected but because of an Undefined Behavior no ? >>> > >>> > Hmm, maybe my "fix" for the bug was too hasty, and I should have >>> done >>> > this instead: >>> > >>> > --- a/libstdc++-v3/include/bits/c++config >>> > +++ b/libstdc++-v3/include/bits/c++config >>> > @@ -684,7 +684,7 @@ namespace std >>> > >>> >  #undef _GLIBCXX_HAS_BUILTIN >>> > >>> > -#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED >>> > +#if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED && __cplusplus >= >>> > 201402L >>> >  # define __glibcxx_assert_1(_Condition)               \ >>> >     if (__builtin_is_constant_evaluated())    \ >>> >      {                                        \ >>> > >>> > That would allow us to keep the std::array runtime assertions for >>> > C++11, and only disable them in constexpr contexts. >>> >>> I already tried to restore this check in C++11 runtime without >>> success >>> but I didn't try this approach. >>> >>> I'll have a try but C++11 forces constexpr to be just a return >>> statement >>> so I fear that it won't appreciate the additional assertion. >>> >>> >>> >>>Ah yes, we'd need something like Daniel suggested, and it's not >>>worth it just for C++11. >>> >>>Just limiting the tests to c++14 is fine. >>> >>> >>Attached patch committed then. > >Thanks. I'm committing this anyway, because although it won't fix those tests, it is useless to check __builtin_is_constant_evaluated() in C++11 mode. Tested powerpc64le-linux, normal mode and debug mode. Pushed to trunk.
diff --git a/libstdc++-v3/testsuite/23_containers/array/debug/back2_neg.cc b/libstdc++-v3/testsuite/23_containers/array/debug/back2_neg.cc index b14a3ec8c04..0066c671c42 100644 --- a/libstdc++-v3/testsuite/23_containers/array/debug/back2_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/array/debug/back2_neg.cc @@ -16,7 +16,7 @@ // <http://www.gnu.org/licenses/>. // // { dg-options "-D_GLIBCXX_ASSERTIONS" } -// { dg-do run { target c++11 xfail *-*-* } } +// { dg-do run { target c++14 xfail *-*-* } } #include <array> diff --git a/libstdc++-v3/testsuite/23_containers/array/debug/front2_neg.cc b/libstdc++-v3/testsuite/23_containers/array/debug/front2_neg.cc index e099e6eb46b..a6118cfce3a 100644 --- a/libstdc++-v3/testsuite/23_containers/array/debug/front2_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/array/debug/front2_neg.cc @@ -16,7 +16,7 @@ // <http://www.gnu.org/licenses/>. // // { dg-options "-D_GLIBCXX_ASSERTIONS" } -// { dg-do run { target c++11 xfail *-*-* } } +// { dg-do run { target c++14 xfail *-*-* } } #include <array> diff --git a/libstdc++-v3/testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc b/libstdc++-v3/testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc index 4e93c8a7d68..efb28d715e9 100644 --- a/libstdc++-v3/testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc +++ b/libstdc++-v3/testsuite/23_containers/array/debug/square_brackets_operator2_neg.cc @@ -16,7 +16,7 @@ // <http://www.gnu.org/licenses/>. // // { dg-options "-D_GLIBCXX_ASSERTIONS" } -// { dg-do run { target c++11 xfail *-*-* } } +// { dg-do run { target c++14 xfail *-*-* } } #include <array> diff --git a/libstdc++-v3/testsuite/23_containers/vector/debug/multithreaded_swap.cc b/libstdc++-v3/testsuite/23_containers/vector/debug/multithreaded_swap.cc index 0d795147644..a0050ec764c 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/debug/multithreaded_swap.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/debug/multithreaded_swap.cc @@ -26,6 +26,7 @@ // mode as it requires acquiring 2 locks at the same time. #include <vector> +#include <memory> #include <thread> #include <functional> #include <testsuite_hooks.h>