Rollup merge of rust-lang#154229 - Zoxc:erase-auto-traits, r=nnethercote

Ensure `ErasedData` only implements appropriate auto traits

This uses an external type to prevent auto traits to be inferred on `ErasedData`. That inference is incorrect since it may not store the type it declares.

This also implements `DynSend` and `DynSync` on `ErasedData` which are checked by bounds on `erase_val`.

Some diagnostics bounds were missing `DynSync`, which is also added here.

Author: JonathanBrouwer

compiler/rustc_errors/src/decorate_diag.rs

@@ -1,7 +1,7 @@
 /// This module provides types and traits for buffering lints until later in compilation.
 use rustc_ast::node_id::NodeId;
 use rustc_data_structures::fx::FxIndexMap;
-use rustc_data_structures::sync::DynSend;
+use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_error_messages::MultiSpan;
 use rustc_lint_defs::{BuiltinLintDiag, Lint, LintId};
 
@@ -10,7 +10,14 @@ use crate::{Diag, DiagCtxtHandle, Diagnostic, Level};
 /// We can't implement `Diagnostic` for `BuiltinLintDiag`, because decorating some of its
 /// variants requires types we don't have yet. So, handle that case separately.
 pub enum DecorateDiagCompat {
-    Dynamic(Box<dyn for<'a> FnOnce(DiagCtxtHandle<'a>, Level) -> Diag<'a, ()> + DynSend + 'static>),
+    Dynamic(
+        Box<
+            dyn for<'a> FnOnce(DiagCtxtHandle<'a>, Level) -> Diag<'a, ()>
+                + DynSync
+                + DynSend
+                + 'static,
+        >,
+    ),
     Builtin(BuiltinLintDiag),
 }
 
@@ -20,7 +27,7 @@ impl std::fmt::Debug for DecorateDiagCompat {
     }
 }
 
-impl<D: for<'a> Diagnostic<'a, ()> + DynSend + 'static> From<D> for DecorateDiagCompat {
+impl<D: for<'a> Diagnostic<'a, ()> + DynSync + DynSend + 'static> From<D> for DecorateDiagCompat {
     #[inline]
     fn from(d: D) -> Self {
         Self::Dynamic(Box::new(|dcx, level| d.into_diag(dcx, level)))
@@ -83,7 +90,7 @@ impl LintBuffer {
     }
 
     pub fn dyn_buffer_lint<
-        F: for<'a> FnOnce(DiagCtxtHandle<'a>, Level) -> Diag<'a, ()> + DynSend + 'static,
+        F: for<'a> FnOnce(DiagCtxtHandle<'a>, Level) -> Diag<'a, ()> + DynSync + DynSend + 'static,
     >(
         &mut self,
         lint: &'static Lint,

compiler/rustc_errors/src/diagnostic.rs

@@ -7,7 +7,7 @@ use std::panic;
 use std::path::PathBuf;
 use std::thread::panicking;
 
-use rustc_data_structures::sync::DynSend;
+use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_error_messages::{DiagArgMap, DiagArgName, DiagArgValue, IntoDiagArg};
 use rustc_lint_defs::{Applicability, LintExpectationId};
 use rustc_macros::{Decodable, Encodable};
@@ -120,7 +120,9 @@ where
 }
 
 impl<'a> Diagnostic<'a, ()>
