* [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
@ 2023-09-23 20:10 François Dumont
2023-10-02 17:07 ` François Dumont
2023-10-08 13:59 ` Iain Sandoe
0 siblings, 2 replies; 8+ messages in thread
From: François Dumont @ 2023-09-23 20:10 UTC (permalink / raw)
To: gcc, libstdc++; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
I'm eventually fixing those tests the same way we manage this problem in
libstdc++ testsuite.
testsuite: Add optional libstdc++ version namespace in expected
diagnostic
When libstdc++ is build with
--enable-symvers=gnu-versioned-namespace diagnostics are
showing this namespace, currently __8.
gcc/testsuite/ChangeLog:
*
testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
'__8' version namespace in expected diagnostic.
*
testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
*
testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
*
testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C: Likewise.
* testsuite/g++.dg/coroutines/pr97438.C: Likewise.
* testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
Tested under Linux x86_64.
I'm contributing to libstdc++ so I already have write access.
Ok to commit ?
François
[-- Attachment #2: coroutines_tests.patch --]
[-- Type: text/x-patch, Size: 2493 bytes --]
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 73e7f381020..c44b71a04cb 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -4867,7 +4867,7 @@ dnl
dnl Control whether the library should define symbols for old and new ABIs.
dnl This affects definitions of strings, stringstreams and locale facets.
dnl
-dnl --disable-libstdcxx-dual-abi will use old ABI for all types.
+dnl --disable-libstdcxx-dual-abi will use new ABI for all types.
dnl
dnl Defines:
dnl _GLIBCXX_USE_DUAL_ABI (always defined, either to 1 or 0)
@@ -4883,7 +4883,7 @@ AC_DEFUN([GLIBCXX_ENABLE_LIBSTDCXX_DUAL_ABI], [
else
if test x"$enable_libstdcxx_dual_abi" != xyes; then
AC_MSG_NOTICE([dual ABI is disabled])
- default_libstdcxx_abi="gcc4-compatible"
+ default_libstdcxx_abi="new"
fi
fi
GLIBCXX_CONDITIONAL(ENABLE_DUAL_ABI, test $enable_libstdcxx_dual_abi = yes)
@@ -4898,7 +4898,6 @@ dnl Defines:
dnl _GLIBCXX_USE_CXX11_ABI (always defined, either to 1 or 0)
dnl
AC_DEFUN([GLIBCXX_DEFAULT_ABI], [
- if test x$enable_libstdcxx_dual_abi = xyes; then
AC_MSG_CHECKING([for default std::string ABI to use])
AC_ARG_WITH([default-libstdcxx-abi],
AS_HELP_STRING([--with-default-libstdcxx-abi],
@@ -4912,7 +4911,6 @@ AC_DEFUN([GLIBCXX_DEFAULT_ABI], [
],
[default_libstdcxx_abi="new"])
AC_MSG_RESULT(${default_libstdcxx_abi})
- fi
if test $default_libstdcxx_abi = "new"; then
glibcxx_cxx11_abi=1
glibcxx_cxx98_abi=0
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 6e9a532a359..14f9569597a 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -70712,13 +70712,12 @@ $as_echo "$as_me: dual ABI is disabled" >&6;}
if test x"$enable_libstdcxx_dual_abi" != xyes; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: dual ABI is disabled" >&5
$as_echo "$as_me: dual ABI is disabled" >&6;}
- default_libstdcxx_abi="gcc4-compatible"
+ default_libstdcxx_abi="new"
fi
fi
- if test x$enable_libstdcxx_dual_abi = xyes; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for default std::string ABI to use" >&5
$as_echo_n "checking for default std::string ABI to use... " >&6; }
@@ -70737,7 +70736,6 @@ fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: ${default_libstdcxx_abi}" >&5
$as_echo "${default_libstdcxx_abi}" >&6; }
- fi
if test $default_libstdcxx_abi = "new"; then
glibcxx_cxx11_abi=1
glibcxx_cxx98_abi=0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
2023-09-23 20:10 [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode François Dumont
@ 2023-10-02 17:07 ` François Dumont
2023-10-03 9:52 ` Jonathan Wakely
2023-10-08 13:59 ` Iain Sandoe
1 sibling, 1 reply; 8+ messages in thread
From: François Dumont @ 2023-10-02 17:07 UTC (permalink / raw)
To: gcc, libstdc++; +Cc: gcc-patches
Hi
Gentle reminder for this minor patch.
Thanks
On 23/09/2023 22:10, François Dumont wrote:
> I'm eventually fixing those tests the same way we manage this problem
> in libstdc++ testsuite.
>
> testsuite: Add optional libstdc++ version namespace in expected
> diagnostic
>
> When libstdc++ is build with
> --enable-symvers=gnu-versioned-namespace diagnostics are
> showing this namespace, currently __8.
>
> gcc/testsuite/ChangeLog:
>
> *
> testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
> '__8' version namespace in expected diagnostic.
> *
> testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
> *
> testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
> *
> testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C:
> Likewise.
> * testsuite/g++.dg/coroutines/pr97438.C: Likewise.
> * testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
>
> Tested under Linux x86_64.
>
> I'm contributing to libstdc++ so I already have write access.
>
> Ok to commit ?
>
> François
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
2023-10-02 17:07 ` François Dumont
@ 2023-10-03 9:52 ` Jonathan Wakely
2023-10-03 20:14 ` François Dumont
0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2023-10-03 9:52 UTC (permalink / raw)
To: François Dumont; +Cc: gcc, libstdc++, gcc-patches
On Mon, 2 Oct 2023 at 18:07, François Dumont <frs.dumont@gmail.com> wrote:
>
> Hi
>
> Gentle reminder for this minor patch.
It looks like you attached the wrong patch.
>
> Thanks
>
> On 23/09/2023 22:10, François Dumont wrote:
> > I'm eventually fixing those tests the same way we manage this problem
> > in libstdc++ testsuite.
> >
> > testsuite: Add optional libstdc++ version namespace in expected
> > diagnostic
> >
> > When libstdc++ is build with
> > --enable-symvers=gnu-versioned-namespace diagnostics are
> > showing this namespace, currently __8.
> >
> > gcc/testsuite/ChangeLog:
> >
> > *
> > testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
> > '__8' version namespace in expected diagnostic.
> > *
> > testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
> > *
> > testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
> > *
> > testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C:
> > Likewise.
> > * testsuite/g++.dg/coroutines/pr97438.C: Likewise.
> > * testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
> >
> > Tested under Linux x86_64.
> >
> > I'm contributing to libstdc++ so I already have write access.
> >
> > Ok to commit ?
> >
> > François
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
2023-10-03 9:52 ` Jonathan Wakely
@ 2023-10-03 20:14 ` François Dumont
0 siblings, 0 replies; 8+ messages in thread
From: François Dumont @ 2023-10-03 20:14 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: gcc, libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]
Indeed ! Here is the right one.
On 03/10/2023 11:52, Jonathan Wakely wrote:
> On Mon, 2 Oct 2023 at 18:07, François Dumont <frs.dumont@gmail.com> wrote:
>> Hi
>>
>> Gentle reminder for this minor patch.
> It looks like you attached the wrong patch.
>
>
>> Thanks
>>
>> On 23/09/2023 22:10, François Dumont wrote:
>>> I'm eventually fixing those tests the same way we manage this problem
>>> in libstdc++ testsuite.
>>>
>>> testsuite: Add optional libstdc++ version namespace in expected
>>> diagnostic
>>>
>>> When libstdc++ is build with
>>> --enable-symvers=gnu-versioned-namespace diagnostics are
>>> showing this namespace, currently __8.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> *
>>> testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
>>> '__8' version namespace in expected diagnostic.
>>> *
>>> testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
>>> *
>>> testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
>>> *
>>> testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C:
>>> Likewise.
>>> * testsuite/g++.dg/coroutines/pr97438.C: Likewise.
>>> * testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
>>>
>>> Tested under Linux x86_64.
>>>
>>> I'm contributing to libstdc++ so I already have write access.
>>>
>>> Ok to commit ?
>>>
>>> François
[-- Attachment #2: coroutines_tests.patch --]
[-- Type: text/x-patch, Size: 4810 bytes --]
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
index 4706deebf4e..928e0c974e1 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
@@ -6,7 +6,7 @@
#include "coro1-allocators.h"
struct coro1
-f () /* { dg-error {'operator new' is provided by 'std::__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
+f () /* { dg-error {'operator new' is provided by 'std::(__8::)?__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
{
co_return;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
index 252cb5e442c..6bed524aa0a 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
@@ -6,7 +6,7 @@
#include "coro1-allocators.h"
struct coro1
-f () /* { dg-error {'operator delete' is provided by 'std::__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
+f () /* { dg-error {'operator delete' is provided by 'std::(__8::)?__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
{
co_return;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
index 89972b60945..0a545fed0e3 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
@@ -9,7 +9,7 @@
#include "coro1-allocators.h"
struct coro1
-f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */
+f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::(__8::)?__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */
{
co_return;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C
index 9fa3d64a9f2..b36e88f871a 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C
@@ -6,7 +6,7 @@
int used_grooaf = 0;
struct coro1
-f () noexcept // { dg-warning {'operator new' is marked 'throw\(\)' or 'noexcept' but no usable 'get_return_object_on_allocation_failure' is provided by 'std::__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\}} }
+f () noexcept // { dg-warning {'operator new' is marked 'throw\(\)' or 'noexcept' but no usable 'get_return_object_on_allocation_failure' is provided by 'std::(__8::)?__n4861::__coroutine_traits_impl<coro1, void>::promise_type' \{aka 'coro1::promise_type'\}} }
{
PRINT ("coro1: about to return");
co_return;
diff --git a/gcc/testsuite/g++.dg/coroutines/pr97438.C b/gcc/testsuite/g++.dg/coroutines/pr97438.C
index 95376648ed7..ac37118eae7 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr97438.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr97438.C
@@ -25,6 +25,6 @@ public:
}
dummy_coroutine
-foo() { // { dg-error {the coroutine promise type 'std::__n4861::coroutine_traits<dummy_coroutine>::promise_type' declares both 'return_value' and 'return_void'} }
+foo() { // { dg-error {the coroutine promise type 'std::(__8::)?__n4861::coroutine_traits<dummy_coroutine>::promise_type' declares both 'return_value' and 'return_void'} }
co_return 17;
}
diff --git a/gcc/testsuite/g++.dg/coroutines/ramp-return-b.C b/gcc/testsuite/g++.dg/coroutines/ramp-return-b.C
index d0e5d1f3c7f..fcca1d73a02 100644
--- a/gcc/testsuite/g++.dg/coroutines/ramp-return-b.C
+++ b/gcc/testsuite/g++.dg/coroutines/ramp-return-b.C
@@ -19,4 +19,4 @@ task<std::vector<int>>
baz ()
{
co_return std::vector<int>();
-} // { dg-error {use of deleted function 'task<T>::task\(const task<T>&\) \[with T = std::vector<int>\]'} }
+} // { dg-error {use of deleted function 'task<T>::task\(const task<T>&\) \[with T = std::(__8::)?vector<int>\]'} }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
2023-09-23 20:10 [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode François Dumont
2023-10-02 17:07 ` François Dumont
@ 2023-10-08 13:59 ` Iain Sandoe
2023-10-11 4:49 ` François Dumont
1 sibling, 1 reply; 8+ messages in thread
From: Iain Sandoe @ 2023-10-08 13:59 UTC (permalink / raw)
To: François Dumont; +Cc: GCC Development, libstdc++, GCC Patches
Hi François,
> On 23 Sep 2023, at 21:10, François Dumont <frs.dumont@gmail.com> wrote:
>
> I'm eventually fixing those tests the same way we manage this problem in libstdc++ testsuite.
>
> testsuite: Add optional libstdc++ version namespace in expected diagnostic
>
> When libstdc++ is build with --enable-symvers=gnu-versioned-namespace diagnostics are
> showing this namespace, currently __8.
>
> gcc/testsuite/ChangeLog:
>
> * testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
> '__8' version namespace in expected diagnostic.
> * testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
> * testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
> * testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C: Likewise.
> * testsuite/g++.dg/coroutines/pr97438.C: Likewise.
> * testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
>
> Tested under Linux x86_64.
>
> I'm contributing to libstdc++ so I already have write access.
>
> Ok to commit ?
As author of the tests, this LGTM as a suitable fix for now (at least, once the main
patch to fix versioned namespaces lands).
However, IMO, this could become quite painful as more g++ tests make use of std headers
(which is not really optional for facilities like this that are tightly-coupled between the FE and
the library).
For the future, it does seem that a more complete solution might be to introduce a
testsuite-wide definition for the C++ versioned std:: introducer, so that we can update it in one
place as the version changes.
So (as a thought experiment):
- we’d have something of the form “CXX_STD” as a tcl global
- we’d add the presence/absence of versioning to the relevant site.exp (which
means recognising the versioning choice also in the GCC configure)
- we’d migrate tests to using ${CXX_STD} instead of "std::__N” in matches
… I guess an alternative could be to cook up some alternate warning/error/etc
match functions that cater for arbitrary inline namespaces but that seems like a much
more tricky and invasive testsuite change.
thoughts?
Iain
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
2023-10-08 13:59 ` Iain Sandoe
@ 2023-10-11 4:49 ` François Dumont
2023-10-11 7:30 ` Iain Sandoe
0 siblings, 1 reply; 8+ messages in thread
From: François Dumont @ 2023-10-11 4:49 UTC (permalink / raw)
To: Iain Sandoe; +Cc: GCC Development, libstdc++, GCC Patches
On 08/10/2023 15:59, Iain Sandoe wrote:
> Hi François,
>
>> On 23 Sep 2023, at 21:10, François Dumont <frs.dumont@gmail.com> wrote:
>>
>> I'm eventually fixing those tests the same way we manage this problem in libstdc++ testsuite.
>>
>> testsuite: Add optional libstdc++ version namespace in expected diagnostic
>>
>> When libstdc++ is build with --enable-symvers=gnu-versioned-namespace diagnostics are
>> showing this namespace, currently __8.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
>> '__8' version namespace in expected diagnostic.
>> * testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
>> * testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
>> * testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C: Likewise.
>> * testsuite/g++.dg/coroutines/pr97438.C: Likewise.
>> * testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
>>
>> Tested under Linux x86_64.
>>
>> I'm contributing to libstdc++ so I already have write access.
>>
>> Ok to commit ?
> As author of the tests, this LGTM as a suitable fix for now (at least, once the main
> patch to fix versioned namespaces lands).
I just realized it was a "go", no ? Then why after the main patch ?
The "main patch" do not fix the versioned namespace. It just makes it
adopt the cxx11 abi.
This patch fixes a problem that is as old as the tests and that is
totally unrelated with the main one. I just wanted to improve the
situation so that versioned namespace mode do not look more buggy than
necessary when someone (like you) run those.
>
> However, IMO, this could become quite painful as more g++ tests make use of std headers
> (which is not really optional for facilities like this that are tightly-coupled between the FE and
> the library).
>
> For the future, it does seem that a more complete solution might be to introduce a
> testsuite-wide definition for the C++ versioned std:: introducer, so that we can update it in one
> place as the version changes.
>
> So (as a thought experiment):
> - we’d have something of the form “CXX_STD” as a tcl global
> - we’d add the presence/absence of versioning to the relevant site.exp (which
> means recognising the versioning choice also in the GCC configure)
> - we’d migrate tests to using ${CXX_STD} instead of "std::__N” in matches
>
> … I guess an alternative could be to cook up some alternate warning/error/etc
> match functions that cater for arbitrary inline namespaces but that seems like a much
> more tricky and invasive testsuite change.
>
> thoughts?
I considered amending gcc/testsuite/lib/prune.exp to simply remove the
version from the diagnostic. But the reply on why it was not working
scared me, so this patch.
https://gcc.gnu.org/pipermail/gcc/2023-September/242526.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
2023-10-11 4:49 ` François Dumont
@ 2023-10-11 7:30 ` Iain Sandoe
2023-10-11 17:22 ` François Dumont
0 siblings, 1 reply; 8+ messages in thread
From: Iain Sandoe @ 2023-10-11 7:30 UTC (permalink / raw)
To: François Dumont; +Cc: GCC Development, libstdc++, GCC Patches
Hi François,
> On 11 Oct 2023, at 05:49, François Dumont <frs.dumont@gmail.com> wrote:
> On 08/10/2023 15:59, Iain Sandoe wrote:
>>> On 23 Sep 2023, at 21:10, François Dumont <frs.dumont@gmail.com> wrote:
>>>
>>> I'm eventually fixing those tests the same way we manage this problem in libstdc++ testsuite.
>>>
>>> testsuite: Add optional libstdc++ version namespace in expected diagnostic
>>>
>>> When libstdc++ is build with --enable-symvers=gnu-versioned-namespace diagnostics are
>>> showing this namespace, currently __8.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
>>> '__8' version namespace in expected diagnostic.
>>> * testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
>>> * testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
>>> * testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C: Likewise.
>>> * testsuite/g++.dg/coroutines/pr97438.C: Likewise.
>>> * testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
>>>
>>> Tested under Linux x86_64.
>>>
>>> I'm contributing to libstdc++ so I already have write access.
>>>
>>> Ok to commit ?
>> As author of the tests, this LGTM as a suitable fix for now (at least, once the main
>> patch to fix versioned namespaces lands).
>
> I just realized it was a "go", no ? Then why after the main patch ?
>
> The "main patch" do not fix the versioned namespace. It just makes it adopt the cxx11 abi.
>
> This patch fixes a problem that is as old as the tests and that is totally unrelated with the main one. I just wanted to improve the situation so that versioned namespace mode do not look more buggy than necessary when someone (like you) run those.
Maybe a misunderstanding on my part. I was under the impression that versioned-namespace was currently unusable because it forces the old string ABI. If that is not the case, then I guess the changes are OK now.
I am pretty concerned about the maintainability of this tho, hence this …
>> However, IMO, this could become quite painful as more g++ tests make use of std headers
>> (which is not really optional for facilities like this that are tightly-coupled between the FE and
>> the library).
>>
>> For the future, it does seem that a more complete solution might be to introduce a
>> testsuite-wide definition for the C++ versioned std:: introducer, so that we can update it in one
>> place as the version changes.
>>
>> So (as a thought experiment):
>> - we’d have something of the form “CXX_STD” as a tcl global
>> - we’d add the presence/absence of versioning to the relevant site.exp (which
>> means recognising the versioning choice also in the GCC configure)
>> - we’d migrate tests to using ${CXX_STD} instead of "std::__N” in matches
>>
>> … I guess an alternative could be to cook up some alternate warning/error/etc
>> match functions that cater for arbitrary inline namespaces but that seems like a much
>> more tricky and invasive testsuite change.
>>
>> thoughts?
>
> I considered amending gcc/testsuite/lib/prune.exp to simply remove the version from the diagnostic. But the reply on why it was not working scared me, so this patch.
>
> https://gcc.gnu.org/pipermail/gcc/2023-September/242526.html
Ah, I didn’t see that mail - will try to take a look at the weekend.
Iain
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode
2023-10-11 7:30 ` Iain Sandoe
@ 2023-10-11 17:22 ` François Dumont
0 siblings, 0 replies; 8+ messages in thread
From: François Dumont @ 2023-10-11 17:22 UTC (permalink / raw)
To: Iain Sandoe; +Cc: GCC Development, libstdc++, GCC Patches
Hi Iain
On 11/10/2023 09:30, Iain Sandoe wrote:
> Hi François,
>
>> On 11 Oct 2023, at 05:49, François Dumont <frs.dumont@gmail.com> wrote:
>> On 08/10/2023 15:59, Iain Sandoe wrote:
>>>> On 23 Sep 2023, at 21:10, François Dumont <frs.dumont@gmail.com> wrote:
>>>>
>>>> I'm eventually fixing those tests the same way we manage this problem in libstdc++ testsuite.
>>>>
>>>> testsuite: Add optional libstdc++ version namespace in expected diagnostic
>>>>
>>>> When libstdc++ is build with --enable-symvers=gnu-versioned-namespace diagnostics are
>>>> showing this namespace, currently __8.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> * testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Add optional
>>>> '__8' version namespace in expected diagnostic.
>>>> * testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise.
>>>> * testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise.
>>>> * testsuite/g++.dg/coroutines/coro-bad-grooaf-01-grooaf-expected.C: Likewise.
>>>> * testsuite/g++.dg/coroutines/pr97438.C: Likewise.
>>>> * testsuite/g++.dg/coroutines/ramp-return-b.C: Likewise.
>>>>
>>>> Tested under Linux x86_64.
>>>>
>>>> I'm contributing to libstdc++ so I already have write access.
>>>>
>>>> Ok to commit ?
>>> As author of the tests, this LGTM as a suitable fix for now (at least, once the main
>>> patch to fix versioned namespaces lands).
>> I just realized it was a "go", no ? Then why after the main patch ?
>>
>> The "main patch" do not fix the versioned namespace. It just makes it adopt the cxx11 abi.
>>
>> This patch fixes a problem that is as old as the tests and that is totally unrelated with the main one. I just wanted to improve the situation so that versioned namespace mode do not look more buggy than necessary when someone (like you) run those.
> Maybe a misunderstanding on my part. I was under the impression that versioned-namespace was currently unusable because it forces the old string ABI. If that is not the case, then I guess the changes are OK now.
Said this way it sure makes this mode usability quite limited.
It's only functional to (almost) pass make check-c++ :-)
>
> I am pretty concerned about the maintainability of this tho, hence this …
>
>>> However, IMO, this could become quite painful as more g++ tests make use of std headers
>>> (which is not really optional for facilities like this that are tightly-coupled between the FE and
>>> the library).
>>>
>>> For the future, it does seem that a more complete solution might be to introduce a
>>> testsuite-wide definition for the C++ versioned std:: introducer, so that we can update it in one
>>> place as the version changes.
>>>
>>> So (as a thought experiment):
>>> - we’d have something of the form “CXX_STD” as a tcl global
>>> - we’d add the presence/absence of versioning to the relevant site.exp (which
>>> means recognising the versioning choice also in the GCC configure)
>>> - we’d migrate tests to using ${CXX_STD} instead of "std::__N” in matches
>>>
>>> … I guess an alternative could be to cook up some alternate warning/error/etc
>>> match functions that cater for arbitrary inline namespaces but that seems like a much
>>> more tricky and invasive testsuite change.
>>>
>>> thoughts?
>> I considered amending gcc/testsuite/lib/prune.exp to simply remove the version from the diagnostic. But the reply on why it was not working scared me, so this patch.
>>
>> https://gcc.gnu.org/pipermail/gcc/2023-September/242526.html
> Ah, I didn’t see that mail - will try to take a look at the weekend.
Ok, I'll instead chase for the patches on libstdc++ side then.
Moreover adopting cxx11 abi in versioned-namespace mode will imply a
version bump which would force to patch those files again if we do not
find another approach before.
Thanks,
François
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-11 17:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-23 20:10 [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode François Dumont
2023-10-02 17:07 ` François Dumont
2023-10-03 9:52 ` Jonathan Wakely
2023-10-03 20:14 ` François Dumont
2023-10-08 13:59 ` Iain Sandoe
2023-10-11 4:49 ` François Dumont
2023-10-11 7:30 ` Iain Sandoe
2023-10-11 17:22 ` François Dumont
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).