lint: revamp ImproperCTypes diagnostic architecture for nested notes …

…and help messages
rust-lang · Nov 5, 2024 · e6b1f2d · e6b1f2d
1 parent 2aa26d8
commit e6b1f2d
Showing 2 changed files with 145 additions and 43 deletions.
diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
@@ -1806,13 +1806,44 @@ pub(crate) enum AmbiguousWidePointerComparisonsAddrSuggestion<'a> {
     },
 }
 
+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 ty: Ty<'a>,
     pub desc: &'a str,
     pub label: Span,
-    pub help: Option<DiagMessage>,
-    pub note: DiagMessage,
-    pub span_note: Option<Span>,
+    pub reasons: Vec<ImproperCTypesLayer<'a>>,
 }
 
 // Used because of the complexity of Option<DiagMessage>, DiagMessage, and Option<Span>
@@ -1822,12 +1853,8 @@ impl<'a> LintDiagnostic<'a, ()> for ImproperCTypes<'_> {
         diag.arg("ty", self.ty);
         diag.arg("desc", self.desc);
         diag.span_label(self.label, fluent::lint_label);
-        if let Some(help) = self.help {
-            diag.help(help);
-        }
-        diag.note(self.note);
-        if let Some(note) = self.span_note {
-            diag.span_note(note, fluent::lint_note);
+        for reason in self.reasons.into_iter() {
+            diag.subdiagnostic(reason);
         }
     }
 }

diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs
@@ -21,8 +21,9 @@ use {rustc_ast as ast, rustc_hir as hir};
 use crate::lints::{
     AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion,
     AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad,
-    AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons,
-    InvalidNanComparisonsSuggestion, UnusedComparisons, VariantSizeDifferencesDiag,
+    AtomicOrderingStore, ImproperCTypes, ImproperCTypesLayer, InvalidAtomicOrderingDiag,
+    InvalidNanComparisons, InvalidNanComparisonsSuggestion, UnusedComparisons,
+    VariantSizeDifferencesDiag,
 };
 use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent};
 
@@ -593,7 +594,19 @@ struct CTypesVisitorState<'tcx> {
 enum FfiResult<'tcx> {
     FfiSafe,
     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>(
@@ -803,12 +816,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
     /// 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 {
         if let ty::Array(..) = ty.kind() {
-            self.emit_ffi_unsafe_type_lint(
+            self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
                 ty,
-                sp,
-                fluent::lint_improper_ctypes_array_reason,
-                Some(fluent::lint_improper_ctypes_array_help),
-            );
+                note: fluent::lint_improper_ctypes_array_reason,
+                help: Some(fluent::lint_improper_ctypes_array_help),
+                inner_ty: None,
+                span_note: None,
+            }]);
             true
         } else {
             false
@@ -865,9 +879,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
             all_phantom &= match self.check_field_type_for_ffi(acc, field, args) {
                 FfiSafe => false,
                 // `()` fields are FFI-safe!
-                FfiUnsafe { ty, .. } if ty.is_unit() => false,
+                FfiUnsafe { ty, .. } | FfiUnsafeWrapper { ty, .. } if ty.is_unit() => false,
                 FfiPhantom(..) => true,
-                r @ FfiUnsafe { .. } => return r,
+                r @ (FfiUnsafe { .. } | FfiUnsafeWrapper { .. }) => return r,
             }
         }
 
@@ -1155,8 +1169,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         &mut self,
         ty: Ty<'tcx>,
         sp: Span,
-        note: DiagMessage,
-        help: Option<DiagMessage>,
+        mut reasons: Vec<ImproperCTypesLayer<'tcx>>,
     ) {
         let lint = match self.mode {
             CItemKind::Declaration => IMPROPER_CTYPES,
@@ -1166,21 +1179,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
             CItemKind::Declaration => "block",
             CItemKind::Definition => "fn",
         };
-        let span_note = if let ty::Adt(def, _) = ty.kind()
-            && let Some(sp) = self.cx.tcx.hir().span_if_local(def.did())
-        {
-            Some(sp)
-        } else {
-            None
-        };
-        self.cx.emit_span_lint(lint, sp, ImproperCTypes {
-            ty,
-            desc,
-            label: sp,
-            help,
-            note,
-            span_note,
-        });
+        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())
+            {
+                Some(sp)
+            } else {
+                None
+            };
+        }
+
+        self.cx.emit_span_lint(lint, sp, ImproperCTypes { ty, desc, label: sp, reasons });
     }
 
     fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
@@ -1209,7 +1218,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
             .visit_with(&mut ProhibitOpaqueTypes)
             .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
         } else {
             false
@@ -1248,15 +1263,75 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         match self.check_type_for_ffi(&mut acc, ty) {
             FfiResult::FfiSafe => {}
             FfiResult::FfiPhantom(ty) => {
-                self.emit_ffi_unsafe_type_lint(
+                self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer {
                     ty,
-                    sp,
-                    fluent::lint_improper_ctypes_only_phantomdata,
-                    None,
-                );
+                    note: fluent::lint_improper_ctypes_only_phantomdata,
+                    span_note: None, // filled later
+                    help: None,
+                    inner_ty: None,
+                }]);
             }
             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);
             }
         }
     }