[draft] Store the path in io::Error without extra allocations.

[m-ou-se]

This stores the path of the file/directory in io::Error, without doing any allocations and without increasing the size of io::Error.

We need to allocate for doing a syscall on a path anyway, so this just keeps that allocation around in the io::Error if the syscall fails.

This implements it only on unix. But it can work similarly on other platforms as well.

It does not store the path for the rename and (sym)link operations, as they involve two paths, and you can't really tell which of the two paths caused the error.

This only shows the path in the Debug implementation of io::Error. It's not clear what else we should do with it. Putting it in Display will make many programs show the path twice, and have less control over the output format. Adding a .path() accessor to io::Error is also not great, because it'll have to be a Option, and won't be accessible through the Error triat.

[m-ou-se]

cc @yaahc

[m-ou-se]

@CryZe Since you reacted with a 👎, I'd be curious to hear your thoughts on this.

[yaahc]

@CryZe I will note that mara has only added the path info to the Debug format, not to Display, precisely because people already add the path at a higher level where they have that context on the semantics of it's usage. Does this change your reaction?

[CryZe]

Yeah I guess that may be fine then.

[bors]

☔ The latest upstream changes (presumably #87329) made this pull request unmergeable. Please resolve the merge conflicts.

[sunfishcode]

It's possible that in the future, std could be optimized to avoid allocing the path buffer in common cases, using a technique like this which can be about 2.5% faster in microbenchmarks that include the system call. If Rust does that, would it still be ok allocing in the error path to continue to support storing the path in the Error?

[thomcc]

Since it was brought up to me, #87869 and this aren't really incompatible beyond it needing to allocate in the 64 bit case (on 32 bit targets it would be almost identical).

(IMO that's probably fine, since this is in the error case, and that having a smaller io::Error optimizes the success path more than allocating in the error case would pessimize anything)

[ben0x539]

When making the OsPathBuf allocation, can't you just leave some room for the error kind field in there, even if it's not needed in the success case? (With 64 bit, maybe half of the string length usize unless we assume path names can be too long for u32 length field?)

[ben0x539]

It does not store the path for the rename and (sym)link operations, as they involve two paths, and you can't really tell which of the two paths caused the error.

Can we store both of them, like, allocated contiguously?

[saethlin]

@sunfishcode The link in your comment above is now broken, at a guess because it wasn't a permalink that included the git commit. Did you mean this: https://github.com/bytecodealliance/rustix/blob/e1ec8abf106e976bc05ce45d308602728e84ccf3/src/path/arg.rs#L933-L953

[sunfishcode]

@saethlin Yes; that's the right link. But also, there is now a PR implenting this in Rust, which may land soon: #93668

[Dylan-DPC]

Closing this as inactive and was still a draft.