Implement Add for String + String

[iopq]

I know some people don’t like the fact that + is overloaded for concatenation. But since String + &str is already implemented, I think that ship has sailed. It is no longer possible to take this out and add another concatenation operator.

I propose that Add be implemented for the String + String case as well. It might not be a big deal in most cases like a + &b - this is not too much extra typing. Where it’s more annoying is in cases like .fold1(String::add) with fold1 from itertools which doesn’t work. Here typing .fold1(|x, y| x + &y) is a lot longer and more complicated. I went ahead and just did this:

pub trait Concatenable {
    fn concat(self, other: Self) -> Self;
}

impl Concatenable for String {
    fn concat(mut self, other: String) -> String {
        self.push_str(&*other);
        self
    }
}

so I could do .fold1(String::concat) whenever I want to. This is much more readable, it has three parts, one of which is a path separator. On the other hand, the closure has 9 separate elements that I have to read, so I have to slow down and check that it does what I think it does.

I think there is no downside to actually doing this and the implementation is pretty trivial. Are there any arguments against it?

[DanielKeep]

Just because someone made a mistake once doesn’t mean it’s OK to further compound it.

If you want concatenation, argue for concatenation. Don’t misappropriate an operator to mean something different. That’s how you end up with iostreams, and I hope no one wants Rust to end up looking like that. Next you’ll be wanting to use a-b for a.replace(b, "") and % for &a[a.len()-b..]. Do you see how slippery this slope is? Even crazy people with mountain climbing gear wouldn’t go near it.

Having an actual concatenation operator opens the door to also concatenating Vecs and arrays and iterators and tuples and probably other things, too. It could be used in pattern matching.

I am, have been, and always shall be vehemently against misusing + for concatenation.

[iopq]

Too late, it’s already in the language. Can’t take it out anymore, just make it have better ergonomics. I don’t get the slippery slope argument, I just want something that’s already in stable Rust, but I want it to apply to an additional type.

[DanielKeep]

Too late, it’s already in the language. Can’t take it out anymore, …

We can’t take it out, but that is in no way an argument for expanding it to more types.

I don’t get the slippery slope argument, …

It’s called “being self-aware that the point being argued is not nearly important enough to warrant one’s emotional reaction by highlighting how it is unlikely to extend beyond the specific case being discussed.”

See also: sarcasm.

… I just want something that’s already in stable Rust, but I want it to apply to an additional type.

I don’t want to give people any more reasons to use + on strings, irrespective of context. I want it to be painful to use so we’re more likely to get a fundamentally better replacement.

[leonardo]

I want it to be painful to use so we’re more likely to get a fundamentally better replacement.

I think the ~ of D language is better for a noncommutative operation, but given the design mistake of using + for String and &str, I think we have already lost the battle.

[oli-obk]

we can easily add some code to deprecate trait impls and move forward with a proper concatenation strategy.

[rkruppe]

Regardless of how it’s spelled (+ or ~ or method call or …), I have some reservations about concatenating String with String. In programs of this form:

// concatenate s1 and s2
// keep using s2

… it is preferable to use String + &str and simply reuse s2, rather than creating an explicit clone of s2 before concatenating. An overload for String + String makes it easier to unthinkingly do the pointless clone. Now, if one ever needed to do String + String (e.g. for performance reasons), this downside would not be a blocker, but since it’s apparently just an ergonomics issues, it is a trade off.

[stebalien]

Related: https://github.com/rust-lang/rust/pull/36430

[llogiq]

Note that I didn’t suggest adding String to Cow yet (although that could be a logical next step). Perhaps more importantly, if we make Cow have better ergonomics than String, more people will use it?

[stebalien]

I know. I’m just saying it’s related.

[iopq]

Is it possible to add a lint to this behavior saying you should do s1 + &s2 if you want to keep using s2?

[llogiq]

Sure, but what for? You’d need to actively .clone() the second String anyway, or the best thing the lint would do is improve on the error reporting the borrow checker gives anyway.

[iopq]

It should warn if you’re clone()ing things that don’t need to be cloned. I’ve seen a lot of clone() abuse by beginning Rust programmers that can’t get their program to compile otherwise.

[withoutboats]

This is not a great way to communicate! Your opinion is that the + operator shouldn’t be overloaded for the concatenation operation, but many people disagree. I recommend trying to express subjective ideas with a subjective tone - like “I think the concatenation add for String that already exists is a mistake, and I think adding this would further compound the problem that it presents.”

The choice to express yourself as if your position is inarguably objective may not seem like a big deal, but it’s the accumulation of small tone choices that push the community toward toxic acrimony.

[withoutboats]

I like concatenative add, but I also don’t like adding this impl for the reason @rkruppe points out.

Exact messaging could be improved, but if you String + String right now, what the compiler can suggest is that you reference the second String. But if you add String and then use the second String again, getting use after move errors, there’s a non-trivial chance the user will solve the problem with string + string.clone() instead of string + &string.

[zepto]

In Erlang, the append operator for “strings” is ++ not + and so is unambiguous and can be caught at compile time. Worth considering?

[iopq]

Only if the Rust team wants to deprecate the Add impl on String + &str

Edit: I can’t get spaces to work around the word on

Otherwise it’s pointless

[zepto]

@iopq - pointless is a bit strong - what if it proves truly a worthwhile avenue to explore? I’d rather respond to a fault in the idea than some restriction in a current implementation.

[EdorianDark]

I think Add should not be implemented for String. String + &str should be depricated.

[nikomatsakis]

If the only point is to guide people away from doing a clone, perhaps we should consider using a lint for that purpose. i.e., I think we could detect common scenarios where people are cloning when a reference would have done.