Skip to content
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

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

Ansariel
Copy link
Contributor

@Ansariel Ansariel commented Sep 6, 2024

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.

Copy link
Contributor

@marchcat marchcat left a 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.

@marchcat marchcat added this to the Develop milestone Sep 9, 2024
@marchcat marchcat merged commit d91d39f into secondlife:develop Sep 9, 2024
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 9, 2024
@@ -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;
Copy link
Collaborator

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;
Copy link
Collaborator

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;
Copy link
Collaborator

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).

@Ansariel Ansariel deleted the develop-c20-wip branch September 13, 2024 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants