From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by sourceware.org (Postfix) with ESMTPS id 8F9333858C50; Tue, 24 Oct 2023 04:55:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8F9333858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8F9333858C50 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::331 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698123315; cv=none; b=p0DkQOvvLLOl+LhHOCArBQy3FnTmdqrUtniNxdkwiK2yONzpDuMBTKWaHm4gGb+fus65LYXIINOz4kodGb+BZ2W6npwfsG3/MvlEqDE7qAMZQ8HMW4xWo3SOC08DFS9wiwxOg89P8yqj5PnqefJYlDpFmBS8wNfKS1SsDlivnz8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698123315; c=relaxed/simple; bh=RSV2fe56X2oIXwnT33B4RuXKUiA1XU/9XfvN+cjKhhw=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=c4zUWo4q9lChq6tn00ofe22/JWLhvhwokzPsDz9rqGxw3tr+nnNIuueCAe+61hHgilhlAOi/9Yumu0Nh0xnB+aj3uEgeRlxnQaYboYXFHP4NgeKRqIrmmq+Sg9Qz8HlruENj8MVpK+hkA6Iq0GgKTtNj+GUum+kOkXGk9MB2GPg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-x331.google.com with SMTP id 5b1f17b1804b1-40806e4106dso23036185e9.1; Mon, 23 Oct 2023 21:55:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1698123310; x=1698728110; darn=gcc.gnu.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:from:subject:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=RHA3cQ/12QSI/U9ZiRxI6QU5BleDGdSGUWPuqrNu1Gw=; b=McX7QZahmNZ7/KasEiT0U8mc514YwHf0it4Z00g4v5+tlQBkdEGDHPiFtRS5nhL82b VqsDwax3AzWHbDyvMUFH9QYg3J5/J8uhbM6qSx7zBsyPsb/j1g1bh7xqeg8ztXX+zjpd gYuon7Bw3GUttT6oobNaNT/UDUWhsfPfAqUOW9w99fGLWhPV2AtOG+RlaBJU7nY2z0Un gyPL1V189Uk6Qw4kmVOl9vccKanOEDuOIt/Q/KewEtnwV6HFUyR1sUj3Nw7sM1NuySZS Hh3KMwDXATmsh9HGO0MRsQ3nD/xtDhQ1V7nH3Q5zz7WTQm2LIRZYCWwuq/Go+SoUtAuo 77XA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698123310; x=1698728110; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:from:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=RHA3cQ/12QSI/U9ZiRxI6QU5BleDGdSGUWPuqrNu1Gw=; b=Cx8J6pEs/VVQ1LslHedOf5pGKClNHKoUrkFs2rOlThzPs+GpwdUmV71QgLzg7UQMBo y29KdgEf07zEEYNJ/aAGd3kme6Hr+pfz+u7NqFcLWcyL6/bcymy3uMdC0fOnj1/X74l4 psaG/mPw/GQPf1Y23Xn3ir3CScCJgyIylgkqxjSo6ZPIhixKCNxJu6L7RCI+gtXrFTXd udSPDXoL3XwMi6pjXAS0GePAcVyD9Uj7vVnK5jv+IYab6hHnIvDdLr1EkT94rQSkiunB p999Vti0gIKcry+0Y75mxvIgGd0Y/XlF5+hJub9gXhjIYJX81SNmJ50+cPlOWiRDWD4F pJXw== X-Gm-Message-State: AOJu0YyRIIzEVz3JWW7lcS4x5uzLdy9HIhq0mtcStuWXko1p15IsRz/S zM55ReAAkK1Ob1X3//c4UobyJ0pEAFU= X-Google-Smtp-Source: AGHT+IGQ+xn1Df9RTG5qYhrKO+CRIHH0y4oJEM5DXAbwfOeyOvd2Ewlb2YjeFCSzuwlsv0ppSXlnzQ== X-Received: by 2002:a05:600c:1d26:b0:405:3924:3cad with SMTP id l38-20020a05600c1d2600b0040539243cadmr10630807wms.15.1698123309747; Mon, 23 Oct 2023 21:55:09 -0700 (PDT) Received: from [10.127.0.57] ([89.207.171.167]) by smtp.gmail.com with ESMTPSA id r4-20020a05600c458400b00405391f485fsm10826343wmo.41.2023.10.23.21.55.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 Oct 2023 21:55:09 -0700 (PDT) Message-ID: Date: Tue, 24 Oct 2023 06:55:07 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] sso-string@gnu-versioned-namespace [PR83077] From: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= To: libstdc++ Cc: gcc-patches References: <1dc681f4-41b7-d171-02ac-b0194617bdee@gmail.com> <91dc6383-6bff-ce6c-b24d-81cd2ab2dce8@gmail.com> <5306fc1f-603b-43ef-beec-420cbe0ebfca@gmail.com> Content-Language: en-US In-Reply-To: <5306fc1f-603b-43ef-beec-420cbe0ebfca@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,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 Still no one to complete this review ? Thanks On 07/10/2023 21:32, François Dumont wrote: > I've been told that previous patch generated with 'git diff -b' was > not applying properly so here is the same patch again with a simple > 'git diff'. > > > On 07/10/2023 14:25, François Dumont wrote: >> Hi >> >> Here is a rebased version of this patch. >> >> There are few test failures when running 'make check-c++' but nothing >> new. >> >> Still, there are 2 patches awaiting validation to fix some of them, >> PR c++/111524 to fix another bunch and I fear that we will have to >> live with the others. >> >>     libstdc++: [_GLIBCXX_INLINE_VERSION] Use cxx11 abi [PR83077] >> >>     Use cxx11 abi when activating versioned namespace mode. To do >> support >>     a new configuration mode where !_GLIBCXX_USE_DUAL_ABI and >> _GLIBCXX_USE_CXX11_ABI. >> >>     The main change is that std::__cow_string is now defined whenever >> _GLIBCXX_USE_DUAL_ABI >>     or _GLIBCXX_USE_CXX11_ABI is true. Implementation is using >> available std::string in >>     case of dual abi and a subset of it when it's not. >> >>     On the other side std::__sso_string is defined only when >> _GLIBCXX_USE_DUAL_ABI is true >>     and _GLIBCXX_USE_CXX11_ABI is false. Meaning that >> std::__sso_string is a typedef for the >>     cow std::string implementation when dual abi is disabled and cow >> string is being used. >> >>     libstdcxx-v3/ChangeLog: >> >>             PR libstdc++/83077 >>             * acinclude.m4 [GLIBCXX_ENABLE_LIBSTDCXX_DUAL_ABI]: >> Default to "new" libstdcxx abi >>             when enable_symvers is gnu-versioned-namespace. >>             * 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. >>             * include/bits/cow_string.h [!_GLIBCXX_USE_CXX11_ABI]: >> Define a light version of COW >>             basic_string as __std_cow_string for use in stdexcept. >>             * include/std/stdexcept [_GLIBCXX_USE_CXX11_ABI]: Define >> __cow_string. >>             (__cow_string(const char*)): New. >>             (__cow_string::c_str()): New. >>             * 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. Also move cow-local_init.cc, >> cxx11-hash_tr1.cc, >>             cxx11-ios_failure.cc entries to... >>             (sources): ...this. >>             (extra_string_inst_sources): Move cow-fstream-inst.cc, >> cow-sstream-inst.cc, cow-string-inst.cc, >>             cow-string-io-inst.cc, cow-wtring-inst.cc, >> cow-wstring-io-inst.cc, cxx11-locale-inst.cc, >>             cxx11-wlocale-inst.cc entries to... >>             (inst_sources): ...this. >>             * 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]: >> Include . >>             [_GLIBCXX_USE_DUAL_ABI || >> _GLIBCXX_USE_CXX11_ABI](__cow_string): Redefine before >>             including . Define >> _GLIBCXX_DEFINE_STDEXCEPT_INSTANTIATIONS so that >>             __cow_string definition in is skipped. >>             [_GLIBCXX_USE_CXX11_ABI]: Skip Transaction Memory TS >> definitions. >>             * src/c++11/string-inst.cc: Add sizeof/alignof >> static_assert on stdexcept >>             __cow_string definition. >>             (_GLIBCXX_DEFINING_CXX11_ABI_INSTANTIATIONS): Define >> following _GLIBCXX_USE_CXX11_ABI >>             value. >>             [_GLIBCXX_USE_CXX11_ABI && >> !_GLIBCXX_DEFINING_CXX11_ABI_INSTANTIATIONS]: >>             Define _GLIBCXX_DEFINING_COW_STRING_INSTANTIATIONS. >> Include . >>             Define basic_string as __std_cow_string for the current >> translation unit. >>             * 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. >>             * 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. >>             * src/c++11/cxx11-wlocale-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/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. >>             * src/c++98/Makefile.in: Regenerate. >>             * src/c++98/cow-istream-string.cc: Include >> . >>             [_GLIBCXX_USE_CXX11_ABI]: Skip definitions. >>             * src/c++98/hash_tr1.cc [_GLIBCXX_USE_CXX11_ABI]: Skip >> definitions. >>             * src/c++98/ios_failure.cc >> [_GLIBCXX_USE_CXX11_ABI][_GLIBCXX_USE_DUAL_ABI]: Skip definitions. >>             * src/c++98/istream-string.cc [!_GLIBCXX_USE_DUAL_ABI]: >> Build only when configured >>             _GLIBCXX_USE_CXX11_ABI is equal to currently built abi. >>             * src/c++98/locale_facets.cc >> [_GLIBCXX_USE_CXX11_ABI](__verify_grouping): Remove. >>             * src/c++98/stdexcept.cc >>             [_GLIBCXX_USE_CXX11_ABI](logic_error(const string&): Remove. >>             [_GLIBCXX_USE_CXX11_ABI](domain_error(const string&): >> Remove. >>             [_GLIBCXX_USE_CXX11_ABI](invalid_argument(const string&): >> Remove. >>             [_GLIBCXX_USE_CXX11_ABI](length_error(const string&): >> Remove. >>             [_GLIBCXX_USE_CXX11_ABI](out_of_range(const string&): >> Remove. >>             [_GLIBCXX_USE_CXX11_ABI](runtime_error(const string&): >> Remove. >>             [_GLIBCXX_USE_CXX11_ABI](range_error(const string&): Remove. >>             [_GLIBCXX_USE_CXX11_ABI](overflow_error(const string&): >> Remove. >>             [_GLIBCXX_USE_CXX11_ABI](underflow_error(const string&): >> Remove. >>             * src/c++98/compatibility.cc [_GLIBCXX_USE_CXX11_ABI]: >> Skip all definitions appart from >>             istream::ignore(streamsize). >> >> Tested under Linux x64_86, ok to commit ? >> >> François >> >> >> On 17/08/2023 19:22, Jonathan Wakely wrote: >>> On Sun, 13 Aug 2023 at 14:27, François Dumont via Libstdc++ >>> wrote: >>>> Here is the fixed patch tested in all 3 modes: >>>> >>>> - _GLIBCXX_USE_DUAL_ABI >>>> >>>> - !_GLIBCXX_USE_DUAL_ABI && !_GLIBCXX_USE_CXX11_ABI >>>> >>>> - !_GLIBCXX_USE_DUAL_ABI && _GLIBCXX_USE_CXX11_ABI >>>> >>>> I don't know what you have in mind for the change below but I >>>> wanted to >>>> let you know that I tried to put COW std::basic_string into a nested >>>> __cow namespace when _GLIBCXX_USE_CXX11_ABI. But it had more impact on >>>> string-inst.cc so I preferred the macro substitution approach. >>> I was thinking of implementing the necessary special members functions >>> of __cow_string directly, so they are ABI compatible with the COW >>> std::basic_string but don't actually reuse the code. That would mean >>> we don't need to compile and instantiate the whole COW string just to >>> use a few members from it. But that can be done later, the macro >>> approach seems OK for now. >>> >>>> There are some test failing when !_GLIBCXX_USE_CXX11_ABI that are >>>> unrelated with my changes. I'll propose fixes in coming days. >>> Which tests? I run the entire testsuite with >>> -D_GLIBCXX_USE_CXX11_ABI=0 several times per day and I'm not seeing >>> failures. >>> >>> I'll review the patch ASAP, thanks for working on it. >>>