std: Remove index notation on slice iterators

[alexcrichton]

These implementations were intended to be unstable, but currently the stability
attributes cannot handle a stable trait with an unstable impl block. This
commit also audits the rest of the standard library for explicitly-#[unstable]
impl blocks. No others were removed but some annotations were changed to
#[stable] as they're defacto stable anyway.

One particularly interesting impl marked #[stable] as part of this commit
is the Add<&[T]> impl for Vec<T>, which uses push_all and implicitly
clones all elements of the vector provided.

Closes#24791

[breaking-change]

[rust-highfive]

r? @gankro

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

r? @aturon

[alexcrichton]

Note that I figured that the Add impl for Vec was fine because we're likely to accept rust-lang/rfcs#839

[aturon]

@alexcrichton But the main holdup with that RFC remains the Copy/Clone distinction.

I think there are also some other concerns about this Add impl, such as "abuse of operator overloading" (not saying I agree with that, but I've seen it mentioned). I'm a little uncomfortable stabilizing this (and thereby establishing precedent) without more thought.

[aturon]

re: the unstable comment -- should we consider future-proofing this by returning a newtype around unit?

[alexcrichton]

Oh wow I totally missed that! I think that it may actually be appropriate in this case as String::from_str can never fail. It would in theory be more appropriately represented as Result<String, Void>, but adding Void probably isn't super well motivated by this use case.

[aturon]

Right, but this might be one reason to leave it opaque for the time being -- so that if we get Void (which has come up from time to time, and is a tiny API) we could switch over to it.

[alexcrichton]

Hm ok, I've switched it over to an #[unstable]ParseError newtype in the module.

[aturon]

I don't recall where the Add implementations were introduced -- was there an RFC on this topic?

[alexcrichton]

@aturon hm yes those are good points, I would also be fine removing the impl for now. I don't remember an RFC specifically to add them, but rather just fallout of the changes to Add at some point in the past.

[alexcrichton]

I've pushed a new commit which removes the Add implementation for vectors.

[alexcrichton]

@bors: r=aturon b1976f1

[aturon]

Note: @alexcrichton and I discussed this further, and determined that the Add implementations here are de facto stable, and fit the spirit of collections reform (which stabilized similar overloads for sets). So he did a final revision to stabilize these impls, rather than removing them.

[bors]

⌛ Testing commit b1976f1 with merge f6574c5...

[bluss]

@aturon thanks for the info. Shouldn't we have an RFC-style discussion about it?

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

[aturon]

@bluss The sense was that these operators are widely used enough and match the collections reform reasonably enough to be stabilized as-is. (The complaints I've heard about them -- operator "abuse" -- apply equally to the set operations which were approved in the earlier RFC.) We're still working out exactly what requires an RFC, but this simple ungating didn't seem to warrant it.

That all said, if you have objections I would love to hear about your concerns, and we could certainly consider removing the implementations for now (though that would likely break code).

[bluss]

I've had these objections for a long time, and I'm sure I've explained before.

• Concatenation should have its own operator. For example ++ or <>.
  • Certain sequences/containers are mathmatical and will implement + for addition. Using + for concatenation of our vector just confuses this. Compare how + is used for concatenation of python lists while being used for addition of numpy arrays, both very common types. And some I work with have trouble separating the two types (what's the difference between a list and an "array" again?).
  • Using + for concatenation is overloading, but it's overloading one step too far. Not just by extending an operation to another type, but redefining it.
• Note that A concat B != B concat A, another reason + might not be the best symbol for this.
  • In Rust, it's even worse, we don't even implement &[T] + Vec<T> even if Vec<T> + &[T] is implemented. We don't even implement Vec<T> + Vec<T> either! We can't say that Vec implements addition, because it doesn't. It implements using the plus symbol with some specific types, look in the docs to see which (and take deref coercion into account yourself).

[bluss]

cc @bstrie I think you wanted to know about Add for Vec.

[aturon]

@bluss Sorry to make you repeat yourself! I think I must have missed the previous discussion on this topic.

@alexcrichton Perhaps we should consider removing the impls for now, after all, and have them go through an explicit RFC process as @bluss is requesting? I feel like it's a borderline case (came in without an RFC), but it's certainly an optional enough feature that removing it shouldn't cause much pain.

@gankro I'd also appreciate your thoughts here.

[bluss]

I'm not trying to obstruct, just stating my thoughts faithfully. I am sure many are happy with the current status of the +.

[aturon]

@bluss Oh, totally! For my part, it seemed like a pretty minor thing but still better to go through the RFC process if we have doubts.

[alexcrichton]

I'd be fine removing the impl, but I also do not feel strongly one way or another.

[bstrie]

@bluss I'm not necessarily a fan of using + for concatenation, but I think having an operator solely for concatenation is very much overkill. I'd rather simply have a method before having a one-off operator. I'm also not especially bothered by it not being commutative. It's also the most borderline case of operator abuse there is, seeing as how widespread + for concatenation is.

My personal concern is efficiency. If we're going to favor the operation with syntax, then it has to be fast. I don't want this being a perf footgun that we have to tell people to avoid.

[bstrie]

Does decision on rust-lang/rfcs#839 need to happen before 1.0?

[llogiq]

@bstrie Please note that vector addition means something very different than vector concatenation.

It could very well be argued that a + b for a : &[T], b : &[T] where T : Add<T, Output=T> should add the vectors element-wise. At least that fits the mathematical definition of vector addition.

Languages like php, javascript, python and java which conflate addition and concatenation all have some gotchas related to it. Lua has a '_' operator for concatenation (which we unfortunately cannot copy), while PL/SQL uses '||' (which unfortunately is hardwired or, so we cannot overload it). ++ already has precedent in C/C++ as an pre-/suffix, so using it for concatenation would probably surprise those versed in C/C++. Excel formulas have '&', which to me looks like a possible good solution.

I quite like the idea of a concatenation operator. Especially with string handling, which is quite a common task, it could simplify things a bit. However, I feel that the choice of '+' is a poor one, and a better choice will need more thought.

[bstrie]

@llogiq Overloading & to mean both "bitwise and" and "vector concatenation" is no more defensible. :P Might as well just use a method instead of introducing cryptic operators that have no precedent in any other language.

[llogiq]

@bstrie yes agreed, though since bitwise and isn't used as often as arithmetic addition, it perhaps carries less connotations. As I said, finding a good solution needs more thought.

If we don't want to overload an existing operator, possible candidates include ℅, ~~, ^^, ++, --, **, <>, ><, ~ (as infix)

Candidates for overloading are &, |, .., << (C++ anyone) and a lot of others.