Skip to content

Commit e6b1f2d

Browse files
committed
lint: revamp ImproperCTypes diagnostic architecture for nested notes and help messages
1 parent 2aa26d8 commit e6b1f2d

File tree

2 files changed

+145
-43
lines changed

2 files changed

+145
-43
lines changed

compiler/rustc_lint/src/lints.rs

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,13 +1806,44 @@ pub(crate) enum AmbiguousWidePointerComparisonsAddrSuggestion<'a> {
18061806
},
18071807
}
18081808

1809+
pub(crate) struct ImproperCTypesLayer<'a> {
1810+
pub ty: Ty<'a>,
1811+
pub inner_ty: Option<Ty<'a>>,
1812+
pub note: DiagMessage,
1813+
pub span_note: Option<Span>,
1814+
pub help: Option<DiagMessage>,
1815+
}
1816+
1817+
impl<'a> Subdiagnostic for ImproperCTypesLayer<'a> {
1818+
fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
1819+
self,
1820+
diag: &mut Diag<'_, G>,
1821+
f: &F,
1822+
) {
1823+
diag.arg("ty", self.ty);
1824+
if let Some(ty) = self.inner_ty {
1825+
diag.arg("inner_ty", ty);
1826+
}
1827+
1828+
if let Some(help) = self.help {
1829+
let msg = f(diag, help.into());
1830+
diag.help(msg);
1831+
}
1832+
1833+
let msg = f(diag, self.note.into());
1834+
diag.note(msg);
1835+
if let Some(note) = self.span_note {
1836+
let msg = f(diag, fluent::lint_note.into());
1837+
diag.span_note(note, msg);
1838+
};
1839+
}
1840+
}
1841+
18091842
pub(crate) struct ImproperCTypes<'a> {
18101843
pub ty: Ty<'a>,
18111844
pub desc: &'a str,
18121845
pub label: Span,
1813-
pub help: Option<DiagMessage>,
1814-
pub note: DiagMessage,
1815-
pub span_note: Option<Span>,
1846+
pub reasons: Vec<ImproperCTypesLayer<'a>>,
18161847
}
18171848

18181849
// Used because of the complexity of Option<DiagMessage>, DiagMessage, and Option<Span>
@@ -1822,12 +1853,8 @@ impl<'a> LintDiagnostic<'a, ()> for ImproperCTypes<'_> {
18221853
diag.arg("ty", self.ty);
18231854
diag.arg("desc", self.desc);
18241855
diag.span_label(self.label, fluent::lint_label);
1825-
if let Some(help) = self.help {
1826-
diag.help(help);
1827-
}
1828-
diag.note(self.note);
1829-
if let Some(note) = self.span_note {
1830-
diag.span_note(note, fluent::lint_note);
1856+
for reason in self.reasons.into_iter() {
1857+
diag.subdiagnostic(reason);
18311858
}
18321859
}
18331860
}

compiler/rustc_lint/src/types.rs

Lines changed: 109 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ use {rustc_ast as ast, rustc_hir as hir};
2121
use crate::lints::{
2222
AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion,
2323
AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad,
24-
AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons,
25-
InvalidNanComparisonsSuggestion, UnusedComparisons, VariantSizeDifferencesDiag,
24+
AtomicOrderingStore, ImproperCTypes, ImproperCTypesLayer, InvalidAtomicOrderingDiag,
25+
InvalidNanComparisons, InvalidNanComparisonsSuggestion, UnusedComparisons,
26+
VariantSizeDifferencesDiag,
2627
};
2728
use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};
2829

@@ -593,7 +594,19 @@ struct CTypesVisitorState<'tcx> {
593594
enum FfiResult<'tcx> {
594595
FfiSafe,
595596
FfiPhantom(Ty<'tcx>),
596-
FfiUnsafe { ty: Ty<'tcx>, reason: DiagMessage, help: Option<DiagMessage> },
597+
FfiUnsafe {
598+
ty: Ty<'tcx>,
599+
reason: DiagMessage,
600+
help: Option<DiagMessage>,
601+
},
602+
// NOTE: this `allow` is only here for one retroactively-added commit
603+
#[allow(dead_code)]
604+
FfiUnsafeWrapper {
605+
ty: Ty<'tcx>,
606+
reason: DiagMessage,
607+
help: Option<DiagMessage>,
608+
wrapped: Box<FfiResult<'tcx>>,
609+
},
597610
}
598611

599612
pub(crate) fn nonnull_optimization_guaranteed<'tcx>(
@@ -803,12 +816,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
803816
/// Check if the type is array and emit an unsafe type lint.
804817
fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
805818
if let ty::Array(..) = ty.kind() {
806-
self.emit_ffi_unsafe_type_lint(
819+
self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
807820
ty,
808-
sp,
809-
fluent::lint_improper_ctypes_array_reason,
810-
Some(fluent::lint_improper_ctypes_array_help),
811-
);
821+
note: fluent::lint_improper_ctypes_array_reason,
822+
help: Some(fluent::lint_improper_ctypes_array_help),
823+
inner_ty: None,
824+
span_note: None,
825+
}]);
812826
true
813827
} else {
814828
false
@@ -865,9 +879,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
865879
all_phantom &= match self.check_field_type_for_ffi(acc, field, args) {
866880
FfiSafe => false,
867881
// `()` fields are FFI-safe!
868-
FfiUnsafe { ty, .. } if ty.is_unit() => false,
882+
FfiUnsafe { ty, .. } | FfiUnsafeWrapper { ty, .. } if ty.is_unit() => false,
869883
FfiPhantom(..) => true,
870-
r @ FfiUnsafe { .. } => return r,
884+
r @ (FfiUnsafe { .. } | FfiUnsafeWrapper { .. }) => return r,
871885
}
872886
}
873887

