transpile: Convert final return statement into tail expression

c2rust-transpile/src/cfg/mod.rs

@@ -2114,13 +2114,8 @@ impl CfgBuilder {
         };
 
         // Run relooper
-        let mut stmts = translator.convert_cfg(
-            &format!("<substmt_{:?}>", stmt_id),
-            graph,
-            store,
-            live_in,
-            false,
-        )?;
+        let mut stmts =
+            translator.convert_cfg(&format!("<substmt_{:?}>", stmt_id), graph, store, live_in)?;
 
         let inner_span = stmts.first().map(|stmt| stmt.span());
 

c2rust-transpile/src/cfg/structures.rs

@@ -2,7 +2,7 @@
 
 use super::*;
 use log::warn;
-use syn::{spanned::Spanned as _, ExprBreak, ExprIf, ExprReturn, ExprUnary, Stmt};
+use syn::{spanned::Spanned as _, ExprBreak, ExprIf, ExprUnary, Stmt};
 
 use crate::rust_ast::{comment_store, set_span::SetSpan, BytePos, SpanExt};
 
@@ -12,7 +12,6 @@ pub fn structured_cfg(
     comment_store: &mut comment_store::CommentStore,
     current_block: Box<Expr>,
     debug_labels: bool,
-    cut_out_trailing_ret: bool,
 ) -> TranslationResult<Vec<Stmt>> {
     let ast: StructuredAST<Box<Expr>, Pat, Label, Stmt> =
         structured_cfg_help(vec![], &IndexSet::new(), root, &mut IndexSet::new())?;
@@ -21,15 +20,7 @@ pub fn structured_cfg(
         debug_labels,
         current_block,
     };
-    let (mut stmts, _span) = s.to_stmt(ast, comment_store);
-
-    // If the very last statement in the vector is a `return`, we can either cut it out or replace
-    // it with the returned value.
-    if cut_out_trailing_ret {
-        if let Some(Stmt::Expr(Expr::Return(ExprReturn { expr: None, .. }), _)) = stmts.last() {
-            stmts.pop();
-        }
-    }
+    let (stmts, _span) = s.to_stmt(ast, comment_store);
 
     Ok(stmts)
 }

c2rust-transpile/src/translator/mod.rs

@@ -13,12 +13,13 @@ use log::{error, info, trace, warn};
 use proc_macro2::{Punct, Spacing::*, Span, TokenStream, TokenTree};
 use syn::spanned::Spanned as _;
 use syn::{
-    AttrStyle, BareVariadic, Block, Expr, ExprBinary, ExprBlock, ExprBreak, ExprCast, ExprField,
-    ExprIndex, ExprParen, ExprUnary, FnArg, ForeignItem, ForeignItemFn, ForeignItemMacro,
-    ForeignItemStatic, ForeignItemType, Ident, Item, ItemConst, ItemEnum, ItemExternCrate, ItemFn,
-    ItemForeignMod, ItemImpl, ItemMacro, ItemMod, ItemStatic, ItemStruct, ItemTrait,
-    ItemTraitAlias, ItemType, ItemUnion, ItemUse, Lit, MacroDelimiter, PathSegment, ReturnType,
-    Stmt, Type, TypeTuple, UseTree, Visibility,
+    Arm, AttrStyle, BareVariadic, Block, Expr, ExprBinary, ExprBlock, ExprBreak, ExprCast,
+    ExprField, ExprIf, ExprIndex, ExprMatch, ExprParen, ExprReturn, ExprTuple, ExprUnary,
+    ExprUnsafe, FnArg, ForeignItem, ForeignItemFn, ForeignItemMacro, ForeignItemStatic,
+    ForeignItemType, Ident, Item, ItemConst, ItemEnum, ItemExternCrate, ItemFn, ItemForeignMod,
+    ItemImpl, ItemMacro, ItemMod, ItemStatic, ItemStruct, ItemTrait, ItemTraitAlias, ItemType,
+    ItemUnion, ItemUse, Lit, MacroDelimiter, PathSegment, ReturnType, Stmt, Type, TypeTuple,
+    UseTree, Visibility,
 };
 use syn::{BinOp, UnOp}; // To override `c_ast::{BinOp,UnOp}` from glob import.
 
@@ -340,12 +341,7 @@ pub fn stmts_block(mut stmts: Vec<Stmt>) -> Block {
             }),
             None,
         )) if stmts.is_empty() => return block,
-        Some(mut s) => {
-            if let Stmt::Expr(e, None) = s {
-                s = Stmt::Expr(e, Some(Default::default()));
-            }
-            stmts.push(s);
-        }
+        Some(s) => stmts.push(s),
     }
     mk().block(stmts)
 }
