Require operator== to bind without errors before implementing PartialOrd

rs_bindings_from_cc/generate_bindings/database/db.rs

@@ -222,14 +222,15 @@ memoized::query_group! {
 
         #[return_ref]
         /// Returns the generated bindings for a function with the given name and param
-        /// types. If none exists, returns None.
+        /// types, along with whether the binding succeeded without errors
+        /// (`GeneratedFunction::status.is_ok()`). If none exists, returns None.
         ///
         /// Implementation: rs_bindings_from_cc/generate_bindings/generate_function.rs?q=function:get_binding
         fn get_binding_memoized(
             &self,
             expected_function_name: Rc<UnqualifiedIdentifier>,
             expected_param_types: Rc<[RsTypeKind]>,
-        ) -> Option<&(Ident, ImplKind)>;
+        ) -> Option<&(Ident, ImplKind, bool)>;
 
         #[return_ref]
         /// Returns a collection of unqualified member functions of the given record.
@@ -278,7 +279,7 @@ impl<'db> BindingsGenerator<'db> {
         &self,
         expected_function_name: UnqualifiedIdentifier,
         expected_param_types: Rc<[RsTypeKind]>,
-    ) -> Option<&(Ident, ImplKind)> {
+    ) -> Option<&(Ident, ImplKind, bool)> {
         self.get_binding_memoized(Rc::new(expected_function_name), expected_param_types)
     }
 

rs_bindings_from_cc/generate_bindings/generate_function.rs

