RFC: cmp and ops reform

[aturon]

This RFC proposes a number of design improvements to the cmp and
ops modules in preparation for 1.0. The impetus for these
improvements, besides the need for stabilization, is that we've added
several important language features (like multidispatch) that greatly
impact the design. Highlights:

• Make basic unary and binary operators work by value and use associated types.
• Generalize comparison operators to work across different types; drop Equiv.
• Refactor slice notation in favor of range notation so that special
  traits are no longer needed.
• Add IndexSet to better support maps.
• Clarify ownership semantics throughout.

Rendered

[japaric]

Shouldn't Self be String (in which case, this would be append) or &'b str (I thought we preferred &str over &String were possible) here?

[Kimundi]

Huge +1 from me, especially with the stuff about merging index and slicing. Having them implemented as the same trait via multidispatch and a range operator makes stuff so much more modular.

[japaric]

Shouldn't be for Sized?, otherwise we won't be able to implement it for [T].

Also, are we going to remove the built-in indexing support for slices (i.e. slice[0] works but [T]/&[T] doesn't implement the Index trait), and replace it with impl Index<uint> for [T]?

[aturon]

I've pushed an update addressing most of the typos/questions people had so far.

@sfackler, yes, this RFC is a good opportunity to nail down those invariants. I'll add that shortly.

[nikomatsakis]

On Tue, Nov 04, 2014 at 12:01:29PM -0800, Aaron Turon wrote:

I've pushed an update addressing most of the typos/questions people had so far.

@aturon: Just for the record, here are some things I noted, some of
which I have communicated to you via IRC.

• What happens if a type implements IndexMut but not IndexSet? We
  could potentially "fallback" but in general type inferencer works
  more smoothly if we can unilaterally use a trait and not have to do
  "probing". I'd prefer to just use IndexSet consistently.
  Conceivably we could make IndexMut : IndexSet, I don't know of any
  reason that you might implement IndexMut but not IndexSet since
  there is at worst a trivial desugaring.
• Do we want to include expression forms of ... (inclusive range)?
• There are of course three Fn traits. We should discuss the precise
  1.0 stability story there, but what you wrote is fine for now I think.

[Gankra]

@nikomatsakis A Bitv can implement IndexSet, but not Index or IndexMut. You can't get references to its contents (since they're internally just bits).

derp

[aturon]

@nikomatsakis

What happens if a type implements IndexMut but not IndexSet? We
could potentially "fallback" but in general type inferencer works
more smoothly if we can unilaterally use a trait and not have to do
"probing". I'd prefer to just use IndexSet consistently.
Conceivably we could make IndexMut : IndexSet, I don't know of any
reason that you might implement IndexMut but not IndexSet since
there is at worst a trivial desugaring.

I'm in favor of IndexMut: IndexSet as the simplest way out of this question. Anyone see a problem with that approach?

Do we want to include expression forms of ... (inclusive range)?

Seems reasonable. Of course we can add this backwards-compatibly. Somewhat subtle notation, but it's hard to do better.

There are of course three Fn traits. We should discuss the precise
1.0 stability story there, but what you wrote is fine for now I think.

Yes, this RFC uses "exemplars" throughout rather than exhaustively listing all the traits. In any case the plan laid out here is admittedly a bit sketchy on the details (i.e., are stability attributes enough to do this staging?)

[Gankra]

Took me a bit to wrap my head around all the consequences, but now it all makes sense to me. +1, removes a lot of magic.

[huonw]

Could these be unified as:

struct Range<Idx> {
    low: Option<Idx>,
    high: Option<Idx>
}

and then i..j is (Some, Some), i.. is (Some, None) etc.? Downsides I can think of are losing the ability to be specific about exactly what sort of slicing works, but benefits include not forgetting to implement one of them.

[sfackler]

I wrote up a range type for rust-postgres here that's a bit more robust - with support for inclusive and exclusive bounds as well as the empty range. It's more than what's needed for this syntax, but if we're going to have the types, it might be worth making them more fleshed out: https://github.com/sfackler/rust-postgres/blob/master/src/types/range.rs

[aturon]

@huonw Right, that seems to be the tradeoff: for x in ..3 would be a panic rather than type error.

Still, probably worth it to do something like you're suggesting.

[Gankra]

I'd rather not overengineer this much. I like that the separate types mean that the right impl statically dispatches, whereas the Option case requires runtime branching to "pick" the impl. Also as @aturon notes failing to impl a variant isn't a runtime error.

[huonw]

Which version is overengineering? (It seems to me that any of them could be regarded as overengineering depending on your perspective.)

