Skip to content

Commit ffbbe4a

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 3d4ce20 commit ffbbe4a

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
@@ -215,7 +215,6 @@ impl<'tcx> FfiResult<'tcx> {
215215
/// For instance, if we have a repr(C) struct in a function's argument, FFI unsafeties inside the struct
216216
/// are to be blamed on the struct and not the members.
217217
/// This is where we use this wrapper, to tell "all FFI-unsafeties in there are caused by this `ty`"
218-
#[expect(unused)]
219218
fn with_overrides(mut self, override_cause_ty: Option<Ty<'tcx>>) -> FfiResult<'tcx> {
220219
use FfiResult::*;
221220

@@ -737,7 +736,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
737736
// `()` fields are FFI-safe!
738737
//FfiUnsafe { ty, .. } if ty.is_unit() => false, // TODO get back here
739738
FfiPhantom(..) => true,
740-
r @ FfiUnsafe { .. } => return r,
739+
r @ FfiUnsafe { .. } => {
740+
return r.wrap_all(
741+
ty,
742+
fluent::lint_improper_ctypes_struct_dueto,
743+
None,
744+
);
745+
}
741746
}
742747
}
743748

@@ -788,7 +793,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
788793
);
789794
}
790795

791-
if def.non_enum_variant().fields.is_empty() {
796+
let ffires = if def.non_enum_variant().fields.is_empty() {
792797
FfiResult::new_with_reason(
793798
ty,
794799
if def.is_struct() {
@@ -804,7 +809,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
804809
)
805810
} else {
806811
self.visit_variant_fields(state, ty, def, def.non_enum_variant(), args)
807-
}
812+
};
813+
814+
// from now on in the function, we lint the actual insides of the struct/union: if something is wrong,
815+
// then the "fault" comes from inside the struct itself.
816+
// even if we add more details to the lint, the initial line must specify that the FFI-unsafety is because of the struct
817+
// - if the struct is from the same crate, there is another warning on its definition anyway
818+
// (unless it's about Boxes and references without Option<_>
819+
// which is partly why we keep the details as to why that struct is FFI-unsafe)
820+
// - if the struct is from another crate, then there's not much that can be done anyways
821+
//
822+
// this enum is visited in the middle of another lint,
823+
// so we override the "cause type" of the lint
824+
ffires.with_overrides(Some(ty))
808825
}
809826

810827
fn visit_enum(
@@ -851,10 +868,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
851868
}
852869
});
853870
if let ControlFlow::Break(result) = ret {
854-
return result;
871+
// this enum is visited in the middle of another lint,
872+
// so we override the "cause type" of the lint
873+
// (for more detail, see comment in ``visit_struct_union`` before its call to ``result.with_overrides``)
874+
result.with_overrides(Some(ty))
875+
} else {
876+
FfiSafe
855877
}
856-
857-
FfiSafe
858878
}
859879

860880
/// 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)