-    for Box<dyn for<'b> FnOnce(DiagCtxtHandle<'b>, Level) -> Diag<'b, ()> + DynSend + 'static>
+    for Box<
+        dyn for<'b> FnOnce(DiagCtxtHandle<'b>, Level) -> Diag<'b, ()> + DynSync + DynSend + 'static,
+    >
 {
     fn into_diag(self, dcx: DiagCtxtHandle<'a>, level: Level) -> Diag<'a, ()> {
         self(dcx, level)

compiler/rustc_middle/src/query/erase.rs

@@ -7,24 +7,37 @@
 
 use std::ffi::OsStr;
 use std::intrinsics::transmute_unchecked;
+use std::marker::PhantomData;
 use std::mem::MaybeUninit;
 
 use rustc_ast::tokenstream::TokenStream;
 use rustc_data_structures::steal::Steal;
+use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_span::{ErrorGuaranteed, Spanned};
 
 use crate::mir::mono::{MonoItem, NormalizationErrorInMono};
 use crate::ty::{self, Ty, TyCtxt};
 use crate::{mir, thir, traits};
 
+unsafe extern "C" {
+    type NoAutoTraits;
+}
+
 /// Internal implementation detail of [`Erased`].
 #[derive(Copy, Clone)]
 pub struct ErasedData<Storage: Copy> {
     /// We use `MaybeUninit` here to make sure it's legal to store a transmuted
     /// value that isn't actually of type `Storage`.
     data: MaybeUninit<Storage>,
+    /// `Storage` is an erased type, so we use an external type here to opt-out of auto traits
+    /// as those would be incorrect.
+    no_auto_traits: PhantomData<NoAutoTraits>,
 }
 
+// SAFETY: The bounds on `erase_val` ensure the types we erase are `DynSync` and `DynSend`
+unsafe impl<Storage: Copy> DynSync for ErasedData<Storage> {}
+unsafe impl<Storage: Copy> DynSend for ErasedData<Storage> {}
+
 /// Trait for types that can be erased into [`Erased<Self>`].
 ///
 /// Erasing and unerasing values is performed by [`erase_val`] and [`restore_val`].
@@ -54,13 +67,11 @@ pub type Erased<T: Erasable> = ErasedData<impl Copy>;
 ///
 /// `Erased<T>` and `Erased<U>` are type-checked as distinct types, but codegen
 /// can see whether they actually have the same storage type.
-///
-/// FIXME: This might have soundness issues with erasable types that don't
-/// implement the same auto-traits as `[u8; _]`; see
-/// <https://github.com/rust-lang/rust/pull/151715#discussion_r2740113250>
 #[inline(always)]
 #[define_opaque(Erased)]
-pub fn erase_val<T: Erasable>(value: T) -> Erased<T> {
+// The `DynSend` and `DynSync` bounds on `T` are used to
+// justify the safety of the implementations of these traits for `ErasedData`.
+pub fn erase_val<T: Erasable + DynSend + DynSync>(value: T) -> Erased<T> {
     // Ensure the sizes match
     const {
         if size_of::<T>() != size_of::<T::Storage>() {
@@ -78,6 +89,7 @@ pub fn erase_val<T: Erasable>(value: T) -> Erased<T> {
         //
         // SAFETY: It is safe to transmute to MaybeUninit for types with the same sizes.
         data: unsafe { transmute_unchecked::<T, MaybeUninit<T::Storage>>(value) },
+        no_auto_traits: PhantomData,
     }
 }
 
@@ -88,7 +100,7 @@ pub fn erase_val<T: Erasable>(value: T) -> Erased<T> {
 #[inline(always)]
 #[define_opaque(Erased)]
 pub fn restore_val<T: Erasable>(erased_value: Erased<T>) -> T {
-    let ErasedData { data }: ErasedData<<T as Erasable>::Storage> = erased_value;
+    let ErasedData { data, .. }: ErasedData<<T as Erasable>::Storage> = erased_value;
     // See comment in `erase_val` for why we use `transmute_unchecked`.
     //
     // SAFETY: Due to the use of impl Trait in `Erased` the only way to safely create an instance

compiler/rustc_session/src/parse.rs

@@ -7,7 +7,7 @@ use std::sync::Arc;
 use rustc_ast::attr::AttrIdGenerator;
 use rustc_ast::node_id::NodeId;
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
-use rustc_data_structures::sync::{AppendOnlyVec, DynSend, Lock};
+use rustc_data_structures::sync::{AppendOnlyVec, DynSend, DynSync, Lock};
 use rustc_errors::annotate_snippet_emitter_writer::AnnotateSnippetEmitter;
 use rustc_errors::emitter::{EmitterWithNote, stderr_destination};
 use rustc_errors::{
@@ -332,7 +332,7 @@ impl ParseSess {
     }
 
     pub fn dyn_buffer_lint<
-        F: for<'a> FnOnce(DiagCtxtHandle<'a>, Level) -> Diag<'a, ()> + DynSend + 'static,
+        F: for<'a> FnOnce(DiagCtxtHandle<'a>, Level) -> Diag<'a, ()> + DynSync + DynSend + 'static,
     >(
         &self,
         lint: &'static Lint,