-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libc++] Refactor num_get optimization to not be ABI breaking #121690
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesFull diff: https://github.com/llvm/llvm-project/pull/121690.diff 3 Files Affected:
diff --git a/libcxx/include/__configuration/abi.h b/libcxx/include/__configuration/abi.h
index c6ef6fdcdf96e6..5e40e308c8926c 100644
--- a/libcxx/include/__configuration/abi.h
+++ b/libcxx/include/__configuration/abi.h
@@ -57,8 +57,6 @@
// because it changes the mangling of the virtual function located in the vtable, which
// changes how it gets signed.
# define _LIBCPP_ABI_BAD_FUNCTION_CALL_GOOD_WHAT_MESSAGE
-// Enable optimized version of __do_get_(un)signed which avoids redundant copies.
-# define _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
// Give reverse_iterator<T> one data member of type T, not two.
// Also, in C++17 and later, don't derive iterator types from std::iterator.
# define _LIBCPP_ABI_NO_ITERATOR_BASES
diff --git a/libcxx/include/locale b/libcxx/include/locale
index 981f25ed1e98cf..4fb01ce061c0b2 100644
--- a/libcxx/include/locale
+++ b/libcxx/include/locale
@@ -400,8 +400,9 @@ struct __num_get : protected __num_get_base {
unsigned*& __g_end,
unsigned& __dc,
_CharT* __atoms);
-# ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
- static string __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
+
+ [[__deprecated__("This exists only for ABI compatability")]] static string
+ __stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep);
static int __stage2_int_loop(
_CharT __ct,
int __base,
@@ -414,55 +415,32 @@ struct __num_get : protected __num_get_base {
unsigned*& __g_end,
_CharT* __atoms);
-# else
- static string __stage2_int_prep(ios_base& __iob, _CharT& __thousands_sep) {
+ _LIBCPP_HIDE_FROM_ABI static string __stage2_int_prep(ios_base& __iob, _CharT& __thousands_sep) {
locale __loc = __iob.getloc();
const numpunct<_CharT>& __np = use_facet<numpunct<_CharT> >(__loc);
__thousands_sep = __np.thousands_sep();
return __np.grouping();
}
- const _CharT* __do_widen(ios_base& __iob, _CharT* __atoms) const { return __do_widen_p(__iob, __atoms); }
-
- static int __stage2_int_loop(
- _CharT __ct,
- int __base,
- char* __a,
- char*& __a_end,
- unsigned& __dc,
- _CharT __thousands_sep,
- const string& __grouping,
- unsigned* __g,
- unsigned*& __g_end,
- const _CharT* __atoms);
+ _LIBCPP_HIDE_FROM_ABI const _CharT* __do_widen(ios_base& __iob, _CharT* __atoms) const {
+ return __do_widen_p(__iob, __atoms);
+ }
private:
template <typename _Tp>
- const _Tp* __do_widen_p(ios_base& __iob, _Tp* __atoms) const {
+ _LIBCPP_HIDE_FROM_ABI const _Tp* __do_widen_p(ios_base& __iob, _Tp* __atoms) const {
locale __loc = __iob.getloc();
use_facet<ctype<_Tp> >(__loc).widen(__src, __src + __int_chr_cnt, __atoms);
return __atoms;
}
- const char* __do_widen_p(ios_base& __iob, char* __atoms) const {
+ _LIBCPP_HIDE_FROM_ABI const char* __do_widen_p(ios_base& __iob, char* __atoms) const {
(void)__iob;
(void)__atoms;
return __src;
}
-# endif
};
-# ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
-template <class _CharT>
-string __num_get<_CharT>::__stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep) {
- locale __loc = __iob.getloc();
- std::use_facet<ctype<_CharT> >(__loc).widen(__src, __src + __int_chr_cnt, __atoms);
- const numpunct<_CharT>& __np = std::use_facet<numpunct<_CharT> >(__loc);
- __thousands_sep = __np.thousands_sep();
- return __np.grouping();
-}
-# endif
-
template <class _CharT>
string __num_get<_CharT>::__stage2_float_prep(
ios_base& __iob, _CharT* __atoms, _CharT& __decimal_point, _CharT& __thousands_sep) {
@@ -475,18 +453,17 @@ string __num_get<_CharT>::__stage2_float_prep(
}
template <class _CharT>
-int
-# ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
-__num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
- unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
- unsigned* __g, unsigned*& __g_end, _CharT* __atoms)
-# else
-__num_get<_CharT>::__stage2_int_loop(_CharT __ct, int __base, char* __a, char*& __a_end,
- unsigned& __dc, _CharT __thousands_sep, const string& __grouping,
- unsigned* __g, unsigned*& __g_end, const _CharT* __atoms)
-
-# endif
-{
+int __num_get<_CharT>::__stage2_int_loop(
+ _CharT __ct,
+ int __base,
+ char* __a,
+ char*& __a_end,
+ unsigned& __dc,
+ _CharT __thousands_sep,
+ const string& __grouping,
+ unsigned* __g,
+ unsigned*& __g_end,
+ _CharT* __atoms) {
if (__a_end == __a && (__ct == __atoms[24] || __ct == __atoms[25])) {
*__a_end++ = __ct == __atoms[24] ? '+' : '-';
__dc = 0;
@@ -856,14 +833,9 @@ _InputIterator num_get<_CharT, _InputIterator>::__do_get_signed(
// Stage 2
char_type __thousands_sep;
const int __atoms_size = __num_get_base::__int_chr_cnt;
-# ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
char_type __atoms1[__atoms_size];
const char_type* __atoms = this->__do_widen(__iob, __atoms1);
string __grouping = this->__stage2_int_prep(__iob, __thousands_sep);
-# else
- char_type __atoms[__atoms_size];
- string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep);
-# endif
string __buf;
__buf.resize(__buf.capacity());
char* __a = &__buf[0];
@@ -879,7 +851,17 @@ _InputIterator num_get<_CharT, _InputIterator>::__do_get_signed(
__a = &__buf[0];
__a_end = __a + __tmp;
}
- if (this->__stage2_int_loop(*__b, __base, __a, __a_end, __dc, __thousands_sep, __grouping, __g, __g_end, __atoms))
+ if (this->__stage2_int_loop(
+ *__b,
+ __base,
+ __a,
+ __a_end,
+ __dc,
+ __thousands_sep,
+ __grouping,
+ __g,
+ __g_end,
+ const_cast<char_type*>(__atoms)))
break;
}
if (__grouping.size() != 0 && __g_end - __g < __num_get_base::__num_get_buf_sz)
@@ -905,14 +887,9 @@ _InputIterator num_get<_CharT, _InputIterator>::__do_get_unsigned(
// Stage 2
char_type __thousands_sep;
const int __atoms_size = __num_get_base::__int_chr_cnt;
-# ifdef _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
char_type __atoms1[__atoms_size];
const char_type* __atoms = this->__do_widen(__iob, __atoms1);
string __grouping = this->__stage2_int_prep(__iob, __thousands_sep);
-# else
- char_type __atoms[__atoms_size];
- string __grouping = this->__stage2_int_prep(__iob, __atoms, __thousands_sep);
-# endif
string __buf;
__buf.resize(__buf.capacity());
char* __a = &__buf[0];
@@ -928,7 +905,17 @@ _InputIterator num_get<_CharT, _InputIterator>::__do_get_unsigned(
__a = &__buf[0];
__a_end = __a + __tmp;
}
- if (this->__stage2_int_loop(*__b, __base, __a, __a_end, __dc, __thousands_sep, __grouping, __g, __g_end, __atoms))
+ if (this->__stage2_int_loop(
+ *__b,
+ __base,
+ __a,
+ __a_end,
+ __dc,
+ __thousands_sep,
+ __grouping,
+ __g,
+ __g_end,
+ const_cast<char_type*>(__atoms)))
break;
}
if (__grouping.size() != 0 && __g_end - __g < __num_get_base::__num_get_buf_sz)
diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index a1e10401f0b299..0b41136ea1e0a7 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -5645,6 +5645,16 @@ void moneypunct_byname<wchar_t, true>::init(const char* nm) {
void __do_nothing(void*) {}
+// Legacy ABI __num_get functions - the new ones are _LIBCPP_HIDE_FROM_ABI
+template <class _CharT>
+string __num_get<_CharT>::__stage2_int_prep(ios_base& __iob, _CharT* __atoms, _CharT& __thousands_sep) {
+ locale __loc = __iob.getloc();
+ std::use_facet<ctype<_CharT> >(__loc).widen(__src, __src + __int_chr_cnt, __atoms);
+ const numpunct<_CharT>& __np = std::use_facet<numpunct<_CharT> >(__loc);
+ __thousands_sep = __np.thousands_sep();
+ return __np.grouping();
+}
+
template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS collate<char>;
_LIBCPP_IF_WIDE_CHARACTERS(template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS collate<wchar_t>;)
|
ldionne
approved these changes
Jan 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with comments applied.
0dd7fc7
to
6f7a1d6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.