-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc++] Remove deprecated char_traits base template #72694
Conversation
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesThis patch has quite a bit of history. First, it must be noted that the Standard only specifies specializations of char_traits for char, char8_t, char16_t, char32_t and wchar_t. However, before this patch, we would provide a base template that accepted anything, and as a result code like We marked the base template as deprecated in LLVM 15 or 16 and were planning on removing it in LLVM 17, which we did in e30a148. However, it turned out that the deprecation warning had never been visible in user code since Clang did not surface that warning from system headers. As a result, this caught people by surprise and we decided to reintroduce the base template in LLVM 17 in cce062d. Since then, #70353 changed Clang so that such deprecation warnings would be visible from user code. Hence, this patch closes the loop and removes the deprecated specializations. TODO: This will be landed in the LLVM 19 time frame, not before. Full diff: /~https://github.com/llvm/llvm-project/pull/72694.diff 3 Files Affected:
diff --git a/libcxx/include/__string/char_traits.h b/libcxx/include/__string/char_traits.h
index 005df9a98f17a28..baf2d2346a59601 100644
--- a/libcxx/include/__string/char_traits.h
+++ b/libcxx/include/__string/char_traits.h
@@ -71,108 +71,6 @@ exposition-only to document what members a char_traits specialization should pro
};
*/
-//
-// Temporary extension to provide a base template for std::char_traits.
-// TODO(LLVM-19): Remove this class.
-//
-#if !defined(_LIBCPP_CHAR_TRAITS_REMOVE_BASE_SPECIALIZATION)
-template <class _CharT>
-struct _LIBCPP_DEPRECATED_("char_traits<T> for T not equal to char, wchar_t, char8_t, char16_t or char32_t is non-standard and is provided for a temporary period. It will be removed in LLVM 19, so please migrate off of it.")
- char_traits
-{
- using char_type = _CharT;
- using int_type = int;
- using off_type = streamoff;
- using pos_type = streampos;
- using state_type = mbstate_t;
-
- static inline void _LIBCPP_CONSTEXPR_SINCE_CXX17 _LIBCPP_HIDE_FROM_ABI
- assign(char_type& __c1, const char_type& __c2) _NOEXCEPT {__c1 = __c2;}
- static inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool eq(char_type __c1, char_type __c2) _NOEXCEPT
- {return __c1 == __c2;}
- static inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool lt(char_type __c1, char_type __c2) _NOEXCEPT
- {return __c1 < __c2;}
-
- static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17
- int compare(const char_type* __s1, const char_type* __s2, size_t __n) {
- for (; __n; --__n, ++__s1, ++__s2)
- {
- if (lt(*__s1, *__s2))
- return -1;
- if (lt(*__s2, *__s1))
- return 1;
- }
- return 0;
- }
- _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_SINCE_CXX17
- size_t length(const char_type* __s) {
- size_t __len = 0;
- for (; !eq(*__s, char_type(0)); ++__s)
- ++__len;
- return __len;
- }
- _LIBCPP_INLINE_VISIBILITY static _LIBCPP_CONSTEXPR_SINCE_CXX17
- const char_type* find(const char_type* __s, size_t __n, const char_type& __a) {
- for (; __n; --__n)
- {
- if (eq(*__s, __a))
- return __s;
- ++__s;
- }
- return nullptr;
- }
- static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
- char_type* move(char_type* __s1, const char_type* __s2, size_t __n) {
- if (__n == 0) return __s1;
- char_type* __r = __s1;
- if (__s1 < __s2)
- {
- for (; __n; --__n, ++__s1, ++__s2)
- assign(*__s1, *__s2);
- }
- else if (__s2 < __s1)
- {
- __s1 += __n;
- __s2 += __n;
- for (; __n; --__n)
- assign(*--__s1, *--__s2);
- }
- return __r;
- }
- _LIBCPP_INLINE_VISIBILITY
- static _LIBCPP_CONSTEXPR_SINCE_CXX20
- char_type* copy(char_type* __s1, const char_type* __s2, size_t __n) {
- if (!__libcpp_is_constant_evaluated()) {
- _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(
- __s2 < __s1 || __s2 >= __s1 + __n, "char_traits::copy overlapped range");
- }
- char_type* __r = __s1;
- for (; __n; --__n, ++__s1, ++__s2)
- assign(*__s1, *__s2);
- return __r;
- }
- _LIBCPP_INLINE_VISIBILITY
- static _LIBCPP_CONSTEXPR_SINCE_CXX20
- char_type* assign(char_type* __s, size_t __n, char_type __a) {
- char_type* __r = __s;
- for (; __n; --__n, ++__s)
- assign(*__s, __a);
- return __r;
- }
-
- static inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int_type not_eof(int_type __c) _NOEXCEPT
- {return eq_int_type(__c, eof()) ? ~eof() : __c;}
- static inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR char_type to_char_type(int_type __c) _NOEXCEPT
- {return char_type(__c);}
- static inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int_type to_int_type(char_type __c) _NOEXCEPT
- {return int_type(__c);}
- static inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool eq_int_type(int_type __c1, int_type __c2) _NOEXCEPT
- {return __c1 == __c2;}
- static inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR int_type eof() _NOEXCEPT
- {return int_type(EOF);}
-};
-#endif // !defined(_LIBCPP_CHAR_TRAITS_REMOVE_BASE_SPECIALIZATION)
-
// char_traits<char>
template <>
diff --git a/libcxx/test/libcxx/strings/char.traits/char.traits.specializations/arbitrary_char_type.deprecated.verify.cpp b/libcxx/test/libcxx/strings/char.traits/char.traits.specializations/arbitrary_char_type.deprecated.verify.cpp
deleted file mode 100644
index ec6f34ef5462e65..000000000000000
--- a/libcxx/test/libcxx/strings/char.traits/char.traits.specializations/arbitrary_char_type.deprecated.verify.cpp
+++ /dev/null
@@ -1,21 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// <string>
-
-// template<> struct char_traits<T> for arbitrary T
-
-// Make sure we issue deprecation warnings.
-
-#include <string>
-
-void f() {
- std::char_traits<unsigned char> t1; (void)t1; // expected-warning{{'char_traits<unsigned char>' is deprecated}}
- std::char_traits<signed char> t2; (void)t2; // expected-warning{{'char_traits<signed char>' is deprecated}}
- std::char_traits<unsigned long> t3; (void)t3; // expected-warning{{'char_traits<unsigned long>' is deprecated}}
-}
diff --git a/libcxx/test/libcxx/strings/char.traits/char.traits.specializations/arbitrary_char_type.pass.cpp b/libcxx/test/libcxx/strings/char.traits/char.traits.specializations/arbitrary_char_type.pass.cpp
deleted file mode 100644
index c2de29d22b2fe4e..000000000000000
--- a/libcxx/test/libcxx/strings/char.traits/char.traits.specializations/arbitrary_char_type.pass.cpp
+++ /dev/null
@@ -1,146 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// <string>
-
-// template<> struct char_traits<T> for arbitrary T
-
-// Non-standard but provided temporarily for users to migrate.
-
-// ADDITIONAL_COMPILE_FLAGS: -Wno-deprecated
-
-#include <string>
-#include <cassert>
-#include <type_traits>
-
-#include "test_macros.h"
-
-template <class Char>
-TEST_CONSTEXPR_CXX20 bool test() {
- static_assert(std::is_same<typename std::char_traits<Char>::char_type, Char>::value, "");
- static_assert(std::is_same<typename std::char_traits<Char>::int_type, int>::value, "");
- static_assert(std::is_same<typename std::char_traits<Char>::off_type, std::streamoff>::value, "");
- static_assert(std::is_same<typename std::char_traits<Char>::pos_type, std::streampos>::value, "");
- static_assert(std::is_same<typename std::char_traits<Char>::state_type, std::mbstate_t>::value, "");
-
- assert(std::char_traits<Char>::to_int_type(Char('a')) == Char('a'));
- assert(std::char_traits<Char>::to_int_type(Char('A')) == Char('A'));
- assert(std::char_traits<Char>::to_int_type(0) == 0);
-
- assert(std::char_traits<Char>::to_char_type(Char('a')) == Char('a'));
- assert(std::char_traits<Char>::to_char_type(Char('A')) == Char('A'));
- assert(std::char_traits<Char>::to_char_type(0) == 0);
-
- assert(std::char_traits<Char>::eof() == EOF);
-
- assert(std::char_traits<Char>::not_eof(Char('a')) == Char('a'));
- assert(std::char_traits<Char>::not_eof(Char('A')) == Char('A'));
- assert(std::char_traits<Char>::not_eof(0) == 0);
- assert(std::char_traits<Char>::not_eof(std::char_traits<Char>::eof()) !=
- std::char_traits<Char>::eof());
-
- assert(std::char_traits<Char>::lt(Char('\0'), Char('A')) == (Char('\0') < Char('A')));
- assert(std::char_traits<Char>::lt(Char('A'), Char('\0')) == (Char('A') < Char('\0')));
- assert(std::char_traits<Char>::lt(Char('a'), Char('a')) == (Char('a') < Char('a')));
- assert(std::char_traits<Char>::lt(Char('A'), Char('a')) == (Char('A') < Char('a')));
- assert(std::char_traits<Char>::lt(Char('a'), Char('A')) == (Char('a') < Char('A')));
-
- assert( std::char_traits<Char>::eq(Char('a'), Char('a')));
- assert(!std::char_traits<Char>::eq(Char('a'), Char('A')));
-
- assert( std::char_traits<Char>::eq_int_type(Char('a'), Char('a')));
- assert(!std::char_traits<Char>::eq_int_type(Char('a'), Char('A')));
- assert(!std::char_traits<Char>::eq_int_type(std::char_traits<Char>::eof(), Char('A')));
- assert( std::char_traits<Char>::eq_int_type(std::char_traits<Char>::eof(), std::char_traits<Char>::eof()));
-
- {
- Char s1[] = {1, 2, 3, 0};
- Char s2[] = {0};
- assert(std::char_traits<Char>::length(s1) == 3);
- assert(std::char_traits<Char>::length(s2) == 0);
- }
-
- {
- Char s1[] = {1, 2, 3};
- assert(std::char_traits<Char>::find(s1, 3, Char(1)) == s1);
- assert(std::char_traits<Char>::find(s1, 3, Char(2)) == s1+1);
- assert(std::char_traits<Char>::find(s1, 3, Char(3)) == s1+2);
- assert(std::char_traits<Char>::find(s1, 3, Char(4)) == 0);
- assert(std::char_traits<Char>::find(s1, 3, Char(0)) == 0);
- assert(std::char_traits<Char>::find(NULL, 0, Char(0)) == 0);
- }
-
- {
- Char s1[] = {1, 2, 3};
- Char s2[3] = {0};
- assert(std::char_traits<Char>::copy(s2, s1, 3) == s2);
- assert(s2[0] == Char(1));
- assert(s2[1] == Char(2));
- assert(s2[2] == Char(3));
- assert(std::char_traits<Char>::copy(NULL, s1, 0) == NULL);
- assert(std::char_traits<Char>::copy(s1, NULL, 0) == s1);
- }
-
- {
- Char s1[] = {1, 2, 3};
- assert(std::char_traits<Char>::move(s1, s1+1, 2) == s1);
- assert(s1[0] == Char(2));
- assert(s1[1] == Char(3));
- assert(s1[2] == Char(3));
- s1[2] = Char(0);
- assert(std::char_traits<Char>::move(s1+1, s1, 2) == s1+1);
- assert(s1[0] == Char(2));
- assert(s1[1] == Char(2));
- assert(s1[2] == Char(3));
- assert(std::char_traits<Char>::move(NULL, s1, 0) == NULL);
- assert(std::char_traits<Char>::move(s1, NULL, 0) == s1);
- }
-
- {
- Char s1[] = {0};
- assert(std::char_traits<Char>::compare(s1, s1, 0) == 0);
- assert(std::char_traits<Char>::compare(NULL, NULL, 0) == 0);
-
- Char s2[] = {1, 0};
- Char s3[] = {2, 0};
- assert(std::char_traits<Char>::compare(s2, s2, 1) == 0);
- assert(std::char_traits<Char>::compare(s2, s3, 1) < 0);
- assert(std::char_traits<Char>::compare(s3, s2, 1) > 0);
- }
-
- {
- Char s2[3] = {0};
- assert(std::char_traits<Char>::assign(s2, 3, Char(5)) == s2);
- assert(s2[0] == Char(5));
- assert(s2[1] == Char(5));
- assert(s2[2] == Char(5));
- assert(std::char_traits<Char>::assign(NULL, 0, Char(5)) == NULL);
- }
-
- {
- Char c = Char('\0');
- std::char_traits<Char>::assign(c, Char('a'));
- assert(c == Char('a'));
- }
-
- return true;
-}
-
-int main(int, char**) {
- test<unsigned char>();
- test<signed char>();
- test<unsigned long>();
-
-#if TEST_STD_VER > 17
- static_assert(test<unsigned char>());
- static_assert(test<signed char>());
- static_assert(test<unsigned long>());
-#endif
-
- return 0;
-}
|
@cor3ntin Thanks a bunch for fixing the underlying Clang issue and enabling this! |
Could you please give me a few days to test this internally? |
Sure, I mean this is going to land in the LLVM 19 time frame only anyways. But I would recommend doing the work now because otherwise this will become a problem for folks living at head when this lands. CC @llvm/libcxx-vendors |
72311b9
to
bd8bb40
Compare
This patch has quite a bit of history. First, it must be noted that the Standard only specifies specializations of char_traits for char, char8_t, char16_t, char32_t and wchar_t. However, before this patch, we would provide a base template that accepted anything, and as a result code like `std::basic_string<long long>` would compile but nobody knows what it really does. It basically compiles by accident. We marked the base template as deprecated in LLVM 15 or 16 and were planning on removing it in LLVM 17, which we did in e30a148. However, it turned out that the deprecation warning had never been visible in user code since Clang did not surface that warning from system headers. As a result, this caught people by surprise and we decided to reintroduce the base template in LLVM 17 in cce062d. Since then, llvm#70353 changed Clang so that such deprecation warnings would be visible from user code. Hence, this patch closes the loop and removes the deprecated specializations.
bd8bb40
to
ce615e6
Compare
This should be ready for review now. |
CC @llvm/libcxx-vendors since this may break some (invalid) code. |
Just for reference, an example of such breakage: jtv/libpqxx#751 |
I'm going for it -- we've been trying to remove this for like 2 years and we went through a deprecation period (LLVM 18), so I think we should land this as early as possible in the release cycle. |
This no longer has an effect since llvm/llvm-project#72694 which we rolled in in https://chromium-review.googlesource.com/c/chromium/src/+/5261503 in early February. No behavior change. Change-Id: I20f32af0296f17a917c64a53f5e0f78a07359a39 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5753085 Commit-Queue: Arthur Eubanks <aeubanks@google.com> Auto-Submit: Nico Weber <thakis@chromium.org> Reviewed-by: Arthur Eubanks <aeubanks@google.com> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/main@{#1335791}
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specialization for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allows any type to be passed for a long time. It looks there have been several attempts to remove this but they restored it afterwards due to some complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned_char>` is not allowed anymore.
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specialization for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allowed any type to be passed for a long time. It looks there have been several attempts to remove this after which they restored it due to complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned_char>` is not allowed anymore. This removes all uses of `std::basic_string<unsigned_char>` from embind. This needs to be done to update libc++ to LLVM 19 (emscripten-core#22994). I'm uploading this as a separate PR because this removes a functionality from embind.
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specialization for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allowed any type to be passed for a long time. It looks there have been several attempts to remove this after which they restored it due to complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned_char>` is not allowed anymore. This removes all uses of `std::basic_string<unsigned_char>` from embind. This needs to be done to update libc++ to LLVM 19 (emscripten-core#22994). I'm uploading this as a separate PR because this removes a functionality from embind.
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specializations for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allowed any type to be passed for a long time. It looks there have been several attempts to remove this after which they restored it due to complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned char>` is not allowed anymore. This removes all uses of `std::basic_string<unsigned char>` from embind. This needs to be done to update libc++ to LLVM 19 (#22994). I'm uploading this as a separate PR because this removes a functionality from embind.
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specializations for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allowed any type to be passed for a long time. It looks there have been several attempts to remove this after which they restored it due to complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned char>` is not allowed anymore. This removes all uses of `std::basic_string<unsigned char>` from embind. This needs to be done to update libc++ to LLVM 19 (emscripten-core#22994). I'm uploading this as a separate PR because this removes a functionality from embind.
This patch has quite a bit of history. First, it must be noted that the Standard only specifies specializations of char_traits for char, char8_t, char16_t, char32_t and wchar_t. However, before this patch, we would provide a base template that accepted anything, and as a result code like
std::basic_string<long long>
would compile but nobody knows what it really does. It basically compiles by accident.We marked the base template as deprecated in LLVM 15 or 16 and were planning on removing it in LLVM 17, which we did in e30a148. However, it turned out that the deprecation warning had never been visible in user code since Clang did not surface that warning from system headers. As a result, this caught people by surprise and we decided to reintroduce the base template in LLVM 17 in cce062d.
Since then, #70353 changed Clang so that such deprecation warnings would be visible from user code. Hence, this patch closes the loop and removes the deprecated specializations.