On 17/05/21 18:14 +0100, Jonathan Wakely wrote:
>On 12/05/21 17:16 +0100, Jonathan Wakely wrote:
>>On 12/05/21 18:51 +0300, Antony Polukhin via Libstdc++ wrote:
>>>ср, 12 мая 2021 г. в 18:38, Antony Polukhin <antoshkka@gmail.com>:
>>>>
>>>>ср, 12 мая 2021 г. в 17:44, Jonathan Wakely <jwakely@redhat.com>:
>>>>>
>>>>>On 12/05/21 12:58 +0300, Antony Polukhin wrote:
>>>>>>ср, 12 мая 2021 г. в 12:18, Jonathan Wakely <jwakely@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 attachment
>>>
>>>Replaced {} with () in test to be C++98 compatible
>>
>>Looks great, thanks.
>>
>>I'll test and commit this tomorrow.
>
>Not quite "tomorrow", but it's pushed to trunk now. Thanks again!

I've also pushed this fix for the new test, so it passes with
-std=gnu++98.

Tested x86_64-linux.