From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp001-out.apm-internet.net (smtp001-out.apm-internet.net [85.119.248.222]) by sourceware.org (Postfix) with ESMTPS id 70B333858C01 for ; Wed, 11 Oct 2023 07:30:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 70B333858C01 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sandoe.co.uk Received: (qmail 64349 invoked from network); 11 Oct 2023 07:30:23 -0000 X-APM-Out-ID: 16970094236434 X-APM-Authkey: 257869/1(257869/1) 4 Received: from unknown (HELO smtpclient.apple) (81.138.1.83) by smtp001.apm-internet.net with SMTP; 11 Oct 2023 07:30:23 -0000 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.4\)) Subject: Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode From: Iain Sandoe In-Reply-To: Date: Wed, 11 Oct 2023 08:30:23 +0100 Cc: GCC Development , libstdc++ , GCC Patches Content-Transfer-Encoding: quoted-printable Message-Id: <35AC1729-7D3F-4DEB-A940-A7BCF0B0E2E0@sandoe.co.uk> References: <4ebb6936-d652-84a5-0028-6ca0a5d2d238@gmail.com> <324ED72B-E687-4B5E-AD6E-C7A3BBDEB223@sandoe.co.uk> To: =?utf-8?Q?Fran=C3=A7ois_Dumont?= X-Mailer: Apple Mail (2.3696.120.41.1.4) X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,KAM_COUK,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Fran=C3=A7ois, > On 11 Oct 2023, at 05:49, Fran=C3=A7ois Dumont = wrote: > On 08/10/2023 15:59, Iain Sandoe wrote: >>> On 23 Sep 2023, at 21:10, Fran=C3=A7ois Dumont = wrote: >>>=20 >>> I'm eventually fixing those tests the same way we manage this = problem in libstdc++ testsuite. >>>=20 >>> testsuite: Add optional libstdc++ version namespace in expected = diagnostic >>>=20 >>> When libstdc++ is build with = --enable-symvers=3Dgnu-versioned-namespace diagnostics are >>> showing this namespace, currently __8. >>>=20 >>> gcc/testsuite/ChangeLog: >>>=20 >>> * = 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. >>>=20 >>> Tested under Linux x86_64. >>>=20 >>> I'm contributing to libstdc++ so I already have write access. >>>=20 >>> 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). >=20 > I just realized it was a "go", no ? Then why after the main patch ? >=20 > The "main patch" do not fix the versioned namespace. It just makes it = adopt the cxx11 abi. >=20 > 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 = =E2=80=A6 >> 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). >>=20 >> 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. >>=20 >> So (as a thought experiment): >> - we=E2=80=99d have something of the form =E2=80=9CCXX_STD=E2=80=9D = as a tcl global >> - we=E2=80=99d add the presence/absence of versioning to the = relevant site.exp (which >> means recognising the versioning choice also in the GCC configure) >> - we=E2=80=99d migrate tests to using ${CXX_STD} instead of = "std::__N=E2=80=9D in matches >>=20 >> =E2=80=A6 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. >>=20 >> thoughts? >=20 > 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. >=20 > https://gcc.gnu.org/pipermail/gcc/2023-September/242526.html Ah, I didn=E2=80=99t see that mail - will try to take a look at the = weekend. Iain