1
Fork 0

lint: revamp ImproperCTypes diagnostic architecture for nested notes and help messages

This commit is contained in:
niacdoial 2024-11-05 22:54:48 +01:00
parent c94848c046
commit f021d99cc8
2 changed files with 146 additions and 45 deletions

View file

@ -1851,13 +1851,44 @@ pub(crate) struct UnpredictableFunctionPointerComparisonsSuggestion<'a> {
pub right: Span, pub right: Span,
} }
pub(crate) struct ImproperCTypesLayer<'a> {
pub ty: Ty<'a>,
pub inner_ty: Option<Ty<'a>>,
pub note: DiagMessage,
pub span_note: Option<Span>,
pub help: Option<DiagMessage>,
}
impl<'a> Subdiagnostic for ImproperCTypesLayer<'a> {
fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
self,
diag: &mut Diag<'_, G>,
f: &F,
) {
diag.arg("ty", self.ty);
if let Some(ty) = self.inner_ty {
diag.arg("inner_ty", ty);
}
if let Some(help) = self.help {
let msg = f(diag, help.into());
diag.help(msg);
}
let msg = f(diag, self.note.into());
diag.note(msg);
if let Some(note) = self.span_note {
let msg = f(diag, fluent::lint_note.into());
diag.span_note(note, msg);
};
}
}
pub(crate) struct ImproperCTypes<'a> { pub(crate) struct ImproperCTypes<'a> {
pub ty: Ty<'a>, pub ty: Ty<'a>,
pub desc: &'a str, pub desc: &'a str,
pub label: Span, pub label: Span,
pub help: Option<DiagMessage>, pub reasons: Vec<ImproperCTypesLayer<'a>>,
pub note: DiagMessage,
pub span_note: Option<Span>,
} }
// Used because of the complexity of Option<DiagMessage>, DiagMessage, and Option<Span> // Used because of the complexity of Option<DiagMessage>, DiagMessage, and Option<Span>
@ -1867,12 +1898,8 @@ impl<'a> LintDiagnostic<'a, ()> for ImproperCTypes<'_> {
diag.arg("ty", self.ty); diag.arg("ty", self.ty);
diag.arg("desc", self.desc); diag.arg("desc", self.desc);
diag.span_label(self.label, fluent::lint_label); diag.span_label(self.label, fluent::lint_label);
if let Some(help) = self.help { for reason in self.reasons.into_iter() {
diag.help(help); diag.subdiagnostic(reason);
}
diag.note(self.note);
if let Some(note) = self.span_note {
diag.span_note(note, fluent::lint_note);
} }
} }
} }

View file

