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


commit b514fce354b5309a9c232a3fe9347e3abde4385f
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Tue Jun 1 19:01:37 2021

    libstdc++: Fix new test for C++98 mode [PR 89728]
    
    The isblank class is not supported until C++11.
    
    Signed-off-by: Jonathan Wakely <jwak...@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/22_locale/ctype/is/string/89728_neg.cc: Only test
            isblank for C++11 and later.

diff --git a/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc b/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc
index 9f15620c9a8..89843b68494 100644
--- a/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc
+++ b/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc
@@ -45,7 +45,9 @@ void test01()
   std::isxdigit(make_str<char, 7>(), loc);	// { dg-error "required from here" }
   std::isalnum(make_str<char, 8>(), loc);	// { dg-error "required from here" }
   std::isgraph(make_str<char, 9>(), loc);	// { dg-error "required from here" }
-  std::isblank(make_str<char, 10>(), loc);	// { dg-error "required from here" }
+#if __cplusplus >= 201103
+  std::isblank(make_str<char, 10>(), loc);	// { dg-error "required from here" "" { target c++11 } }
+#endif
   std::toupper(make_str<char, 11>(), loc);	// { dg-error "required from here" }
   std::tolower(make_str<char, 12>(), loc);	// { dg-error "required from here" }
 }
@@ -66,7 +68,9 @@ void test02()
   std::isxdigit(make_str<wchar_t, 7>(), loc);	// { dg-error "required from here" }
   std::isalnum(make_str<wchar_t, 8>(), loc);	// { dg-error "required from here" }
   std::isgraph(make_str<wchar_t, 9>(), loc);	// { dg-error "required from here" }
-  std::isblank(make_str<wchar_t, 10>(), loc);	// { dg-error "required from here" }
+#if __cplusplus >= 201103
+  std::isblank(make_str<wchar_t, 10>(), loc);	// { dg-error "required from here" "" { target c++11 } }
+#endif
   std::toupper(make_str<wchar_t, 11>(), loc);	// { dg-error "required from here" }
   std::tolower(make_str<wchar_t, 12>(), loc);	// { dg-error "required from here" }
 }

Reply via email to