@@ -2404,6 +2400,7 @@ impl<'c> Translation<'c> {
                 };
                 let mut converted_body =
                     self.convert_function_body(ctx, name, body_ids, return_type, ret)?;
+                strip_tail_return(&mut converted_body);
 
                 // If `alloca` was used in the function body, include a variable to hold the
                 // allocations.
@@ -2520,7 +2517,6 @@ impl<'c> Translation<'c> {
         graph: cfg::Cfg<cfg::Label, cfg::StmtOrDecl>,
         store: cfg::DeclStmtStore,
         live_in: IndexSet<CDeclId>,
-        cut_out_trailing_ret: bool,
     ) -> TranslationResult<Vec<Stmt>> {
         if self.tcfg.dump_function_cfgs {
             graph
@@ -2582,7 +2578,6 @@ impl<'c> Translation<'c> {
             &mut self.comment_store.borrow_mut(),
             current_block,
             self.tcfg.debug_relooper_labels,
-            cut_out_trailing_ret,
         )?);
         Ok(stmts)
     }
@@ -2598,7 +2593,7 @@ impl<'c> Translation<'c> {
         // Function body scope
         self.with_scope(|| {
             let (graph, store) = cfg::Cfg::from_stmts(self, ctx, body_ids, ret, ret_ty)?;
-            self.convert_cfg(name, graph, store, IndexSet::new(), true)
+            self.convert_cfg(name, graph, store, IndexSet::new())
         })
     }
 
@@ -5266,3 +5261,81 @@ impl<'c> Translation<'c> {
         }
     }
 }
+
+// If the very last statement in the vector is a `return`, either cut it out or replace it with
+// the returned value.
+fn strip_tail_return(stmts: &mut Vec<Stmt>) {
+    if let Some(Stmt::Expr(expr, semi)) = stmts.last_mut() {
+        *semi = None;
+        strip_tail_return_expr(expr);
+
+        // If the expression was replaced with an empty tuple (), then just delete it altogether.
+        if matches!(expr, Expr::Tuple(ExprTuple { elems, .. }) if elems.is_empty()) {
+            stmts.pop();
+        }
+    }
+}
+
+// If a return is found, replace it with the returned expression.
+// If an expression of another kind is found, and it contains a subexpression that becomes the
+// final value of the whole, then recurse down into it.
+fn strip_tail_return_expr(expr: &mut Expr) {
+    match expr {
+        old_expr @ Expr::Return(ExprReturn { .. }) => {
+            // placeholder value to allow swapping
+            let temp = mem::replace(old_expr, Expr::Verbatim(Default::default()));
+            // TODO: Rust 1.65: use let-else
+            let expr = match temp {
+                Expr::Return(ExprReturn { expr, .. }) => expr,
+                _ => unreachable!(),
+            };
+
+            if let Some(expr) = expr {
+                // Replace return + expression with the expression.
+                *old_expr = *expr;
+            } else {
+                // Replace standalone return with ()
+                *old_expr = *mk().tuple_expr(vec![]);
+            }
+        }
+
+        // Simple blocks, recurse down
+        // TODO: add when syn is updated
+        // | Expr::Const(ExprConst { block, .. })
+        Expr::Block(ExprBlock { block, .. }) | Expr::Unsafe(ExprUnsafe { block, .. }) => {
+            strip_tail_return(&mut block.stmts);
+        }
+
+        // Recurse down both branches of the `if`
+        Expr::If(ExprIf {
+            then_branch,
+            else_branch,
+            ..
+        }) => {
+            strip_tail_return(&mut then_branch.stmts);
+
+            // If the function returns a value, then there must be an else_branch,
+            // but if it returns void then there doesn't have to be. For example
+            //
+            //     if condition {
+            //         // stuff
+            //         return;
+            //     }
+            // } // end of function
+            if let Some((_, else_branch)) = else_branch {
+                strip_tail_return_expr(else_branch);
+            }
+        }
+
+        // Recurse down all arms of the `match`
+        Expr::Match(ExprMatch { arms, .. }) => {
+            for Arm { body, .. } in arms {
+                strip_tail_return_expr(body);
+            }
+        }
+
+        // Other expression types do not have blocks with tail expressions, whose value becomes
+        // that of the whole expression.
+        _ => (),
+    }
+}

c2rust-transpile/tests/snapshots/exprs.c

@@ -38,3 +38,12 @@ void compound_literal(){
     /// https://github.com/immunant/c2rust/issues/1234
     int i = (enum {A, B, C}){1};
 }
+
+void statement_expr() {
+    ({
+        puts("should execute");
+        return;
+    });
+
+    puts("should be unreachable!");
+}

c2rust-transpile/tests/snapshots/snapshots__transpile-aarch64@vm_x86.c.snap

