Fix __eq metamethods throwing on type mismatch

crates/luaffi/src/lib.rs

@@ -458,6 +458,7 @@ pub struct MetatypeFunctionBuilder<'r, 'm> {
     cargs: String,    // C call arguments
     prelude: String,  // lua function body prelude
     postlude: String, // lua function body postlude
+    is_eqmm: bool,
 }
 
 impl<'r, 'm> MetatypeFunctionBuilder<'r, 'm> {
@@ -469,9 +470,15 @@ impl<'r, 'm> MetatypeFunctionBuilder<'r, 'm> {
             cargs: String::new(),
             prelude: String::new(),
             postlude: String::new(),
+            is_eqmm: false,
         }
     }
 
+    pub fn set_eqmm(&mut self, eq: bool) -> &mut Self {
+        self.is_eqmm = eq; // are we building an __eq metamethod?
+        self
+    }
+
     pub fn param<T: FromFfi>(&mut self, name: impl Display) -> &mut Self {
         let Self {
             metatype: MetatypeBuilder { reg, .. },
@@ -510,6 +517,26 @@ impl<'r, 'm> MetatypeFunctionBuilder<'r, 'm> {
         self
     }
 
+    pub fn param_ctchecked<T: FromFfi, U: Type + Metatype<Target = U>>(
+        &mut self,
+        name: impl Display,
+    ) -> &mut Self {
+        let ct = U::name();
+        if self.is_eqmm {
+            // special case for building __eq metamethods which are expected to return false if the
+            // arguments aren't of the expected type, because otherwise the `==` and `~=` operators
+            // could throw instead of returning false.
+            write!(
+                self.prelude,
+                r#"if not __istype(__ct.{ct}, {name}) then return false; end; "#
+            )
+        } else {
+            write!(self.prelude, r#"assert(__istype(__ct.{ct}, {name})); "#)
+        }
+        .unwrap();
+        self.param::<T>(name)
+    }
+
     pub fn param_str(
         &mut self,
         name: impl Display,
@@ -520,7 +547,9 @@ impl<'r, 'm> MetatypeFunctionBuilder<'r, 'm> {
         //
         // this passes one lua `string` argument as two C `const uint8_t *ptr` and `uintptr_t len`
         // arguments, bypassing the slower generic `&[u8]: FromFfi` path which constructs a
-        // temporary cdata to pass the string and its length in one argument.
+        // temporary cdata to pass the string and its length in one argument. this is used when the
+        // #[metatype] macro detects a string-like parameter, currently defined to be &[u8], &str,
+        // &BStr (from the bstr crate), or those types wrapped in Option<T>.
         let Self {
             lparams,
             cparams,
@@ -780,8 +809,7 @@ macro_rules! impl_bigint_intoabi {
             fn postlude(ret: &str) -> impl Display {
                 // this isn't "correct" per se, but it's much more ergonomic to work with numbers in
                 // lua than with long longs wrapped in cdata. we gracefully accept the loss of
-                // precision here and that 53 bits of precision for big integers are enough. (the
+                // precision here and that 53 bits of precision for big integers are enough.
-                // vain of Lua 5.3 integer subtype ;D )
                 display!("{ret} = tonumber({ret}); ")
             }
         }
@@ -837,22 +865,6 @@ impl_ptr!(*mut T, true);
 impl_ptr!(Option<&T>, false);
 impl_ptr!(Option<&mut T>, true);
 
-macro_rules! impl_ptr_annotation {
-    ($ty:ty) => {
-        impl<T> Annotation for $ty
-        where
-            T: Annotation,
-        {
-            fn annotation() -> impl Display {
-                "lightuserdata"
-            }
-        }
-    };
-}
-
-impl_ptr_annotation!(*const T);
-impl_ptr_annotation!(*mut T);
-
 macro_rules! impl_ref_annotation {
     ($ty:ty) => {
         impl<T> Annotation for $ty

crates/luaffi_impl/src/metatype.rs

@@ -87,7 +87,7 @@ fn generate_impls(_args: &Args, imp: &mut ItemImpl) -> Result<TokenStream> {
             );
         }
 
-        add_ffi_function(&mut registry, &func)?;
+        generate_ffi_function(&mut registry, &func)?;
     }
 
     // process extern "Lua" lua functions
@@ -100,10 +100,10 @@ fn generate_impls(_args: &Args, imp: &mut ItemImpl) -> Result<TokenStream> {
             );
         }
 
-        if let Some(Metamethod::Gc) = func.attrs.metamethod {
+        if func.attrs.metamethod == Some(Metamethod::Gc) {
             lua_drop = Some(func);
         } else {
-            add_lua_function(&mut registry, &func)?;
+            generate_lua_function(&mut registry, &func)?;
         }
     }
 
@@ -155,6 +155,25 @@ enum Metamethod {
     Ipairs,
 }
 
+impl Metamethod {
+    fn narg(&self) -> Option<usize> {
+        Some(match *self {
+            Self::Eq
+            | Self::Lt
+            | Self::Le
+            | Self::Concat
+            | Self::Add
+            | Self::Sub
+            | Self::Mul
+            | Self::Div
+            | Self::Mod
+            | Self::Pow => 2,
+            Self::Gc | Self::Len | Self::Unm | Self::ToString | Self::Pairs | Self::Ipairs => 1,
+            Self::Call | Self::New => return None,
+        })
+    }
+}
+
 impl TryFrom<&Ident> for Metamethod {
     type Error = Error;
 
@@ -284,11 +303,23 @@ fn parse_ffi_functions(imp: &mut ItemImpl) -> Result<Vec<FfiFunction>> {
                 }
             }
 
+            let attrs = parse_ffi_function_attrs(&mut func.attrs)?;
+
+            if let Some(mm) = attrs.metamethod
+                && let Some(narg) = mm.narg()
+            {
+                syn_assert!(
+                    params.len() == narg,
+                    func.sig.inputs,
+                    "expected {narg} parameters for `{mm}` metamethod"
+                );
+            }
+
             parsed.push(FfiFunction {
                 name: func.sig.ident.clone(),
                 params,
                 ret,
-                attrs: parse_ffi_function_attrs(&mut func.attrs)?,
+                attrs,
                 is_async: func.sig.asyncness.is_some(),
             });
 
@@ -304,14 +335,10 @@ fn parse_ffi_function_attrs(attrs: &mut Vec<Attribute>) -> Result<FfiFunctionAtt
     let mut i = 0;
     while let Some(attr) = attrs.get(i) {
         if let Some(name) = attr.path().get_ident()
-            && let Ok(method) = Metamethod::try_from(&name.unraw())
+            && let Ok(mm) = Metamethod::try_from(&name.unraw())
         {
-            match method {
+            syn_assert!(mm != Metamethod::Gc, attr, "implement `Drop` instead");
-                Metamethod::Gc => syn_error!(attr, "implement `Drop` instead"),
+            parsed.metamethod = Some(mm);
-                _ => {}
-            }
-
-            parsed.metamethod = Some(method);
             attrs.remove(i);
         } else {
             i += 1;
@@ -370,7 +397,7 @@ fn get_ffi_ret_type(ty: &Type) -> FfiReturnType {
     }
 }
 
-fn add_ffi_function(registry: &mut Registry, func: &FfiFunction) -> Result<()> {
+fn generate_ffi_function(registry: &mut Registry, func: &FfiFunction) -> Result<()> {
     let ffi = ffi_crate();
     let ty = &registry.ty;
     let func_name = &func.name;
@@ -389,12 +416,22 @@ fn add_ffi_function(registry: &mut Registry, func: &FfiFunction) -> Result<()> {
     let mut asserts = vec![]; // compile-time asserts
     let mut build = vec![]; // ffi builder body
 
-    // for __new metamethods, ignore the first argument (ctype of self, for which there is no
+    match func.attrs.metamethod {
-    // equivalent in C)
+        Some(Metamethod::New) => {
-    if func.attrs.metamethod == Some(Metamethod::New) {
+            // for __new metamethods, ignore the first argument (ctype of self, for which there is
-        build.push(quote!(
+            // no equivalent in C)
-            b.param_ignored();
+            build.push(quote!(
-        ));
+                b.param_ignored();
+            ));
+        }
+        Some(Metamethod::Eq) => {
+            // for __eq metamethods, set special eq mode to return false on argument type mismatch
+            // instead of throwing
+            build.push(quote!(
+                b.set_eqmm(true);
+            ));
+        }
+        _ => {}
     }
 
     for (i, (name, func_param)) in func_params.iter().enumerate() {
@@ -410,9 +447,18 @@ fn add_ffi_function(registry: &mut Registry, func: &FfiFunction) -> Result<()> {
                 func_args.push(quote_spanned!(span =>
                     <#func_param as #ffi::FromFfi>::convert(#shim_param)
                 ));
-                build.push(quote_spanned!(span =>
+                build.push(if func.attrs.metamethod == Some(Metamethod::Eq) {
-                    b.param::<#func_param>(#name);
+                    quote_spanned!(span =>
-                ));
+                        // preliminary `return false` for type mismatch in __eq mode. we just check
+                        // against the ctype of Self because that's what __eq should be comparing
+                        // anyway.
+                        b.param_ctchecked::<#func_param, Self>(#name);
+                    )
+                } else {
+                    quote_spanned!(span =>
+                        b.param::<#func_param>(#name);
+                    )
+                });
             }
             ty @ (FfiParameterType::StringLike(str) | FfiParameterType::OptionStringLike(str)) => {
                 let shim_param_len = format_ident!("arg{i}_len");
@@ -494,19 +540,21 @@ fn add_ffi_function(registry: &mut Registry, func: &FfiFunction) -> Result<()> {
         }
     };
 
-    build.push({
+    build.push(if func.is_async {
+        // we can't name the future from an async function, so instead we provide a closure with a
+        // "dummy call" that never gets called so that the builder can infer the return type from
+        // the closure
         let infer_args = iter::repeat_n(quote!(::std::unreachable!()), func_params.len());
-        let infer = if func.is_async {
+        quote!(b.call_inferred(#c_name, || {
-            quote!(|| #ffi::future::lua_future::new(Self::#func_name(#(#infer_args),*)))
+            #ffi::future::lua_future::new(Self::#func_name(#(#infer_args),*))
-        } else {
+        });)
-            quote!(|| Self::#func_name(#(#infer_args),*))
+    } else {
-        };
+        quote!(b.call::<#func_ret>(#c_name);)
-        quote!(b.call_inferred(#c_name, #infer);)
     });
 
     registry.build.push(quote!(#(::std::assert!(#asserts);)*));
     registry.build.push(match func.attrs.metamethod {
-        Some(ref mm) => quote!(b.metatable(#mm, |b| { #(#build)* });),
+        Some(mm) => quote!(b.metatable(#mm, |b| { #(#build)* });),
         None => quote!(b.index(#lua_name, |b| { #(#build)* });),
     });
 
@@ -576,14 +624,28 @@ fn parse_lua_functions(imp: &mut ItemImpl) -> Result<Vec<LuaFunction>> {
                 .collect();
 
             if let Some(ref variadic) = func.sig.variadic {
-                params.push(parse_quote_spanned!(variadic.span() => variadic!())); // luaify builtin macro
+                // need to transform variadic to the `varidic!()` luaify builtin macro because we
+                // luaify a closure below, and closures don't have variadics.
+                params.push(parse_quote_spanned!(variadic.span() => variadic!()));
+            }
+
+            let attrs = parse_lua_function_attrs(&mut func.attrs)?;
+
+            if let Some(mm) = attrs.metamethod
+                && let Some(narg) = mm.narg()
+            {
+                syn_assert!(
+                    params.len() == narg,
+                    func.sig.inputs,
+                    "expected {narg} parameters for `{mm}` metamethod"
+                );
             }
 
             parsed.push(LuaFunction {
                 name: func.sig.ident.clone(),
                 params,
                 body: func.block.clone(),
-                attrs: parse_lua_function_attrs(&mut func.attrs)?,
+                attrs,
             });
 
             document_lua_function(func, parsed.last().unwrap());
@@ -599,9 +661,9 @@ fn parse_lua_function_attrs(attrs: &mut Vec<Attribute>) -> Result<LuaFunctionAtt
     let mut i = 0;
     while let Some(attr) = attrs.get(i) {
         if let Some(name) = attr.path().get_ident()
-            && let Ok(method) = Metamethod::try_from(&name.unraw())
+            && let Ok(mm) = Metamethod::try_from(&name.unraw())
         {
-            parsed.metamethod = Some(method);
+            parsed.metamethod = Some(mm);
             attrs.remove(i);
         } else {
             i += 1;
@@ -659,7 +721,7 @@ fn stub_lua_function(func: &mut ImplItemFn) -> Result<()> {
     Ok(())
 }
 
-fn add_lua_function(registry: &mut Registry, func: &LuaFunction) -> Result<()> {
+fn generate_lua_function(registry: &mut Registry, func: &LuaFunction) -> Result<()> {
     let ffi = ffi_crate();
     let luaify = quote!(#ffi::__internal::luaify!);
     let func_name = &func.name;
@@ -668,7 +730,7 @@ fn add_lua_function(registry: &mut Registry, func: &LuaFunction) -> Result<()> {
     let name = func_name.unraw().to_string();
 
     registry.build.push(match func.attrs.metamethod {
-        Some(ref mm) => quote!(b.metatable_raw(#mm, #luaify(|#(#params),*| #body));),
+        Some(mm) => quote!(b.metatable_raw(#mm, #luaify(|#(#params),*| #body));),
         None => quote!(b.index_raw(#name, #luaify(|#(#params),*| #body));),
     });
 
@@ -698,12 +760,6 @@ fn inject_merged_drop(registry: &mut Registry, lua: Option<&LuaFunction>) -> Res
     let c_name_str = c_name.to_string();
 
     if let Some(lua) = lua {
-        syn_assert!(
-            lua.params.len() == 1,
-            lua.name,
-            "finaliser must take exactly one parameter"
-        );
-
         let param = pat_ident(&lua.params[0])?;
         let body = &lua.body;
 
@@ -866,8 +922,8 @@ fn generate_stab_fallible(item: &ImplItemFn) -> Option<Attribute> {
     }
 }
 
-fn generate_stab_metamethod(method: Option<Metamethod>) -> Option<(Attribute, Attribute)> {
+fn generate_stab_metamethod(mm: Option<Metamethod>) -> Option<(Attribute, Attribute)> {
-    method.map(|method| {
+    mm.map(|mm| {
         let stab = generate_stab(StabConfig {
             text: "Metamethod",
             desc: "This function is a metamethod.",
@@ -875,7 +931,7 @@ fn generate_stab_metamethod(method: Option<Metamethod>) -> Option<(Attribute, At
             strong: false,
         });
 
-        let help = match method {
+        let help = match mm {
             Metamethod::Eq => "This function is a metamethod which is called by the `==` operator.",
             Metamethod::Len => "This function is a metamethod which is called by the `#` operator.",
             Metamethod::Lt => "This function is a metamethod which is called by the `<` operator.",