Implement Deref<Target=ByteStr> for CStr #138498
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This is a really interesting idea. One potential point of concern: the deref to This is probably fine, and I would hope that any FFI cases that require the NUL use |
|
@joshtriplett sounds reasonable, except I'm not sure where to document this. I can't document it on the trait impl. Would it go in the |
|
I think it would go on CStr. Even before BStr is stable, the resulting transitive Deref to a slice will work, and that merits the documentation. |
This produces a deref chain of `CStr` -> `BStr` -> `[u8]` which is present in the Rust-for-Linux analogues of these types. Add `AsRef<ByteStr>` as well. Link: rust-lang#134915 Link: Rust-for-Linux/linux#1075 Link: https://lore.kernel.org/all/20250221142816.0c015e9f@eugeo/ Link: Rust-for-Linux/linux#1146
|
@joshtriplett done. |
|
Looks reasonable. Since trait impls are insta-stable, starting an FCP on this: @rfcbot merge |
|
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
@rfcbot concern &cstr as thin ptr Afaik, there is still a plan to make &CStr a thin pointer in the future. If we do that, then a deref to [u8] would mean counting the length (strlen) every time. |
|
@m-ou-se That's a really good point. That is enough overhead that we shouldn't have this Deref at all if we're going to change CStr to a thin pointer. |
|
@m-ou-se @joshtriplett that overhead is probably significant enough that even |
|
Yes. That's why CStr::to_bytes() is called "to" not "as". |
|
This isn't the first case where we've had interesting work gated on whether we're actually going to make CStr a thin pointer. It seems like we need to evaluate the decision about whether we're ever going to do that and want to reserve the ability to do so. (Other alternatives include making a separate type for a We should talk about that at a future libs-api meeting. |
The compiler does not gate this correctly: Trait impls are currently not checked for stability and this impl will allow access to use std::ffi::CStr;
fn foo(s: &CStr) {
let bstr = &**s;
println!("{bstr}"); // accesses `ByteStr as Display` without a feature gate!
}
fn main() {
foo(c"cstr");
} |
|
@lukas-code Trait impls aren't able to be feature-gated in general, and it's already established that feature-gating a type doesn't prevent you from getting a value of that type if a function returns it, it just prevents you from naming the type (e.g. to construct it). |
|
All Could we instead unstably add |
|
Apologies for letting this languish; we certainly could add |
This produces a deref chain of
CStr->BStr->[u8]which ispresent in the Rust-for-Linux analogues of these types.
Link: #134915
Link: Rust-for-Linux/linux#1075
Link: https://lore.kernel.org/all/20250221142816.0c015e9f@eugeo/
Link: Rust-for-Linux/linux#1146
r? @joshtriplett
cc @nbdd0121
I'm not entirely sure what it means for a stable type to deref to an unstable one - hopefully the compiler does the right thing and gates this impl on all the items being available.