-
Notifications
You must be signed in to change notification settings - Fork 3
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
std::path::Component makes platform specific assumptions; cannot be extended #18
Comments
Excellent concrete case to bring up. This enum is already a bit silly cause some variants only come at the beginning of the path and some only in the middle. This means any use of the components iterator will probably blow up in absurd cases anyways, so exhaustiveness of the enum is not that important. I'd deprecate it and make something wholly new and better. Relatedly, I think the portability lint should effect enum exhaustiveness like so: enum Foo {
#[cfg(x)] Bar,
#[cfg(or(x,y))] Baz,
}
#[cfg(x)]
fn ok_for_x(a: Foo) {
let Bar(b) = a;
}
#[cfg(or(x,y))]
fn ok_for_either(a: Foo) {
match a {
Bar(_) => (),
Baz(_) => (),
}
}
#[cfg(x)]
fn bad_for_either(a: Foo) {
let Bar(b) = a;
} of course, this is only sound if the portability lint is an error rather than warning, so it will be a while before we can do such things. |
Another issue: what if I'm on Linux (or whatever) and I want to work with Windows-specific paths? (Or the other way around) |
@jethrogb That's valid too, though to be clear this already fails for that because the |
Ran into this with the Nintendo 3DS of all things. The homebrew lib I wrap over for out-of-tree |
Related issue: rust-lang/rust#52331 |
I think there are a few ways to fix this.
I'm thinking perhaps the best choice here is to deprecate without replacement, while leaving the option to add a new API in the future if anyone can think of a sane one and there seems to be enough demand to include it in The cost of deprecating probably isn't major, because I don't think it's used much. Is there a good way to determine/estimate what percentage/number of If it is used more commonly than I expect but users only need strings for the components, then I might change my preference to option 5. Edit: On the other hand, accessing path components seems like a pretty fundamental feature of a path type. But the most common cases are handled by other methods, and I expect very few crates (if any) individually handle each variant of Edit: Added option 6. |
Please no. Components is a perfectly reasonable API, and at least I personally have used them multiple times to, for instance, inspect if a path starts with a Root or a Prefix, and if so pop said prefix in an idiomatic/functional way. It's a really nice API whose only problem is that it makes overeager assumptions about what a Path should look like with no path for extensibility. It's worth noting that, for the specific case of Redox laid out in rust-lang/rust#52331 (which I have a specific interest in since I'm maintaining an std port with a platform that has a similar path system), kennytm proposed a design that is not a breaking change and works OK, though it does slightly overload what a Prefix is. It's what I ended up doing in my STD fork. |
Fair enough. I'm probably underestimated the utility of the function, and I suppose it is necessary to have some kind of API for this. The fundamental issue here is that
An issue I have with @kennytm's suggestion is the deprecation only on Redox. It also affects other potential platforms, and it is also desirable to warn developers about the portability issue even when they're not compiling for such a platform. And is deprecation warnings for just one target something Rust currently does? It seems like an odd thing to do generally, even if it's technically possible. |
I would understand that a deprecation warning done like that to be an indication that something needs to be done better in terms of organization, rather than just a stopgap telling people not to use a feature because it isn't applicable on a certain platform. However, it would certainly work in the short term. For a more permanent fix, given that the design is not suitable for parsing paths for other OS's, I'd recommend modifying |
The
Component
enum includes aPrefix
variant that only appears on Windows, which is not ideal. Optimally, OS specific functionality should not be exposed on other OSs. Worse, operating systems with paths that are noticeably different from Unix and Windows cannot be represented by this enum.This has been an issue for Redox OS, which includes "schemes" in paths, like
file:/path/to/file
andsys:uname
. We haven't been able to add a variant to the enum, since this would be a breaking change (by breaking exhaustive match statements).Resolving this without breaking backwards compatibility is problematic. Making the enum non-exhaustive and adding new variants presumably would not possible for a future edition of Rust (it would break the ABI of the enum, and there is no sane way to automatically fix exhaustive matches).
I guess that just leaves deprecating
Component
, and adding a better alternative alongside it? This does seem like a genuine portability issue, and I'm sure Redox is not the only OS that would run into difficulty here, though I can't immediately think of any others.The text was updated successfully, but these errors were encountered: