Tracking issue for `slice_concat_ext` stabilization · Issue #27747 · rust-lang/r...
source link: https://github.com/rust-lang/rust/issues/27747
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
The SliceConcatExt trait offers methods concat and join on slices. For somewhat technical reasons, it wasn't possible to make these inherent methods.
The methods themselves are stable, but the trait isn't. However, since the trait is in the prelude, the methods are usable in stable today.
Ideally, the methods would also be available on iterators, but there are performance ramifications for doing so. Impl specialization may help.
Having just run into needing this today it would be great to see the documentation for this referenced in the Vec docs with some examples
@aturon just reading along, and I'm curious if you have time to explain: What are the technical reasons you mentioned at the top? Is this related to why the SliceExt trait exists?
So, the main problem with moving to inherent methods at this point is that the traits provide something akin to method overloading -- both apply to slice types varying only by some bounds (Clone and Borrow). I think originally they were less general, but there was some other holdup to making them inherent, which I no longer recall.
re: SliceExt: the reason for that trait is somewhat obscure. Basically, the std crate ends up re-exporting a number of things from the core crate, but in the case of types like slices, it also wants to add methods of its own. We set things up so that the set of inherent methods for any type in std has to be defined in exactly one of the crates making up std. So core's slices don't have any inherent items, because they're defined in collections. You can find the impl here: http://static.rust-lang.org/doc/master/src/collections/slice.rs.html#160
If you're just using std, you never need to see/work with SliceExt. But if you're using core, the SliceExt trait is how we provide any methods on slices. It's in the core prelude, so you generally don't notice the difference.
... Hope that helps!
Good to know :)
It's kinda annoying to be able to join strings with a str (which can have multiple chars), but joining a slice of slices, you can only join with a single element.
let v = vec!["1", "2", "3"];
v.join(", ")works, but
let v: Vec<&[u8]> = vec![b"1", b"2", b"3"]; v.join(b", ") // Error: expected u8, found array of 2 elements v.join(&b',') // works, but is missing the space char
doesn't work
I tried making the methods inherent with separate inherent impls. But we can't have multiple inherent impls for a primitive. I got the error:
"only a single inherent implementation marked with #[lang = "slice"] is allowed for the [T] primitive"
This is known of course, the issue is #32631.
@leodasvacas I think you mean multiple impl Type {…} blocks, but that’s not the main issue. We can add new methods in the existing impl blocks. However in this case, for example <&[String]>::concat and <&[Vec<T>]>::concat end up being the same method, so a helper trait would still be needed to generalize over both cases.
The libs team discussed this and consensus was to add #[doc(hidden)] to the deprecated (but stable) connect method, but make no further changes or stabilize anything at the moment. Inherent methods with a new helper trait doesn’t seem much better than the current trait methods, and still wouldn’t help with joining/concatenating iterators without first collecting into an intermediate Vec.
Design proposals to generalize this functionality to iterators are welcome.
The methods themselves are stable, but the trait isn't. However, since the trait is in the prelude, the methods are usable in stable today.
Does this mean concat() isn’t usable in a no_std module with explicit std imports and without the slice_concat_ext unstable feature enabled?
@sanmai-NL That is correct. (Modulo #15702, which is a bug and not considered part of the stability promise.)
Does there exist a path to stabilizing SliceConcatExt? Are there any specific outstanding issues associated with it? The specific use case I have for it is that I'd like for it to be able to work on other string types. e.g.,
impl<S: Borrow<BStr>> SliceConcatExt for [S] {
type Output = BString;
// ...
}The work-around is to define a separate trait that is probably identical to SliceConcatExt, but in my crate. But that means users won't benefit from having it in the prelude.
Ping @aturon
Is there a reason this trait isn't implemented for OsString and CString?
Is there any stable way to use join in a no_std enviroment using alloc?
@nico-abram I believe there is none today, since liballoc does not have a prelude that gets implicitly imported.
I a previous comment, I wrote:
We can add new methods in the existing impl blocks. However in this case, for example
<&[String]>::concatand<&[Vec<T>]>::concatend up being the same method, so a helper trait would still be needed to generalize over both cases.Inherent methods with a new helper trait doesn’t seem much better than the current trait methods, and still wouldn’t help with joining/concatenating iterators without first collecting into an intermediate Vec.
This is however an improvement in that the new helper trait does not need to be in scope for the inherent methods to be used. In particular, it does not need to be in the prelude.
#62403 implements this.
impl<S: Borrow<BStr>> SliceConcatExt for [S] { type Output = BString; // ... }
@BurntSushi, impl coherence rules do not allow this impl outside of the crate that defines the trait:
error[E0210]: type parameter `S` must be used as the type parameter for some local type (e.g., `MyStruct<S>`)
You would need separate impls for [BStr], for [BString], etc.
But that means users won't benefit from having it in the prelude.
FWIW, not having this benefit is rather typical of functionality defined outside of the standard library.
The API is now:
Stable: (with connect deprecated)
impl<T> [T] {
pub fn concat<Item: ?Sized>(&self) -> <Self as Concat<Item>>::Output
where Self: Concat<Item>
{
Concat::concat(self)
}
pub fn join<Separator>(&self, sep: Separator) -> <Self as Join<Separator>>::Output
where Self: Join<Separator>
{
Join::join(self, sep)
}
pub fn connect<Separator>(&self, sep: Separator) -> <Self as Join<Separator>>::Output
where Self: Join<Separator>
{
Join::join(self, sep)
}
}Unstable:
/// Note: the `Item` type parameter is not used in this trait,
/// but it allows impls to be more generic.
pub trait Concat<Item: ?Sized> {
type Output;
fn concat(slice: &Self) -> Self::Output;
}
pub trait Join<Separator> {
type Output;
fn join(slice: &Self, sep: Separator) -> Self::Output;
}
impl<T: Clone, V: Borrow<[T]>> Concat<T> for [V] {
type Output = Vec<T>;
fn concat(slice: &Self) -> Vec<T> { … }
}
impl<T: Clone, V: Borrow<[T]>> Join<&T> for [V] {
type Output = Vec<T>;
fn join(slice: &Self, sep: &T) -> Vec<T> { … }
}
impl<T: Clone, V: Borrow<[T]>> Join<&[T]> for [V] {
type Output = Vec<T>;
fn join(slice: &Self, sep: &[T]) -> Vec<T> { … }
}
impl<S: Borrow<str>> Concat<str> for [S] {
type Output = String;
fn concat(slice: &Self) -> String { … }
}
impl<S: Borrow<str>> Join<&str> for [S] {
type Output = String;
fn join(slice: &Self, sep: &str) -> String { … }
}The above have the same signatures as corresponding trait methods. They mostly exist so that traits don’t need to be in scope for the methods to be used.
Possible to add, but causes inference regressions:
impl<S: Borrow<str>> Join<char> for [S] {
type Output = String;
fn join(slice: &Self, sep: char) -> String { … }
}I’m wondering about the fact that in Rust you can’t do:
let s = ['a', 'b'].join(", ");But it works with itertools’ join().
itertools’s join is very different: it relies on the Display trait and always returns String. So it can’t support cases where [T]::join returns a Vec.
That said, it might be possible to add:
impl Join<&str> for [char] {
type Output = String;
fn join(slice: &Self, sep: &str) -> String { … }
}If you’re interested in pursuing this, feel free to submit a PR that adds this impl and request a Crater run to look for potential inference regressions.
Well I’m getting that
error[E0119]: conflicting implementations of trait `slice::Join<&str>` for type `[char]`:
--> src/liballoc/str.rs:94:1
|
85 | impl<S: Borrow<str>> Join<&str> for [S] {
| --------------------------------------- first implementation here
...
94 | impl Join<&str> for [char] {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `[char]`
|
= note: upstream crates may add a new impl of trait `core::borrow::Borrow<str>` for type `char` in future versions
So it maybe not possible? I guess I should I take this somewhere else if it is.
Interesting, I would have thought those wouldn’t conflict since char does not implement Borrow<str>. It suppose this is because Borrow is defined in libcore and so is a "foreign" trait for liballoc which is therefore not allowed to assume that impl Borrow<str> for char won’t be added in the future.
So yeah, it looks like this impl is not possible without changing trait coherence rules.
Just a thought. Would it be possible to work in something like:
format!("Here we are: {}", big_array.lazy_join(", "));where lazy_join returns a impl Display?
Yeah that sounds possible. It’s a different feature though. And one you could implement on crates.io with an extension trait.
Team member @Amanieu has proposed to close this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
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.
Note that AFAIK, this is still an issue: #27747 (comment)
Note that AFAIK, this is still an issue: #27747 (comment)
I'm confused by what you mean @BurntSushi. Isn't that impl still an orphan rules violation as pointed out in #27747 (comment)? What's the issue and how does this impact the FCP?
@yaahc Ah sorry, yeah, I forgot about that. Yeah, regrettably, I agree to close this. It would be nice to improve things here, but it looks like a bigger issue than I thought and isn't going to be solved by this trait alone.
Copy link
rfcbot commented 10 days ago
This is now entering its final comment period, as per the review above.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK