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

Fix Unicode handling when writing to an ostream on GCC on Windows #2875

Closed
wants to merge 1 commit into from
Closed

Fix Unicode handling when writing to an ostream on GCC on Windows #2875

wants to merge 1 commit into from

Conversation

dimztimz
Copy link
Contributor

I noticed that you recently added this 8751a03 which works just on MSVC. This patch does the same on GCC on Windows (Mingw).

The relevant info about the internals of GCC's standard library is here https://github.com/gcc-mirror/gcc/blob/master/libstdc++-v3/src/c++98/ios_init.cc

@vitaut
Copy link
Contributor

vitaut commented May 1, 2022

Thanks for the PR but the commit you are referring to is just an experiment, it doesn't need to be ported anywhere.

@vitaut vitaut closed this May 1, 2022
@dimztimz dimztimz deleted the ostream branch May 1, 2022 00:51
@dimztimz
Copy link
Contributor Author

dimztimz commented Jul 5, 2022

Thanks for the PR but the commit you are referring to is just an experiment, it doesn't need to be ported anywhere.

The experiment shipped in v9.0.0.

@vitaut
Copy link
Contributor

vitaut commented Jul 5, 2022

It is included in the release but none of this is recommended. There are better APIs that don't involve iostreams.

@dimztimz
Copy link
Contributor Author

dimztimz commented Jul 5, 2022

But now you have divergant behavior. One behavior on MSVC and another on GCC (on Windows).

@vitaut
Copy link
Contributor

vitaut commented Jul 9, 2022

The divergence is preexisting but now it's more limited. In any case this is mostly a proof of concept and

There are better APIs that don't involve iostreams.

@dimztimz
Copy link
Contributor Author

dimztimz commented Jul 9, 2022

Ok, let me understand this better. The patch I linked, 8751a03, is POC implementation for the following function.

void vprint_unicode(ostream& os, string_view fmt, format_args args);

The function is proposed in your paper Formatted output. Only Windows needs special handing to implement this function. Until that patch, it did not had a reference implementation, yet it was proposed. There are 3 standard libraries that work with Windows. Your patch implements the function only for MSVC and uses some unorthodox tricks to access some private members of the streambuf in cout. There is no other way.

My patch additionally implements the same function when GCC+MinGW is used. It uses completely documented interfaces by GCC, no tricks here. If you have something against this function then really don't propose it at all the paper. But if its there it would be good to work as described on all compilers that are available on Windows, not just on MSVC.

@vitaut
Copy link
Contributor

vitaut commented Jul 16, 2022

It uses completely documented interfaces by GCC

If this is the case then I'm not opposed to adding the functionality. My main concern was adding more hacks like in the MSVC case. So feel free to resubmit the PR (I can't reopen this one for some reason) but please check that the mojibake problem exists in libstdc++ case on Windows and this fix is actually needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants