* [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
@ 2021-05-11 18:27 Antony Polukhin
2021-05-11 20:00 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Antony Polukhin @ 2021-05-11 18:27 UTC (permalink / raw)
To: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 366 bytes --]
This patch provides compile time diagnostics for common misuse of
[locale.convenience] functions with std::string as a character type.
2021-05-11 Antony Polukhin <antoshkka@gmail.com>
PR libstdc++/89728
* include/bits/locale_facets.h (ctype) Add static assert.
* testsuite/22_locale/ctype/is/string/89728_neg.cc New test.
--
Best regards,
Antony Polukhin
[-- Attachment #2: ctype_compile_time_diag.txt --]
[-- Type: text/plain, Size: 4861 bytes --]
diff --git a/libstdc++-v3/include/bits/locale_facets.h b/libstdc++-v3/include/bits/locale_facets.h
index 03724cf..012857f 100644
--- a/libstdc++-v3/include/bits/locale_facets.h
+++ b/libstdc++-v3/include/bits/locale_facets.h
@@ -136,6 +136,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return __s;
}
+ template<typename>
+ struct __is_string
+ {
+ enum _Value { _value = 0 };
+ };
+
+ template<typename _CharT, typename _Traits, typename _Alloc>
+ struct __is_string<basic_string<_CharT, _Traits, _Alloc> >
+ {
+ enum _Value { _value = 1 };
+ };
// 22.2.1.1 Template class ctype
// Include host and configuration specific ctype enums for ctype_base.
@@ -614,6 +625,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _CharT>
class ctype : public __ctype_abstract_base<_CharT>
{
+#if __cplusplus >= 201103L
+ static_assert(!__is_string<_CharT>::_value,
+ "std::basic_string used as a character type");
+#endif
public:
// Types:
typedef _CharT char_type;
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
new file mode 100644
index 0000000..987fa8e
--- /dev/null
+++ b/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc
@@ -0,0 +1,73 @@
+// { dg-do compile { target c++11 } }
+
+// Copyright (C) 2021 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-error "used as a character type" "" { target *-*-* } 0 }
+
+#include <locale>
+
+template <class Char, int I>
+struct trait: std::char_traits<Char> {};
+
+template <class Char, int I>
+auto make_str()
+{
+ return std::basic_string<Char, trait<Char, I>>{};
+}
+
+void test01()
+{
+ const auto& loc = std::locale::classic();
+
+ std::isspace(std::string{}, loc); // { dg-error "required from here" }
+ std::isprint(make_str<char, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<char, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<char, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<char, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<char, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<char, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<char, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ std::toupper(make_str<char, 11>(), loc); // { dg-error "required from here" }
+ std::tolower(make_str<char, 12>(), loc); // { dg-error "required from here" }
+}
+
+#ifdef _GLIBCXX_USE_WCHAR_T
+void test02()
+{
+ const auto& loc = std::locale::classic();
+
+ std::isspace(std::wstring{}, loc); // { dg-error "required from here" }
+ std::isprint(make_str<wchar_t, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<wchar_t, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<wchar_t, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<wchar_t, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<wchar_t, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<wchar_t, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<wchar_t, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ 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" }
+}
+#endif
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-11 18:27 [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions Antony Polukhin
@ 2021-05-11 20:00 ` Jonathan Wakely
2021-05-12 9:18 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Wakely @ 2021-05-11 20:00 UTC (permalink / raw)
To: Antony Polukhin; +Cc: libstdc++, gcc-patches List
On 11/05/21 21:27 +0300, Antony Polukhin via Libstdc++ wrote:
>This patch provides compile time diagnostics for common misuse of
>[locale.convenience] functions with std::string as a character type.
>
>
>2021-05-11 Antony Polukhin <antoshkka@gmail.com>
>
>PR libstdc++/89728
> * include/bits/locale_facets.h (ctype) Add static assert.
> * testsuite/22_locale/ctype/is/string/89728_neg.cc New test.
>
>--
>Best regards,
>Antony Polukhin
>diff --git a/libstdc++-v3/include/bits/locale_facets.h b/libstdc++-v3/include/bits/locale_facets.h
>index 03724cf..012857f 100644
>--- a/libstdc++-v3/include/bits/locale_facets.h
>+++ b/libstdc++-v3/include/bits/locale_facets.h
>@@ -136,6 +136,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> return __s;
> }
>
>+ template<typename>
>+ struct __is_string
>+ {
>+ enum _Value { _value = 0 };
The _value member needs to use double underscores.
But since this is only used in a static_assert, which is only
available in C++11, I think it can just derive from std::false_type
instead.
>+ };
>+
>+ template<typename _CharT, typename _Traits, typename _Alloc>
>+ struct __is_string<basic_string<_CharT, _Traits, _Alloc> >
>+ {
>+ enum _Value { _value = 1 };
>+ };
>
> // 22.2.1.1 Template class ctype
> // Include host and configuration specific ctype enums for ctype_base.
>@@ -614,6 +625,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> template<typename _CharT>
> class ctype : public __ctype_abstract_base<_CharT>
> {
>+#if __cplusplus >= 201103L
>+ static_assert(!__is_string<_CharT>::_value,
>+ "std::basic_string used as a character type");
>+#endif
> public:
> // Types:
> typedef _CharT char_type;
Alternatively, would it be even simpler to just define a partial
specialization of ctype?
template<typename _CharT, typename _Traits, typename _Alloc>
class ctype<basic_string<_CharT, _Traits, _Alloc> >
{
#if __cplusplus >= 201103L
static_assert(something dependent,
"std::basic_string used as a character type");
#endif
private:
ctype();
~ctype();
};
This will work in C++98 too.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-11 20:00 ` Jonathan Wakely
@ 2021-05-12 9:18 ` Jonathan Wakely
2021-05-12 9:58 ` Antony Polukhin
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Wakely @ 2021-05-12 9:18 UTC (permalink / raw)
To: Antony Polukhin; +Cc: libstdc++, gcc-patches List
On 11/05/21 21:00 +0100, Jonathan Wakely wrote:
>Alternatively, would it be even simpler to just define a partial
>specialization of ctype?
>
>template<typename _CharT, typename _Traits, typename _Alloc>
> class ctype<basic_string<_CharT, _Traits, _Alloc> >
> {
>#if __cplusplus >= 201103L
> static_assert(something dependent,
> "std::basic_string used as a character type");
>#endif
> private:
> ctype();
> ~ctype();
> };
>
>This will work in C++98 too.
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-12 9:18 ` Jonathan Wakely
@ 2021-05-12 9:58 ` Antony Polukhin
2021-05-12 12:35 ` Antony Polukhin
2021-05-12 14:44 ` Jonathan Wakely
0 siblings, 2 replies; 11+ messages in thread
From: Antony Polukhin @ 2021-05-12 9:58 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
ср, 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++
Should we care about those users?
--
Best regards,
Antony Polukhin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-12 9:58 ` Antony Polukhin
@ 2021-05-12 12:35 ` Antony Polukhin
2021-05-12 14:44 ` Jonathan Wakely
1 sibling, 0 replies; 11+ messages in thread
From: Antony Polukhin @ 2021-05-12 12:35 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 396 bytes --]
ср, 12 мая 2021 г. в 12:58, Antony Polukhin <antoshkka@gmail.com>:
<...>
> Should we care about those users?
Looks like the answer is "yes". There are tests for that in
22_locale/ctype/requirements/explicit_instantiation.cc and
22_locale/money_get/cons/3.cc
Attaching a patch with review fixes (with ctype specialization for basic_string)
--
Best regards,
Antony Polukhin
[-- Attachment #2: ctype_compile_time_diag-3.txt --]
[-- Type: text/plain, Size: 4849 bytes --]
diff --git a/libstdc++-v3/include/bits/locale_facets.h b/libstdc++-v3/include/bits/locale_facets.h
index 03724cf..bbd7da5 100644
--- a/libstdc++-v3/include/bits/locale_facets.h
+++ b/libstdc++-v3/include/bits/locale_facets.h
@@ -671,6 +671,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _CharT>
locale::id ctype<_CharT>::id;
+ // Compile time diagnostics for common misuse of [locale.convenience]
+ // functions with basic_string as a character type.
+ template<typename _CharT, typename _Traits, typename _Alloc>
+ class ctype<basic_string<_CharT, _Traits, _Alloc> >
+ {
+#if __cplusplus >= 201103L
+ static_assert(sizeof(_CharT) && false,
+ "std::basic_string used as a character type");
+#endif
+ ctype();
+ ~ctype();
+ };
+
/**
* @brief The ctype<char> specialization.
* @ingroup locales
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
new file mode 100644
index 0000000..268e03a
--- /dev/null
+++ b/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc
@@ -0,0 +1,75 @@
+// { dg-do compile }
+
+// Copyright (C) 2021 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-error "basic_string used as a character type" "" { target c++11 } 0 }
+// { dg-error "is not a member of" "" { target *-*-* } 0 }
+// { dg-error "has no member named" "" { target *-*-* } 0 }
+
+#include <locale>
+
+template <class Char, int I>
+struct trait: std::char_traits<Char> {};
+
+template <class Char, int I>
+std::basic_string<Char, trait<Char, I> > make_str()
+{
+ return std::basic_string<Char, trait<Char, I> >();
+}
+
+void test01()
+{
+ const std::locale& loc = std::locale::classic();
+
+ std::isspace(std::string{}, loc); // { dg-error "required from here" }
+ std::isprint(make_str<char, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<char, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<char, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<char, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<char, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<char, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<char, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ std::toupper(make_str<char, 11>(), loc); // { dg-error "required from here" }
+ std::tolower(make_str<char, 12>(), loc); // { dg-error "required from here" }
+}
+
+#ifdef _GLIBCXX_USE_WCHAR_T
+void test02()
+{
+ const std::locale& loc = std::locale::classic();
+
+ std::isspace(std::wstring{}, loc); // { dg-error "required from here" }
+ std::isprint(make_str<wchar_t, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<wchar_t, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<wchar_t, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<wchar_t, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<wchar_t, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<wchar_t, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<wchar_t, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ 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" }
+}
+#endif
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-12 9:58 ` Antony Polukhin
2021-05-12 12:35 ` Antony Polukhin
@ 2021-05-12 14:44 ` Jonathan Wakely
2021-05-12 15:38 ` Antony Polukhin
1 sibling, 1 reply; 11+ messages in thread
From: Jonathan Wakely @ 2021-05-12 14:44 UTC (permalink / raw)
To: Antony Polukhin; +Cc: libstdc++, gcc-patches List
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?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-12 14:44 ` Jonathan Wakely
@ 2021-05-12 15:38 ` Antony Polukhin
2021-05-12 15:51 ` Antony Polukhin
0 siblings, 1 reply; 11+ messages in thread
From: Antony Polukhin @ 2021-05-12 15:38 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]
ср, 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
--
Best regards,
Antony Polukhin
[-- Attachment #2: ctype_compile_time_diag-7.txt --]
[-- Type: text/plain, Size: 4540 bytes --]
diff --git a/libstdc++-v3/include/bits/locale_facets.h b/libstdc++-v3/include/bits/locale_facets.h
index 03724cf..5ca431e 100644
--- a/libstdc++-v3/include/bits/locale_facets.h
+++ b/libstdc++-v3/include/bits/locale_facets.h
@@ -671,6 +671,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _CharT>
locale::id ctype<_CharT>::id;
+ // Incomplete to provide a compile time diagnostics for common misuse
+ // of [locale.convenience] functions with basic_string as a character type.
+ template<typename _CharT, typename _Traits, typename _Alloc>
+ class ctype<basic_string<_CharT, _Traits, _Alloc> >;
+
/**
* @brief The ctype<char> specialization.
* @ingroup locales
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
new file mode 100644
index 0000000..ccab3a2
--- /dev/null
+++ b/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc
@@ -0,0 +1,73 @@
+// { dg-do compile }
+
+// Copyright (C) 2021 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-error "complete" "" { target *-*-* } 0 }
+
+#include <locale>
+
+template <class Char, int I>
+struct trait: std::char_traits<Char> {};
+
+template <class Char, int I>
+std::basic_string<Char, trait<Char, I> > make_str()
+{
+ return std::basic_string<Char, trait<Char, I> >();
+}
+
+void test01()
+{
+ const std::locale& loc = std::locale::classic();
+
+ std::isspace(std::string{}, loc); // { dg-error "required from here" }
+ std::isprint(make_str<char, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<char, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<char, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<char, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<char, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<char, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<char, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ std::toupper(make_str<char, 11>(), loc); // { dg-error "required from here" }
+ std::tolower(make_str<char, 12>(), loc); // { dg-error "required from here" }
+}
+
+#ifdef _GLIBCXX_USE_WCHAR_T
+void test02()
+{
+ const std::locale& loc = std::locale::classic();
+
+ std::isspace(std::wstring{}, loc); // { dg-error "required from here" }
+ std::isprint(make_str<wchar_t, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<wchar_t, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<wchar_t, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<wchar_t, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<wchar_t, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<wchar_t, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<wchar_t, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ 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" }
+}
+#endif
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-12 15:38 ` Antony Polukhin
@ 2021-05-12 15:51 ` Antony Polukhin
2021-05-12 16:16 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Antony Polukhin @ 2021-05-12 15:51 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 2943 bytes --]
ср, 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
--
Best regards,
Antony Polukhin
[-- Attachment #2: ctype_compile_time_diag-8.txt --]
[-- Type: text/plain, Size: 4540 bytes --]
diff --git a/libstdc++-v3/include/bits/locale_facets.h b/libstdc++-v3/include/bits/locale_facets.h
index 03724cf..5ca431e 100644
--- a/libstdc++-v3/include/bits/locale_facets.h
+++ b/libstdc++-v3/include/bits/locale_facets.h
@@ -671,6 +671,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _CharT>
locale::id ctype<_CharT>::id;
+ // Incomplete to provide a compile time diagnostics for common misuse
+ // of [locale.convenience] functions with basic_string as a character type.
+ template<typename _CharT, typename _Traits, typename _Alloc>
+ class ctype<basic_string<_CharT, _Traits, _Alloc> >;
+
/**
* @brief The ctype<char> specialization.
* @ingroup locales
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
new file mode 100644
index 0000000..9f15620
--- /dev/null
+++ b/libstdc++-v3/testsuite/22_locale/ctype/is/string/89728_neg.cc
@@ -0,0 +1,73 @@
+// { dg-do compile }
+
+// Copyright (C) 2021 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-error "complete" "" { target *-*-* } 0 }
+
+#include <locale>
+
+template <class Char, int I>
+struct trait: std::char_traits<Char> {};
+
+template <class Char, int I>
+std::basic_string<Char, trait<Char, I> > make_str()
+{
+ return std::basic_string<Char, trait<Char, I> >();
+}
+
+void test01()
+{
+ const std::locale& loc = std::locale::classic();
+
+ std::isspace(std::string(), loc); // { dg-error "required from here" }
+ std::isprint(make_str<char, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<char, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<char, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<char, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<char, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<char, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<char, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ std::toupper(make_str<char, 11>(), loc); // { dg-error "required from here" }
+ std::tolower(make_str<char, 12>(), loc); // { dg-error "required from here" }
+}
+
+#ifdef _GLIBCXX_USE_WCHAR_T
+void test02()
+{
+ const std::locale& loc = std::locale::classic();
+
+ std::isspace(std::wstring(), loc); // { dg-error "required from here" }
+ std::isprint(make_str<wchar_t, 0>(), loc); // { dg-error "required from here" }
+ std::iscntrl(make_str<wchar_t, 1>(), loc); // { dg-error "required from here" }
+ std::isupper(make_str<wchar_t, 2>(), loc); // { dg-error "required from here" }
+ std::islower(make_str<wchar_t, 3>(), loc); // { dg-error "required from here" }
+ std::isalpha(make_str<wchar_t, 4>(), loc); // { dg-error "required from here" }
+ std::isdigit(make_str<wchar_t, 5>(), loc); // { dg-error "required from here" }
+ std::ispunct(make_str<wchar_t, 6>(), loc); // { dg-error "required from here" }
+ 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" }
+ 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" }
+}
+#endif
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-12 15:51 ` Antony Polukhin
@ 2021-05-12 16:16 ` Jonathan Wakely
2021-05-17 17:14 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Wakely @ 2021-05-12 16:16 UTC (permalink / raw)
To: Antony Polukhin; +Cc: libstdc++, gcc-patches List
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.
(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).
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-12 16:16 ` Jonathan Wakely
@ 2021-05-17 17:14 ` Jonathan Wakely
2021-06-01 18:04 ` Jonathan Wakely
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Wakely @ 2021-05-17 17:14 UTC (permalink / raw)
To: Antony Polukhin; +Cc: libstdc++, gcc-patches List
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!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions
2021-05-17 17:14 ` Jonathan Wakely
@ 2021-06-01 18:04 ` Jonathan Wakely
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Wakely @ 2021-06-01 18:04 UTC (permalink / raw)
To: Antony Polukhin; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 3324 bytes --]
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.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2096 bytes --]
commit b514fce354b5309a9c232a3fe9347e3abde4385f
Author: Jonathan Wakely <jwakely@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 <jwakely@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" }
}
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-01 18:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11 18:27 [PATCH] PR libstdc++/89728 diagnose some missuses of [locale.convenience] functions Antony Polukhin
2021-05-11 20:00 ` Jonathan Wakely
2021-05-12 9:18 ` Jonathan Wakely
2021-05-12 9:58 ` Antony Polukhin
2021-05-12 12:35 ` Antony Polukhin
2021-05-12 14:44 ` Jonathan Wakely
2021-05-12 15:38 ` Antony Polukhin
2021-05-12 15:51 ` Antony Polukhin
2021-05-12 16:16 ` Jonathan Wakely
2021-05-17 17:14 ` Jonathan Wakely
2021-06-01 18:04 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).