Recover error strings on Unix from_lossy_utf8

[workingjubilee]

Some language settings can result in unreliable UTF-8 being produced.
This can result in failing to emit the error string, panicking instead.
from_lossy_utf8 allows us to assume these strings usually will be fine.

This fixes #99535.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[the8472]

Aiui some legacy encodings such as shift-jis aren't an ascii-superset. So if there's a system using such a locale they'd still garbage and this wouldn't really be a fix.

[workingjubilee]

Alas, "Not output garbage" is not the goal of this PR.
"Not panic when the OS feeds us something we consider garbage" is the goal of this PR.
Emitting garbage is better than an unnecessary panic.
Currently, even a garbage error message is not emitted, because we panic before emitting it.

A more complicated fix requires more nuanced handling, you are correct, but immediately after writing this, I started doing many more smaller changes to improve things further until I realized the OsStr interfaces that we probably should be using for this case were actually quite underdeveloped and that in order for me to vet the implementation, I would also have to at minimum test Windows, and so I need to get my Windows machine booting. I do intend to make such followups, but I saw no reason to further delay things.

The primary location to find Shift-JIS is in fact on Windows platforms, which have an even more eccentric interpretation of it than you might imagine. I am quite familiar with how Shift-JIS breaks systems, as it is what gives the phenomenon of incorrect character emission the name mojibake.

[ChrisDenton]

I'm not understanding the relevance of Windows here? In Rust for Windows, OsStr is WTF-8 encoded.

In Windows itself all local encodings are lossily converted to/from UTF-16. I guess the only issue would be reading the stdout of a third party tool that spits out a locale-specific encoding.

[workingjubilee]

@ChrisDenton The only relevance is that to Actually Fix the Issue so that we don't ever emit garbage would require changing the signature of fn error_string. After doing such a significant change, I would prefer to test it on multiple installations of different operating systems. And to do what I would really prefer might require extending OsStr, which I would want to do further testing to validate, again, on multiple platforms.

And I may or may not get around to doing all that.

However, here the standard library is panicking needlessly while trying to construct a mere error report. Emitting junk data is genuinely preferable. "Really fixing it" can be left to future work.

[workingjubilee]

To be clear: Before I ever touched programming, I did localization. If you have questions about fonts, locales, languages, or encodings, I will happily answer them. But I would rather save any such explanations to the comments and commit messages of my intended future PR.

[thomcc]

@workingjubilee asked me to take a look at this, and I think this makes sense given the justification in #99609 (comment), that this is just to avoid panicking and producing a good error message on a best-effort basis.

That said I won't snipe the review from the assigned reviewer, but r=me if they don't want to take it.

[ChrisDenton]

I'll also try to be clear. I do not have a problem with this PR at all. I think it's a good improvement over the current behavior. I would however be much more nervous about a more substantial fix that potentially also affects Windows as the original issue demonstrated a *nix specific problem. But I agree, this conversation can be better had at a more appropriate time.

[thomcc]

@bors r+

[bors]

📌 Commit bcf780e has been approved by thomcc

It is now in the queue for this repository.

[joshtriplett]

Sorry to have missed this. LGTM as well. Thanks @thomcc.

[bors]

⌛ Testing commit bcf780e with merge 8e9c93d...

[bors]

☀️ Test successful - checks-actions
Approved by: thomcc
Pushing 8e9c93d to master...

[rust-timer]

Finished benchmarking commit (8e9c93d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.3%	[-3.3%, -3.3%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.1%	[4.1%, 4.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-3.7%	[-3.7%, -3.7%]	1
All ❌✅ (primary)	-2.4%	[-2.4%, -2.4%]	1

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³