Pre-RFC core::ops traits for relational operators

I'm not sure I follow. PartialOrd isn't magical, it is just a trait, and it is the API that libstd uses to interface with the standard collections.

If you want to implement different orderings for your types, just do exactly what PartialOrd does:

  • add your own traits for your own orderings, e.g., MyOrdTrait
  • your own proc macros to automatically derive them, e.g., derive(MyOrdTrait)
  • add small wrapper that maps a T: MyOrdTrait into PartialOrd, e.g., struct Wrapper<T: MyOrdTrait>(T) with a impl<T: MyOrdTrait> PartialOrd for Wrapper<T>, to be able to interface with libstd.

With this, your Person example is solving by adding a derive(MyOrdTrait) to Person.


@mcy

If your type's comparison semantics really are non-trivial, add methods, rather than overloading expectations

My expectation and that of many of my users is that a < b works for types for which it is a meaningful operation, even if those types do not have a partial ordering relation.

ISO C++, in my view, is not a great example of a principled design process.

I'm confused with what you are trying to convey with your C++ examples. C++ supports types that have all kinds of ordering relations by allowing users to specify the relational operators however they want, and also supports convenience notation for some common ordering relations via the <=> operator. If you choose <=>, you can't specialize the operators.

Rust is quite similar, in that if you have a common ordering relation like a strict or non-strict partial order, or a total order, you can just write partial_eq and partial_cmp and call it a day. If you have an ordering relation that isn't one of this, but the result of the operators is bool, you can "hack" it in by, e.g., manually specifying PartialOrd::{lt, le, gt, ge}. But if the result isn't bool, you can't use the operators at all, and that's again the expectation of the users of those types.

Why? In my view, it is a mistake to decouple the six comparison operators from each other,

As far as I can tell, the motivation for this can be solved in other ways that don't involve decoupling the six comparisons. [...] WeakCmp

The expectation, of course, is that for SIMD, the compiler is able to aggressively inline these and macro-fuse the resulting calls to std::arch intrinsics in the obvious manner, so that !(a < b) reduces to a >= b at the instruction level.

For a performance oriented feature, such optimizations must either be guaranteed, or a escape hatch must be available. This is why users are allowed to overwrite the PartialOrd::{lt, le, gt, ge} operations.

If instead of 6 different traits, you prefer a single Trait with 6 different methods to implement, that would be fine with me.

When you propose WeakCmp it feels like you are proposing this to be a different type of ordering relation. It is unclear what ordering relation that would be, and how it could work for SIMD vectors (or subset relations, for example).

I don't think they should have been PartialOrd/Eq in the first place

This is offtopic, but floats form a strict partial order, and PartialOrd is the libstd API that collection use to require types with either a strict or a non-strict partial order, so this sounds appropriate to me. Floats also have a total order, but for floats this order is different from their strict partial order, so a problem of the current API is that this use case is not supported. AFAICT that's orthogonal to this discussion.


@droundy

I'd like to share here that in numpy the behavior of comparison operators is indeed terribly unexpected to students (I teach computational physics), and is in my opinion one of the worst features of the DSL. Students who make the mistake of comparing arrays inevitably need help up extricate them from the situation. The result ends up in an if or while statement, and the resulting error message gives them terrible advice.

You are correct in that this is a real issue new numpy users have, but I disagree in the relational operator overloads being the cause of this issue, because new users of other linear algebra libraries, like Eigen, do not have this problem. The main difference is that Eigen is strongly typed, while numpy isn't. This means that when a new user of Eigen tries to assign the result of comparing two arrays with a < b to a bool, they get a nice and obvious compiler error, while numpy user get, as you put it, "terrible advice".

1 Like

That, I worry, is a manner of opinion, depending on what you think the purpose of a < b is.

Unfortunately, that's incorrect. C++ will not generate a definition for e.g. operator< if you've defined one yourself. You are welcome to mix and match and let ADL play loose.

I don't think there should be any expectation that an escape hatch is ergonomic.

Also, I just realized that this feature will probably play hell with @varkor's efforts to kill the turbofish, but I don't know what the status of that is.

If someone has the know-how to hook up the lang items, here is a playground defining what the comparison traits could look like under this proposal. A patch to see how this would impact inference and errors without further adjustment would help this proposal, as a big unknown here is how it would impact such.

I believe the status is that it's probably not happening. Though the problematic edge case is (a<b, c>(d)) so an edition barrier would be required for the parse to change. < and > already can't just be a generic operator symbol due to their use in generics, so I don't think allowing _ > _ to type at types other than bool impacts interaction with disambiguator-free turbofish.

1 Like

While this looks neat for SIMD operations, wouldn’t it lead to poor perf for arbitrary arrays? Is rustc smart enough to fuse the broadcasting loops and avoid allocating the two temporary collections? In other words, wouldn’t this make it easier for users to write code that performs poorly?