@@ -295,7 +295,9 @@ fn api_func_shape_for_operator_ne(
     // If operator== is present, don't generate ne, rely on rust's default ne.
     let eq_binding =
         db.get_binding(UnqualifiedIdentifier::Operator(Operator::new("==")), param_types.into());
-    if let Some((_, ImplKind::Trait { trait_name: TraitName::PartialEq { .. }, .. })) = eq_binding {
+    if let Some((_, ImplKind::Trait { trait_name: TraitName::PartialEq { .. }, .. }, _)) =
+        eq_binding
+    {
         // operator== is present, skipping bindings for operator!=
         return Ok(None);
     }
@@ -548,7 +550,7 @@ fn api_func_shape_for_operator_partial_ord(
             UnqualifiedIdentifier::Operator(Operator::new(preferred_op_name)),
             param_types.into(),
         );
-        if let Some((_, ImplKind::Trait { trait_name: TraitName::PartialOrd { .. }, .. })) =
+        if let Some((_, ImplKind::Trait { trait_name: TraitName::PartialOrd { .. }, .. }, true)) =
             preferred_binding
         {
             bail_to_errors!(
@@ -592,7 +594,7 @@ fn api_func_shape_for_operator_partial_ord(
     let partialeq_binding =
         db.get_binding(UnqualifiedIdentifier::Operator(Operator::new("==")), param_types.into());
     match partialeq_binding {
-        Some((_, ImplKind::Trait { trait_name: TraitName::PartialEq { .. }, .. })) => {}
+        Some((_, ImplKind::Trait { trait_name: TraitName::PartialEq { .. }, .. }, true)) => {}
         _ => errors.add(anyhow!("operator{op_name} where operator== is missing.")),
     }
     let func_name = make_rs_ident(op.rs_method_name());
@@ -1295,7 +1297,7 @@ pub fn get_binding(
     db: &BindingsGenerator,
     expected_function_name: std::rc::Rc<UnqualifiedIdentifier>,
     expected_param_types: Rc<[RsTypeKind]>,
-) -> Option<(Ident, ImplKind)> {
+) -> Option<(Ident, ImplKind, bool)> {
     // Lifetime names depend on the names of the C++ parameters (e.g. `&'__this T` in a member
     // function VS `&'lhs T` in a non-member function), so with this feature, lifetimes are ignored
     // when finding the corresponding function. Lifetimes are not ignored when generating the
@@ -1304,32 +1306,27 @@ pub fn get_binding(
         .ir()
         .target_crubit_features(db.ir().current_target())
         .contains(crubit_feature::CrubitFeature::ParamNameInsensitiveOperatorLookup);
-    db.ir()
-        .get_functions_by_name(&expected_function_name)
-        .filter(|function| {
-            generate_function(db, (*function).clone(), None).ok().flatten().is_some()
-        })
-        .find_map(|function| {
-            let Ok((mut function_param_types, return_type)) =
-                rs_type_kinds_for_func(db, function.clone())
-            else {
-                return None;
-            };
-            if function_param_types.len() != expected_param_types.len()
-                || !function_param_types.iter().zip(expected_param_types.iter()).all(
-                    |(actual, expected)| {
-                        if ignore_lifetimes {
-                            actual.eq_ignoring_lifetimes(expected, false)
-                        } else {
-                            actual == expected
-                        }
-                    },
-                )
-            {
-                return None;
-            }
-            api_func_shape_if_some(db, function, &mut function_param_types, &return_type)
-        })
+    db.ir().get_functions_by_name(&expected_function_name).find_map(|function| {
+        let generated_function = db.generate_function((*function).clone(), None).ok().flatten()?;
+        let (mut function_param_types, return_type) =
+            rs_type_kinds_for_func(db, function.clone()).ok()?;
+        if function_param_types.len() != expected_param_types.len()
+            || !function_param_types.iter().zip(expected_param_types.iter()).all(
+                |(actual, expected)| {
+                    if ignore_lifetimes {
+                        actual.eq_ignoring_lifetimes(expected, false)
+                    } else {
+                        actual == expected
+                    }
+                },
+            )
+        {
+            return None;
+        }
+        let (ident, impl_kind) =
+            api_func_shape_if_some(db, function, &mut function_param_types, &return_type)?;
+        Some((ident, impl_kind, generated_function.status.is_ok()))
+    })
 }
 
 /// Implementation of `BindingsGenerator::is_record_clonable`.

rs_bindings_from_cc/generate_bindings/generate_function_test.rs

@@ -1724,6 +1724,43 @@ fn test_impl_lt_missing_eq_impl() -> Result<()> {
     Ok(())
 }
 
+/// When `operator==` fails to bind (for example, because its return type is `int` rather than
+/// `bool`, producing an uncallable `PartialEq` impl with an unsatisfied `BindingFailedFor_...`
+/// bound), `PartialOrd` must not be implemented for `operator<`.
+#[gtest]
+fn test_impl_lt_when_eq_has_invalid_return_type() -> Result<()> {
+    let rs_api = generate_bindings_tokens_from_cc(
+        r#"
+        struct SomeStruct final {
+            inline int operator==(const SomeStruct& other) const {
+                return i == other.i;
+            }
+            inline int operator!=(const SomeStruct& other) const {
+                return i != other.i;
+            }
+            inline bool operator<(const SomeStruct& other) const {
+                return i < other.i;
+            }
+            int i;
+        };"#,
+    )?
+    .rs_api;
+    assert_rs_not_matches!(rs_api, quote! { impl PartialOrd });
+    let rs_api_str = rs_api.to_string();
+    expect_that!(
+        rs_api_str,
+        contains_substring(
+            "comparison operator return type must be `bool`, found: ::ffi_11::c_int"
+        )
+    );
+    expect_that!(rs_api_str, contains_substring("operator< where operator== is missing"));
+    expect_that!(
+        rs_api_str,
+        not(contains_substring("Cannot generate bindings for overloaded function"))
+    );
+    Ok(())
+}
+
 #[gtest]
 fn test_impl_gt_for_member_function() -> Result<()> {
     let BindingsTokens { rs_api, rs_api_impl } = generate_bindings_tokens_from_cc(

rs_bindings_from_cc/test/struct/operators/operators.h

@@ -302,4 +302,37 @@ struct OnlyGe final {
 bool operator==(const OnlyGe& lhs, const OnlyGe& rhs);
 bool operator>=(const OnlyGe& lhs, const OnlyGe& rhs);
 
+// `operator==` returns `int` instead of `bool`, so its `PartialEq` impl has an
+// unsatisfied `BindingFailedFor_...` where bound. `PartialOrd` must not be
+// generated for the relational operators (which would otherwise fail to compile
+// because `PartialOrd` requires a satisfied `PartialEq` supertrait).
+struct IntEqWithBoolRelational final {
+  int i;
+
+  friend int operator==(const IntEqWithBoolRelational& lhs,
+                        const IntEqWithBoolRelational& rhs) {
+    return lhs.i == rhs.i;
+  }
+  friend int operator!=(const IntEqWithBoolRelational& lhs,
+                        const IntEqWithBoolRelational& rhs) {
+    return lhs.i != rhs.i;
+  }
+  friend bool operator<(const IntEqWithBoolRelational& lhs,
+                        const IntEqWithBoolRelational& rhs) {
+    return lhs.i < rhs.i;
+  }
+  friend bool operator<=(const IntEqWithBoolRelational& lhs,
+                         const IntEqWithBoolRelational& rhs) {
+    return lhs.i <= rhs.i;
+  }
+  friend bool operator>(const IntEqWithBoolRelational& lhs,
+                        const IntEqWithBoolRelational& rhs) {
+    return lhs.i > rhs.i;
+  }
+  friend bool operator>=(const IntEqWithBoolRelational& lhs,
+                         const IntEqWithBoolRelational& rhs) {
+    return lhs.i >= rhs.i;
+  }
+};
+
 #endif  // CRUBIT_RS_BINDINGS_FROM_CC_TEST_STRUCT_OPERATORS_OPERATORS_H_

rs_bindings_from_cc/test/struct/operators/operators_test.rs

@@ -393,3 +393,8 @@ fn test_only_ge() {
     expect_eq!(s2.partial_cmp(&s1), Some(core::cmp::Ordering::Greater));
     expect_eq!(s1.partial_cmp(&s1), Some(core::cmp::Ordering::Equal));
 }
+
+#[gtest]
+fn test_int_eq_with_bool_relational_does_not_implement_partial_ord() {
+    assert_not_impl_any!(IntEqWithBoolRelational: PartialEq, PartialOrd);
+}