Skip to content

Commit 100f567

Browse files
committed
ImproperCTypes: change what type is blamed, use nested help messages
A major change to the content of linting messages, but not where they occur. Now, the "uses type `_`" part of the message mentions the type directly visible where the error occurs, and the nested note/help messages trace the link to the actual source of the FFI-unsafety
1 parent 8bc7d92 commit 100f567

19 files changed

+143
-55
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,8 @@ lint_improper_ctypes_str_reason = string slices have no C equivalent
402402
lint_improper_ctypes_struct_fieldless_help = consider adding a member to this struct
403403
404404
lint_improper_ctypes_struct_fieldless_reason = this struct has no fields
405+
lint_improper_ctypes_struct_dueto = this struct/enum/union (`{$ty}`) is FFI-unsafe due to a `{$inner_ty}` field
406+
405407
lint_improper_ctypes_struct_layout_help = consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
406408
407409
lint_improper_ctypes_struct_layout_reason = this struct has unspecified layout

compiler/rustc_lint/src/types/improper_ctypes.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,6 @@ impl<'tcx> FfiResult<'tcx> {
362362
/// For instance, if we have a repr(C) struct in a function's argument, FFI unsafeties inside the struct
363363
/// are to be blamed on the struct and not the members.
364364
/// This is where we use this wrapper, to tell "all FFI-unsafeties in there are caused by this `ty`"
365-
#[expect(unused)]
366365
fn with_overrides(mut self, override_cause_ty: Option<Ty<'tcx>>) -> FfiResult<'tcx> {
367366
use FfiResult::*;
368367

@@ -889,7 +888,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
889888
FfiSafe => false,
890889
// `()` fields are FFI-safe!
891890
FfiPhantom(..) => true,
892-
r @ FfiUnsafe { .. } => return r,
891+
r @ FfiUnsafe { .. } => {
892+
return r.wrap_all(
893+
ty,
894+
fluent::lint_improper_ctypes_struct_dueto,
895+
None,
896+
);
897+
}
893898
}
894899
}
895900

@@ -940,7 +945,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
940945
);
941946
}
942947

943-
if def.non_enum_variant().fields.is_empty() {
948+
let ffires = if def.non_enum_variant().fields.is_empty() {
944949
FfiResult::new_with_reason(
945950
ty,
946951
if def.is_struct() {
@@ -956,7 +961,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
956961
)
957962
} else {
958963
self.visit_variant_fields(state, ty, def, def.non_enum_variant(), args)
959-
}
964+
};
965+
966+
// from now on in the function, we lint the actual insides of the struct/union: if something is wrong,
967+
// then the "fault" comes from inside the struct itself.
968+
// even if we add more details to the lint, the initial line must specify that the FFI-unsafety is because of the struct
969+
// - if the struct is from the same crate, there is another warning on its definition anyway
970+
// (unless it's about Boxes and references without Option<_>
971+
// which is partly why we keep the details as to why that struct is FFI-unsafe)
972+
// - if the struct is from another crate, then there's not much that can be done anyways
973+
//
974+
// this enum is visited in the middle of another lint,
975+
// so we override the "cause type" of the lint
976+
ffires.with_overrides(Some(ty))
960977
}
961978

962979
fn visit_enum(
@@ -1003,10 +1020,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
10031020
}
10041021
});
10051022
if let ControlFlow::Break(result) = ret {
1006-
return result;
1023+
// this enum is visited in the middle of another lint,
1024+
// so we override the "cause type" of the lint
1025+
// (for more detail, see comment in ``visit_struct_union`` before its call to ``result.with_overrides``)
1026+
result.with_overrides(Some(ty))
1027+
} else {
1028+
FfiSafe
10071029
}
1008-
1009-
FfiSafe
10101030
}
10111031

10121032
/// Checks if the given type is "ffi-safe" (has a stable, well-defined

tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ type Foo = extern "C" fn(::std::ffi::CStr);
88
//~^ WARN `extern` callback uses type
99
extern "C" {
1010
fn meh(blah: Foo);
11-
//~^ WARN `extern` block uses type
11+
// ^ FIXME: the error isn't seen here but at least it's reported elsewhere
1212
}
1313

