From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by sourceware.org (Postfix) with ESMTPS id 8A73A3858D1E; Wed, 11 Oct 2023 17:22:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8A73A3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-x32e.google.com with SMTP id 5b1f17b1804b1-4065dea9a33so1475485e9.3; Wed, 11 Oct 2023 10:22:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1697044950; x=1697649750; darn=gcc.gnu.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=yTeduO0vgTnrMAnTc4bn/ny455PDtk9b/3L6u2e3aro=; b=TE8j6PIOATbi03FVdBbIZ2CVFd2Yz7Q550P65qNaseH3xnBhxmozsVUwl0XjSdiunF cHyhBh8SKMCGxO3pZEe9XCWjFBnzSAgaGfZCJhpOBAV6DTVJWxf6ELhbAaOGGmN35x0U 6OaId8ekR6BI2UGIZXbtkrMDRQ+8zdYKyZrId5dQd4iMQgoV20Gv3Cz+TTsG0zDRstW0 z0cvSRuEOhiLa2i4KEF9givDr6CVXVP+4pBPVmZhzNykEcFb/Gmam0+OYvEcBh64KF8C QFyukgvTA+ConJPBGbs1TARlLs2U1RMCxOctLuaibmGD6aSzc/IMsTbjJKCJb1BZuaYT O02g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697044950; x=1697649750; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=yTeduO0vgTnrMAnTc4bn/ny455PDtk9b/3L6u2e3aro=; b=aqImO1HjPfnFYS/KcDcljJ6sni6T9nmpWgGg1VxyI6gNolGiIZEJLHlw/P9eYZ8zU1 nW9jFVqZOoiz0wdaUNw7CbO43EzKE8UfX7AInFxq3vJ/OokhOqYkna7UZlodQGsecUE0 WQnkw8mFymv3sINYBLFe1lUjBk2w2P0EvuL46rXjGAMet1i7s4MoRWZdjQ3efv2uizC7 tfxr3a7lsIyJjqWvN3kNUEqabtVgmqwv0RsJijtTTfJnHZdXvJ+8D0Do41nDrRQwOUWv ljGhvFjAnONVc1sE9fQ+20hqaMrM8QuzZ+aKUuQwiTK1jf2HZILo1cd7lg16NNno3Km1 74HQ== X-Gm-Message-State: AOJu0Yyl2PtuzTc9nO1OuJS5y50OLsh4ppSb1qrqV5FlOn3KLVR2lBwO TSlB0kQTzdQbtdp9yQHZDvemaxidJLXlkw== X-Google-Smtp-Source: AGHT+IEpfRZUGdii1HIINzFX00DTbijlpC8vfIGwQw+Ua1N4gvMMJa46U5iIU02VWx02tOjPWJtk2g== X-Received: by 2002:a5d:68c1:0:b0:31f:f9de:6a4e with SMTP id p1-20020a5d68c1000000b0031ff9de6a4emr18538952wrw.69.1697044949670; Wed, 11 Oct 2023 10:22:29 -0700 (PDT) Received: from [10.78.2.85] ([89.207.171.155]) by smtp.gmail.com with ESMTPSA id k1-20020adff5c1000000b003258934a4bcsm15985882wrp.42.2023.10.11.10.22.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 11 Oct 2023 10:22:29 -0700 (PDT) Message-ID: <351f61e0-c1c1-4c5b-beb5-8447b01b55ea@gmail.com> Date: Wed, 11 Oct 2023 19:22:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Fix coroutine tests for libstdc++ gnu-version-namespace mode To: Iain Sandoe Cc: GCC Development , libstdc++ , GCC Patches References: <4ebb6936-d652-84a5-0028-6ca0a5d2d238@gmail.com> <324ED72B-E687-4B5E-AD6E-C7A3BBDEB223@sandoe.co.uk> <35AC1729-7D3F-4DEB-A940-A7BCF0B0E2E0@sandoe.co.uk> Content-Language: en-US From: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= In-Reply-To: <35AC1729-7D3F-4DEB-A940-A7BCF0B0E2E0@sandoe.co.uk> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham 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 Iain On 11/10/2023 09:30, Iain Sandoe wrote: > Hi François, > >> On 11 Oct 2023, at 05:49, François Dumont wrote: >> On 08/10/2023 15:59, Iain Sandoe wrote: >>>> On 23 Sep 2023, at 21: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 ? >>> 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