[idea] Fallible operators

I've run into an interesting issue with my code recently. Here is an extremely simplified example of the problem:

struct Foo {
    identifier: u128,
    timestamp: std::time::Instant
}

impl std::cmp::PartialEq<Foo> for Foo {
    fn eq(&self, other: &Foo) -> bool {
        assert_eq!(self.identifier, other.identifier);
        self.timestamp == other.timestamp
    }
}

As you can see, if the identifiers don't match, then the comparison is invalid. In those cases where recovery is possible, it would be better to return Result instead. While this is pretty easy to solve for PartialEq, it got me thinking about other operators already defined in rust. In most cases, it appears that this is easy to handle (e.g., implement std::ops::Add such that Output = Result<Self>), but I noticed that for some operators, such as std::ops::BitAndAssign, there is no output type, so no way to avoid a panic if the operation type checks but doesn't make sense for some reason.

The current way to handle this kind of situation is to use std::panic::catch_unwind, but that doesn't feel very ergonomic. So, would it make sense to add in fallible versions of those traits? Picking on std::ops::BitAndAssign in particular, I was thinking of something like:

// From @dtolnay's [anyhow](https://crates.io/crates/anyhow) crate,
// but the standard library would have to do something different so
// that it was independent of anyhow.
use anyhow::Result;

#[lang = "bitand_assign_fallible"]
pub trait BitAndAssignFallible<Rhs = Self> {
    fn bitand_assign(&mut self, rhs: Rhs) -> Result<()>;
}

which would be used like:

foo &= bar?;

This would require some changes to the compiler, but I don't know enough about the internals to know if this would be easy, or painful. I also don't know if others would consider this to be a useful change.

Partial list of traits that could be changed this way

This is ambiguous, because this syntax is already valid. (it applies ? to bar then passes the result to BitAndAssign).

I don't think you should implement *Assign operators if you can fail, just stick to the normal operators. If you must have the *Assign operators, then you should either panic or have Self signal that the operation failed. This seems to be a super niche use-case, so I don't think there is enough justification for basically duplicating 9 traits.

6 Likes

I'm pretty sure the answer to any such use cases is simply to use functions/methods instead of operators. A non-trivial return type is a pretty strong indicator that an operator is too concise for what you're trying to do.

In particular, catch_unwind is really only meant to be used to avoid letting panics escape into places like FFI where they'd cause insta-UB, or in multi-thread scenarios to convert a thread panic into an error value for some master thread to see. Using them to work around the fact that you made a panicking operator instead of a Result-returning function is definitely not intended.

9 Likes

That is already what Partial means, that some comparisons can not be made. Simply not implementing Eq for this type would already express the relevant facts to me, the implementation forwards to a total comparison but instead of protecting it with an assertion it could simply return false. The additional semantics of incomparability being an error are better and simpler expressed with a properly named method and Result<_,_>. I'm also puzzled why you choose to overload the comparison operator in the first place. Usually this is done for the sake of readability for common operations but having to write extra code to handle the additional intended code path has the opposite effect of making it much harder to read. The short code example as given does not provide sufficient incentive for a change to operators.

8 Likes

Note also that PartialCmp is pretty much fallible comparison. There's no operator that calls partial_cmp automatically, but you can use an explicit partial_cmp call to compare two objects in a fallible manner, returning Option<Ordering>.

If partial_cmp is #[inline] or otherwise in the same translation unit, a.partial_cmp(&b) == Some(Ordering::Equal) should trivially optimize down to just doing the comparison (and "can I compare" check).

I agree that for the OpAssign operators, they shouldn't be used if the regular operator doesn't return Self. Operators in general should be implemented only if they follow the "obvious" meaning for that type (and by "obvious" I mean the widely accepted mathematical meaning of that operator on that mathematical type).

2 Likes

Ah, I hadn't realized it would be ambiguous. That's enough to kill the idea right there. I withdraw my idea from consideration, but want to answer everyone's comments below.

I think you might have misunderstood what I was trying to explain. The types can be compared, its just that some values cannot be correctly compared. As I said in the original example, this is easy to solve for PartialEq, but not as easy to solve for BitAndAssign.

That said, I really should have used a different example; that example was the smallest I could think of off hand.

Yeah, the more I think about it, the more I think you guys are right. The reason I started thinking about it was because of how I'm implementing various operator traits on some of my data structures. I have a lot of structs that only have one or two fields being compared out of some number that are collected together in the struct. In some cases the types can be compared, but certain values can't be compared to one another. PartialCmp solves this easily for my problems, but because I was thinking about it, I took a quick look to see if there were operators defined where you don't have the option of returning Result or Option (anything where you choose your return type doesn't have this issue). The ones I found quickly are the ones I listed. But since my method would be ambiguous, and since I do not want to propose new operators like =?, I think that this idea should go away.

Thank you all for reading and commenting on it!

5 Likes

Well, some sort of design around this could be used for basic arithmetic operations to signal overflow—so perhaps not that niche...

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