@@ -1155,8 +1169,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
11551169
&mut self,
11561170
ty: Ty<'tcx>,
11571171
sp: Span,
1158-
note: DiagMessage,
1159-
help: Option<DiagMessage>,
1172+
mut reasons: Vec<ImproperCTypesLayer<'tcx>>,
11601173
) {
11611174
let lint = match self.mode {
11621175
CItemKind::Declaration => IMPROPER_CTYPES,
@@ -1166,21 +1179,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
11661179
CItemKind::Declaration => "block",
11671180
CItemKind::Definition => "fn",
11681181
};
1169-
let span_note = if let ty::Adt(def, _) = ty.kind()
1170-
&& let Some(sp) = self.cx.tcx.hir().span_if_local(def.did())
1171-
{
1172-
Some(sp)
1173-
} else {
1174-
None
1175-
};
1176-
self.cx.emit_span_lint(lint, sp, ImproperCTypes {
1177-
ty,
1178-
desc,
1179-
label: sp,
1180-
help,
1181-
note,
1182-
span_note,
1183-
});
1182+
for reason in reasons.iter_mut() {
1183+
reason.span_note = if let ty::Adt(def, _) = reason.ty.kind()
1184+
&& let Some(sp) = self.cx.tcx.hir().span_if_local(def.did())
1185+
{
1186+
Some(sp)
1187+
} else {
1188+
None
1189+
};
1190+
}
1191+
1192+
self.cx.emit_span_lint(lint, sp, ImproperCTypes { ty, desc, label: sp, reasons });
11841193
}
11851194

11861195
fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
@@ -1209,7 +1218,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12091218
.visit_with(&mut ProhibitOpaqueTypes)
12101219
.break_value()
12111220
{
1212-
self.emit_ffi_unsafe_type_lint(ty, sp, fluent::lint_improper_ctypes_opaque, None);
1221+
self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
1222+
ty,
1223+
note: fluent::lint_improper_ctypes_opaque,
1224+
span_note: Some(sp),
1225+
help: None,
1226+
inner_ty: None,
1227+
}]);
12131228
true
12141229
} else {
12151230
false
@@ -1248,15 +1263,75 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12481263
match self.check_type_for_ffi(&mut acc, ty) {
12491264
FfiResult::FfiSafe => {}
12501265
FfiResult::FfiPhantom(ty) => {
1251-
self.emit_ffi_unsafe_type_lint(
1266+
self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
12521267
ty,
1253-
sp,
1254-
fluent::lint_improper_ctypes_only_phantomdata,
1255-
None,
1256-
);
1268+
note: fluent::lint_improper_ctypes_only_phantomdata,
1269+
span_note: None, // filled later
1270+
help: None,
1271+
inner_ty: None,
1272+
}]);
12571273
}
12581274
FfiResult::FfiUnsafe { ty, reason, help } => {
1259-
self.emit_ffi_unsafe_type_lint(ty, sp, reason, help);
1275+
self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
1276+
ty,
1277+
help,
1278+
note: reason,
1279+
span_note: None, // filled later
1280+
inner_ty: None,
1281+
}]);
1282+
}
1283+
ffir @ FfiResult::FfiUnsafeWrapper { .. } => {
1284+
let mut last_ty = None;
1285+
let mut ffiresult_recursor = Some(&ffir);
1286+
let mut cimproper_layers: Vec<ImproperCTypesLayer<'tcx>> = vec![];
1287+
1288+
// this whole while block converts the arbitrarily-deep
1289+
// FfiResult stack to a ImproperCTypesLayer Vec
1290+
while let Some(ref ffir_rec) = ffiresult_recursor {
1291+
match ffir_rec {
1292+
FfiResult::FfiPhantom(ty) => {
1293+
last_ty = Some(ty.clone());
1294+
let len = cimproper_layers.len();
1295+
if len > 0 {
1296+
cimproper_layers[len - 1].inner_ty = last_ty.clone();
1297+
}
1298+
cimproper_layers.push(ImproperCTypesLayer {
1299+
ty: ty.clone(),
1300+
inner_ty: None,
1301+
help: None,
1302+
note: fluent::lint_improper_ctypes_only_phantomdata,
1303+
span_note: None, // filled later
1304+
});
1305+
ffiresult_recursor = None;
1306+
}
1307+
FfiResult::FfiUnsafe { ty, reason, help }
1308+
| FfiResult::FfiUnsafeWrapper { ty, reason, help, .. } => {
1309+
last_ty = Some(ty.clone());
1310+
let len = cimproper_layers.len();
1311+
if len > 0 {
1312+
cimproper_layers[len - 1].inner_ty = last_ty.clone();
1313+
}
1314+
cimproper_layers.push(ImproperCTypesLayer {
1315+
ty: ty.clone(),
1316+
inner_ty: None,
1317+
help: help.clone(),
1318+
note: reason.clone(),
1319+
span_note: None, // filled later
1320+
});
1321+
1322+
if let FfiResult::FfiUnsafeWrapper { wrapped, .. } = ffir_rec {
1323+
ffiresult_recursor = Some(wrapped.as_ref());
1324+
} else {
1325+
ffiresult_recursor = None;
1326+
}
1327+
}
1328+
FfiResult::FfiSafe => {
1329+
bug!("malformed FfiResult stack: it should be unsafe all the way down")
1330+
}
1331+
};
1332+
}
1333+
let last_ty = last_ty.unwrap(); // populated by any run of the `while` block
1334+
self.emit_ffi_unsafe_type_lint(last_ty, sp, cimproper_layers);
12601335
}
12611336
}
12621337
}

0 commit comments

Comments
 (0)