@@ -68,5 +68,5 @@ pub unsafe extern "C" fn VM_CallCompiled(
         return 0 as ::core::ffi::c_int;
     }
     (*vm).programStack = stackOnEntry;
-    return *opStack.offset(opStackOfs as isize);
+    *opStack.offset(opStackOfs as isize)
 }

c2rust-transpile/tests/snapshots/snapshots__transpile-linux@macros.c.snap

@@ -22,12 +22,12 @@ extern "C" {
 pub type size_t = usize;
 #[no_mangle]
 pub unsafe extern "C" fn errno_is_error() -> bool {
-    return *__errno_location() != 0 as ::core::ffi::c_int;
+    *__errno_location() != 0 as ::core::ffi::c_int
 }
 #[no_mangle]
 pub unsafe extern "C" fn size_of_const() -> ::core::ffi::c_int {
     let mut a: [::core::ffi::c_int; 10] = [0; 10];
-    return SIZE as ::core::ffi::c_int;
+    SIZE as ::core::ffi::c_int
 }
 pub const SIZE: usize = ::core::mem::size_of::<[::core::ffi::c_int; 10]>();
 pub const POS: [::core::ffi::c_char; 3] =

c2rust-transpile/tests/snapshots/snapshots__transpile-linux@rnd.c.snap

@@ -32,5 +32,5 @@ pub unsafe extern "C" fn get_rand_seed() -> uint32_t {
         .wrapping_mul(cur_rand_seed)
         .wrapping_add(INCREMENT);
     let mut ret: uint32_t = abs(cur_rand_seed as ::core::ffi::c_int) as uint32_t;
-    return ret;
+    ret
 }

c2rust-transpile/tests/snapshots/snapshots__transpile-linux@rotate.c.snap

@@ -15,15 +15,13 @@ input_file: c2rust-transpile/tests/snapshots/os-specific/rotate.c
 pub unsafe extern "C" fn rotate_left_64(
     mut x: ::core::ffi::c_ulonglong,
 ) -> ::core::ffi::c_ulonglong {
-    return (x as ::core::ffi::c_ulong)
-        .rotate_left(4 as ::core::ffi::c_int as ::core::ffi::c_ulong as u32)
-        as ::core::ffi::c_ulonglong;
+    (x as ::core::ffi::c_ulong).rotate_left(4 as ::core::ffi::c_int as ::core::ffi::c_ulong as u32)
+        as ::core::ffi::c_ulonglong
 }
 #[no_mangle]
 pub unsafe extern "C" fn rotate_right_64(
     mut x: ::core::ffi::c_ulonglong,
 ) -> ::core::ffi::c_ulonglong {
-    return (x as ::core::ffi::c_ulong)
-        .rotate_right(4 as ::core::ffi::c_int as ::core::ffi::c_ulong as u32)
-        as ::core::ffi::c_ulonglong;
+    (x as ::core::ffi::c_ulong).rotate_right(4 as ::core::ffi::c_int as ::core::ffi::c_ulong as u32)
+        as ::core::ffi::c_ulonglong
 }

c2rust-transpile/tests/snapshots/snapshots__transpile-macos@macros.c.snap

@@ -23,12 +23,12 @@ pub type __darwin_size_t = usize;
 pub type size_t = __darwin_size_t;
 #[no_mangle]
 pub unsafe extern "C" fn errno_is_error() -> bool {
-    return *__error() != 0 as ::core::ffi::c_int;
+    *__error() != 0 as ::core::ffi::c_int
 }
 #[no_mangle]
 pub unsafe extern "C" fn size_of_const() -> ::core::ffi::c_int {
     let mut a: [::core::ffi::c_int; 10] = [0; 10];
-    return SIZE as ::core::ffi::c_int;
+    SIZE as ::core::ffi::c_int
 }
 pub const SIZE: usize = ::core::mem::size_of::<[::core::ffi::c_int; 10]>();
 pub const POS: [::core::ffi::c_char; 3] =

c2rust-transpile/tests/snapshots/snapshots__transpile-macos@rnd.c.snap

@@ -30,5 +30,5 @@ pub unsafe extern "C" fn get_rand_seed() -> uint32_t {
         .wrapping_mul(cur_rand_seed)
         .wrapping_add(INCREMENT);
     let mut ret: uint32_t = abs(cur_rand_seed as ::core::ffi::c_int) as uint32_t;
-    return ret;
+    ret
 }

c2rust-transpile/tests/snapshots/snapshots__transpile-macos@rotate.c.snap

@@ -15,11 +15,11 @@ input_file: c2rust-transpile/tests/snapshots/os-specific/rotate.c
 pub unsafe extern "C" fn rotate_left_64(
     mut x: ::core::ffi::c_ulonglong,
 ) -> ::core::ffi::c_ulonglong {
-    return x.rotate_left(4 as ::core::ffi::c_int as ::core::ffi::c_ulonglong as u32);
+    x.rotate_left(4 as ::core::ffi::c_int as ::core::ffi::c_ulonglong as u32)
 }
 #[no_mangle]
 pub unsafe extern "C" fn rotate_right_64(
     mut x: ::core::ffi::c_ulonglong,
 ) -> ::core::ffi::c_ulonglong {
-    return x.rotate_right(4 as ::core::ffi::c_int as ::core::ffi::c_ulonglong as u32);
+    x.rotate_right(4 as ::core::ffi::c_int as ::core::ffi::c_ulonglong as u32)
 }

c2rust-transpile/tests/snapshots/snapshots__transpile-x86_64@vm_x86.c.snap

@@ -78,5 +78,5 @@ pub unsafe extern "C" fn VM_CallCompiled(
         return 0 as ::core::ffi::c_int;
     }
     (*vm).programStack = stackOnEntry;
-    return *opStack.offset(opStackOfs as isize);
+    *opStack.offset(opStackOfs as isize)
 }

