Re: [PATCH] Use cxx11 abi in versioned namespace

[Jonathan Wakely <jwakely@redhat.com>]

On Mon, 16 Jan 2023 at 17:47, François Dumont <frs.dumont@gmail.com> wrote:
>
> On 13/01/23 17:33, Jonathan Wakely wrote:
> > On Mon, 5 Dec 2022 at 21:14, François Dumont via Libstdc++
> > <libstdc++@gcc.gnu.org>  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.
>
> --disable-libstdcxx-dual-abi means use cow string implementation. With
> this patch I am introducing somehow another disable-dual-abi mode but
> this time to use cxx11 string implementation.

Oh right, I misremembered. I thought you could choose the ABI when
disabling dual-abi.



>
> At Makefile.am level it was difficult to match both cases so I'm doing
> it at compilation time with most of time the condition:
>
> #if ! _GLIBCXX_USE_CXX11_ABI || ! _GLIBCXX_USE_DUAL_ABI
>
> I'll make it clearer at ChangeLog level.
>
> >>>              * 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<num_get<>>,
> >>> std::use_facet<num_put<>>): Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](std::has_facet<num_get<>>,
> >>> std::has_facet<num_put<>>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](std::num_get<C,
> >>> istreambuf_iterator<C>>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](std::num_put<C,
> >>> ostreambuf_iterator<C>>): 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<C, false>):
> >>> Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](__moneypunct_cache<C, true>):
> >>> Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](__numpunct_cache<C>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](__timepunct<C>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](__timepunct_cache<C>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](time_put<C,
> >>> ostreambuf_iterator<C>>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](time_put_byname<C,
> >>> ostreambuf_iterator<C>>): Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](__ctype_abstract_base<C>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](ctype_byname<C>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](__codecvt_abstract_base<C, char,
> >>> mbstate_t>): Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](codecvt_byname<C, char,
> >>> mbstate_t>): Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](use_facet<ctype<C>>(const locale&)):
> >>> Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](use_facet<codecvt<C, char,
> >>> mbstate_t>>(const locale&)): Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](use_facet<__timepunct<C>>(const locale&)):
> >>> Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](use_facet<time_put<C>>(const locale&)):
> >>> Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](has_facet<ctype<C>>(const locale&)):
> >>> Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](has_facet<codecvt<C, char,
> >>> mbstate_t>>(const locale&)): Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](has_facet<__timepunct<C>>(const locale&)):
> >>> Instantiate.
> >>> [!_GLIBCXX_USE_DUAL_ABI](has_facet<time_put<C>>(const locale&)):
> >>> Instantiate.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](__add_grouping<C>): Define.
> >>>              [!_GLIBCXX_USE_DUAL_ABI](__pad<C, char_traits<C>>):
> >>> 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?
> >
> >>>              * src/c++98/Makefile.in: Regenerate.
> >>>              * src/c++98/cow-istream-string.cc: Include
> >>> <bits/c++config.h>.
> >>>              [_GLIBCXX_USE_CXX11_ABI]: Skip definitions.
> > This file doesn't need to be compiled when the dual abi is disabled.
>
> It depends if it is disabled because of --disable-libstdcxx-dual-abi or
> because of --enable-symvers=gnu-versioned-namespace.
>
>
> >>>              * 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.
> >>>              * testsuite/libstdc++-prettyprinters/cxx17.cc: Add
> >>> std::any check with std::wstring.
> >>>
> >>> François
> >>>
> >>> On 30/05/22 22:58, François Dumont wrote:
> >>>> Hi
> >>>>
> >>>> Here is the patch to use cxx11 abi for the versioned namespace mode.
> >>>>
> >>>> It is still suffering from a side effect on gdb pretty printers with
> >>>> 3 errors like:
> >>>>
> >>>> got: type =
> >>>> std::unique_ptr<std::vector<std::unique_ptr<std::list<std::__8::basic_string<char,
> >>>> std::__8::char_traits<char>, std::__8::allocator<char> >>[]>>[99]>
> >>>> FAIL: libstdc++-prettyprinters/80276.cc whatis p4
> >>>>
> >>>> However once I apply the version bump errors are gone.
> >>>>
> >>>> Let me know if it is ok to apply all at once or if you prefer to
> >>>> avoid the version bump.
> >>>>
> >>>> François
>
>