Skip to content

Commit 3e5e51a

Browse files
alexcrichtonalexcrichton
authored
Squash more warnings in generated Rust code (#1214)
Add `-Dwarnings` to the codegen tests and fixup various issues around `unsafe` and such by shuffling around `unsafe` blocks.
1 parent f68f480 commit 3e5e51a

File tree

6 files changed

+54
-30
lines changed

6 files changed

+54
-30
lines changed

‎crates/guest-rust/rt/src/async_support/stream_support.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ impl<T> StreamReader<T> {
330330
}
331331

332332
#[doc(hidden)]
333-
pub fn from_handle_and_vtable(handle: u32, vtable: &'static StreamVtable<T>) -> Self {
333+
pub unsafe fn from_handle_and_vtable(handle: u32, vtable: &'static StreamVtable<T>) -> Self {
334334
super::with_entry(handle, |entry| match entry {
335335
Entry::Vacant(entry) => {
336336
entry.insert(Handle::Read);

‎crates/rust/src/bindgen.rs

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -464,21 +464,21 @@ impl Bindgen for FunctionBindgen<'_, '_> {
464464

465465
let result = if is_own {
466466
let name = self.r#gen.type_path(dealiased_resource, true);
467-
format!("unsafe {{ {name}::from_handle({op} as u32) }}")
467+
format!("{name}::from_handle({op} as u32)")
468468
} else if self.r#gen.is_exported_resource(*resource) {
469469
let name = resolve.types[*resource]
470470
.name
471471
.as_deref()
472472
.unwrap()
473473
.to_upper_camel_case();
474-
format!("unsafe {{ {name}Borrow::lift({op} as u32 as usize) }}")
474+
format!("{name}Borrow::lift({op} as u32 as usize)")
475475
} else {
476476
let tmp = format!("handle{}", self.tmp());
477477
self.handle_decls.push(format!("let {tmp};"));
478478
let name = self.r#gen.type_path(dealiased_resource, true);
479479
format!(
480480
"{{\n
481-
{tmp} = unsafe {{ {name}::from_handle({op} as u32) }};
481+
{tmp} = {name}::from_handle({op} as u32);
482482
&{tmp}
483483
}}"
484484
)
@@ -509,8 +509,8 @@ impl Bindgen for FunctionBindgen<'_, '_> {
509509
.unwrap();
510510
let path = self.r#gen.path_to_root();
511511
results.push(format!(
512-
"unsafe {{ {async_support}::FutureReader::from_handle_and_vtable\
513-
({op} as u32, &{path}wit_future::vtable{ordinal}::VTABLE) }}"
512+
"{async_support}::FutureReader::from_handle_and_vtable\
513+
({op} as u32, &{path}wit_future::vtable{ordinal}::VTABLE)"
514514
))
515515
}
516516

@@ -897,11 +897,10 @@ impl Bindgen for FunctionBindgen<'_, '_> {
897897
self.push_str("let ret = ");
898898
results.push("ret".to_string());
899899
}
900-
self.push_str("unsafe { ");
901900
self.push_str(&func);
902901
self.push_str("(");
903902
self.push_str(&operands.join(", "));
904-
self.push_str(") };\n");
903+
self.push_str(");\n");
905904
}
906905

907906
Instruction::AsyncCallWasm { name, .. } => {
@@ -1025,13 +1024,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
10251024
Instruction::AsyncCallReturn { name, params } => {
10261025
let func = self.declare_import(name, params, &[]);
10271026

1028-
uwriteln!(
1029-
self.src,
1030-
"\
1031-
unsafe {{ {func}({}) }};
1032-
",
1033-
operands.join(", ")
1034-
);
1027+
uwriteln!(self.src, "{func}({});", operands.join(", "));
10351028
self.emit_cleanup();
10361029
self.src.push_str("});\n");
10371030
}

‎crates/rust/src/interface.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -940,12 +940,12 @@ pub mod vtable{ordinal} {{
940940
self.src,
941941
"\
942942
#[doc(hidden)]
943-
#[allow(non_snake_case)]
943+
#[allow(non_snake_case, unused_unsafe)]
944944
pub unsafe fn _export_{name_snake}_cabi<T: {trait_name}>\
945945
",
946946
);
947947
let params = self.print_export_sig(func, async_);
948-
self.push_str(" {");
948+
self.push_str(" { unsafe {");
949949

950950
if !self.r#gen.opts.disable_run_ctors_once_workaround {
951951
let run_ctors_once = self.path_to_run_ctors_once();
@@ -1004,7 +1004,7 @@ pub mod vtable{ordinal} {{
10041004
self.src.push_str("\n");
10051005
}
10061006
self.src.push_str(&String::from(src));
1007-
self.src.push_str("}\n");
1007+
self.src.push_str("} }\n");
10081008

10091009
if async_ {
10101010
let async_support = self.r#gen.async_support_path();
@@ -1030,7 +1030,7 @@ pub mod vtable{ordinal} {{
10301030
"
10311031
);
10321032
let params = self.print_post_return_sig(func);
1033-
self.src.push_str("{\n");
1033+
self.src.push_str("{ unsafe {\n");
10341034

10351035
let mut f = FunctionBindgen::new(self, params, async_, self.wasm_import_module, false);
10361036
abi::post_return(f.r#gen.resolve, func, &mut f, async_);
@@ -1043,7 +1043,7 @@ pub mod vtable{ordinal} {{
10431043
assert!(!needs_cleanup_list);
10441044
assert!(handle_decls.is_empty());
10451045
self.src.push_str(&String::from(src));
1046-
self.src.push_str("}\n");
1046+
self.src.push_str("} }\n");
10471047
}
10481048
}
10491049

@@ -1228,6 +1228,7 @@ pub mod vtable{ordinal} {{
12281228
sig.self_arg = Some("&self".into());
12291229
sig.self_is_first_param = true;
12301230
}
1231+
self.src.push_str("#[allow(unused_variables)]\n");
12311232
self.print_signature(func, true, &sig);
12321233
self.src.push_str("{ unreachable!() }\n");
12331234
}
@@ -1307,6 +1308,10 @@ pub mod vtable{ordinal} {{
13071308
// TODO: re-add this when docs are back
13081309
// self.rustdoc_params(&func.results, "Return");
13091310

1311+
// TODO: should probably return `impl Future` in traits instead of
1312+
// having an `async fn` but that'll require more plumbing here.
1313+
self.push_str("#[allow(async_fn_in_trait)]\n");
1314+
13101315
if !sig.private {
13111316
self.push_str("pub ");
13121317
}
@@ -2717,7 +2722,7 @@ impl<'a> {camel}Borrow<'a>{{
27172722
#[doc(hidden)]
27182723
pub unsafe fn _lift(val: {repr}) -> {name} {{
27192724
if !cfg!(debug_assertions) {{
2720-
return ::core::mem::transmute(val);
2725+
return unsafe {{ ::core::mem::transmute(val) }};
27212726
}}
27222727
27232728
match val {{

‎crates/rust/src/lib.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -560,8 +560,10 @@ pub unsafe fn cabi_dealloc(ptr: *mut u8, size: usize, align: usize) {
560560
if size == 0 {
561561
return;
562562
}
563-
let layout = alloc::Layout::from_size_align_unchecked(size, align);
564-
alloc::dealloc(ptr, layout);
563+
unsafe {
564+
let layout = alloc::Layout::from_size_align_unchecked(size, align);
565+
alloc::dealloc(ptr, layout);
566+
}
565567
}
566568
",
567569
);
@@ -575,7 +577,7 @@ pub unsafe fn string_lift(bytes: Vec<u8>) -> String {
575577
if cfg!(debug_assertions) {
576578
String::from_utf8(bytes).unwrap()
577579
} else {
578-
String::from_utf8_unchecked(bytes)
580+
unsafe { String::from_utf8_unchecked(bytes) }
579581
}
580582
}
581583
",
@@ -603,7 +605,7 @@ pub unsafe fn char_lift(val: u32) -> char {
603605
if cfg!(debug_assertions) {
604606
core::char::from_u32(val).unwrap()
605607
} else {
606-
core::char::from_u32_unchecked(val)
608+
unsafe { core::char::from_u32_unchecked(val) }
607609
}
608610
}
609611
",

‎crates/test/src/lib.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,11 @@ impl Runner<'_> {
441441
TestKind::Runtime(_) => None,
442442
TestKind::Codegen(p) => Some((name, p)),
443443
}) {
444+
if let Some(filter) = &self.opts.filter {
445+
if !filter.is_match(name) {
446+
continue;
447+
}
448+
}
444449
let config = match fs::read_to_string(test) {
445450
Ok(wit) => config::parse_test_config::<config::WitConfig>(&wit, "//@")
446451
.with_context(|| format!("failed to parse test config from {test:?}"))?,
@@ -459,20 +464,29 @@ impl Runner<'_> {
459464
continue;
460465
}
461466

467+
let mut args = Vec::new();
468+
for arg in language.obj().default_bindgen_args_for_codegen() {
469+
args.push(arg.to_string());
470+
}
471+
462472
codegen_tests.push((
463473
language.clone(),
464474
test,
465475
name.to_string(),
466-
Vec::new(),
476+
args.clone(),
467477
config.clone(),
468478
));
469479

470-
for (args_kind, args) in language.obj().codegen_test_variants() {
480+
for (args_kind, new_args) in language.obj().codegen_test_variants() {
481+
let mut args = args.clone();
482+
for arg in new_args.iter() {
483+
args.push(arg.to_string());
484+
}
471485
codegen_tests.push((
472486
language.clone(),
473487
test,
474488
format!("{name}-{args_kind}"),
475-
args.iter().map(|s| s.to_string()).collect::<Vec<_>>(),
489+
args,
476490
config.clone(),
477491
));
478492
}
@@ -982,6 +996,12 @@ trait LanguageMethods {
982996
&[]
983997
}
984998

999+
/// Same as `default_bindgen_args` but specifically applied during codegen
1000+
/// tests, such as generating stub impls by default.
1001+
fn default_bindgen_args_for_codegen(&self) -> &[&str] {
1002+
&[]
1003+
}
1004+
9851005
/// Returns the name of this bindings generator when passed to
9861006
/// `wit-bindgen`.
9871007
///

‎crates/test/src/rust.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,11 @@ impl LanguageMethods for Rust {
7272
}
7373

7474
fn default_bindgen_args(&self) -> &[&str] {
75-
&["--generate-all"]
75+
&["--generate-all", "--format"]
76+
}
77+
78+
fn default_bindgen_args_for_codegen(&self) -> &[&str] {
79+
&["--stubs"]
7680
}
7781

7882
fn prepare(&self, runner: &mut Runner<'_>) -> Result<()> {
@@ -162,7 +166,6 @@ path = 'lib.rs'
162166
.join(format!("{}.rs", compile.component.kind)),
163167
)
164168
.arg(compile.component.path.file_name().unwrap())
165-
.arg("-Dwarnings")
166169
.arg("-o")
167170
.arg(&output);
168171
match compile.component.kind {
@@ -250,6 +253,7 @@ impl Runner<'_> {
250253
))
251254
.arg("--target")
252255
.arg(&opts.rust_target)
256+
.arg("-Dwarnings")
253257
.arg("-Cdebuginfo=1");
254258
for dep in state.wit_bindgen_deps.iter() {
255259
cmd.arg(&format!("-Ldependency={}", dep.display()));

0 commit comments

Comments
 (0)