Auto merge of rust-lang#3210 - rust-lang:rustup-2023-12-05, r=RalfJung

Automatic Rustup

Author: bors

compiler/rustc_arena/src/lib.rs

@@ -197,23 +197,24 @@ impl<T> TypedArena<T> {
         start_ptr
     }
 
+    /// Allocates the elements of this iterator into a contiguous slice in the `TypedArena`.
+    ///
+    /// Note: for reasons of reentrancy and panic safety we collect into a `SmallVec<[_; 8]>` before
+    /// storing the elements in the arena.
     #[inline]
     pub fn alloc_from_iter<I: IntoIterator<Item = T>>(&self, iter: I) -> &mut [T] {
-        // This implementation is entirely separate to
-        // `DroplessIterator::alloc_from_iter`, even though conceptually they
-        // are the same.
+        // Despite the similarlty with `DroplessArena`, we cannot reuse their fast case. The reason
+        // is subtle: these arenas are reentrant. In other words, `iter` may very well be holding a
+        // reference to `self` and adding elements to the arena during iteration.
         //
-        // `DroplessIterator` (in the fast case) writes elements from the
-        // iterator one at a time into the allocated memory. That's easy
-        // because the elements don't implement `Drop`. But for `TypedArena`
-        // they do implement `Drop`, which means that if the iterator panics we
-        // could end up with some allocated-but-uninitialized elements, which
-        // will then cause UB in `TypedArena::drop`.
+        // For this reason, if we pre-allocated any space for the elements of this iterator, we'd
+        // have to track that some uninitialized elements are followed by some initialized elements,
+        // else we might accidentally drop uninitialized memory if something panics or if the
+        // iterator doesn't fill all the length we expected.
         //
-        // Instead we use an approach where any iterator panic will occur
-        // before the memory is allocated. This function is much less hot than
-        // `DroplessArena::alloc_from_iter`, so it doesn't need to be
-        // hyper-optimized.
+        // So we collect all the elements beforehand, which takes care of reentrancy and panic
+        // safety. This function is much less hot than `DroplessArena::alloc_from_iter`, so it
+        // doesn't need to be hyper-optimized.
         assert!(mem::size_of::<T>() != 0);
 
         let mut vec: SmallVec<[_; 8]> = iter.into_iter().collect();
@@ -485,8 +486,9 @@ impl DroplessArena {
 
     /// # Safety
     ///
-    /// The caller must ensure that `mem` is valid for writes up to
-    /// `size_of::<T>() * len`.
+    /// The caller must ensure that `mem` is valid for writes up to `size_of::<T>() * len`, and that
+    /// that memory stays allocated and not shared for the lifetime of `self`. This must hold even
+    /// if `iter.next()` allocates onto `self`.
     #[inline]
     unsafe fn write_from_iter<T, I: Iterator<Item = T>>(
         &self,
@@ -516,6 +518,8 @@ impl DroplessArena {
 
     #[inline]
     pub fn alloc_from_iter<T, I: IntoIterator<Item = T>>(&self, iter: I) -> &mut [T] {
+        // Warning: this function is reentrant: `iter` could hold a reference to `&self` and
+        // allocate additional elements while we're iterating.
         let iter = iter.into_iter();
         assert!(mem::size_of::<T>() != 0);
         assert!(!mem::needs_drop::<T>());
@@ -524,18 +528,23 @@ impl DroplessArena {
 
         match size_hint {
             (min, Some(max)) if min == max => {
-                // We know the exact number of elements the iterator will produce here
+                // We know the exact number of elements the iterator expects to produce here.
                 let len = min;
 
                 if len == 0 {
                     return &mut [];
                 }
 
                 let mem = self.alloc_raw(Layout::array::<T>(len).unwrap()) as *mut T;
+                // SAFETY: `write_from_iter` doesn't touch `self`. It only touches the slice we just
+                // reserved. If the iterator panics or doesn't output `len` elements, this will
+                // leave some unallocated slots in the arena, which is fine because we do not call
+                // `drop`.
                 unsafe { self.write_from_iter(iter, len, mem) }
             }
             (_, _) => {
                 outline(move || -> &mut [T] {
+                    // Takes care of reentrancy.
                     let mut vec: SmallVec<[_; 8]> = iter.collect();
                     if vec.is_empty() {
                         return &mut [];

compiler/rustc_ast_lowering/src/asm.rs

@@ -14,8 +14,8 @@ use rustc_ast::*;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::definitions::DefPathData;
 use rustc_session::parse::feature_err;
+use rustc_span::symbol::kw;
 use rustc_span::{sym, Span};
 use rustc_target::asm;
 use std::collections::hash_map::Entry;
@@ -227,7 +227,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                             self.create_def(
                                 parent_def_id.def_id,
                                 node_id,
-                                DefPathData::AnonConst,
+                                kw::Empty,
                                 DefKind::AnonConst,
                                 *op_sp,
                             );

compiler/rustc_ast_lowering/src/expr.rs

@@ -13,10 +13,9 @@ use rustc_ast::*;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::definitions::DefPathData;
 use rustc_session::errors::report_lit_error;
 use rustc_span::source_map::{respan, Spanned};
-use rustc_span::symbol::{sym, Ident, Symbol};
+use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::DUMMY_SP;
 use rustc_span::{DesugaringKind, Span};
 use thin_vec::{thin_vec, ThinVec};
@@ -376,7 +375,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 self.create_def(
                     parent_def_id.def_id,
                     node_id,
-                    DefPathData::AnonConst,
+                    kw::Empty,
                     DefKind::AnonConst,
                     f.span,
                 );

compiler/rustc_ast_lowering/src/item.rs

@@ -3,7 +3,6 @@ use super::ResolverAstLoweringExt;
 use super::{AstOwner, ImplTraitContext, ImplTraitPosition};
 use super::{FnDeclKind, LoweringContext, ParamMode};
 
-use hir::definitions::DefPathData;
 use rustc_ast::ptr::P;
 use rustc_ast::visit::AssocCtxt;
 use rustc_ast::*;
@@ -1367,7 +1366,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 let def_id = self.create_def(
                     self.local_def_id(parent_node_id),
                     param_node_id,
-                    DefPathData::TypeNs(sym::host),
+                    sym::host,
                     DefKind::ConstParam,
                     span,
                 );
@@ -1427,13 +1426,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
         if let Some((span, hir_id, def_id)) = host_param_parts {
             let const_node_id = self.next_node_id();
-            let anon_const = self.create_def(
-                def_id,
-                const_node_id,
-                DefPathData::AnonConst,
-                DefKind::AnonConst,
-                span,
-            );
+            let anon_const =
+                self.create_def(def_id, const_node_id, kw::Empty, DefKind::AnonConst, span);
 
             let const_id = self.next_id();
             let const_expr_id = self.next_id();

compiler/rustc_ast_lowering/src/lib.rs

@@ -58,7 +58,6 @@ use rustc_errors::{DiagnosticArgFromDisplay, StashKey};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
 use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
-use rustc_hir::definitions::DefPathData;
 use rustc_hir::{ConstArg, GenericArg, ItemLocalId, ParamName, TraitCandidate};
 use rustc_index::{Idx, IndexSlice, IndexVec};
 use rustc_middle::{
@@ -499,20 +498,20 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         &mut self,
         parent: LocalDefId,
         node_id: ast::NodeId,
-        data: DefPathData,
+        name: Symbol,
         def_kind: DefKind,
         span: Span,
     ) -> LocalDefId {
         debug_assert_ne!(node_id, ast::DUMMY_NODE_ID);
         assert!(
             self.opt_local_def_id(node_id).is_none(),
-            "adding a def'n for node-id {:?} and data {:?} but a previous def'n exists: {:?}",
+            "adding a def'n for node-id {:?} and def kind {:?} but a previous def'n exists: {:?}",
             node_id,
-            data,
+            def_kind,
             self.tcx.hir().def_key(self.local_def_id(node_id)),
         );
 
-        let def_id = self.tcx.at(span).create_def(parent, data, def_kind).def_id();
+        let def_id = self.tcx.at(span).create_def(parent, name, def_kind).def_id();
 
         debug!("create_def: def_id_to_node_id[{:?}] <-> {:?}", def_id, node_id);
         self.resolver.node_id_to_def_id.insert(node_id, def_id);
@@ -809,7 +808,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 let _def_id = self.create_def(
                     self.current_hir_id_owner.def_id,
                     param,
-                    DefPathData::LifetimeNs(kw::UnderscoreLifetime),
+                    kw::UnderscoreLifetime,
                     DefKind::LifetimeParam,
                     ident.span,
                 );
@@ -1227,7 +1226,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                                 let def_id = self.create_def(
                                     parent_def_id.def_id,
                                     node_id,
-                                    DefPathData::AnonConst,
+                                    kw::Empty,
                                     DefKind::AnonConst,
                                     span,
                                 );
@@ -1465,7 +1464,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         self.create_def(
                             self.current_hir_id_owner.def_id,
                             *def_node_id,
-                            DefPathData::TypeNs(ident.name),
+                            ident.name,
                             DefKind::TyParam,
                             span,
                         );
@@ -1619,7 +1618,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let opaque_ty_def_id = self.create_def(
             self.current_hir_id_owner.def_id,
             opaque_ty_node_id,
-            DefPathData::ImplTrait,
+            kw::Empty,
             DefKind::OpaqueTy,
             opaque_ty_span,
         );
@@ -1674,7 +1673,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 let duplicated_lifetime_def_id = self.create_def(
                     opaque_ty_def_id,
                     duplicated_lifetime_node_id,
-                    DefPathData::LifetimeNs(lifetime.ident.name),
+                    lifetime.ident.name,
                     DefKind::LifetimeParam,
                     lifetime.ident.span,
                 );
@@ -2549,7 +2548,7 @@ impl<'hir> GenericArgsCtor<'hir> {
         let def_id = lcx.create_def(
             lcx.current_hir_id_owner.def_id,
             id,
-            DefPathData::AnonConst,
+            kw::Empty,
             DefKind::AnonConst,
             span,
         );

compiler/rustc_codegen_llvm/src/builder.rs

@@ -489,6 +489,15 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
 
     #[instrument(level = "trace", skip(self))]
     fn load_operand(&mut self, place: PlaceRef<'tcx, &'ll Value>) -> OperandRef<'tcx, &'ll Value> {
+        if place.layout.is_unsized() {
+            let tail = self.tcx.struct_tail_with_normalize(place.layout.ty, |ty| ty, || {});
+            if matches!(tail.kind(), ty::Foreign(..)) {
+                // Unsized locals and, at least conceptually, even unsized arguments must be copied
+                // around, which requires dynamically determining their size. Therefore, we cannot
+                // allow `extern` types here. Consult t-opsem before removing this check.
+                panic!("unsized locals must not be `extern` types");
+            }
+        }
         assert_eq!(place.llextra.is_some(), place.layout.is_unsized());
 
         if place.layout.is_zst() {

compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs

@@ -594,7 +594,7 @@ fn push_unqualified_item_name(
         DefPathData::CrateRoot => {
             output.push_str(tcx.crate_name(def_id.krate).as_str());
         }
-        DefPathData::ClosureExpr => {
+        DefPathData::Closure => {
             let label = coroutine_kind_label(tcx.coroutine_kind(def_id));
 
             push_disambiguated_special_name(

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -414,6 +414,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandValue<V> {
                 // value is through `undef`/`poison`, and the store itself is useless.
             }
             OperandValue::Ref(r, None, source_align) => {
+                assert!(dest.layout.is_sized(), "cannot directly store unsized values");
                 if flags.contains(MemFlags::NONTEMPORAL) {
                     // HACK(nox): This is inefficient but there is no nontemporal memcpy.
                     let ty = bx.backend_type(dest.layout);

compiler/rustc_codegen_ssa/src/mir/place.rs

@@ -143,7 +143,8 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
         // Simple cases, which don't need DST adjustment:
         //   * no metadata available - just log the case
         //   * known alignment - sized types, `[T]`, `str` or a foreign type
-        //   * packed struct - there is no alignment padding
+        // Note that looking at `field.align` is incorrect since that is not necessarily equal
+        // to the dynamic alignment of the type.
         match field.ty.kind() {
             _ if self.llextra.is_none() => {
                 debug!(
@@ -154,14 +155,6 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
             }
             _ if field.is_sized() => return simple(),
             ty::Slice(..) | ty::Str | ty::Foreign(..) => return simple(),
-            ty::Adt(def, _) => {
-                if def.repr().packed() {
-                    // FIXME(eddyb) generalize the adjustment when we
-                    // start supporting packing to larger alignments.
-                    assert_eq!(self.layout.align.abi.bytes(), 1);
-                    return simple();
-                }
-            }
             _ => {}
         }
 
@@ -186,7 +179,16 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
         let unaligned_offset = bx.cx().const_usize(offset.bytes());
 
         // Get the alignment of the field
-        let (_, unsized_align) = glue::size_and_align_of_dst(bx, field.ty, meta);
+        let (_, mut unsized_align) = glue::size_and_align_of_dst(bx, field.ty, meta);
+
+        // For packed types, we need to cap alignment.
+        if let ty::Adt(def, _) = self.layout.ty.kind()
+            && let Some(packed) = def.repr().pack
+        {
+            let packed = bx.const_usize(packed.bytes());
+            let cmp = bx.icmp(IntPredicate::IntULT, unsized_align, packed);
+            unsized_align = bx.select(cmp, unsized_align, packed)
+        }
 
         // Bump the unaligned offset up to the appropriate alignment
         let offset = round_up_const_value_to_alignment(bx, unaligned_offset, unsized_align);

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -282,9 +282,7 @@ impl<'mir, 'tcx, Prov: Provenance, Extra> Frame<'mir, 'tcx, Prov, Extra> {
 impl<'tcx> fmt::Display for FrameInfo<'tcx> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         ty::tls::with(|tcx| {
-            if tcx.def_key(self.instance.def_id()).disambiguated_data.data
-                == DefPathData::ClosureExpr
-            {
+            if tcx.def_key(self.instance.def_id()).disambiguated_data.data == DefPathData::Closure {
                 write!(f, "inside closure")
             } else {
                 // Note: this triggers a `good_path_delayed_bug` state, which means that if we ever
@@ -299,7 +297,7 @@ impl<'tcx> fmt::Display for FrameInfo<'tcx> {
 impl<'tcx> FrameInfo<'tcx> {
     pub fn as_note(&self, tcx: TyCtxt<'tcx>) -> errors::FrameNote {
         let span = self.span;
-        if tcx.def_key(self.instance.def_id()).disambiguated_data.data == DefPathData::ClosureExpr {
+        if tcx.def_key(self.instance.def_id()).disambiguated_data.data == DefPathData::Closure {
             errors::FrameNote { where_: "closure", span, instance: String::new(), times: 0 }
         } else {
             let instance = format!("{}", self.instance);