Re: [PATCH] Use cxx11 abi in versioned namespace

[Jonathan Wakely <jwakely@redhat.com>]

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.


> >             * 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.

> >             * 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
> >