src: constrain MaybeStackBuffer::ToString and ToStringView to standard char types#62507
Conversation
nodejs-github-bot
added
c++
om-ghante
force-pushed
the
fix/constrain-maybstackbuffer-tostring-char-traits
branch
from
ecb3efb to
7c9594c
Compare
| std::is_same_v<T, char8_t> || std::is_same_v<T, char16_t> || | ||
| std::is_same_v<T, char32_t>) { | ||
| return {out(), length()}; | ||
| } |
There was a problem hiding this comment.
These could be condensed into a single concept couldn't it? Just to avoid the duplication.
|
Have you tried your changes? I applied the patch and still see a deprecation warning. |
om-ghante
force-pushed
the
fix/constrain-maybstackbuffer-tostring-char-traits
branch
from
7c9594c to
b42db3d
Compare
…d char types On newer libc++ (shipped with macOS Xcode 16+), std::char_traits<T> for T not equal to char, wchar_t, char8_t, char16_t, or char32_t is deprecated and will be removed in a future release. When MaybeStackBuffer is instantiated with unsigned char or uint8_t (e.g. in test/cctest/test_util.cc), the ToString() and ToStringView() methods trigger this deprecation warning because they reference std::basic_string<unsigned char> and std::basic_string_view<unsigned char>, even though these methods are never actually called for those types. Add C++20 requires clauses to constrain ToString() and ToStringView() to only be available for standard character types, preventing the deprecated std::char_traits<unsigned char> from being instantiated. This is consistent with the existing use of C++20 concepts elsewhere in the same file (e.g. the is_callable concept).
om-ghante
force-pushed
the
fix/constrain-maybstackbuffer-tostring-char-traits
branch
from
b42db3d to
8eb1cad
Compare
Yes, I have checked my updated changes and the deprecation warning is now gone. The issue with the first patch was that |
|
Thanks. I confirm there is no warning anymore with the latest push. I defer to @nodejs/cpp-reviewers for the code review. |
4 participants
On newer libc++ (shipped with macOS Xcode 16+),
std::char_traits<T>forTnotequal to
char,wchar_t,char8_t,char16_t, orchar32_tis deprecated andwill be removed in a future release.
When the MaybeStackBuffer template is instantiated with
unsigned charoruint8_t(e.g., in test/cctest/test_util.cc), the compiler instantiates the declarations
for all member functions. The ToString() and ToStringView() methods historically returned
std::basic_string<T>andstd::basic_string_view<T>, which triggered thisdeprecation warning (
-Wdeprecated-declarations) during class instantiation becausethe return type references
std::char_traits<unsigned char>.Changes
This PR resolves the warning by:
standard_char_type.member function templates with a constrained default template parameter:
template <standard_char_type U = T>.By making these member function templates, the C++ compiler defers instantiating
the return type (
std::basic_string<U>) until the function is actually called. Sincethese methods are never actually called for
unsigned charoruint8_tvariantsin the Node.js codebase, the deprecated
std::char_traits<unsigned char>is neverevaluated or instantiated, completely bypassing the warning.
Refs: #62506