-
Notifications
You must be signed in to change notification settings - Fork 54
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
Changes towards C++20 compatibility #2520
Conversation
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.
Thank you, @Ansariel!
Mac builds are green for this one, merging.
@@ -226,7 +226,7 @@ bool LLApp::parseCommandOptions(int argc, wchar_t** wargv) | |||
if(wargv[ii][0] != '-') | |||
{ | |||
LL_INFOS() << "Did not find option identifier while parsing token: " | |||
<< wargv[ii] << LL_ENDL; | |||
<< (intptr_t)wargv[ii] << LL_ENDL; |
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.
This allows the line to compile, but isn't useful to the user encountering this error: it displays the "token" in question as the int value of the pointer.
How about ll_convert<std::string>(wargv[ii])
to convert the wchar_t*
to UTF-8 std::string
?
@@ -261,7 +261,7 @@ void LLViewerEventRecorder::logKeyUnicodeEvent(llwchar uni_char) { | |||
|
|||
event.insert("event",LLSD("keyDown")); | |||
|
|||
LL_DEBUGS() << "[VITA] unicode key: " << uni_char << LL_ENDL; | |||
LL_DEBUGS() << "[VITA] unicode key: " << (int)uni_char << LL_ENDL; |
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.
What this uni_char
reference seems to want is the same expression we find on lines 250 and 253: wstring_to_utf8str(LLWString(1, uni_char))
. Because it's referenced unconditionally on line 253, we could reasonably assign that expression to a variable, e.g. utf8_key
, then reference it on lines 250, 253 and 264.
That said, since we find still other instances elsewhere, it might be worth adding this ll_convert_impl
specialization to llstring.h right after the declaration of wchar_to_utf8chars()
:
// return a UTF-8 string representation of a single llwchar, which we
// occasionally require:
// cheaper than ll_convert<std::string>(LLWString(1, inchar))
template <>
struct ll_convert_impl<std::string, llwchar>
{
std::string operator()(llwchar in)
{
// wchar_to_utf8chars() never requires a buffer larger than 8 chars
char buffer[8];
auto len = wchar_to_utf8chars(in, buffer);
return { buffer, std::string::size_type(len) };
}
};
Then we can use ll_convert_to<std::string>(uni_char)
instead of the wstring_to_utf8str()
expression.
@@ -503,7 +503,7 @@ S32 LLEmbeddedItems::getIndexFromEmbeddedChar(llwchar wch) | |||
} | |||
else | |||
{ | |||
LL_WARNS() << "Embedded char " << wch << " not found, using 0" << LL_ENDL; | |||
LL_WARNS() << "Embedded char " << (int)wch << " not found, using 0" << LL_ENDL; |
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.
See above remarks about ll_convert_to<std::string>(wch)
.
Since there are still issues on macOS side preventing the actual switch to C++20 (see #2330), I broke out some changes that also work on C++17 and that will be needed in the future. I left out some changes I am not sure of if they would cause issues on macOS currently.