c2rust-transpile/tests/snapshots/snapshots__transpile@alloca.c.snap

@@ -37,5 +37,5 @@ pub unsafe extern "C" fn alloca_sum(
         alloca2 = alloca_allocations.last_mut().unwrap().as_mut_ptr() as *mut ::core::ffi::c_int;
         *alloca2 = val2;
     }
-    return *alloca1 + *alloca2;
+    *alloca1 + *alloca2
 }

c2rust-transpile/tests/snapshots/snapshots__transpile@atomics.c.snap

@@ -35,6 +35,6 @@ pub unsafe extern "C" fn c11_atomics(mut x: ::core::ffi::c_int) -> ::core::ffi::
         ::core::intrinsics::atomic_cxchgweak_seqcst_seqcst(&raw mut x, *&raw mut expected, desired);
     *&raw mut expected = fresh1.0;
     fresh1.1;
-    return x;
+    x
 }
 pub const __ATOMIC_SEQ_CST: ::core::ffi::c_int = 5 as ::core::ffi::c_int;

c2rust-transpile/tests/snapshots/snapshots__transpile@exprs.c.snap

@@ -11,7 +11,7 @@ input_file: c2rust-transpile/tests/snapshots/exprs.c
     unused_assignments,
     unused_mut
 )]
-#![feature(raw_ref_op)]
+#![feature(label_break_value, raw_ref_op)]
 extern "C" {
     fn puts(str: *const ::core::ffi::c_char) -> ::core::ffi::c_int;
 }
@@ -21,7 +21,7 @@ pub const B: C2RustUnnamed = 1;
 pub const A: C2RustUnnamed = 0;
 unsafe extern "C" fn side_effect() -> ::core::ffi::c_int {
     puts(b"the return of side effect\0" as *const u8 as *const ::core::ffi::c_char);
-    return 10 as ::core::ffi::c_int;
+    10 as ::core::ffi::c_int
 }
 #[no_mangle]
 pub unsafe extern "C" fn unary_without_side_effect() {
@@ -58,3 +58,9 @@ pub unsafe extern "C" fn unary_with_side_effect() {
 pub unsafe extern "C" fn compound_literal() {
     let mut i: ::core::ffi::c_int = B as ::core::ffi::c_int;
 }
+#[no_mangle]
+pub unsafe extern "C" fn statement_expr() {
+    puts(b"should execute\0" as *const u8 as *const ::core::ffi::c_char);
+    return;
+    puts(b"should be unreachable!\0" as *const u8 as *const ::core::ffi::c_char);
+}

c2rust-transpile/tests/snapshots/snapshots__transpile@factorial.c.snap

@@ -19,5 +19,5 @@ pub unsafe extern "C" fn factorial(mut n: ::core::ffi::c_ushort) -> ::core::ffi:
         result = (result as ::core::ffi::c_int * i as ::core::ffi::c_int) as ::core::ffi::c_ushort;
         i = i.wrapping_add(1);
     }
-    return result;
+    result
 }

c2rust-transpile/tests/snapshots/snapshots__transpile@gotos.c.snap

@@ -18,5 +18,5 @@ pub unsafe extern "C" fn sum(mut count: ::core::ffi::c_int) -> ::core::ffi::c_in
         x += count;
         count -= 1;
     }
-    return x;
+    x
 }

c2rust-transpile/tests/snapshots/snapshots__transpile@macrocase.c.snap

@@ -29,5 +29,5 @@ pub unsafe extern "C" fn ZSTD_dParam_getBounds(mut dParam: ZSTD_dParameter) -> :
         }
         _ => {}
     }
-    return bounds;
+    bounds
 }