transpile: Convert final return statement into tail expression

c2rust-transpile/src/cfg/mod.rs

@@ -558,6 +558,8 @@ pub enum ImplicitReturnType {
     ///
     /// TODO: document
     StmtExpr(ExprContext, CExprId, Label),
+
+    StmtExprVoid,
 }
 
 /// A complete control-flow graph
@@ -632,6 +634,7 @@ impl Cfg<Label, StmtOrDecl> {
                             mk().break_expr_value(Some(brk_label.pretty_print()), Some(val)),
                         )));
                     }
+                    ImplicitReturnType::StmtExprVoid => (),
                 };
 
                 cfg_builder.add_wip_block(wip, End);
@@ -2114,13 +2117,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

@@ -15,12 +15,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.
 
@@ -342,12 +343,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)
 }
@@ -2377,7 +2373,8 @@ impl<'c> Translation<'c> {
                     _ => panic!("function body expects to be a compound statement"),
                 };
                 let mut converted_body =
-                    self.convert_function_body(ctx, name, body_ids, return_type, ret)?;
+                    self.convert_block_with_scope(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.
@@ -2494,7 +2491,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
@@ -2556,12 +2552,11 @@ impl<'c> Translation<'c> {
             &mut self.comment_store.borrow_mut(),
             current_block,
             self.tcfg.debug_relooper_labels,
-            cut_out_trailing_ret,
         )?);
         Ok(stmts)
     }
 
-    fn convert_function_body(
+    fn convert_block_with_scope(
         &self,
         ctx: ExprContext,
         name: &str,
@@ -2572,7 +2567,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())
         })
     }
 
@@ -4237,15 +4232,21 @@ impl<'c> Translation<'c> {
                 let mut stmts = match self.ast_context[result_id].kind {
                     CStmtKind::Expr(expr_id) => {
                         let ret = cfg::ImplicitReturnType::StmtExpr(ctx, expr_id, lbl.clone());
-                        self.convert_function_body(ctx, &name, &substmt_ids[0..(n - 1)], None, ret)?
+                        self.convert_block_with_scope(
+                            ctx,
+                            &name,
+                            &substmt_ids[0..(n - 1)],
+                            None,
+                            ret,
+                        )?
                     }
 
-                    _ => self.convert_function_body(
+                    _ => self.convert_block_with_scope(
                         ctx,
                         &name,
                         substmt_ids,
                         None,
-                        cfg::ImplicitReturnType::Void,
+                        cfg::ImplicitReturnType::StmtExprVoid,
                     )?,
                 };
 
@@ -5255,3 +5256,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@call_only_once.c.snap

@@ -29,7 +29,7 @@ pub const __ASSERT_FUNCTION: [::core::ffi::c_char; 32] = unsafe {
 static mut called: ::core::ffi::c_int = 0 as ::core::ffi::c_int;
 unsafe extern "C" fn called_only_once() -> ::core::ffi::c_int {
     called += 1;
-    return 1 as ::core::ffi::c_int;
+    1 as ::core::ffi::c_int
 }
 #[no_mangle]
 pub unsafe extern "C" fn assert_call_only_once() -> ::core::ffi::c_int {
@@ -45,5 +45,5 @@ pub unsafe extern "C" fn assert_call_only_once() -> ::core::ffi::c_int {
             );
         }
     };
-    return called;
+    called
 }

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@call_only_once.c.snap

@@ -22,7 +22,7 @@ extern "C" {
 static mut called: ::core::ffi::c_int = 0 as ::core::ffi::c_int;
 unsafe extern "C" fn called_only_once() -> ::core::ffi::c_int {
     called += 1;
-    return 1 as ::core::ffi::c_int;
+    1 as ::core::ffi::c_int
 }
 #[no_mangle]
 pub unsafe extern "C" fn assert_call_only_once() -> ::core::ffi::c_int {
@@ -35,5 +35,5 @@ pub unsafe extern "C" fn assert_call_only_once() -> ::core::ffi::c_int {
         );
     } else {
     };
-    return called;
+    called
 }

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
 }