[Gankra]

Inclusive/Exclusive range types seems like overkill to me here. Especially since the syntax to get something like (a, b] isn't clear.

The Options I could take or leave, save the possibility of static dispatch otherwise.

[arcto]

I'd rather see a type error than a runtime panic, in every instance possible. If ..3 isn't logically meaningful then let that be a type error. That's better for correctness and for performance.

[huonw]

Encoding information in the type system is valuable, but we have to balance making things too hard/annoying to use with a huge pile of types.

[nrc]

I'm a big fan of &v now - I've been converting a bunch of [T, ..n] to &[T] and writing &v feels 'right'. I'd like to extend that to vecs. I think I would prefer to lose [] - I've already been confused by it a few times (an although I used to like [..] it is starting to look pretty ugly to me now).

[brendanzab]

I would say we should either have deref coercions or keep []. &*v is very ugly from an aesthetic point of view.

[aturon]

It sounds like we should consider the deref coercions RFC. Perhaps if that's accepted before this one is discussed, I can change this RFC to drop [].

[nrc]

Could you clarify what this means for the symmetry of operators please? Taking == as an example, if I write a == b will the coercion/autoref'ing applied to the operands be symmetric? After removing the [T, ..n] -> &[T] coercion, comparing slices is really ugly - e.g., if a and b are both [T, ..n] you have to write a == &b.

[brendanzab]

+1 for me. Cuts down a great deal of sugar. I very much like the by-value operators - it's a shame that in using & we can't take advantage of Rust's efficient move-semantics for certain types.

[ftxqxd]

