Implement Add for String + String

[ehiggs]

Maybe we just need to make it easier to find those and know how to use them?

Yes! Maybe even just aliasing them as StringConcatExt ans re-exporting it under std::string::...

[quote]let foo = collection_of_strings.into_iter().filter(no_russian).join(", "); [/quote] That helps in the case of collecting a bunch of strings using iterators. But if someone is looking to build a string they might not think to call iter and then look for various extension traits for what they want. When I search for string in the docs, I don’t see anything about SliceConcatExt. I had originally meant that a StringBuilder could also, as a nice to have feature, work with iterators. But that’s gravy after it’s been discovered.

[zackw]

I wrote:

let foo : String = collection_of_strings.into_iter().filter(no_russian).collect();

I thought I’d look into making this work, and I discovered that it already works. There are FromIterator<T> for String impls for the obvious string classes.

fn main() {
  let v = vec!["foo", "bar", "baz", "quux"];
  let s : String = v.into_iter().filter(|s| s.starts_with("b")).collect();
  println!("{}", s);
}

compiles and runs and prints “barbaz”, as expected. So we just need join-with-separator now.

[ehiggs]

@bluss has this in his rust-itertools crate.

https://docs.rs/itertools/0.5.9/itertools/fn.join.html

use itertools::join;

assert_eq!(join(&[1, 2, 3], ", "), "1, 2, 3");

But it’s not in std.

[bluss]

Does it have a chance to go into libstd? It’s a bit fuzzier than std’s usual since it accepts anything that implements Display. (And Iterator is in libcore, so it can’t be an iterator method).

[NN1]

What is the conclusion here ? AFAIU String + &str is going to be deprecated in the future and other concatenation options such as String + String or &str + String will be never implemented. Is it correct ?

[H2CO3]

I see no indication or intent of String + &str being deprecated.

[NN1]

And the other options will not be implemented , right ? Personally I don’t see any issue with String + String, &str + String. This would definitely make newcomers happier and there is no performance degradation here unless you write explicitly clone() or anything like that.

[withoutboats]

My opinion now, in contrast to two and a half years ago, is that we should add this impl to make things easier for people.

[ekuber]

@withoutboats I think we already have our cake (have obvious allocations around String/&str) and eat it too (be friendly to newcomers):

error[E0369]: binary operation `+` cannot be applied to type `&str`
 --> src/main.rs:2:13
  |
2 |     let _ = "" + "";
  |             ^^^^^^^ `+` can't be used to concatenate two `&str` strings
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
  |
2 |     let _ = "".to_owned() + "";
  |             ^^^^^^^^^^^^^

error[E0308]: mismatched types
 --> src/main.rs:3:29
  |
3 |     let _ = String::new() + String::new();
  |                             ^^^^^^^^^^^^^
  |                             |
  |                             expected &str, found struct `std::string::String`
  |                             help: consider borrowing here: `&String::new()`
  |
  = note: expected type `&str`
             found type `std::string::String`

error[E0369]: binary operation `+` cannot be applied to type `&str`
 --> src/main.rs:4:13
  |
4 |     let _ = "" + String::new();
  |             ^^^^^^^^^^^^^^^^^^ `+` can't be used to concatenate a `&str` with a `String`
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
  |
4 |     let _ = "".to_owned() + &String::new();
  |             ^^^^^^^^^^^^^   ^^^^^^^^^^^^^^

It explains the reason for this not being supported while giving the user a clear avenue to fix it. It simultaneously helps get over the bump and expands the understanding of the underlying language. I think changing the ergonomics of the language by adding Add<String> to the language would be worth it only if we could simultaneously introduce lints to detect unnecessary allocations.

[scottmcm]

would be worth it only if we could simultaneously introduce lints to detect unnecessary allocations.

I’m not convinced. The a.to_owned() + b recommendation is already an unnecessary allocation (compared to, say, [a, b].join("")).

Supporting a + b can even reduce allocations, because if it’s changed to a + &b and b actually had enough capacity for the combined string, it’s allocating for no reason.

[ekuber]

Supporting a + b can even reduce allocations, because if it’s changed to a + &b and b actually had enough capacity for the combined string, it’s allocating for no reason.

We could have impl Add<String> for &'a str or similar to handle that case. The other two cases (&str + &str and maybe String + String) I think it is reasonable to raise rustc's hands in the air and say “you deal with it, what did you really want?”.

[petrochenkov]

FWIW, C++ has all four overloads of operator + (analogous to &str + &str/&str + String/String + &str/String + String) to manage allocations optimally in each case.

[withoutboats]

String + String would not allocate. There would still be an obvious allocation if a user cloned the second string instead of referencing it.

[H2CO3]

Doesn’t a.to_owned() + b reuse the buffer of the temporary a.to_owned() by reusing its buffer? In that case, how is it an unnecessary allocation?

[scottmcm]

Doesn’t a.to_owned() + b reuse the buffer of the temporary a.to_owned() by reusing its buffer? In that case, how is it an unnecessary allocation?

Because a.to_owned() doesn’t have any excess capacity, so it’s one allocation for the to_owned, and then another when reserving enough space for the addition.

Whereas [a, b].join("") is one allocation, since it pre-computes the capacity.

[dhm]

When I search for string in the docs, I don’t see anything about SliceConcatExt

@ehiggs I wish the * -> RetType search pattern was better documented, it’s one “hidden” gem of the rust docs: example for your case (slice::join and slice::concat appear among the first 10 results).

Regarding overloading Add, I agree with @DanielKeep on this one: since adding chars is not very intuitive, adding sequences of chars is not very ambiguous, thus people can see the appeal of it. But quid of Vec<char>? And Vec<u8>? And Vec<T>? I agree it’s a slippery slope and only a matter of time before those are requested too. But when T : Add, the “sum” of two sequences of T is now ambiguous.

The solution? Do have a fn (String, String) -> String operation (this would solve OP’s use case, for instance), but do not call it Add::add. If you want sugar, other sigils can be used (someone mentioned ~, Ocaml uses @ for list concatenation, but my favorite sugar would be abstracting .. with a trait that remains compatible with the current fn (Idx, Idx) -> Range<Idx> behavior, but that would also allow using it for more general concatenations). But please do not use addition notation.

[Tom-Phinney]

One obvious analogy is matrix multiply, where the Mul trait conceptually applies to both pairwise element-by-element multiplication of two identically-sized matrices and to the cross-product of the two matrices. Both operators return a third identically-sized matrix. However, overloading Mul for both purposes so that the convenient sigil * could be used would lead to ambiguous code.

[soc]

I concur.

Having only + available leads to people misusing it to mean both “add this single thing” and “add the multiple things contained in this thing”, which has been a pain in every language that tried it. (Kotlin is a good example of that.)

Let’s

• use + to only mean “add this single thing” (addition)
• use ++ to only mean “add multiple things” (concatenation)
• deprecate the current + on strings

[mjbshaw]

use ++ to only mean “add multiple things” (concatenation)

That has already been shown to not be backwards compatible.

I would love to have + for String + String. I don’t want new sigils, though. Rust has enough. If you wanted to concatenate multiple Strings (not just &str, which can already be done), it might be doable with a new trait (e.g. TupleConcatExt) that has a concat() method for tuples where each element of the tuple satisfies Borrow<str> or something like that (I haven’t thought through this, it might be a terrible idea).

But multi-string concatenation is only tangentially related to + for String + String. I hope we can get String + String without being paralyzed by fear of people writing suboptimal String + String + String code.

[repax]

Addition is commutative, but concatenation is not. They’re also really not the same thing imho.