1414
fn main() {

tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,11 @@ warning: `extern` callback uses type `CStr`, which is not FFI-safe
44
LL | type Foo = extern "C" fn(::std::ffi::CStr);
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
66
|
7-
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
7+
= note: the function pointer to `extern "C" fn(CStr)` is FFI-unsafe due to `CStr`
8+
= help: consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
9+
and use (depending on the use case) `CStr::as_ptr()`, `CString::into_raw()` then `CString::from_raw()`, or a dedicated buffer
810
= note: `CStr`/`CString` do not have a guaranteed layout
911
= note: `#[warn(improper_c_callbacks)]` (part of `#[warn(improper_c_boundaries)]`) on by default
1012

11-
warning: `extern` block uses type `CStr`, which is not FFI-safe
12-
--> $DIR/extern-C-non-FFI-safe-arg-ice-52334.rs:10:18
13-
|
14-
LL | fn meh(blah: Foo);
15-
| ^^^ not FFI-safe
16-
|
17-
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
18-
= note: `CStr`/`CString` do not have a guaranteed layout
19-
= note: `#[warn(improper_ctypes)]` on by default
20-
21-
warning: 2 warnings emitted
13+
warning: 1 warning emitted
2214

tests/ui/extern/extern-C-str-arg-ice-80125.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub struct Struct(ExternCallback);
77

88
#[no_mangle]
99
pub extern "C" fn register_something(bind: ExternCallback) -> Struct {
10-
//~^ WARN `extern` fn uses type `str`, which is not FFI-safe
10+
// ^ FIXME: the error isn't seen here, but at least it's reported elsewhere
1111
//~^^ WARN `extern` fn uses type `Struct`, which is not FFI-safe
1212
Struct(bind)
1313
}

tests/ui/extern/extern-C-str-arg-ice-80125.stderr

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,11 @@ warning: `extern` callback uses type `str`, which is not FFI-safe
44
LL | type ExternCallback = extern "C" fn(*const u8, u32, str);
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
66
|
7+
= note: the function pointer to `extern "C" fn(*const u8, u32, str)` is FFI-unsafe due to `str`
78
= help: consider using `*const u8` and a length instead
89
= note: string slices have no C equivalent
910
= note: `#[warn(improper_c_callbacks)]` (part of `#[warn(improper_c_boundaries)]`) on by default
1011

11-
warning: `extern` fn uses type `str`, which is not FFI-safe
12-
--> $DIR/extern-C-str-arg-ice-80125.rs:9:44
13-
|
14-
LL | pub extern "C" fn register_something(bind: ExternCallback) -> Struct {
15-
| ^^^^^^^^^^^^^^ not FFI-safe
16-
|
17-
= help: consider using `*const u8` and a length instead
18-
= note: string slices have no C equivalent
19-
= note: `#[warn(improper_c_fn_definitions)]` (part of `#[warn(improper_c_boundaries)]`) on by default
20-
2112
warning: `extern` fn uses type `Struct`, which is not FFI-safe
2213
--> $DIR/extern-C-str-arg-ice-80125.rs:9:63
2314
|
@@ -31,6 +22,7 @@ note: the type is defined here
3122
|
3223
LL | pub struct Struct(ExternCallback);
3324
| ^^^^^^^^^^^^^^^^^
25+
= note: `#[warn(improper_c_fn_definitions)]` (part of `#[warn(improper_c_boundaries)]`) on by default
3426

35-
warning: 3 warnings emitted
27+
warning: 2 warnings emitted
3628

tests/ui/lint/extern-C-fnptr-lints-slices.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | pub type F = extern "C" fn(&[u8]);
55
| ^^^^^^^^^^^^^^^^^^^^ not FFI-safe
66
|
77
= note: the function pointer to `for<'a> extern "C" fn(&'a [u8])` is FFI-unsafe due to `&[u8]`
8-
= help: consider using a raw pointer to the slice's first element (and a length) instead
8+
= help: consider using a raw pointer instead
99
= note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
1010
note: the lint level is defined here
1111
--> $DIR/extern-C-fnptr-lints-slices.rs:1:8

tests/ui/lint/improper_ctypes/lint-113436-1.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ pub struct Bar {
2020
}
2121

2222
extern "C" fn bar(x: Bar) -> Bar {
23-
//~^ ERROR `extern` fn uses type `NotSafe`, which is not FFI-safe
24-
//~^^ ERROR `extern` fn uses type `NotSafe`, which is not FFI-safe
23+
//~^ ERROR `extern` fn uses type `Bar`, which is not FFI-safe
24+
//~^^ ERROR `extern` fn uses type `Bar`, which is not FFI-safe
2525
todo!()
2626
}
2727

tests/ui/lint/improper_ctypes/lint-113436-1.stderr

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
1-
error: `extern` fn uses type `NotSafe`, which is not FFI-safe
1+
error: `extern` fn uses type `Bar`, which is not FFI-safe
22
--> $DIR/lint-113436-1.rs:22:22
33
|
44
LL | extern "C" fn bar(x: Bar) -> Bar {
55
| ^^^ not FFI-safe
66
|
7+
= note: this struct/enum/union (`Bar`) is FFI-unsafe due to a `NotSafe` field
8+
note: the type is defined here
9+
--> $DIR/lint-113436-1.rs:16:1
10+
|
11+
LL | pub struct Bar {
12+
| ^^^^^^^^^^^^^^
713
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
814
= note: this struct has unspecified layout
915
note: the type is defined here
@@ -17,12 +23,18 @@ note: the lint level is defined here
1723
LL | #![deny(improper_c_fn_definitions)]
1824
| ^^^^^^^^^^^^^^^^^^^^^^^^^
1925

20-
error: `extern` fn uses type `NotSafe`, which is not FFI-safe
26+
error: `extern` fn uses type `Bar`, which is not FFI-safe
2127
--> $DIR/lint-113436-1.rs:22:30
2228
|
2329
LL | extern "C" fn bar(x: Bar) -> Bar {
2430
| ^^^ not FFI-safe
2531
|
32+
= note: this struct/enum/union (`Bar`) is FFI-unsafe due to a `NotSafe` field
33+
note: the type is defined here
34+
--> $DIR/lint-113436-1.rs:16:1
35+
|
36+
LL | pub struct Bar {
37+
| ^^^^^^^^^^^^^^
2638
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
2739
= note: this struct has unspecified layout
2840
note: the type is defined here

tests/ui/lint/improper_ctypes/lint-73249-3.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub struct A {
1818
}
1919

2020
extern "C" {
21-
pub fn lint_me() -> A; //~ ERROR: uses type `Qux`
21+
pub fn lint_me() -> A; //~ ERROR: `extern` block uses type `A`
2222
}
2323

2424
fn main() {}

0 commit comments

Comments
 (0)