[Bug libstdc++/108323] combine does not change the locale name

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108323

--- Comment #4 from GCC Commits  ---
The master branch has been updated by Jonathan Wakely :

https://gcc.gnu.org/g:947a9c801e818f412ad4c669a49297c3512b3a6e

commit r15-781-g947a9c801e818f412ad4c669a49297c3512b3a6e
Author: Jonathan Wakely 
Date:   Tue Jan 30 14:48:28 2024 +

libstdc++: Fix effects of combining locales [PR108323]

This fixes a bug in locale::combine where we fail to meet the standard's
requirement that the result is unnamed. It also implements two library
issues related to the names of combined locales (2295 and 3676).

libstdc++-v3/ChangeLog:

PR libstdc++/108323
* include/bits/locale_classes.tcc (locale(const locale&, Facet*)):
Return a copy of the first argument when the facet pointer is
null, as per LWG 2295.
(locale::combine): Ensure the result is unnamed.
* src/c++11/localename.cc (_M_replace_categories): Ignore
whether the second locale has a name when cat == none, as per
LWG 3676.
* src/c++98/locale.cc (_M_install_facet): Use __builtin_expect
to predict that the facet pointer is non-null.
* testsuite/22_locale/locale/cons/names.cc: New test.

[Bug libstdc++/108323] combine does not change the locale name

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108323

--- Comment #3 from Jonathan Wakely  ---
Also, locale::combine (and has_facet and use_facet, and locale::global and
locale::classic) should use [[nodiscard]].

[Bug libstdc++/108323] combine does not change the locale name

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108323

Jonathan Wakely  changed:

   What|Removed |Added

   Last reconfirmed|2024-01-30 00:00:00 |2024-2-1

--- Comment #2 from Jonathan Wakely  ---
Kinda related, the implementation of locale::combine uses _M_replace_facet
which seems to have a bug:

  void
  locale::_Impl::
  _M_replace_facet(const _Impl* __imp, const locale::id* __idp)
  {
size_t __index = __idp->_M_id();
if ((__index > (__imp->_M_facets_size - 1))
|| !__imp->_M_facets[__index])
  __throw_runtime_error(__N("locale::_Impl::_M_replace_facet"));
_M_install_facet(__idp, __imp->_M_facets[__index]);
  }

This only checks that a facet with the specified id is present. This is not
sufficient, because it could be a different derived type that has the same id.

e.g.

struct custom_ctype : ctype { ... };
auto loc = std::locale().combine(std::locale());

This should throw std::runtime_error because has_facet is false.

For combine, _M_replace_facet should use std::has_facet as specified in the
standard. I haven't checked if that would be correct for the other uses of
_M_replace_facet.

[Bug libstdc++/108323] combine does not change the locale name

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108323

Jonathan Wakely  changed:

   What|Removed |Added

   Last reconfirmed|2023-01-07 00:00:00 |2024-1-30

--- Comment #1 from Jonathan Wakely  ---
This should be fixed at the same time as
https://cplusplus.github.io/LWG/issue2295

And we should also add a static_assert(__is_facet<_Facet>::value, "") for
https://cplusplus.github.io/LWG/issue436

[Bug libstdc++/108323] combine does not change the locale name

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108323

Jonathan Wakely  changed:

   What|Removed |Added

 Ever confirmed|0   |1
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2023-01-07