immunant / c2rust

Migrate C code to Rust
https://c2rust.com/
Other
3.94k stars 233 forks source link

function macro definition not output #1122

Open GPHemsley opened 2 weeks ago

GPHemsley commented 2 weeks ago

I realize that it doesn't promise to compile, but when using --translate-fn-macros to maintain function macros, no line is included for the actual definition of the macro, leading to "cannot find macro `...` in this scope" errors instead of errors about the actual macro code.

Since the original impetus for the addition of this functionality was to possibly improve the rewrite effort (f1c1c38), it would be good to give the developer a pointer to where and how the macro should be defined.

Luckily, the tests already have a file that can demonstrate the issue:

./target/release/c2rust-transpile --translate-const-macros --translate-fn-macros tests/macros/src/define.c

In the corresponding output, TEST_FN_MACRO! and inc! are not defined anywhere.

--- tests/macros/src/define_without_fn.rs   2024-09-07 01:37:59.243037053 -0400
+++ tests/macros/src/define_with_fn.rs  2024-09-07 01:32:26.060152070 -0400
@@ -12,7 +12,7 @@
 }
 #[no_mangle]
 pub unsafe extern "C" fn test_fn_macro(mut x: libc::c_int) -> libc::c_int {
-    return x * x;
+    return TEST_FN_MACRO!(x);
 }
 pub const NULL: libc::c_int = 0 as libc::c_int;
 pub const TEST_CONST1: libc::c_int = 1 as libc::c_int;
@@ -64,16 +64,13 @@
     let mut a: libc::c_int = 0 as libc::c_int;
     let mut b: *mut libc::c_int = &mut a;
     ({
-        *b += 1;
-        *b;
-        *b;
-        *b
-    });
-    return ({
-        *b += 1;
+        let ref mut fresh0 = inc!(b);
+        *fresh0 += 1;
+        *fresh0;
         *b;
         *b
     });
+    return inc!(b);
 }
 #[no_mangle]
 pub unsafe extern "C" fn test_switch(mut x: libc::c_int) -> libc::c_int {

For reference, the original definitions and their relative placement: https://github.com/immunant/c2rust/blob/9eaf8a15ede349a3ed9bff4af4638a80c88b8b65/tests/macros/src/define.c#L3-L7 https://github.com/immunant/c2rust/blob/9eaf8a15ede349a3ed9bff4af4638a80c88b8b65/tests/macros/src/define.c#L56-L71

GPHemsley commented 1 week ago

I've been looking into this some, and it occurs to me that converting function-like C macros to Rust macros may not be the right thing to do.

Object-like C macros are turned into Rust constants. Shouldn't function-like C macros be turned into functions? The macro-ness of each has more to do with the C code than the Rust code; if macro behavior is still needed in Rust, can't that be handled inside the Rust function?

Or is the limitation specifically around the lack of a function signature?

Is there a reason this:

#define TEST_FN_MACRO(x) ((x) * (x))

can't be converted to this:

pub fn TEST_FN_MACRO(x: libc::c_int) -> libc::c_int { x * x }

?

kkysen commented 1 week ago

In the general case, we can't always convert C macros to Rust functions because they can do very weird things. Even in this example, the C macro is generic over any type that can be multiplied to itself, so the correct Rust function should be generic with a Mul bound. Deducing that from the body of the macro or its use sites is difficult, though.