Make [deny(unsafe_op_in_unsafe_fn)] optional (#1720)

We should not emit an extra `unsafe` block around most function bodies
by default. The c2rust output is less verbose and easier to consume by
TRACTOR tools if we silence the lint instead. We also do not want to
inadvertently change the TRACTOR T&E verbosity baseline.

Add a new flag `--deny-unsafe_op_in_unsafe_fn` so users opt into the
feature implemented in PR #1662.
Emission of unsafe blocks can be default-on for Rust 2027 editions if
necessary.

Author: ahomescu

c2rust-ast-builder/src/builder.rs

@@ -455,6 +455,7 @@ pub struct Builder {
     ext: Extern,
     attrs: Vec<Attribute>,
     span: Span,
+    deny_unsafe_op_in_unsafe_fn: bool,
 }
 
 impl Default for Builder {
@@ -468,6 +469,7 @@ impl Default for Builder {
             ext: Extern::None,
             attrs: Vec::new(),
             span: Span::call_site(),
+            deny_unsafe_op_in_unsafe_fn: false,
         }
     }
 }
@@ -507,6 +509,13 @@ impl Builder {
         self.unsafety(Unsafety::Unsafe)
     }
 
+    pub fn deny_unsafe_op_in_unsafe_fn(self) -> Self {
+        Builder {
+            deny_unsafe_op_in_unsafe_fn: true,
+            ..self
+        }
+    }
+
     pub fn constness<C: Make<Constness>>(self, constness: C) -> Self {
         let constness = constness.make(&self);
         Builder { constness, ..self }
@@ -1413,14 +1422,11 @@ impl Builder {
     {
         let sig = sig.make(&self);
 
-        if sig.unsafety.is_some() && !block.stmts.is_empty() {
-            // In edition 2024, `#[warn(unsafe_op_in_unsafe_fn)]` is on,
-            // so we need to wrap any `unsafe` operation in an `unsafe` block.
-            // For now, just wrap the whole function body in an `unsafe` block,
-            // which we can later narrow down to individual `unsafe` operations.
-            // In previous editions, this does not warn by default,
-            // but it can be turned on, and it's generally good practice anyways,
-            // so we do this unconditionally (i.e., no edition check needed).
+        if sig.unsafety.is_some() && !block.stmts.is_empty() && self.deny_unsafe_op_in_unsafe_fn {
+            // When `#[deny(unsafe_op_in_unsafe_fn)]` is in effect, unsafe operations
+            // inside an `unsafe fn` must be wrapped in an `unsafe` block.
+            // Wrap the whole function body for now; this can later be narrowed to
+            // individual operations.
             let unsafe_expr = mk().unsafe_block_expr(block);
             block = mk().block(vec![mk().expr_stmt(unsafe_expr)]);
         }

c2rust-transpile/src/lib.rs

@@ -107,6 +107,7 @@ pub struct TranspilerConfig {
     pub preserve_unused_functions: bool,
     pub log_level: log::LevelFilter,
     pub edition: RustEdition,
+    pub deny_unsafe_op_in_unsafe_fn: bool,
 
     /// Run `c2rust-postprocess` after transpiling and potentially refactoring.
     pub postprocess: bool,

c2rust-transpile/src/translator/mod.rs

@@ -1592,20 +1592,28 @@ impl<'c> Translation<'c> {
         let mut features = vec![];
         features.extend(self.features.borrow().iter());
         features.extend(self.type_converter.borrow().features_used());
-        let mut pragmas: PragmaVec = vec![
-            (
-                "allow",
-                vec![
-                    "non_upper_case_globals",
-                    "non_camel_case_types",
-                    "non_snake_case",
-                    "dead_code",
-                    "unused_mut",
-                    "unused_assignments",
-                ],
-            ),
-            ("deny", vec!["unsafe_op_in_unsafe_fn"]),
+
+        let mut allow = vec![
+            "non_upper_case_globals",
+            "non_camel_case_types",
+            "non_snake_case",
+            "dead_code",
+            "unused_mut",
+            "unused_assignments",
         ];
+        let mut pragmas: PragmaVec = vec![];
+        if self.tcfg.deny_unsafe_op_in_unsafe_fn {
+            // Edition 2024 defaults to deny `unsafe_op_in_unsafe_fn` so the
+            // deny pragma only has an effect on older versions. Go for brevity.
+            if self.tcfg.edition < Edition2024 {
+                pragmas.push(("deny", vec!["unsafe_op_in_unsafe_fn"]));
+            }
+        } else if self.tcfg.edition >= Edition2024 {
+            // Allow generation of less verbose code (fn bodies not wrapped in unsafe).
+            allow.push("unsafe_op_in_unsafe_fn");
+        }
+        pragmas.push(("allow", allow));
+
         if self.tcfg.cross_checks {
             features.append(&mut vec!["plugin"]);
             pragmas.push(("cross_check", vec!["yes"]));
@@ -1643,6 +1651,14 @@ impl<'c> Translation<'c> {
         ))
     }
 
+    fn mk(&self) -> Builder {
+        let mut b = mk();
+        if self.tcfg.deny_unsafe_op_in_unsafe_fn {
+            b = b.deny_unsafe_op_in_unsafe_fn();
+        }
+        b
+    }
+
     fn mk_cross_check(&self, mk: Builder, args: Vec<&str>) -> Builder {
         if self.tcfg.cross_checks {
             mk.call_attr("cross_check", args)
@@ -1847,7 +1863,7 @@ impl<'c> Translation<'c> {
         let fn_decl = mk().fn_decl(fn_name.clone(), vec![], None, fn_ty.clone());
         let fn_bare_decl = (vec![], None, fn_ty);
         let fn_block = mk().block(sectioned_static_initializers);
-        let fn_attributes = self.mk_cross_check(mk(), vec!["none"]);
+        let fn_attributes = self.mk_cross_check(self.mk(), vec!["none"]);
         let fn_item = fn_attributes
             .unsafe_()
             .extern_("C")
@@ -2434,6 +2450,12 @@ impl<'c> Translation<'c> {
                     mk().extern_("C")
                 };
 
+                // In Edition2024, `unsafe_op_in_unsafe_fn` is deny-by-default so we emit an allow pragma
+                // to silence warnings. Was this overridden by the `--deny_unsafe_op_in_unsafe_fn` flag?
+                if self.tcfg.deny_unsafe_op_in_unsafe_fn {
+                    mk_ = mk_.deny_unsafe_op_in_unsafe_fn();
+                }
+
                 for attr in attrs {
                     mk_ = match attr {
                         c_ast::Attribute::AlwaysInline => mk_.call_attr("inline", vec!["always"]),

c2rust-transpile/tests/snapshots.rs

@@ -56,6 +56,7 @@ fn config(edition: RustEdition) -> TranspilerConfig {
         preserve_unused_functions: false,
         log_level: log::LevelFilter::Warn,
         edition,
+        deny_unsafe_op_in_unsafe_fn: false,
         postprocess: false,
         emit_build_files: false,
         binaries: Vec::new(),

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

@@ -10,36 +10,33 @@ expression: cat tests/snapshots/alloca.2021.rs
     unused_assignments,
     unused_mut
 )]
-#![deny(unsafe_op_in_unsafe_fn)]
 #[no_mangle]
 pub static mut TRUE: ::core::ffi::c_int = 1 as ::core::ffi::c_int;
 #[no_mangle]
 pub unsafe extern "C" fn alloca_sum(
     mut val1: ::core::ffi::c_int,
     mut val2: ::core::ffi::c_int,
 ) -> ::core::ffi::c_int {
-    unsafe {
-        let mut c2rust_alloca_allocations: Vec<Vec<u8>> = Vec::new();
-        let mut alloca1: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
-        let mut alloca2: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
-        if TRUE != 0 {
-            c2rust_alloca_allocations.push(::std::vec::from_elem(
-                0,
-                ::core::mem::size_of::<::core::ffi::c_int>() as usize,
-            ));
-            alloca1 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
-                as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
-            *alloca1 = val1;
-        }
-        if TRUE != 0 {
-            c2rust_alloca_allocations.push(::std::vec::from_elem(
-                0,
-                ::core::mem::size_of::<::core::ffi::c_int>() as usize,
-            ));
-            alloca2 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
-                as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
-            *alloca2 = val2;
-        }
-        return *alloca1 + *alloca2;
+    let mut c2rust_alloca_allocations: Vec<Vec<u8>> = Vec::new();
+    let mut alloca1: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
+    let mut alloca2: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
+    if TRUE != 0 {
+        c2rust_alloca_allocations.push(::std::vec::from_elem(
+            0,
+            ::core::mem::size_of::<::core::ffi::c_int>() as usize,
+        ));
+        alloca1 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
+            as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
+        *alloca1 = val1;
     }
+    if TRUE != 0 {
+        c2rust_alloca_allocations.push(::std::vec::from_elem(
+            0,
+            ::core::mem::size_of::<::core::ffi::c_int>() as usize,
+        ));
+        alloca2 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
+            as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
+        *alloca2 = val2;
+    }
+    return *alloca1 + *alloca2;
 }

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

@@ -7,39 +7,37 @@ expression: cat tests/snapshots/alloca.2024.rs
     non_camel_case_types,
     non_snake_case,
     non_upper_case_globals,
+    unsafe_op_in_unsafe_fn,
     unused_assignments,
     unused_mut
 )]
-#![deny(unsafe_op_in_unsafe_fn)]
 #[unsafe(no_mangle)]
 pub static mut TRUE: ::core::ffi::c_int = 1 as ::core::ffi::c_int;
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn alloca_sum(
     mut val1: ::core::ffi::c_int,
     mut val2: ::core::ffi::c_int,
 ) -> ::core::ffi::c_int {
-    unsafe {
-        let mut c2rust_alloca_allocations: Vec<Vec<u8>> = Vec::new();
-        let mut alloca1: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
-        let mut alloca2: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
-        if TRUE != 0 {
-            c2rust_alloca_allocations.push(::std::vec::from_elem(
-                0,
-                ::core::mem::size_of::<::core::ffi::c_int>() as usize,
-            ));
-            alloca1 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
-                as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
-            *alloca1 = val1;
-        }
-        if TRUE != 0 {
-            c2rust_alloca_allocations.push(::std::vec::from_elem(
-                0,
-                ::core::mem::size_of::<::core::ffi::c_int>() as usize,
-            ));
-            alloca2 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
-                as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
-            *alloca2 = val2;
-        }
-        return *alloca1 + *alloca2;
+    let mut c2rust_alloca_allocations: Vec<Vec<u8>> = Vec::new();
+    let mut alloca1: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
+    let mut alloca2: *mut ::core::ffi::c_int = ::core::ptr::null_mut::<::core::ffi::c_int>();
+    if TRUE != 0 {
+        c2rust_alloca_allocations.push(::std::vec::from_elem(
+            0,
+            ::core::mem::size_of::<::core::ffi::c_int>() as usize,
+        ));
+        alloca1 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
+            as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
+        *alloca1 = val1;
     }
+    if TRUE != 0 {
+        c2rust_alloca_allocations.push(::std::vec::from_elem(
+            0,
+            ::core::mem::size_of::<::core::ffi::c_int>() as usize,
+        ));
+        alloca2 = c2rust_alloca_allocations.last_mut().unwrap().as_mut_ptr()
+            as *mut ::core::ffi::c_void as *mut ::core::ffi::c_int;
+        *alloca2 = val2;
+    }
+    return *alloca1 + *alloca2;
 }