From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by sourceware.org (Postfix) with ESMTPS id 0D17E3858C62; Mon, 16 Jan 2023 17:52:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0D17E3858C62 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-wr1-x435.google.com with SMTP id e3so19020969wru.13; Mon, 16 Jan 2023 09:52:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=efhXO52h+zcwfoYMS1lq5zaBJQqGHzf7eJEK/gExs4Q=; b=oCM1apnMRRde9z+x12A3sF2qHa21mRs1Lea5W8v9asNQuPcx8kHP1NuMc2joUAL+i7 zTV8HXpsr6w8LQppXRUv3zE+dE5/2o49yN3Pj9O7wnGpJID0/Ki6CtZZwflHH5mg6qEu 2Wqh79avl8lqVMVovURuHNdDnjLajjiIHyECiD/7bu+C+MuXYxXxnHNTGRotRKboIEZZ +GgnpbsZNNs2kNs2kRCQl0fUgvuKRUyNiHz5aUGW0Tw9OLZp5XGxwST8R3sT8WCc8iwK jyl95UmW35+sDJR+DaLjQ4BZwni/5ISPhyF1mRHIt0hDbpI02W1rGuBea9To+6TGTzHu N2JQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=efhXO52h+zcwfoYMS1lq5zaBJQqGHzf7eJEK/gExs4Q=; b=UflI1EaCoqrEaRBMUQGyAC3gOeHbity40qjB7Jy99w9RCjrW10GmTYmk5IqOlIX9jL 6rg8xBXqTV1kmLm4CpSzI1pxx46GGkwzkDzaVlunyA7x9DcBhxMdy21WnMPQMLCVd7nR mFn+v40DNQ3NIAmLQCWfEFNmcEjE5a8EGcYcvujgZa2PtgDu9KUv84HrsiHa+XZPsAjp 7Bec6mLxsasP+HfQxGh6gWCvbsNWs8AXkX3CM/s4Qjdz8O+OqJFeT4PEiVq7/Ry9drAD sve5Q37ADPwYwXTQBKZlJddMjgfesEB9Hs4hGAlaTOKq/7ozFeLNUs1lf/lSkl49tp+L kypQ== X-Gm-Message-State: AFqh2krWWx8hRkR/G7Jtiw+dxWylN5W1iQo27sEuAiDTyUrh/XzRDoeV uiecsrhsnIYG5VucBQ1ni28= X-Google-Smtp-Source: AMrXdXvXfl2C3wSoGc3C/7Xf8yQ0VAzdhXthU+RP880LRPIyXxOITLCgcYG3Swgf0F4XgFY3qi8M4w== X-Received: by 2002:adf:f10b:0:b0:2bd:e215:4372 with SMTP id r11-20020adff10b000000b002bde2154372mr287817wro.20.1673891547754; Mon, 16 Jan 2023 09:52:27 -0800 (PST) Received: from [10.5.1.164] ([109.190.253.14]) by smtp.googlemail.com with ESMTPSA id m5-20020a056000024500b00267bcb1bbe5sm27651903wrz.56.2023.01.16.09.52.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Jan 2023 09:52:27 -0800 (PST) Message-ID: <40b9a8bf-f075-8f79-5620-8c976b267690@gmail.com> Date: Mon, 16 Jan 2023 18:52:26 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH] Use cxx11 abi in versioned namespace Content-Language: fr, en-US To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <2d387cf7-8b99-3f04-6b99-d9dab5309693@gmail.com> <66ebb430-e280-3501-1f20-aab66f6f4d7b@gmail.com> <98eeb943-9f1d-72b6-4ce1-cb2a802550f7@gmail.com> From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,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: On 13/01/23 18:06, Jonathan Wakely wrote: > On Fri, 13 Jan 2023 at 16:33, Jonathan Wakely wrote: >> On Mon, 5 Dec 2022 at 21:14, François Dumont via Libstdc++ >> wrote: >>> I just rebased this patch. >>> >>> All good apart from the to_chars/from_chars symbols issue. >>> >>> François >>> >>> >>> On 11/10/22 19:28, François Dumont wrote: >>>> Hi >>>> >>>> Now that pretty printer is fixed (once patch validated) I'd like >>>> to propose this patch again. >>>> >>>> Note that I'am adding a check on pretty printer with a std::any on >>>> a std::wstring. I did so because of the FIXME in printers.py which is >>>> dealing with 'std::string' explicitely. Looks like in my case, where >>>> there is no 'std::string' but just a 'std::__8::string' we do not need >>>> the workaround. >>>> >>>> Once again I am attaching also the version namespace bump patch as >>>> I think that adopting the cxx11 abi in this mode is a good enough >>>> reason to bump it. If you agress let me know if I should squash the >>>> commits before pushing. >> Yes, I think this change would justify bumping the version. >> >>>> libstdc++: [_GLIBCXX_INLINE_VERSION] Use cxx11 abi >>>> >>>> Use cxx11 abi when activating versioned namespace mode. >>>> >>>> libstdcxx-v3/ChangeLog: >>>> >>>> * acinclude.m4 [GLIBCXX_ENABLE_LIBSTDCXX_DUAL_ABI]: >>>> Default to "new" libstdcxx abi. >>>> * config/locale/dragonfly/monetary_members.cc >>>> [!_GLIBCXX_USE_DUAL_ABI]: Define money_base >>>> members. >>>> * config/locale/generic/monetary_members.cc >>>> [!_GLIBCXX_USE_DUAL_ABI]: Likewise. >>>> * config/locale/gnu/monetary_members.cc >>>> [!_GLIBCXX_USE_DUAL_ABI]: Likewise. >>>> * config/locale/gnu/numeric_members.cc >>>> [!_GLIBCXX_USE_DUAL_ABI](__narrow_multibyte_chars): Define. >>>> * configure: Regenerate. >>>> * include/bits/c++config >>>> [_GLIBCXX_INLINE_VERSION](_GLIBCXX_NAMESPACE_CXX11, >>>> _GLIBCXX_BEGIN_NAMESPACE_CXX11): Define >>>> empty. >>>> [_GLIBCXX_INLINE_VERSION](_GLIBCXX_END_NAMESPACE_CXX11, >>>> _GLIBCXX_DEFAULT_ABI_TAG): Likewise. >>>> * python/libstdcxx/v6/printers.py >>>> (StdStringPrinter::__init__): Set self.new_string to True >>>> when std::__8::basic_string type is >>>> found. >>>> * src/Makefile.am >>>> [ENABLE_SYMVERS_GNU_NAMESPACE](ldbl_alt128_compat_sources): Define empty. >>>> * src/Makefile.in: Regenerate. >>>> * src/c++11/Makefile.am (cxx11_abi_sources): Rename into... >>>> (dual_abi_sources): ...this, new. Also move several >>>> sources to... >>>> (sources): ...this. >>>> (extra_string_inst_sources): Move several sources to... >>>> (inst_sources): ...this. >> I don't understand this part. Moving those files to sources and >> inst_sources will mean they are always compiled, right? But we don't >> want them compiled for --disable-libstdcxx-dual-abi >> >> In those files you've changed the #if conditions so they are empty if >> the dual ABI is disabled, but why do they need to be compiled at all? >> This isn't clear from the patch or the description or the changelog. >> >> >>>> * src/c++11/Makefile.in: Regenerate. >>>> * src/c++11/cow-fstream-inst.cc [_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions. >>>> * src/c++11/cow-locale_init.cc [_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions. >>>> * src/c++11/cow-sstream-inst.cc [_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions. >>>> * src/c++11/cow-stdexcept.cc >>>> [_GLIBCXX_USE_CXX11_ABI](error_category::_M_message): >>>> Skip definition. >>>> [_GLIBCXX_USE_CXX11_ABI]: Skip Transaction Memory TS >>>> definitions. >>>> * src/c++11/cow-string-inst.cc [_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions. >>>> * src/c++11/cow-string-io-inst.cc >>>> [_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>>> * src/c++11/cow-wstring-inst.cc [_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions. >>>> * src/c++11/cow-wstring-io-inst.cc >>>> [_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>>> * src/c++11/cxx11-hash_tr1.cc [!_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions. >>>> * src/c++11/cxx11-ios_failure.cc >>>> [!_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>>> [!_GLIBCXX_USE_DUAL_ABI] (__ios_failure): Remove. >> For this file I think your changes make sense, because the definitions >> of the gcc4-compatible and cxx11 ABI are different, we're not just >> compiling it twice. >> >> >>>> * src/c++11/cxx11-locale-inst.cc: Cleanup, just include >>>> locale-inst.cc. >>>> * src/c++11/cxx11-stdexcept.cc [!_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions. >>>> [!_GLIBCXX_USE_DUAL_ABI](__cow_string): Remove. >>>> * src/c++11/cxx11-wlocale-inst.cc >>>> [!_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>>> * src/c++11/fstream-inst.cc [!_GLIBCXX_USE_CXX11_ABI]: >>>> Skip definitions >>>> * src/c++11/locale-inst-numeric.h >>>> [!_GLIBCXX_USE_DUAL_ABI](std::use_facet>, >>>> std::use_facet>): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](std::has_facet>, >>>> std::has_facet>): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](std::num_get>>> istreambuf_iterator>): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](std::num_put>>> ostreambuf_iterator>): Instantiate. >>>> * src/c++11/locale-inst.cc [!_GLIBCXX_USE_DUAL_ABI]: Build >>>> only when configured >>>> _GLIBCXX_USE_CXX11_ABI is equal to currently built abi. >>>> [!_GLIBCXX_USE_DUAL_ABI](__moneypunct_cache): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__moneypunct_cache): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__numpunct_cache): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__timepunct): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__timepunct_cache): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](time_put>>> ostreambuf_iterator>): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](time_put_byname>>> ostreambuf_iterator>): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__ctype_abstract_base): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](ctype_byname): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__codecvt_abstract_base>>> mbstate_t>): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](codecvt_byname>>> mbstate_t>): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](use_facet>(const locale&)): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](use_facet>>> mbstate_t>>(const locale&)): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](use_facet<__timepunct>(const locale&)): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](use_facet>(const locale&)): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](has_facet>(const locale&)): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](has_facet>>> mbstate_t>>(const locale&)): Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](has_facet<__timepunct>(const locale&)): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](has_facet>(const locale&)): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__add_grouping): Define. >>>> [!_GLIBCXX_USE_DUAL_ABI](__pad>): >>>> Instantiate. >>>> [!_GLIBCXX_USE_DUAL_ABI](__int_to_char(C*, unsigned long, >>>> const C*, ios_base::fmtflags, bool)): >>>> Define. >>>> [!_GLIBCXX_USE_DUAL_ABI](__int_to_char(C*, unsigned long >>>> long, const C*, ios_base::fmtflags, bool)): >>>> Define. >>>> * src/c++11/cxx11-wlocale-inst.cc >>>> [!_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>>> * src/c++98/Makefile.am (cxx11_abi_sources): Remove, >>>> unique cow-istream-string.cc entry move to... >>>> (inst_sources): ...this. >> Why? > I know all this ENABLE_DUAL_ABI stuff is a horrible mess, and I should > really try to clean it up, and it's impressive you made this work at > all. But I think the solution you've found makes the mess even worse, > unfortunately. > > Another problem is here: > > --- a/libstdc++-v3/src/c++98/stdexcept.cc > +++ b/libstdc++-v3/src/c++98/stdexcept.cc > @@ -26,7 +26,7 @@ > // ISO C++ 14882: 19.1 Exception classes > // > > -// All exception classes still use the classic COW std::string. > +// In dual abi all exception classes still use the classic COW std::string. > #define _GLIBCXX_USE_CXX11_ABI 0 > > Exceptions must have a non-throwing copy constructor, which is why > they still need to use a reference-counted string. If we disable the > dual ABI and only build the cxx11 string, we need to figure out a way > to compile enough of the COW string to keep exceptions working. > Ouch, I did not know. Maybe there should be a static_assert about this constraint, you would have saved a patch review. This is going to give me a big headache to find a solution to this problem !