On 12/05/21 18:51 +0300, Antony Polukhin via Libstdc++ wrote:
ср, 12 мая 2021 г. в 18:38, Antony Polukhin <antosh...@gmail.com>:ср, 12 мая 2021 г. в 17:44, Jonathan Wakely <jwak...@redhat.com>: > > On 12/05/21 12:58 +0300, Antony Polukhin wrote: > >ср, 12 мая 2021 г. в 12:18, Jonathan Wakely <jwak...@redhat.com>: > ><...> > >> Or just leave it undefined, as libc++ seems to do according to your > >> comment in PR 89728: > >> > >> error: implicit instantiation of undefined template 'std::__1::ctype<std::__1::basic_string<char> >' > >> > >> Was your aim to have a static_assert that gives a more descriptive > >> error? We could leave it undefined in C++98 and have the static assert > >> for C++11 and up. > > > >Leaving it undefined would be the best. It would allow SFINAE on ctype > >and a compile time error is informative enough. > > > >However, there may be users who instantiate ctype<ThierChar> in a > >shared library without ctype<ThierChar> template specializations in > >the main executable. Making the default ctype undefined would break > >their compilation: > > > >#include <locale> > >// no ctype<ThierChar> specialization > >c = std::tolower(ThierChar{42}, locale_from_shared_library()); // OK > >right now in libstdc++, fails on libc++ > > What I meant was leaving the partial specialization undefined, not the > primary template, i.e. > > --- a/libstdc++-v3/include/bits/locale_facets.h > +++ b/libstdc++-v3/include/bits/locale_facets.h > @@ -1476,6 +1476,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > }; > #endif //_GLIBCXX_USE_WCHAR_T > > + template<typename _CharT, typename _Traits, typename _Alloc> > + class ctype<basic_string<_CharT, _Traits, _Alloc> >; > + > /// class ctype_byname [22.2.1.2]. > template<typename _CharT> > class ctype_byname : public ctype<_CharT> > > This makes your test fail with errors like this: > > In file included from /home/jwakely/gcc/12/include/c++/12.0.0/locale:40, > from loc.C:1: > /home/jwakely/gcc/12/include/c++/12.0.0/bits/locale_facets.h: In instantiation of 'bool std::isspace(_CharT, const std::locale&) [with _CharT = std::__cxx11::basic_string<char>]': > loc.C:16:15: required from here > /home/jwakely/gcc/12/include/c++/12.0.0/bits/locale_facets.h:2600:47: error: invalid use of incomplete type 'const class std::ctype<std::__cxx11::basic_string<char> >' > 2600 | { return use_facet<ctype<_CharT> >(__loc).is(ctype_base::space, __c); } > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ > > But it shouldn't affect the uses of ctype<TheirChar>. > > What do you think? Good idea. That way the compiler message points directly to the misused function. Patch is in attachmentReplaced {} with () in test to be C++98 compatible
Looks great, thanks. I'll test and commit this tomorrow. (And I haven't forgotten about your other patch, I'm still trying to convince myself it won't cause problems for code which is technically UB but "works" today).
