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.

Sign up for GitHub

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

AsRef and related conversions for CString #30616

Merged
merged 3 commits into from
Dec 31, 2015
Merged

AsRef and related conversions for CString #30616

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43ab6c7
AsRef and related conversions for CString
arcnmx Dec 29, 2015
965556d
impl From<&CStr> for CString
arcnmx Dec 31, 2015
53878e7
CStr impl stability
arcnmx Dec 31, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions src/libstd/ffi/c_str.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use iter::Iterator;
use libc;
use mem;
use memchr;
use ops::Deref;
use ops;
use option::Option::{self, Some, None};
use os::raw::c_char;
use result::Result::{self, Ok, Err};
Expand Down Expand Up @@ -282,7 +282,7 @@ impl CString {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Deref for CString {
impl ops::Deref for CString {
type Target = CStr;

fn deref(&self) -> &CStr {
Expand Down Expand Up @@ -522,6 +522,37 @@ impl ToOwned for CStr {
}
}

#[stable(feature = "cstring_asref", since = "1.7.0")]
impl<'a> From<&'a CStr> for CString {
fn from(s: &'a CStr) -> CString {
s.to_owned()
}
}

#[stable(feature = "cstring_asref", since = "1.7.0")]
impl ops::Index<ops::RangeFull> for CString {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should implement indexing and slicing with other kind of ranges as well then. Seems strange to only support slicing by RangeFull. Also, these should probably be implemented on CStr and relied on Deref to be found?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OsString does the same thing, though that's intentionally an opaque type so the situation is slightly different in that slicing it is actually "impossible"...

I'm iffy on whether we should allow slicing into a CStr, just because it doesn't seem like a very common operation. It's also not necessarily clear whether you actually want to slice as_bytes() or as_bytes_with_nul(), and &cstr[..1] is impossible to implement without panicking if it doesn't cover the entire string. It also hides the cost of the strlen which is supposed to be necessary in the future... I'm not entirely opposed to it though!

This impl was meant as a conversion trait (the good old &deref/&index[..]/AsRef/.as_slice() bunch), the main advantage being that it isn't type-ambiguous. The discussion at #27729 might be a bit relevant there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index<RangeTo> and Index<Range> can't really work, you would have to insert a 0 byte in the middle of the string. And implementing only Index<RangeFrom> also seems strange (though potentially useful).

type Output = CStr;

#[inline]
fn index(&self, _index: ops::RangeFull) -> &CStr {
self
}
}

#[stable(feature = "cstring_asref", since = "1.7.0")]
impl AsRef<CStr> for CStr {
fn as_ref(&self) -> &CStr {
self
}
}

#[stable(feature = "cstring_asref", since = "1.7.0")]
impl AsRef<CStr> for CString {
fn as_ref(&self) -> &CStr {
self
}
}

#[cfg(test)]
mod tests {
use prelude::v1::*;
Expand Down