@ -22,10 +22,10 @@ mod improper_ctypes;
use crate::lints::{ use crate::lints::{
AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion,
AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad, AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad,
AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons, AtomicOrderingStore, ImproperCTypes, ImproperCTypesLayer, InvalidAtomicOrderingDiag,
InvalidNanComparisonsSuggestion, UnpredictableFunctionPointerComparisons, InvalidNanComparisons, InvalidNanComparisonsSuggestion,
UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, UnpredictableFunctionPointerComparisons, UnpredictableFunctionPointerComparisonsSuggestion,
VariantSizeDifferencesDiag, UnusedComparisons, VariantSizeDifferencesDiag,
}; };
use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};
@ -727,7 +727,19 @@ struct CTypesVisitorState<'tcx> {
enum FfiResult<'tcx> { enum FfiResult<'tcx> {
FfiSafe, FfiSafe,
FfiPhantom(Ty<'tcx>), FfiPhantom(Ty<'tcx>),
FfiUnsafe { ty: Ty<'tcx>, reason: DiagMessage, help: Option<DiagMessage> }, FfiUnsafe {
ty: Ty<'tcx>,
reason: DiagMessage,
help: Option<DiagMessage>,
},
// NOTE: this `allow` is only here for one retroactively-added commit
#[allow(dead_code)]
FfiUnsafeWrapper {
ty: Ty<'tcx>,
reason: DiagMessage,
help: Option<DiagMessage>,
wrapped: Box<FfiResult<'tcx>>,
},
} }
pub(crate) fn nonnull_optimization_guaranteed<'tcx>( pub(crate) fn nonnull_optimization_guaranteed<'tcx>(
@ -933,12 +945,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
/// Check if the type is array and emit an unsafe type lint. /// Check if the type is array and emit an unsafe type lint.
fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
if let ty::Array(..) = ty.kind() { if let ty::Array(..) = ty.kind() {
self.emit_ffi_unsafe_type_lint( self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
ty, ty,
sp, note: fluent::lint_improper_ctypes_array_reason,
fluent::lint_improper_ctypes_array_reason, help: Some(fluent::lint_improper_ctypes_array_help),
Some(fluent::lint_improper_ctypes_array_help), inner_ty: None,
); span_note: None,
}]);
true true
} else { } else {
false false
@ -995,9 +1008,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
all_phantom &= match self.check_field_type_for_ffi(acc, field, args) { all_phantom &= match self.check_field_type_for_ffi(acc, field, args) {
FfiSafe => false, FfiSafe => false,
// `()` fields are FFI-safe! // `()` fields are FFI-safe!
FfiUnsafe { ty, .. } if ty.is_unit() => false, FfiUnsafe { ty, .. } | FfiUnsafeWrapper { ty, .. } if ty.is_unit() => false,
FfiPhantom(..) => true, FfiPhantom(..) => true,
r @ FfiUnsafe { .. } => return r, r @ (FfiUnsafe { .. } | FfiUnsafeWrapper { .. }) => return r,
} }
} }
@ -1278,8 +1291,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
&mut self, &mut self,
ty: Ty<'tcx>, ty: Ty<'tcx>,
sp: Span, sp: Span,
note: DiagMessage, mut reasons: Vec<ImproperCTypesLayer<'tcx>>,
help: Option<DiagMessage>,
) { ) {
let lint = match self.mode { let lint = match self.mode {
CItemKind::Declaration => IMPROPER_CTYPES, CItemKind::Declaration => IMPROPER_CTYPES,
@ -1289,21 +1301,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
CItemKind::Declaration => "block", CItemKind::Declaration => "block",
CItemKind::Definition => "fn", CItemKind::Definition => "fn",
}; };
let span_note = if let ty::Adt(def, _) = ty.kind() for reason in reasons.iter_mut() {
reason.span_note = if let ty::Adt(def, _) = reason.ty.kind()
&& let Some(sp) = self.cx.tcx.hir().span_if_local(def.did()) && let Some(sp) = self.cx.tcx.hir().span_if_local(def.did())
{ {
Some(sp) Some(sp)
} else { } else {
None None
}; };
self.cx.emit_span_lint(lint, sp, ImproperCTypes { }
ty,
desc, self.cx.emit_span_lint(lint, sp, ImproperCTypes { ty, desc, label: sp, reasons });
label: sp,
help,
note,
span_note,
});
} }
fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
@ -1332,7 +1340,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
.visit_with(&mut ProhibitOpaqueTypes) .visit_with(&mut ProhibitOpaqueTypes)
.break_value() .break_value()
{ {
self.emit_ffi_unsafe_type_lint(ty, sp, fluent::lint_improper_ctypes_opaque, None); self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
ty,
note: fluent::lint_improper_ctypes_opaque,
span_note: Some(sp),
help: None,
inner_ty: None,
}]);
true true
} else { } else {
false false
@ -1371,15 +1385,75 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
match self.check_type_for_ffi(&mut acc, ty) { match self.check_type_for_ffi(&mut acc, ty) {
FfiResult::FfiSafe => {} FfiResult::FfiSafe => {}
FfiResult::FfiPhantom(ty) => { FfiResult::FfiPhantom(ty) => {
self.emit_ffi_unsafe_type_lint( self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
ty, ty,
sp, note: fluent::lint_improper_ctypes_only_phantomdata,
fluent::lint_improper_ctypes_only_phantomdata, span_note: None, // filled later
None, help: None,
); inner_ty: None,
}]);
} }
FfiResult::FfiUnsafe { ty, reason, help } => { FfiResult::FfiUnsafe { ty, reason, help } => {
self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
ty,
help,
note: reason,
span_note: None, // filled later
inner_ty: None,
}]);
}
ffir @ FfiResult::FfiUnsafeWrapper { .. } => {
let mut last_ty = None;
let mut ffiresult_recursor = Some(&ffir);
let mut cimproper_layers: Vec<ImproperCTypesLayer<'tcx>> = vec![];
// this whole while block converts the arbitrarily-deep
// FfiResult stack to a ImproperCTypesLayer Vec
while let Some(ref ffir_rec) = ffiresult_recursor {
match ffir_rec {
FfiResult::FfiPhantom(ty) => {
last_ty = Some(ty.clone());
let len = cimproper_layers.len();
if len > 0 {
cimproper_layers[len - 1].inner_ty = last_ty.clone();
}
cimproper_layers.push(ImproperCTypesLayer {
ty: ty.clone(),
inner_ty: None,
help: None,
note: fluent::lint_improper_ctypes_only_phantomdata,
span_note: None, // filled later
});
ffiresult_recursor = None;
}
FfiResult::FfiUnsafe { ty, reason, help }
| FfiResult::FfiUnsafeWrapper { ty, reason, help, .. } => {
last_ty = Some(ty.clone());
let len = cimproper_layers.len();
if len > 0 {
cimproper_layers[len - 1].inner_ty = last_ty.clone();
}
cimproper_layers.push(ImproperCTypesLayer {
ty: ty.clone(),
inner_ty: None,
help: help.clone(),
note: reason.clone(),
span_note: None, // filled later
});
if let FfiResult::FfiUnsafeWrapper { wrapped, .. } = ffir_rec {
ffiresult_recursor = Some(wrapped.as_ref());
} else {
ffiresult_recursor = None;
}
}
FfiResult::FfiSafe => {
bug!("malformed FfiResult stack: it should be unsafe all the way down")
}
};
}
let last_ty = last_ty.unwrap(); // populated by any run of the `while` block
self.emit_ffi_unsafe_type_lint(last_ty, sp, cimproper_layers);
} }
} }
} }