Has the option of changing this to fn deref(self) -> Result been considered? So all impl Deref for Bar { type Result = Foo; ... }s would be changed to impl<'a> Deref for &'a Bar { type Result = &'a Foo; ... }. The advantage of this is that Deref, DerefMut, and even the hypothetical DerefMove could all be combined into a single trait. The same transformation could happen to Index as well (and maybe even the Fn traits), although IndexSet would still be required for maps. Such a change to all those traits would probably require adjusting the existing compiler machinery that determines which trait to use to instead determine how (if at all) to auto-reference.

I feel that there’s got to be some reason why this isn’t feasible, but I can’t think of one, other than requiring some extra auto-referencing logic. All the other traits are being by-value-ified, which is great, so it would be nice if that could extend to every trait.

[aturon]

I'm not sure how feasible that would be. The compiler has a fair amount of special machinery for choosing between Deref and DerefMut, and similarly for the Index/IndexMut pair. @nikomatsakis or @pcwalton could probably say more.

[japaric]

@nick29581

Taking == as an example, if I write a == b will the coercion/autoref'ing applied to the operands be symmetric?

rust-lang/rust#18486 removes the autoref magic from binary operators. In other words, a OP b (AIUI) desugars to OpTrait::op_method(&a, &b), i.e. it only works if impl OpTrait<B> for A, where a/b has type A/B.

After removing the [T, ..n] -> &[T] coercion, comparing slices is really ugly - e.g., if a and b are both [T, ..n] you have to write a == &b.

rust-lang/rust#18486 adds impl PartialEq for [T, ..N] up to N = 32, so a == b will work if a and b have types [T, ..N] and N <= 32.

@nikomatsakis can confirm/elaborate

[ben0x539]

So generally, a + b will potentially move from a and/or b? Does a += b still work (presumably in a = a + b, a is moved to the addition and then the variable cannot be used for the assignment anymore) [edit: apparently this already stopped working long ago, nevermind]? Are we going to ship impls for &int: Add<&int>, int: Add<&int> and &int: Add<int> etc.? Do Rust libraries offering arbitrary-sized integers or similar exist yet to which we can look for practical experience?

I'm fairly skeptical of providing operator overloading but designing it so using arithmetic operators works differently depending on whether the operands are primitives or not. The existing desugaring that adds a level of indirection is a lot more intuitive to me.

I really don't understand the motivation of DList concatenation that is apparently supposed to behave differently than arithmetic on primitives but use the same syntax. What is the intended syntactical improvement there? let a = a + b; instead of a.append(b);?

#118 is probably relevant.

[seanmonstar]

typo ord, should be ops

(same typo is in intro comment)

[emberian]

IndexSet is very important to me because it allows me to nicely model write-only memory mapping, which is important for graphics APIs.

[aturon]

@ben0x539

So generally, a + b will potentially move from a and/or b?

More precisely: a + b will always move both a and b. But the moved values can be references in the case of non-Copy data (&matrix1 + &matrix2) and, for Copy data, you're moving a copy as per usual.

Are we going to ship impls for &int: Add<&int>, int: Add<&int> and &int: Add<int> etc.?

No. You only need int: Add<int>, because int is Copy. Same for the rest of the primitive types.

Do Rust libraries offering arbitrary-sized integers or similar exist yet to which we can look for practical experience?

There's libnum, and I'm sure many other examples.

I'm fairly skeptical of providing operator overloading but designing it so using arithmetic operators works differently depending on whether the operands are primitives or not. The existing desugaring that adds a level of indirection is a lot more intuitive to me.

I'm not sure I understand this concern. The operators would always take their arguments by value. But in some cases those arguments might be Copy data, e.g. primitive types, in which case passing by value entails copying. This is already the case in Rust generally.

I really don't understand the motivation of DList concatenation that is apparently supposed to behave differently than arithmetic on primitives but use the same syntax. What is the intended syntactical improvement there? let a = a + b; instead of a.append(b);?

DList is just one example where consuming a non-Copy value would make sense.

The broader point is that the by-ref semantics restrict the possible uses of the operators. Using by-value is more general, since you can always implement for references. And for non-Copy operands, explicitly marking whether you are moving or passing by reference is usually helpful for reasoning about ownership locally (and is more consistent with fn arguments in general).

Also, I'm not sure what you mean by "behave differently than arithmetic on primitives"? In general, the way that by-value parameters work varies depending on whether data is Copy or not (with the simple rule that Copy data moves a copy of the data), so this is just making the operators consistent with other parts of the language.

[alexcrichton]

Discussion

Tracking

[SimonSapin]

“These” here only refers to the Fn* traits, not all the traits mentioned in the RFC, right?

[ftxqxd]

Isn’t the range notation ambiguous with the array repeat syntax? That is, [3u, ..4] could either be an array containing four threes, or an array containing one three and one range up to four. I guess the array notation could be changed to [3u, ...4] (which seems wrong, as ... generally represents an inclusive range in Rust, but would represent an exclusive range of indices), but that would rule out the possibility of inclusive range sugar.

[Kimundi]

Hm, that seems troublesome indeed, but could still be managed by some kind of special casing in the lexer.
However, the truly problematic example would be this: [..4, ..4]

Could be either two ranges up to four, or four ranges up to four if ..4 evaluates to a copyable type.

[aturon]

@P1start Wow, good catch... I really wish we had a grammar!

I'm still digesting this a bit to figure out a reasonable path forward here. But everyone reading this, feel free to toss out suggestions.

[alexcrichton]

cc rust-lang/rust#9879, some old bikeshed about changing [foo, ..N], although that may not be the route we want to take.

[glaebhoerl]

A straightforward fix would be to just use _..4 instead of ..4 for single-ended ranges (and likewise 4.._). Not very pretty though.

[nikomatsakis]

On Wed, Nov 26, 2014 at 08:27:27AM -0800, Aaron Turon wrote:

@P1start Wow, good catch... I really wish we had a grammar!

Sigh. Yes. Amazing how hard it is to catch (in retrospect) obvious
ambiguities!

[aturon]

I've filed an amendment to deal with the grammar ambiguity, after some internal discussion.

[arthurprs]

I'm trying to implement Ord to compare a struct Pair(K, V) against K

This is what we have

trait Eq: PartialEq<Self>
trait Ord: Eq + PartialOrd<Self>

but I (and the RFC) expected this

trait Eq<Rhs: ?Sized = Self>: PartialEq<Rhs>
trait Ord<Rhs: ?Sized = Self>: Eq<Rhs> + PartialOrd<Rhs>

Is it a bug or oversight? What's the motivation?

I originally posted this at

@eddyb suggested we made these traits unsafe.

[eddyb]

To expand on that: I believe implementations of Eq and Ord where the two sides aren't the same type are more prone to incorrectness.

Meanwhile, we can't depend on Eq or Ord guarantees for memory safety in optimizations or algorithms (e.g. sorting) because an incorrect implementation can be written without unsafe.
Making those two traits unsafe to implement has been thrown around for some time before we got unsafe trait which feels like the perfect solution.

#[deriving(Eq, Ord)] would only be allowed to impl both the partial traits and the total ones, at the same time, making them correct by default (it might be possible to not even emit unsafe impl from deriving, but I'm not familiar with OIBIT to say for sure).

cc @nikomatsakis @flaper87

[glaebhoerl]

@eddyb @arthurprs There is discussion of the unsafe trait approach in #926.