2 Likes

Ah wow, I need to play more with that. Then C++ is pretty much in the same place Rust is right now with the PartialOrd trait, except for the restriction on the output type.

While this looks neat for SIMD operations, wouldn’t it lead to poor perf for arbitrary arrays?

The PartialOrd implementation for arrays returns bool and I haven't heard of anyone proposing to change that.

For an user-defined array type, if you want (a < b) to return an array, you can just do that.

I'm not sure what this has to do with the < operator, but if you compute the output array by writing a raw loop:

for (c, (a, b)) c.iter_mut().zip(a.iter(), b.iter()) {
    *c = (*a < *b);
}

and that happens to not optimize properly, you can always manually use SIMD intrinsics to do better, like you would do for any other Rust code, including the same Rust code that instead of returning an array would return bool.

@mcy Following your suggestion, I tried to explore what the WeakEq and WeakOrd would look like, but I think one can achieve a very similar result with way less churn by just adding a defaulted associated type Target: Not = bool; to PartialEq and re-using that in the PartialOrd impl.

Adding a default associated type to PartialEq would require that all code that bounds T: PartialEq get a T: PartialEq<Target = bool> implicity for backward compatibility. I'm not sure if that's how default associated types already work or not, or if that's possible at all, but such a solution would satisfy most of my use cases, while preserving the "coupling" of the relational operators with each other, even though what the ordering relations would mean when Target != bool is not clear to me (AFAICT they wouldn't be ordering relations).


The main use case that this does not support is returning different types , e.g., from < and >. This is very useful in C++, where < can be used to return a Less<A, B> "node" and > a Greater<A, B> "node", e.g., such that the type of (a < 0) - (a > 0) becomes Sub<Greater<ArrayRef<'a, Int>, Scalar<Int, 0>>,Less<ArrayRef<'a, Int>, Scalar<Int, 0>>>. AFAICT the only way to support this would be to add the 6 different traits to overload operators independently of one another, but such a solution would be compatible with an extension that adds a default associated Target type to PartialEq, so we could punt this for the time being.

Yes, you could have a proc macro that implements custom operators. Perhaps that's the best way to experiment with the feature and see how actually useful it is.

But macros will be seen as boilerplate. They're tricky for IDE autocomplete. So if the proposal survives past prototype stage, it'd be good for the custom operators to be adopted as a "native" part of Rust syntax.

1 Like

Just to be clear, they're impossible in the general case and basically impossible when they change the Rust grammar.

4 Likes

I have a patched version of rustc that (should) use separate ops traits: https://github.com/CAD97/rust/tree/cmp!

Example:

#![feature(const_generics)]
#![feature(comparison_traits)]

use std::{cmp::LtOp, fmt, mem};

struct A<T, const N: usize>([T; N]);

impl<T: fmt::Debug, const N: usize> fmt::Debug for A<T, N> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        f.debug_list().entries(self.0.iter()).finish()
    }
}

impl<T: PartialOrd, const N: usize> LtOp<A<T, N>> for A<T, N> {
    type Target = A<bool, N>;
    fn lt_op(&self, other: &Self) -> A<bool, N> {
        let mut out: [bool; N] = unsafe { mem::zeroed() };
        for i in 0..N {
            out[i] = self.0[i] < other.0[i];
        }
        A(out)
    }
}

fn main() {
    let up = A([1, 2, 3, 4, 5]);
    let dn = A([5, 4, 3, 2, 1]);
    dbg!(up < dn);
}

prints:

[src/main.rs:28] up < dn = [
    true,
    true,
    false,
    false,
    false,
]

I've used deliberately awkward names to make the patch easier.

2 Likes

Which parts?

If you have a concrete proposal that solves all constraints in a backwards compatible way without any drawbacks I'd love to read it.

@CAD97 thanks, that's more or less what I had in mind. I've updated the top post with a link to your implementation, and tuned the bounds.

I'm not sure I follow why does the PartialOrd trait require the new bounds. Also, there is a cycle, in which the PartialOrd trait requires the bounds but the blanket impl is only available for types that implement PartialOrd. I did not know that the trait system supported these kind of cycles.

Does this pass all rustc tests? That would hint that this might be a backward compatible change.

It's so that T: PartialOrd obviously implies LtOp<Rhs, Target=bool> and so on, the same way it's done with the Fn trait hiearchy.

It's the exact same setup that the Fn trait hiearchy has.

Unfortunately there are some unspecified problems with my dev setup currently that mean even master doesn't get through the whole x.py test suite, so someone else would have to check. I suspect that a lot of UI tests would need adjusting first, though. (At the very lease the #[doc(alias = ">"). and #[rustc_on_unimplemented(..)] attributes would need to be adjusted and provided for the _Op traits.)

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.