Skip to content

Commit d4dc99b

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 c6eb386 commit d4dc99b

19 files changed

+137
-54
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,8 @@ lint_improper_ctypes_str_reason = string slices have no C equivalent
395395
lint_improper_ctypes_struct_fieldless_help = consider adding a member to this struct
396396
397397
lint_improper_ctypes_struct_fieldless_reason = this struct has no fields
398+
lint_improper_ctypes_struct_dueto = this struct/enum/union (`{$ty}`) is FFI-unsafe due to a `{$inner_ty}` field
399+
398400
lint_improper_ctypes_struct_layout_help = consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
399401
400402
lint_improper_ctypes_struct_layout_reason = this struct has unspecified layout

compiler/rustc_lint/src/types/improper_ctypes.rs

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

@@ -1024,7 +1023,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
10241023
all_phantom &= match self.visit_type(state.get_next(ty), field_ty) {
10251024
FfiSafe => false,
10261025
FfiPhantom(..) => true,
1027-
r @ FfiUnsafe { .. } => return r,
1026+
r @ FfiUnsafe { .. } => {
1027+
return r.wrap_all(ty, fluent::lint_improper_ctypes_struct_dueto, None);
1028+
}
10281029
}
10291030
}
10301031

@@ -1075,7 +1076,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
10751076
);
10761077
}
10771078

1078-
if def.non_enum_variant().fields.is_empty() {
1079+
let ffires = if def.non_enum_variant().fields.is_empty() {
10791080
FfiResult::new_with_reason(
10801081
ty,
10811082
if def.is_struct() {
@@ -1091,7 +1092,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
10911092
)
10921093
} else {
10931094
self.visit_variant_fields(state, ty, def, def.non_enum_variant(), args)
1094-
}
1095+
};
1096+
1097+
// from now on in the function, we lint the actual insides of the struct/union: if something is wrong,
1098+
// then the "fault" comes from inside the struct itself.
1099+
// even if we add more details to the lint, the initial line must specify that the FFI-unsafety is because of the struct
1100+
// - if the struct is from the same crate, there is another warning on its definition anyway
1101+
// (unless it's about Boxes and references without Option<_>
1102+
// which is partly why we keep the details as to why that struct is FFI-unsafe)
1103+
// - if the struct is from another crate, then there's not much that can be done anyways
1104+
//
1105+
// this enum is visited in the middle of another lint,
1106+
// so we override the "cause type" of the lint
1107+
ffires.with_overrides(Some(ty))
10951108
}
10961109

10971110
fn visit_enum(
@@ -1138,10 +1151,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
11381151
}
11391152
});
11401153
if let ControlFlow::Break(result) = ret {
1141-
return result;
1154+
// this enum is visited in the middle of another lint,
1155+
// so we override the "cause type" of the lint
1156+
// (for more detail, see comment in ``visit_struct_union`` before its call to ``result.with_overrides``)
1157+
result.with_overrides(Some(ty))
1158+
} else {
1159+
FfiSafe
11421160
}
1143-
1144-
FfiSafe
11451161
}
11461162

11471163
/// 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: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,10 @@ 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+
= note: the function pointer to `extern "C" fn(CStr)` is FFI-unsafe due to `CStr`
78
= help: consider passing a `*mut std::ffi::c_char` instead, converting to/from `CStr` as needed
89
= note: `CStr`/`CString` do not have a guaranteed layout
910
= note: `#[warn(improper_c_callbacks)]` (part of `#[warn(improper_c_boundaries)]`) on by default
1011

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 `*mut std::ffi::c_char` instead, converting to/from `CStr` as needed
18-
= note: `CStr`/`CString` do not have a guaranteed layout
19-
= note: `#[warn(improper_ctypes)]` (part of `#[warn(improper_c_boundaries)]`) on by default
20-
21-
warning: 2 warnings emitted
12+
warning: 1 warning emitted
2213

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)