From 1348d6956dcba3f0f06c12185b185557dc0c8dfb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 15 Mar 2024 21:06:40 +0100 Subject: [PATCH 1/6] collector: recursively traverse 'mentioned' items to evaluate their constants --- compiler/rustc_data_structures/src/sync.rs | 9 +- compiler/rustc_middle/src/mir/mod.rs | 26 ++ .../rustc_mir_build/src/build/custom/mod.rs | 1 + compiler/rustc_mir_transform/src/inline.rs | 26 +- compiler/rustc_mir_transform/src/lib.rs | 5 + .../src/mentioned_items.rs | 57 +++ compiler/rustc_mir_transform/src/shim.rs | 4 +- compiler/rustc_monomorphize/src/collector.rs | 371 +++++++++++++----- .../rustc_monomorphize/src/partitioning.rs | 14 +- .../collect-in-dead-drop.noopt.stderr | 6 +- .../collect-in-dead-drop.opt.stderr | 20 + .../required-consts/collect-in-dead-drop.rs | 7 +- .../collect-in-dead-fn.noopt.stderr | 8 +- .../collect-in-dead-fn.opt.stderr | 23 ++ .../required-consts/collect-in-dead-fn.rs | 7 +- ...ollect-in-dead-fnptr-in-const.noopt.stderr | 20 + .../collect-in-dead-fnptr-in-const.opt.stderr | 20 + .../collect-in-dead-fnptr-in-const.rs | 33 ++ .../collect-in-dead-fnptr.noopt.stderr | 23 ++ .../collect-in-dead-fnptr.opt.stderr | 23 ++ .../required-consts/collect-in-dead-fnptr.rs | 32 ++ .../collect-in-dead-move.noopt.stderr | 6 +- .../collect-in-dead-move.opt.stderr | 20 + .../required-consts/collect-in-dead-move.rs | 7 +- 24 files changed, 631 insertions(+), 137 deletions(-) create mode 100644 compiler/rustc_mir_transform/src/mentioned_items.rs create mode 100644 tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.opt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.rs create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fnptr.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fnptr.opt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-fnptr.rs create mode 100644 tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 32202ac3eded1..eab6d8168ca8e 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -20,6 +20,7 @@ //! | ----------------------- | ------------------- | ------------------------------- | //! | `Lrc` | `rc::Rc` | `sync::Arc` | //! |` Weak` | `rc::Weak` | `sync::Weak` | +//! | `LRef<'a, T>` [^2] | `&'a mut T` | `&'a T` | //! | | | | //! | `AtomicBool` | `Cell` | `atomic::AtomicBool` | //! | `AtomicU32` | `Cell` | `atomic::AtomicU32` | @@ -38,7 +39,7 @@ //! of a `RefCell`. This is appropriate when interior mutability is not //! required. //! -//! [^2] `MTLockRef` is a typedef. +//! [^2] `MTRef`, `MTLockRef` are type aliases. pub use crate::marker::*; use std::collections::HashMap; @@ -208,7 +209,7 @@ cfg_match! { use std::cell::RefCell as InnerRwLock; - pub type MTLockRef<'a, T> = &'a mut MTLock; + pub type LRef<'a, T> = &'a mut T; #[derive(Debug, Default)] pub struct MTLock(T); @@ -274,7 +275,7 @@ cfg_match! { pub use std::sync::Arc as Lrc; pub use std::sync::Weak as Weak; - pub type MTLockRef<'a, T> = &'a MTLock; + pub type LRef<'a, T> = &'a T; #[derive(Debug, Default)] pub struct MTLock(Lock); @@ -314,6 +315,8 @@ cfg_match! { } } +pub type MTLockRef<'a, T> = LRef<'a, MTLock>; + #[derive(Default)] #[cfg_attr(parallel_compiler, repr(align(64)))] pub struct CacheAligned(pub T); diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index d57ffc0f8b5c5..7495c16a89e25 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -20,6 +20,7 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_ID}; use rustc_hir::{self, CoroutineDesugaring, CoroutineKind, ImplicitSelfKind}; use rustc_hir::{self as hir, HirId}; use rustc_session::Session; +use rustc_span::source_map::Spanned; use rustc_target::abi::{FieldIdx, VariantIdx}; use polonius_engine::Atom; @@ -312,6 +313,16 @@ impl<'tcx> CoroutineInfo<'tcx> { } } +/// Some item that needs to monomorphize successfully for a MIR body to be considered well-formed. +#[derive(Copy, Clone, PartialEq, Debug, HashStable, TyEncodable, TyDecodable)] +#[derive(TypeFoldable, TypeVisitable)] +pub enum MentionedItem<'tcx> { + Fn(DefId, GenericArgsRef<'tcx>), + Drop(Ty<'tcx>), + // FIXME: add Vtable { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> }, + // FIXME: do we have to add closures? +} + /// The lowered representation of a single function. #[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable, TypeFoldable, TypeVisitable)] pub struct Body<'tcx> { @@ -375,8 +386,21 @@ pub struct Body<'tcx> { /// Constants that are required to evaluate successfully for this MIR to be well-formed. /// We hold in this field all the constants we are not able to evaluate yet. + /// + /// This is soundness-critical, we make a guarantee that all consts syntactically mentioned in a + /// function have successfully evaluated if the function ever gets executed at runtime. pub required_consts: Vec>, + /// Further items that were mentioned in this function and hence *may* become monomorphized, + /// depending on optimizations. We use this to avoid optimization-dependent compile errors: the + /// collector recursively traverses all "mentioned" items and evaluates all their + /// `requiered_consts`. + /// + /// This is *not* soundness-critical and the contents of this list are *not* a stable guarantee. + /// See the documentation of `CollectionMode` in `compiler/rustc_monomorphize/src/collector.rs` + /// for more context. + pub mentioned_items: Vec>>, + /// Does this body use generic parameters. This is used for the `ConstEvaluatable` check. /// /// Note that this does not actually mean that this body is not computable right now. @@ -453,6 +477,7 @@ impl<'tcx> Body<'tcx> { var_debug_info, span, required_consts: Vec::new(), + mentioned_items: Vec::new(), is_polymorphic: false, injection_phase: None, tainted_by_errors, @@ -482,6 +507,7 @@ impl<'tcx> Body<'tcx> { spread_arg: None, span: DUMMY_SP, required_consts: Vec::new(), + mentioned_items: Vec::new(), var_debug_info: Vec::new(), is_polymorphic: false, injection_phase: None, diff --git a/compiler/rustc_mir_build/src/build/custom/mod.rs b/compiler/rustc_mir_build/src/build/custom/mod.rs index 288b787798b49..109ffedec5506 100644 --- a/compiler/rustc_mir_build/src/build/custom/mod.rs +++ b/compiler/rustc_mir_build/src/build/custom/mod.rs @@ -56,6 +56,7 @@ pub(super) fn build_custom_mir<'tcx>( var_debug_info: Vec::new(), span, required_consts: Vec::new(), + mentioned_items: Vec::new(), is_polymorphic: false, tainted_by_errors: None, injection_phase: None, diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index f6a0945c22297..aee2c99c929db 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -565,7 +565,8 @@ impl<'tcx> Inliner<'tcx> { mut callee_body: Body<'tcx>, ) { let terminator = caller_body[callsite.block].terminator.take().unwrap(); - let TerminatorKind::Call { args, destination, unwind, target, .. } = terminator.kind else { + let TerminatorKind::Call { func, args, destination, unwind, target, .. } = terminator.kind + else { bug!("unexpected terminator kind {:?}", terminator.kind); }; @@ -717,6 +718,29 @@ impl<'tcx> Inliner<'tcx> { Const::Val(..) | Const::Unevaluated(..) => true, }, )); + // Now that we incorporated the callee's `required_consts`, we can remove the callee from + // `mentioned_items` -- but we have to take their `mentioned_items` in return. + // This does some extra work here to save the monomorphization collector work later. + // FIXME: benchmark which option is better. + let callee_item = { + // We need to reconstruct the `required_item` for the callee so that we can find and + // remove it. + let func_ty = func.ty(caller_body, self.tcx); + match func_ty.kind() { + ty::FnDef(def_id, args) => MentionedItem::Fn(*def_id, args), + _ => bug!(), + } + }; + if let Some(idx) = + caller_body.mentioned_items.iter().position(|item| item.node == callee_item) + { + // We found the callee, so remove it and add its items instead. + caller_body.mentioned_items.remove(idx); + caller_body.mentioned_items.extend(callee_body.mentioned_items); + } else { + // If we can't find the callee, there's no point in adding its items. + // Probably it already got removed by being inlined elsewhere in the same function. + } } fn make_call_args( diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 4551380152217..65bc2af2ad7eb 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -89,6 +89,7 @@ mod lint; mod lower_intrinsics; mod lower_slice_len; mod match_branches; +mod mentioned_items; mod multiple_return_terminators; mod normalize_array_len; mod nrvo; @@ -566,6 +567,10 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { tcx, body, &[ + // Before doing anything, remember which items are being mentioned so that the set of items + // visited does not depend on the optimization level. + &mentioned_items::MentionedItems, + // Add some UB checks before any UB gets optimized away. &check_alignment::CheckAlignment, // Before inlining: trim down MIR with passes to reduce inlining work. diff --git a/compiler/rustc_mir_transform/src/mentioned_items.rs b/compiler/rustc_mir_transform/src/mentioned_items.rs new file mode 100644 index 0000000000000..ed363b4f2527a --- /dev/null +++ b/compiler/rustc_mir_transform/src/mentioned_items.rs @@ -0,0 +1,57 @@ +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::{self, ConstOperand, Location, MentionedItem, MirPass}; +use rustc_middle::ty::{self, TyCtxt}; +use rustc_session::Session; +use rustc_span::source_map::Spanned; + +pub struct MentionedItems; + +struct MentionedItemsVisitor<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + body: &'a mir::Body<'tcx>, + mentioned_items: &'a mut Vec>>, +} + +impl<'tcx> MirPass<'tcx> for MentionedItems { + fn is_enabled(&self, _sess: &Session) -> bool { + // If this pass is skipped the collector assume that nothing got mentioned! We could + // potentially skip it in opt-level 0 if we are sure that opt-level will never *remove* uses + // of anything, but that still seems fragile. Furthermore, even debug builds use level 1, so + // special-casing level 0 is just not worth it. + true + } + + fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut mir::Body<'tcx>) { + debug_assert!(body.mentioned_items.is_empty()); + let mut mentioned_items = Vec::new(); + MentionedItemsVisitor { tcx, body, mentioned_items: &mut mentioned_items }.visit_body(body); + body.mentioned_items = mentioned_items; + } +} + +impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> { + fn visit_constant(&mut self, constant: &ConstOperand<'tcx>, _: Location) { + let const_ = constant.const_; + // This is how function items get referenced: via constants of `FnDef` type. This handles + // both functions that are called and those that are just turned to function pointers. + if let ty::FnDef(def_id, args) = const_.ty().kind() { + debug!("adding to required_items: {def_id:?}"); + self.mentioned_items + .push(Spanned { node: MentionedItem::Fn(*def_id, args), span: constant.span }); + } + } + + fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { + self.super_terminator(terminator, location); + match terminator.kind { + // We don't need to handle `Call` as we already handled all function type operands in + // `visit_constant`. But we do need to handle `Drop`. + mir::TerminatorKind::Drop { place, .. } => { + let ty = place.ty(self.body, self.tcx).ty; + let span = self.body.source_info(location).span; + self.mentioned_items.push(Spanned { node: MentionedItem::Drop(ty), span }); + } + _ => {} + } + } +} diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 733e2f93b2535..dd2b24ad6691d 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -17,7 +17,7 @@ use std::iter; use crate::{ abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, deref_separator, - pass_manager as pm, remove_noop_landing_pads, simplify, + mentioned_items, pass_manager as pm, remove_noop_landing_pads, simplify, }; use rustc_middle::mir::patch::MirPatch; use rustc_mir_dataflow::elaborate_drops::{self, DropElaborator, DropFlagMode, DropStyle}; @@ -147,6 +147,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<' tcx, &mut body, &[ + &mentioned_items::MentionedItems, &abort_unwinding_calls::AbortUnwindingCalls, &add_call_guards::CriticalCallEdges, ], @@ -178,6 +179,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<' tcx, &mut result, &[ + &mentioned_items::MentionedItems, &add_moves_for_packed_drops::AddMovesForPackedDrops, &deref_separator::Derefer, &remove_noop_landing_pads::RemoveNoopLandingPads, diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index cd9eb4916ce5d..59cbc770c7918 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -137,21 +137,41 @@ //! just linked to and no node is created; which is exactly what we want, since //! no machine code should be generated in the current crate for such an item. //! -//! Eager and Lazy Collection Mode -//! ------------------------------ -//! Mono item collection can be performed in one of two modes: +//! Eager and Lazy Collection Strategy +//! ---------------------------------- +//! Mono item collection can be performed with one of two strategies: //! -//! - Lazy mode means that items will only be instantiated when actually +//! - Lazy strategy means that items will only be instantiated when actually //! used. The goal is to produce the least amount of machine code //! possible. //! -//! - Eager mode is meant to be used in conjunction with incremental compilation +//! - Eager strategy is meant to be used in conjunction with incremental compilation //! where a stable set of mono items is more important than a minimal -//! one. Thus, eager mode will instantiate drop-glue for every drop-able type +//! one. Thus, eager strategy will instantiate drop-glue for every drop-able type //! in the crate, even if no drop call for that type exists (yet). It will //! also instantiate default implementations of trait methods, something that //! otherwise is only done on demand. //! +//! Collection-time const evaluation and "mentioned" items +//! ------------------------------------------------------ +//! +//! One important role of collection is to evaluate all constants that are used by all the items +//! which are being collected. Codegen can then rely on only encountering constants that evaluate +//! successfully, and if a constant fails to evaluate, the collector has much better context to be +//! able to show where this constant comes up. +//! +//! However, the exact set of "used" items (collected as described above), and therefore the exact +//! set of used constants, can depend on optimizations. Optimizing away dead code may optimize away +//! a function call that uses a failing constant, so an unoptimized build may fail where an +//! optimized build succeeds. This is undesirable. +//! +//! To fix this, the collector has the concept of "mentioned" items. Some time during the MIR +//! pipeline, before any optimization-level-dependent optimizations, we compute a list of all items +//! that syntactically appear in the code. These are considered "mentioned", and even if they are in +//! dead code and get optimized away (which makes them no longer "used"), they are still +//! "mentioned". For every used item, the collector ensures that all mentioned items, recursively, +//! do not use a failing constant. This is reflected via the [`CollectionMode`], which determines +//! whether we are visiting a used item or merely a mentioned item. //! //! Open Issues //! ----------- @@ -165,7 +185,7 @@ //! regardless of whether it is actually needed or not. use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_data_structures::sync::{par_for_each_in, MTLock, MTLockRef}; +use rustc_data_structures::sync::{par_for_each_in, LRef, MTLock}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId}; @@ -173,7 +193,7 @@ use rustc_hir::lang_items::LangItem; use rustc_middle::mir::interpret::{AllocId, ErrorHandled, GlobalAlloc, Scalar}; use rustc_middle::mir::mono::{InstantiationMode, MonoItem}; use rustc_middle::mir::visit::Visitor as MirVisitor; -use rustc_middle::mir::{self, Location}; +use rustc_middle::mir::{self, Location, MentionedItem}; use rustc_middle::query::TyCtxtAt; use rustc_middle::ty::adjustment::{CustomCoerceUnsized, PointerCoercion}; use rustc_middle::ty::layout::ValidityRequirement; @@ -199,7 +219,7 @@ use crate::errors::{ }; #[derive(PartialEq)] -pub enum MonoItemCollectionMode { +pub enum MonoItemCollectionStrategy { Eager, Lazy, } @@ -214,6 +234,35 @@ pub struct UsageMap<'tcx> { type MonoItems<'tcx> = Vec>>; +/// The state that is shared across the concurrent threads that are doing collection. +struct SharedState<'tcx> { + /// Items that have been or are currently being recursively collected. + visited: MTLock>>, + /// Items that have been or are currently being recursively treated as "mentioned", i.e., their + /// consts are evaluated but nothing is added to the collection. + mentioned: MTLock>>, + /// Which items are being used where, for better errors. + usage_map: MTLock>, +} + +/// See module-level docs on some contect for "mentioned" items. +#[derive(Copy, Clone, Debug, PartialEq)] +enum CollectionMode { + /// Collect items that are used, i.e., actually needed for codegen. + /// + /// Which items are used can depend on optimization levels, as MIR optimizations can remove + /// uses. + UsedItems, + /// Collect items that are mentioned. The goal of this mode is that it is independent of + /// optimizations: the set of "mentioned" items is computed before optimizations are run. + /// + /// The exact contents of this set are *not* a stable guarantee. (For instance, it is currently + /// computed after drop-elaboration. If we ever do some optimizations even in debug builds, we + /// might decide to run them before computing mentioned items.) The key property of this set is + /// that it is optimization-independent. + MentionedItems, +} + impl<'tcx> UsageMap<'tcx> { fn new() -> UsageMap<'tcx> { UsageMap { used_map: FxHashMap::default(), user_map: FxHashMap::default() } @@ -253,25 +302,28 @@ impl<'tcx> UsageMap<'tcx> { } } -#[instrument(skip(tcx, mode), level = "debug")] +#[instrument(skip(tcx, strategy), level = "debug")] pub fn collect_crate_mono_items( tcx: TyCtxt<'_>, - mode: MonoItemCollectionMode, + strategy: MonoItemCollectionStrategy, ) -> (FxHashSet>, UsageMap<'_>) { let _prof_timer = tcx.prof.generic_activity("monomorphization_collector"); - let roots = - tcx.sess.time("monomorphization_collector_root_collections", || collect_roots(tcx, mode)); + let roots = tcx + .sess + .time("monomorphization_collector_root_collections", || collect_roots(tcx, strategy)); debug!("building mono item graph, beginning at roots"); - let mut visited = MTLock::new(FxHashSet::default()); - let mut usage_map = MTLock::new(UsageMap::new()); + let mut state = SharedState { + visited: MTLock::new(FxHashSet::default()), + mentioned: MTLock::new(FxHashSet::default()), + usage_map: MTLock::new(UsageMap::new()), + }; let recursion_limit = tcx.recursion_limit(); { - let visited: MTLockRef<'_, _> = &mut visited; - let usage_map: MTLockRef<'_, _> = &mut usage_map; + let state: LRef<'_, _> = &mut state; tcx.sess.time("monomorphization_collector_graph_walk", || { par_for_each_in(roots, |root| { @@ -279,22 +331,22 @@ pub fn collect_crate_mono_items( collect_items_rec( tcx, dummy_spanned(root), - visited, + state, &mut recursion_depths, recursion_limit, - usage_map, + CollectionMode::UsedItems, ); }); }); } - (visited.into_inner(), usage_map.into_inner()) + (state.visited.into_inner(), state.usage_map.into_inner()) } // Find all non-generic items by walking the HIR. These items serve as roots to // start monomorphizing from. #[instrument(skip(tcx, mode), level = "debug")] -fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionMode) -> Vec> { +fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionStrategy) -> Vec> { debug!("collecting roots"); let mut roots = Vec::new(); @@ -303,7 +355,7 @@ fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionMode) -> Vec, mode: MonoItemCollectionMode) -> Vec( tcx: TyCtxt<'tcx>, starting_item: Spanned>, - visited: MTLockRef<'_, FxHashSet>>, + state: LRef<'_, SharedState<'tcx>>, recursion_depths: &mut DefIdMap, recursion_limit: Limit, - usage_map: MTLockRef<'_, UsageMap<'tcx>>, + mode: CollectionMode, ) { - if !visited.lock_mut().insert(starting_item.node) { - // We've been here already, no need to search again. - return; + if mode == CollectionMode::UsedItems { + if !state.visited.lock_mut().insert(starting_item.node) { + // We've been here already, no need to search again. + return; + } + } else { + if state.visited.lock().contains(&starting_item.node) { + // We've already done a *full* visit on this one, no need to do the "mention" visit. + return; + } + if !state.mentioned.lock_mut().insert(starting_item.node) { + // We've been here already, no need to search again. + return; + } + // There's some risk that we first do a 'mention' visit and then a full visit. But there's no + // harm in that, the mention visit will trigger all the queries and the results are cached. } - let mut used_items = Vec::new(); + let mut used_items = MonoItems::new(); + let mut mentioned_items = MonoItems::new(); let recursion_depth_reset; // Post-monomorphization errors MVP @@ -373,37 +442,48 @@ fn collect_items_rec<'tcx>( // FIXME: don't rely on global state, instead bubble up errors. Note: this is very hard to do. let error_count = tcx.dcx().err_count(); + // In `mentioned_items` we collect items that were mentioned in this MIR but possibly do not + // need to be monomorphized. This is done to ensure that optimizing away function calls does not + // hide const-eval errors that those calls would otherwise have triggered. match starting_item.node { MonoItem::Static(def_id) => { - let instance = Instance::mono(tcx, def_id); + recursion_depth_reset = None; - // Sanity check whether this ended up being collected accidentally - debug_assert!(should_codegen_locally(tcx, &instance)); + // Statics always get evaluted (which is possible because they can't be generic), so for + // `MentionedItems` collection there's nothing to do here. + if mode == CollectionMode::UsedItems { + let instance = Instance::mono(tcx, def_id); - let DefKind::Static { nested, .. } = tcx.def_kind(def_id) else { bug!() }; - // Nested statics have no type. - if !nested { - let ty = instance.ty(tcx, ty::ParamEnv::reveal_all()); - visit_drop_use(tcx, ty, true, starting_item.span, &mut used_items); - } + // Sanity check whether this ended up being collected accidentally + debug_assert!(should_codegen_locally(tcx, &instance)); - recursion_depth_reset = None; + let DefKind::Static { nested, .. } = tcx.def_kind(def_id) else { bug!() }; + // Nested statics have no type. + if !nested { + let ty = instance.ty(tcx, ty::ParamEnv::reveal_all()); + visit_drop_use(tcx, ty, true, starting_item.span, &mut used_items); + } - if let Ok(alloc) = tcx.eval_static_initializer(def_id) { - for &prov in alloc.inner().provenance().ptrs().values() { - collect_alloc(tcx, prov.alloc_id(), &mut used_items); + if let Ok(alloc) = tcx.eval_static_initializer(def_id) { + for &prov in alloc.inner().provenance().ptrs().values() { + collect_alloc(tcx, prov.alloc_id(), &mut used_items); + } } - } - if tcx.needs_thread_local_shim(def_id) { - used_items.push(respan( - starting_item.span, - MonoItem::Fn(Instance { - def: InstanceDef::ThreadLocalShim(def_id), - args: GenericArgs::empty(), - }), - )); + if tcx.needs_thread_local_shim(def_id) { + used_items.push(respan( + starting_item.span, + MonoItem::Fn(Instance { + def: InstanceDef::ThreadLocalShim(def_id), + args: GenericArgs::empty(), + }), + )); + } } + + // mentioned_items stays empty since there's no codegen for statics. statics don't get + // optimized, and if they did then the const-eval interpreter would have to worry about + // mentioned_items. } MonoItem::Fn(instance) => { // Sanity check whether this ended up being collected accidentally @@ -420,10 +500,20 @@ fn collect_items_rec<'tcx>( check_type_length_limit(tcx, instance); rustc_data_structures::stack::ensure_sufficient_stack(|| { - collect_used_items(tcx, instance, &mut used_items); + collect_items_of_instance( + tcx, + instance, + &mut used_items, + &mut mentioned_items, + mode, + ) }); } MonoItem::GlobalAsm(item_id) => { + assert!( + mode == CollectionMode::UsedItems, + "should never encounter global_asm when collecting mentioned items" + ); recursion_depth_reset = None; let item = tcx.hir().item(item_id); @@ -459,7 +549,12 @@ fn collect_items_rec<'tcx>( } else { span_bug!(item.span, "Mismatch between hir::Item type and MonoItem type") } + + // mention_items stays empty as nothing gets optimized here. } + }; + if mode == CollectionMode::MentionedItems { + assert!(used_items.is_empty(), "'mentioned' collection should never encounter used items"); } // Check for PMEs and emit a diagnostic if one happened. To try to show relevant edges of the @@ -474,10 +569,37 @@ fn collect_items_rec<'tcx>( formatted_item, }); } - usage_map.lock_mut().record_used(starting_item.node, &used_items); + // Only updating `usage_map` for used items as otherwise we may be inserting the same item + // multiple times (if it is first 'mentioned' and then later actuall used), and the usage map + // logic does not like that. + // This is part of the output of collection and hence only relevant for "used" items. + // ("Mentioned" items are only considered internally during collection.) + if mode == CollectionMode::UsedItems { + state.usage_map.lock_mut().record_used(starting_item.node, &used_items); + } for used_item in used_items { - collect_items_rec(tcx, used_item, visited, recursion_depths, recursion_limit, usage_map); + collect_items_rec( + tcx, + used_item, + state, + recursion_depths, + recursion_limit, + CollectionMode::UsedItems, + ); + } + + // Walk over mentioned items *after* used items, so that if an item is both mentioned and used then + // the loop above has fully collected it, so this loop will skip it. + for mentioned_item in mentioned_items { + collect_items_rec( + tcx, + mentioned_item, + state, + recursion_depths, + recursion_limit, + CollectionMode::MentionedItems, + ); } if let Some((def_id, depth)) = recursion_depth_reset { @@ -596,7 +718,7 @@ fn check_type_length_limit<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) { struct MirUsedCollector<'a, 'tcx> { tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, - output: &'a mut MonoItems<'tcx>, + used_items: &'a mut MonoItems<'tcx>, instance: Instance<'tcx>, /// Spans for move size lints already emitted. Helps avoid duplicate lints. move_size_spans: Vec, @@ -735,6 +857,31 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> { ); self.move_size_spans.push(span); } + + /// Evaluates a *not yet monomorphized* constant. + fn eval_constant( + &mut self, + constant: &mir::ConstOperand<'tcx>, + ) -> Option> { + let const_ = self.monomorphize(constant.const_); + let param_env = ty::ParamEnv::reveal_all(); + // Evaluate the constant. This makes const eval failure a collection-time error (rather than + // a codegen-time error). rustc stops after collection if there was an error, so this + // ensures codegen never has to worry about failing consts. + // (codegen relies on this and ICEs will happen if this is violated.) + match const_.eval(self.tcx, param_env, Some(constant.span)) { + Ok(v) => Some(v), + Err(ErrorHandled::TooGeneric(..)) => span_bug!( + constant.span, + "collection encountered polymorphic constant: {:?}", + const_ + ), + Err(err @ ErrorHandled::Reported(..)) => { + err.emit_note(self.tcx); + return None; + } + } + } } impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { @@ -769,7 +916,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { target_ty, source_ty, span, - self.output, + self.used_items, ); } } @@ -780,7 +927,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { ) => { let fn_ty = operand.ty(self.body, self.tcx); let fn_ty = self.monomorphize(fn_ty); - visit_fn_use(self.tcx, fn_ty, false, span, self.output); + visit_fn_use(self.tcx, fn_ty, false, span, self.used_items); } mir::Rvalue::Cast( mir::CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(_)), @@ -798,7 +945,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { ty::ClosureKind::FnOnce, ); if should_codegen_locally(self.tcx, &instance) { - self.output.push(create_fn_mono_item(self.tcx, instance, span)); + self.used_items.push(create_fn_mono_item(self.tcx, instance, span)); } } _ => bug!(), @@ -809,7 +956,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { let instance = Instance::mono(self.tcx, def_id); if should_codegen_locally(self.tcx, &instance) { trace!("collecting thread-local static {:?}", def_id); - self.output.push(respan(span, MonoItem::Static(def_id))); + self.used_items.push(respan(span, MonoItem::Static(def_id))); } } _ => { /* not interesting */ } @@ -822,26 +969,9 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { /// to ensure that the constant evaluates successfully and walk the result. #[instrument(skip(self), level = "debug")] fn visit_constant(&mut self, constant: &mir::ConstOperand<'tcx>, location: Location) { - let const_ = self.monomorphize(constant.const_); - let param_env = ty::ParamEnv::reveal_all(); - // Evaluate the constant. This makes const eval failure a collection-time error (rather than - // a codegen-time error). rustc stops after collection if there was an error, so this - // ensures codegen never has to worry about failing consts. - // (codegen relies on this and ICEs will happen if this is violated.) - let val = match const_.eval(self.tcx, param_env, Some(constant.span)) { - Ok(v) => v, - Err(ErrorHandled::TooGeneric(..)) => span_bug!( - self.body.source_info(location).span, - "collection encountered polymorphic constant: {:?}", - const_ - ), - Err(err @ ErrorHandled::Reported(..)) => { - err.emit_note(self.tcx); - return; - } - }; - collect_const_value(self.tcx, val, self.output); - MirVisitor::visit_ty(self, const_.ty(), TyContext::Location(location)); + let Some(val) = self.eval_constant(constant) else { return }; + collect_const_value(self.tcx, val, self.used_items); + MirVisitor::visit_ty(self, constant.ty(), TyContext::Location(location)); } fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { @@ -852,7 +982,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { let push_mono_lang_item = |this: &mut Self, lang_item: LangItem| { let instance = Instance::mono(tcx, tcx.require_lang_item(lang_item, Some(source))); if should_codegen_locally(tcx, &instance) { - this.output.push(create_fn_mono_item(tcx, instance, source)); + this.used_items.push(create_fn_mono_item(tcx, instance, source)); } }; @@ -861,25 +991,25 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { let callee_ty = func.ty(self.body, tcx); let callee_ty = self.monomorphize(callee_ty); self.check_fn_args_move_size(callee_ty, args, *fn_span, location); - visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output) + visit_fn_use(self.tcx, callee_ty, true, source, &mut self.used_items) } mir::TerminatorKind::Drop { ref place, .. } => { let ty = place.ty(self.body, self.tcx).ty; let ty = self.monomorphize(ty); - visit_drop_use(self.tcx, ty, true, source, self.output); + visit_drop_use(self.tcx, ty, true, source, self.used_items); } mir::TerminatorKind::InlineAsm { ref operands, .. } => { for op in operands { match *op { mir::InlineAsmOperand::SymFn { ref value } => { let fn_ty = self.monomorphize(value.const_.ty()); - visit_fn_use(self.tcx, fn_ty, false, source, self.output); + visit_fn_use(self.tcx, fn_ty, false, source, self.used_items); } mir::InlineAsmOperand::SymStatic { def_id } => { let instance = Instance::mono(self.tcx, def_id); if should_codegen_locally(self.tcx, &instance) { trace!("collecting asm sym static {:?}", def_id); - self.output.push(respan(source, MonoItem::Static(def_id))); + self.used_items.push(respan(source, MonoItem::Static(def_id))); } } _ => {} @@ -1239,7 +1369,7 @@ fn create_mono_items_for_vtable_methods<'tcx>( struct RootCollector<'a, 'tcx> { tcx: TyCtxt<'tcx>, - mode: MonoItemCollectionMode, + strategy: MonoItemCollectionStrategy, output: &'a mut MonoItems<'tcx>, entry_fn: Option<(DefId, EntryFnType)>, } @@ -1248,7 +1378,7 @@ impl<'v> RootCollector<'_, 'v> { fn process_item(&mut self, id: hir::ItemId) { match self.tcx.def_kind(id.owner_id) { DefKind::Enum | DefKind::Struct | DefKind::Union => { - if self.mode == MonoItemCollectionMode::Eager + if self.strategy == MonoItemCollectionStrategy::Eager && self.tcx.generics_of(id.owner_id).count() == 0 { debug!("RootCollector: ADT drop-glue for `{id:?}`",); @@ -1279,7 +1409,7 @@ impl<'v> RootCollector<'_, 'v> { } } DefKind::Impl { .. } => { - if self.mode == MonoItemCollectionMode::Eager { + if self.strategy == MonoItemCollectionStrategy::Eager { create_mono_items_for_default_impls(self.tcx, id, self.output); } } @@ -1298,9 +1428,9 @@ impl<'v> RootCollector<'_, 'v> { fn is_root(&self, def_id: LocalDefId) -> bool { !self.tcx.generics_of(def_id).requires_monomorphization(self.tcx) - && match self.mode { - MonoItemCollectionMode::Eager => true, - MonoItemCollectionMode::Lazy => { + && match self.strategy { + MonoItemCollectionStrategy::Eager => true, + MonoItemCollectionStrategy::Lazy => { self.entry_fn.and_then(|(id, _)| id.as_local()) == Some(def_id) || self.tcx.is_reachable_non_generic(def_id) || self @@ -1493,26 +1623,67 @@ fn build_skip_move_check_fns(tcx: TyCtxt<'_>) -> Vec { } /// Scans the MIR in order to find function calls, closures, and drop-glue. -#[instrument(skip(tcx, output), level = "debug")] -fn collect_used_items<'tcx>( +/// +/// Anything that's found is added to `output`. Furthermore the "mentioned items" of the MIR are returned. +#[instrument(skip(tcx, used_items, mentioned_items), level = "debug")] +fn collect_items_of_instance<'tcx>( tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, - output: &mut MonoItems<'tcx>, + used_items: &mut MonoItems<'tcx>, + mentioned_items: &mut MonoItems<'tcx>, + mode: CollectionMode, ) { let body = tcx.instance_mir(instance.def); - - // Here we rely on the visitor also visiting `required_consts`, so that we evaluate them - // and abort compilation if any of them errors. - MirUsedCollector { + let mut collector = MirUsedCollector { tcx, - body: body, - output, + body, + used_items, instance, move_size_spans: vec![], visiting_call_terminator: false, skip_move_check_fns: None, + }; + + if mode == CollectionMode::UsedItems { + // Visit everything. Here we rely on the visitor also visiting `required_consts`, so that we + // evaluate them and abort compilation if any of them errors. + collector.visit_body(body); + } else { + // We only need to evaluate all constants, but can ignore the rest of the MIR. + for const_op in &body.required_consts { + if let Some(val) = collector.eval_constant(const_op) { + collect_const_value(tcx, val, mentioned_items); + } + } + } + + // Always gather mentioned items. + for item in &body.mentioned_items { + let item_mono = collector.monomorphize(item.node); + visit_mentioned_item(tcx, &item_mono, item.span, mentioned_items); + } +} + +/// `item` must be already monomorphized +fn visit_mentioned_item<'tcx>( + tcx: TyCtxt<'tcx>, + item: &MentionedItem<'tcx>, + span: Span, + output: &mut MonoItems<'tcx>, +) { + match *item { + MentionedItem::Fn(def_id, args) => { + let instance = Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, args); + // `visit_instance_use` was written for "used" item collection but works just as well + // for "mentioned" item collection. + // We can set `is_direct_call`; that just means we'll skip a bunch of shims that anyway + // can't have their own failing constants. + visit_instance_use(tcx, instance, /*is_direct_call*/ true, span, output); + } + MentionedItem::Drop(ty) => { + visit_drop_use(tcx, ty, /*is_direct_call*/ true, span, output); + } } - .visit_body(body); } #[instrument(skip(tcx, output), level = "debug")] diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 15041b9cd4181..5a92657cb40d7 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -117,7 +117,7 @@ use rustc_session::CodegenUnits; use rustc_span::symbol::Symbol; use crate::collector::UsageMap; -use crate::collector::{self, MonoItemCollectionMode}; +use crate::collector::{self, MonoItemCollectionStrategy}; use crate::errors::{CouldntDumpMonoStats, SymbolAlreadyDefined, UnknownCguCollectionMode}; struct PartitioningCx<'a, 'tcx> { @@ -1087,30 +1087,30 @@ where } fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[CodegenUnit<'_>]) { - let collection_mode = match tcx.sess.opts.unstable_opts.print_mono_items { + let collection_strategy = match tcx.sess.opts.unstable_opts.print_mono_items { Some(ref s) => { let mode = s.to_lowercase(); let mode = mode.trim(); if mode == "eager" { - MonoItemCollectionMode::Eager + MonoItemCollectionStrategy::Eager } else { if mode != "lazy" { tcx.dcx().emit_warn(UnknownCguCollectionMode { mode }); } - MonoItemCollectionMode::Lazy + MonoItemCollectionStrategy::Lazy } } None => { if tcx.sess.link_dead_code() { - MonoItemCollectionMode::Eager + MonoItemCollectionStrategy::Eager } else { - MonoItemCollectionMode::Lazy + MonoItemCollectionStrategy::Lazy } } }; - let (items, usage_map) = collector::collect_crate_mono_items(tcx, collection_mode); + let (items, usage_map) = collector::collect_crate_mono_items(tcx, collection_strategy); // If there was an error during collection (e.g. from one of the constants we evaluated), // then we stop here. This way codegen does not have to worry about failing constants. diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr index 0bf231d09f174..73790f7517db1 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr @@ -1,13 +1,13 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-drop.rs:12:19 + --> $DIR/collect-in-dead-drop.rs:9:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:12:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:9:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-drop.rs:19:17 + --> $DIR/collect-in-dead-drop.rs:16:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr new file mode 100644 index 0000000000000..73790f7517db1 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr @@ -0,0 +1,20 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-drop.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-drop.rs:16:17 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn as std::ops::Drop>::drop` + --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.rs b/tests/ui/consts/required-consts/collect-in-dead-drop.rs index c9ffcec690317..98e9d19189597 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-drop.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.rs @@ -1,15 +1,12 @@ //@revisions: noopt opt -//@[noopt] build-fail +//@ build-fail //@[opt] compile-flags: -O -//FIXME: `opt` revision currently does not stop with an error due to -//. -//@[opt] build-pass //! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is //! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) struct Fail(T); impl Fail { - const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed } // This function is not actually called, but is mentioned implicitly as destructor in dead code in a diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr index 8bb99efe8e48d..52462076ff90b 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr @@ -1,19 +1,19 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-fn.rs:12:19 + --> $DIR/collect-in-dead-fn.rs:9:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:12:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:9:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-fn.rs:22:17 + --> $DIR/collect-in-dead-fn.rs:19:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ note: the above error was encountered while instantiating `fn not_called::` - --> $DIR/collect-in-dead-fn.rs:29:9 + --> $DIR/collect-in-dead-fn.rs:26:9 | LL | not_called::(); | ^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr new file mode 100644 index 0000000000000..30c5c2d5c5caa --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr @@ -0,0 +1,23 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-fn.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-fn.rs:19:17 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn not_called::` + --> $DIR/collect-in-dead-fn.rs:26:9 + | +LL | not_called::(); + | ^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.rs b/tests/ui/consts/required-consts/collect-in-dead-fn.rs index 9e6b151915331..754fcf7b9874d 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-fn.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.rs @@ -1,15 +1,12 @@ //@revisions: noopt opt -//@[noopt] build-fail +//@ build-fail //@[opt] compile-flags: -O -//FIXME: `opt` revision currently does not stop with an error due to -//. -//@[opt] build-pass //! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is //! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) struct Fail(T); impl Fail { - const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed } // This function is not actually called, but it is mentioned in dead code in a function that is diff --git a/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.noopt.stderr new file mode 100644 index 0000000000000..7303b8d608a78 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.noopt.stderr @@ -0,0 +1,20 @@ +error[E0080]: evaluation of `Late::::FAIL` failed + --> $DIR/collect-in-dead-fnptr-in-const.rs:8:22 + | +LL | const FAIL: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fnptr-in-const.rs:8:22 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-fnptr-in-const.rs:9:28 + | +LL | const FNPTR: fn() = || Self::FAIL; + | ^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn Late::::FNPTR::{closure#0}` + --> $SRC_DIR/core/src/ops/function.rs:LL:COL + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.opt.stderr new file mode 100644 index 0000000000000..7303b8d608a78 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.opt.stderr @@ -0,0 +1,20 @@ +error[E0080]: evaluation of `Late::::FAIL` failed + --> $DIR/collect-in-dead-fnptr-in-const.rs:8:22 + | +LL | const FAIL: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fnptr-in-const.rs:8:22 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-fnptr-in-const.rs:9:28 + | +LL | const FNPTR: fn() = || Self::FAIL; + | ^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn Late::::FNPTR::{closure#0}` + --> $SRC_DIR/core/src/ops/function.rs:LL:COL + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.rs b/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.rs new file mode 100644 index 0000000000000..ed073e486c807 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fnptr-in-const.rs @@ -0,0 +1,33 @@ +//@revisions: noopt opt +//@ build-fail +//@[opt] compile-flags: -O +//! This fails without optimizations, so it should also fail with optimizations. + +struct Late(T); +impl Late { + const FAIL: () = panic!(); //~ERROR evaluation of `Late::::FAIL` failed + const FNPTR: fn() = || Self::FAIL; +} + +// This function is not actually called, but it is mentioned in dead code in a function that is +// called. The function then mentions a const that evaluates to a fnptr that points to a function +// that used a const that fails to evaluate. +// This tests that when processing mentioned items, we also check the fnptrs in the final values +// of consts that we encounter. +#[inline(never)] +fn not_called() { + if false { + let _ = Late::::FNPTR; + } +} + +#[inline(never)] +fn called() { + if false { + not_called::(); + } +} + +pub fn main() { + called::(); +} diff --git a/tests/ui/consts/required-consts/collect-in-dead-fnptr.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fnptr.noopt.stderr new file mode 100644 index 0000000000000..bf2f3cf9cc753 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fnptr.noopt.stderr @@ -0,0 +1,23 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-fnptr.rs:8:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fnptr.rs:8:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-fnptr.rs:17:17 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn not_called::` + --> $DIR/collect-in-dead-fnptr.rs:26:28 + | +LL | let _fnptr: fn() = not_called::; + | ^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-fnptr.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fnptr.opt.stderr new file mode 100644 index 0000000000000..bf2f3cf9cc753 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fnptr.opt.stderr @@ -0,0 +1,23 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-fnptr.rs:8:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fnptr.rs:8:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-fnptr.rs:17:17 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn not_called::` + --> $DIR/collect-in-dead-fnptr.rs:26:28 + | +LL | let _fnptr: fn() = not_called::; + | ^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-fnptr.rs b/tests/ui/consts/required-consts/collect-in-dead-fnptr.rs new file mode 100644 index 0000000000000..061addfe10287 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-fnptr.rs @@ -0,0 +1,32 @@ +//@revisions: noopt opt +//@ build-fail +//@[opt] compile-flags: -O +//! This fails without optimizations, so it should also fail with optimizations. + +struct Fail(T); +impl Fail { + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed +} + +// This function is not actually called, but it is mentioned in dead code in a function that is +// called. Make sure we still find this error. +// This ensures that we consider ReifyFnPointer coercions when gathering "mentioned" items. +#[inline(never)] +fn not_called() { + if false { + let _ = Fail::::C; + } +} + +#[inline(never)] +fn called() { + if false { + // We don't call the function, but turn it to a function pointer. + // Make sure it still gest added to `mentioned_items`. + let _fnptr: fn() = not_called::; + } +} + +pub fn main() { + called::(); +} diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr index 5b1df78b23265..2ab1f80e2d3b4 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr @@ -1,13 +1,13 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-move.rs:12:19 + --> $DIR/collect-in-dead-move.rs:9:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:12:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:9:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-move.rs:19:17 + --> $DIR/collect-in-dead-move.rs:16:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr new file mode 100644 index 0000000000000..2ab1f80e2d3b4 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr @@ -0,0 +1,20 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-move.rs:9:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:9:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-move.rs:16:17 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn as std::ops::Drop>::drop` + --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.rs b/tests/ui/consts/required-consts/collect-in-dead-move.rs index f3a6ba8a6577c..2fd6aea58de65 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-move.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-move.rs @@ -1,15 +1,12 @@ //@revisions: noopt opt -//@[noopt] build-fail +//@ build-fail //@[opt] compile-flags: -O -//FIXME: `opt` revision currently does not stop with an error due to -//. -//@[opt] build-pass //! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is //! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) struct Fail(T); impl Fail { - const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed } // This function is not actually called, but is mentioned implicitly as destructor in dead code in a From a6124d7079e6d2340bd300e0d16c79f9fdd5971a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 09:02:18 +0100 Subject: [PATCH 2/6] fix comments in required-consts tests --- .../required-consts/collect-in-dead-drop.noopt.stderr | 6 +++--- .../required-consts/collect-in-dead-drop.opt.stderr | 6 +++--- tests/ui/consts/required-consts/collect-in-dead-drop.rs | 3 +-- .../required-consts/collect-in-dead-fn.noopt.stderr | 8 ++++---- .../consts/required-consts/collect-in-dead-fn.opt.stderr | 8 ++++---- tests/ui/consts/required-consts/collect-in-dead-fn.rs | 3 +-- tests/ui/consts/required-consts/collect-in-dead-forget.rs | 3 +-- .../required-consts/collect-in-dead-move.noopt.stderr | 6 +++--- .../required-consts/collect-in-dead-move.opt.stderr | 6 +++--- tests/ui/consts/required-consts/collect-in-dead-move.rs | 3 +-- .../required-consts/collect-in-dead-vtable.noopt.stderr | 8 ++++---- tests/ui/consts/required-consts/collect-in-dead-vtable.rs | 3 +-- 12 files changed, 29 insertions(+), 34 deletions(-) diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr index 73790f7517db1..796df067e0b6c 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.noopt.stderr @@ -1,13 +1,13 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-drop.rs:9:19 + --> $DIR/collect-in-dead-drop.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:9:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-drop.rs:16:17 + --> $DIR/collect-in-dead-drop.rs:15:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr index 73790f7517db1..796df067e0b6c 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.opt.stderr @@ -1,13 +1,13 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-drop.rs:9:19 + --> $DIR/collect-in-dead-drop.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:9:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-drop.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-drop.rs:16:17 + --> $DIR/collect-in-dead-drop.rs:15:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-drop.rs b/tests/ui/consts/required-consts/collect-in-dead-drop.rs index 98e9d19189597..ca140f7e95e45 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-drop.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-drop.rs @@ -1,8 +1,7 @@ //@revisions: noopt opt //@ build-fail //@[opt] compile-flags: -O -//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is -//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) +//! This fails without optimizations, so it should also fail with optimizations. struct Fail(T); impl Fail { diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr index 52462076ff90b..f0b7984d2fa35 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.noopt.stderr @@ -1,19 +1,19 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-fn.rs:9:19 + --> $DIR/collect-in-dead-fn.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:9:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-fn.rs:19:17 + --> $DIR/collect-in-dead-fn.rs:18:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ note: the above error was encountered while instantiating `fn not_called::` - --> $DIR/collect-in-dead-fn.rs:26:9 + --> $DIR/collect-in-dead-fn.rs:25:9 | LL | not_called::(); | ^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr index 30c5c2d5c5caa..25c9bb0f8c63a 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.opt.stderr @@ -1,19 +1,19 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-fn.rs:9:19 + --> $DIR/collect-in-dead-fn.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:9:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-fn.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-fn.rs:19:17 + --> $DIR/collect-in-dead-fn.rs:18:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ note: the above error was encountered while instantiating `fn not_called::` - --> $DIR/collect-in-dead-fn.rs:26:9 + --> $DIR/collect-in-dead-fn.rs:25:9 | LL | not_called::(); | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-fn.rs b/tests/ui/consts/required-consts/collect-in-dead-fn.rs index 754fcf7b9874d..f1d8c479a45fb 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-fn.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-fn.rs @@ -1,8 +1,7 @@ //@revisions: noopt opt //@ build-fail //@[opt] compile-flags: -O -//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is -//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) +//! This fails without optimizations, so it should also fail with optimizations. struct Fail(T); impl Fail { diff --git a/tests/ui/consts/required-consts/collect-in-dead-forget.rs b/tests/ui/consts/required-consts/collect-in-dead-forget.rs index 720b7a499f781..17479ef8b37c7 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-forget.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-forget.rs @@ -1,8 +1,7 @@ //@revisions: noopt opt //@build-pass //@[opt] compile-flags: -O -//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is -//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) +//! This passes without optimizations, so it can (and should) also pass with optimizations. struct Fail(T); impl Fail { diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr index 2ab1f80e2d3b4..915949bcddf18 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-move.noopt.stderr @@ -1,13 +1,13 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-move.rs:9:19 + --> $DIR/collect-in-dead-move.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:9:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-move.rs:16:17 + --> $DIR/collect-in-dead-move.rs:15:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr index 2ab1f80e2d3b4..915949bcddf18 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-move.opt.stderr @@ -1,13 +1,13 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-move.rs:9:19 + --> $DIR/collect-in-dead-move.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:9:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-move.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-move.rs:16:17 + --> $DIR/collect-in-dead-move.rs:15:17 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-move.rs b/tests/ui/consts/required-consts/collect-in-dead-move.rs index 2fd6aea58de65..d78c13b988cd4 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-move.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-move.rs @@ -1,8 +1,7 @@ //@revisions: noopt opt //@ build-fail //@[opt] compile-flags: -O -//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is -//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) +//! This fails without optimizations, so it should also fail with optimizations. struct Fail(T); impl Fail { diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr index 56b6989b441ed..2a835a4237eb8 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr @@ -1,19 +1,19 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-vtable.rs:12:19 + --> $DIR/collect-in-dead-vtable.rs:11:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:12:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:11:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-vtable.rs:26:21 + --> $DIR/collect-in-dead-vtable.rs:25:21 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ note: the above error was encountered while instantiating `fn as MyTrait>::not_called` - --> $DIR/collect-in-dead-vtable.rs:35:40 + --> $DIR/collect-in-dead-vtable.rs:34:40 | LL | let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here | ^^ diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.rs b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs index f21a1cc1fc2fd..3098661fbd91c 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-vtable.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs @@ -4,8 +4,7 @@ //FIXME: `opt` revision currently does not stop with an error due to //. //@[opt] build-pass -//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is -//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) +//! This fails without optimizations, so it should also fail with optimizations. struct Fail(T); impl Fail { From 39bc32bca4b474c971206d6d56ff34cb14f3376a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 09:37:52 +0100 Subject: [PATCH 3/6] avoid processing mentioned items that are also still used --- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_monomorphize/src/collector.rs | 27 +++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 7495c16a89e25..f19e342525a42 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -314,7 +314,7 @@ impl<'tcx> CoroutineInfo<'tcx> { } /// Some item that needs to monomorphize successfully for a MIR body to be considered well-formed. -#[derive(Copy, Clone, PartialEq, Debug, HashStable, TyEncodable, TyDecodable)] +#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, HashStable, TyEncodable, TyDecodable)] #[derive(TypeFoldable, TypeVisitable)] pub enum MentionedItem<'tcx> { Fn(DefId, GenericArgsRef<'tcx>), diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 59cbc770c7918..ded4751a89c3e 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -719,6 +719,8 @@ struct MirUsedCollector<'a, 'tcx> { tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, used_items: &'a mut MonoItems<'tcx>, + /// See the comment in `collect_items_of_instance` for the purpose of this set. + used_mentioned_items: &'a mut FxHashSet>, instance: Instance<'tcx>, /// Spans for move size lints already emitted. Helps avoid duplicate lints. move_size_spans: Vec, @@ -989,6 +991,10 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { match terminator.kind { mir::TerminatorKind::Call { ref func, ref args, ref fn_span, .. } => { let callee_ty = func.ty(self.body, tcx); + // *Before* monomorphizing, record that we already handled this mention. + if let ty::FnDef(def_id, args) = callee_ty.kind() { + self.used_mentioned_items.insert(MentionedItem::Fn(*def_id, args)); + } let callee_ty = self.monomorphize(callee_ty); self.check_fn_args_move_size(callee_ty, args, *fn_span, location); visit_fn_use(self.tcx, callee_ty, true, source, &mut self.used_items) @@ -1634,10 +1640,22 @@ fn collect_items_of_instance<'tcx>( mode: CollectionMode, ) { let body = tcx.instance_mir(instance.def); + // Naively, in "used" collection mode, all functions get added to *both* `used_items` and + // `mentioned_items`. Mentioned items processing will then notice that they have already been + // visited, but at that point each mentioned item has been monomorphized, added to the + // `mentioned_items` worklist, and checked in the global set of visited items. To removes that + // overhead, we have a special optimization that avoids adding items to `mentioned_items` when + // they are already added in `used_items`. We could just scan `used_items`, but that's a linear + // scan and not very efficient. Furthermore we can only do that *after* monomorphizing the + // mentioned item. So instead we collect all pre-monomorphized `MentionedItem` that were already + // added to `used_items` in a hash set, which can efficiently query in the + // `body.mentioned_items` loop below. + let mut used_mentioned_items = FxHashSet::>::default(); let mut collector = MirUsedCollector { tcx, body, used_items, + used_mentioned_items: &mut used_mentioned_items, instance, move_size_spans: vec![], visiting_call_terminator: false, @@ -1657,10 +1675,13 @@ fn collect_items_of_instance<'tcx>( } } - // Always gather mentioned items. + // Always gather mentioned items. We try to avoid processing items that we have already added to + // `used_items` above. for item in &body.mentioned_items { - let item_mono = collector.monomorphize(item.node); - visit_mentioned_item(tcx, &item_mono, item.span, mentioned_items); + if !collector.used_mentioned_items.contains(&item.node) { + let item_mono = collector.monomorphize(item.node); + visit_mentioned_item(tcx, &item_mono, item.span, mentioned_items); + } } } From 74bf305fc95067a3f50d575d2e87e611af3c6e88 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 13:28:31 +0100 Subject: [PATCH 4/6] mentioned items: also handle vtables --- compiler/rustc_middle/src/mir/mod.rs | 6 +- .../src/mentioned_items.rs | 26 ++++++- compiler/rustc_monomorphize/src/collector.rs | 75 +++++++++++-------- .../collect-in-dead-vtable.noopt.stderr | 10 +-- .../collect-in-dead-vtable.opt.stderr | 23 ++++++ .../required-consts/collect-in-dead-vtable.rs | 12 +-- 6 files changed, 107 insertions(+), 45 deletions(-) create mode 100644 tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index f19e342525a42..7a1e41c50302c 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -319,7 +319,11 @@ impl<'tcx> CoroutineInfo<'tcx> { pub enum MentionedItem<'tcx> { Fn(DefId, GenericArgsRef<'tcx>), Drop(Ty<'tcx>), - // FIXME: add Vtable { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> }, + /// Unsizing casts might require vtables, so we have to record them. + UnsizeCast { + source_ty: Ty<'tcx>, + target_ty: Ty<'tcx>, + }, // FIXME: do we have to add closures? } diff --git a/compiler/rustc_mir_transform/src/mentioned_items.rs b/compiler/rustc_mir_transform/src/mentioned_items.rs index ed363b4f2527a..0e0114c9d2c8c 100644 --- a/compiler/rustc_mir_transform/src/mentioned_items.rs +++ b/compiler/rustc_mir_transform/src/mentioned_items.rs @@ -1,6 +1,6 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{self, ConstOperand, Location, MentionedItem, MirPass}; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty::{self, adjustment::PointerCoercion, TyCtxt}; use rustc_session::Session; use rustc_span::source_map::Spanned; @@ -54,4 +54,28 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> { _ => {} } } + + fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) { + self.super_rvalue(rvalue, location); + match *rvalue { + // We need to detect unsizing casts that required vtables. + mir::Rvalue::Cast( + mir::CastKind::PointerCoercion(PointerCoercion::Unsize), + ref operand, + target_ty, + ) + | mir::Rvalue::Cast(mir::CastKind::DynStar, ref operand, target_ty) => { + let span = self.body.source_info(location).span; + self.mentioned_items.push(Spanned { + node: MentionedItem::UnsizeCast { + source_ty: operand.ty(self.body, self.tcx), + target_ty, + }, + span, + }); + } + // Function pointer casts are already handled by `visit_constant` above. + _ => {} + } + } } diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index ded4751a89c3e..0f33ee3d4e21a 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -734,7 +734,7 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> { where T: TypeFoldable>, { - debug!("monomorphize: self.instance={:?}", self.instance); + trace!("monomorphize: self.instance={:?}", self.instance); self.instance.instantiate_mir_and_normalize_erasing_regions( self.tcx, ty::ParamEnv::reveal_all(), @@ -1338,35 +1338,37 @@ fn create_mono_items_for_vtable_methods<'tcx>( ) { assert!(!trait_ty.has_escaping_bound_vars() && !impl_ty.has_escaping_bound_vars()); - if let ty::Dynamic(trait_ty, ..) = trait_ty.kind() { - if let Some(principal) = trait_ty.principal() { - let poly_trait_ref = principal.with_self_ty(tcx, impl_ty); - assert!(!poly_trait_ref.has_escaping_bound_vars()); - - // Walk all methods of the trait, including those of its supertraits - let entries = tcx.vtable_entries(poly_trait_ref); - let methods = entries - .iter() - .filter_map(|entry| match entry { - VtblEntry::MetadataDropInPlace - | VtblEntry::MetadataSize - | VtblEntry::MetadataAlign - | VtblEntry::Vacant => None, - VtblEntry::TraitVPtr(_) => { - // all super trait items already covered, so skip them. - None - } - VtblEntry::Method(instance) => { - Some(*instance).filter(|instance| should_codegen_locally(tcx, instance)) - } - }) - .map(|item| create_fn_mono_item(tcx, item, source)); - output.extend(methods); - } - - // Also add the destructor. - visit_drop_use(tcx, impl_ty, false, source, output); + let ty::Dynamic(trait_ty, ..) = trait_ty.kind() else { + bug!("create_mono_items_for_vtable_methods: {trait_ty:?} not a trait type"); + }; + if let Some(principal) = trait_ty.principal() { + let poly_trait_ref = principal.with_self_ty(tcx, impl_ty); + assert!(!poly_trait_ref.has_escaping_bound_vars()); + + // Walk all methods of the trait, including those of its supertraits + let entries = tcx.vtable_entries(poly_trait_ref); + debug!(?entries); + let methods = entries + .iter() + .filter_map(|entry| match entry { + VtblEntry::MetadataDropInPlace + | VtblEntry::MetadataSize + | VtblEntry::MetadataAlign + | VtblEntry::Vacant => None, + VtblEntry::TraitVPtr(_) => { + // all super trait items already covered, so skip them. + None + } + VtblEntry::Method(instance) => { + Some(*instance).filter(|instance| should_codegen_locally(tcx, instance)) + } + }) + .map(|item| create_fn_mono_item(tcx, item, source)); + output.extend(methods); } + + // Also add the destructor. + visit_drop_use(tcx, impl_ty, false, source, output); } //=----------------------------------------------------------------------------- @@ -1685,7 +1687,8 @@ fn collect_items_of_instance<'tcx>( } } -/// `item` must be already monomorphized +/// `item` must be already monomorphized. +#[instrument(skip(tcx, span, output), level = "debug")] fn visit_mentioned_item<'tcx>( tcx: TyCtxt<'tcx>, item: &MentionedItem<'tcx>, @@ -1704,6 +1707,18 @@ fn visit_mentioned_item<'tcx>( MentionedItem::Drop(ty) => { visit_drop_use(tcx, ty, /*is_direct_call*/ true, span, output); } + MentionedItem::UnsizeCast { source_ty, target_ty } => { + let (source_ty, target_ty) = + find_vtable_types_for_unsizing(tcx.at(span), source_ty, target_ty); + // This could also be a different Unsize instruction, like + // from a fixed sized array to a slice. But we are only + // interested in things that produce a vtable. + if (target_ty.is_trait() && !source_ty.is_trait()) + || (target_ty.is_dyn_star() && !source_ty.is_dyn_star()) + { + create_mono_items_for_vtable_methods(tcx, target_ty, source_ty, span, output); + } + } } } diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr index 2a835a4237eb8..53d8e0cb996b8 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr @@ -1,21 +1,21 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-vtable.rs:11:19 + --> $DIR/collect-in-dead-vtable.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:11:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-vtable.rs:25:21 + --> $DIR/collect-in-dead-vtable.rs:21:21 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ note: the above error was encountered while instantiating `fn as MyTrait>::not_called` - --> $DIR/collect-in-dead-vtable.rs:34:40 + --> $DIR/collect-in-dead-vtable.rs:30:40 | -LL | let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here +LL | let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here | ^^ error: aborting due to 1 previous error diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr new file mode 100644 index 0000000000000..53d8e0cb996b8 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr @@ -0,0 +1,23 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-vtable.rs:8:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:8:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-vtable.rs:21:21 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn as MyTrait>::not_called` + --> $DIR/collect-in-dead-vtable.rs:30:40 + | +LL | let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here + | ^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.rs b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs index 3098661fbd91c..39d51801c04f7 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-vtable.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs @@ -1,14 +1,11 @@ //@revisions: noopt opt -//@[noopt] build-fail +//@ build-fail //@[opt] compile-flags: -O -//FIXME: `opt` revision currently does not stop with an error due to -//. -//@[opt] build-pass //! This fails without optimizations, so it should also fail with optimizations. struct Fail(T); impl Fail { - const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed } trait MyTrait { @@ -17,8 +14,7 @@ trait MyTrait { // This function is not actually called, but it is mentioned in a vtable in a function that is // called. Make sure we still find this error. -// This relies on mono-item collection checking `required_consts` in functions that are referenced -// in vtables that syntactically appear in collected functions (even inside dead code). +// This ensures that we are properly considering vtables when gathering "mentioned" items. impl MyTrait for Vec { fn not_called(&self) { if false { @@ -31,7 +27,7 @@ impl MyTrait for Vec { fn called() { if false { let v: Vec = Vec::new(); - let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here + let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here } } From 11eaebb780a775868dded494692cb0847ce2f829 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 13:42:54 +0100 Subject: [PATCH 5/6] mentioned items: also handle closure-to-fn-ptr coercions --- compiler/rustc_middle/src/mir/mod.rs | 3 +- .../src/mentioned_items.rs | 16 ++++++++++ compiler/rustc_monomorphize/src/collector.rs | 6 ++++ .../collect-in-dead-closure.noopt.stderr | 23 +++++++++++++++ .../collect-in-dead-closure.opt.stderr | 23 +++++++++++++++ .../collect-in-dead-closure.rs | 29 +++++++++++++++++++ 6 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/ui/consts/required-consts/collect-in-dead-closure.noopt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-closure.opt.stderr create mode 100644 tests/ui/consts/required-consts/collect-in-dead-closure.rs diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 7a1e41c50302c..95110d38c2f64 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -324,7 +324,8 @@ pub enum MentionedItem<'tcx> { source_ty: Ty<'tcx>, target_ty: Ty<'tcx>, }, - // FIXME: do we have to add closures? + /// A closure that is coerced to a function pointer. + Closure(DefId, GenericArgsRef<'tcx>), } /// The lowered representation of a single function. diff --git a/compiler/rustc_mir_transform/src/mentioned_items.rs b/compiler/rustc_mir_transform/src/mentioned_items.rs index 0e0114c9d2c8c..63f898630abc9 100644 --- a/compiler/rustc_mir_transform/src/mentioned_items.rs +++ b/compiler/rustc_mir_transform/src/mentioned_items.rs @@ -74,6 +74,22 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> { span, }); } + // Similarly, record closures that are turned into function pointers. + mir::Rvalue::Cast( + mir::CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(_)), + ref operand, + _, + ) => { + let span = self.body.source_info(location).span; + let source_ty = operand.ty(self.body, self.tcx); + match *source_ty.kind() { + ty::Closure(def_id, args) => { + self.mentioned_items + .push(Spanned { node: MentionedItem::Closure(def_id, args), span }); + } + _ => bug!(), + } + } // Function pointer casts are already handled by `visit_constant` above. _ => {} } diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 0f33ee3d4e21a..4f0902bc6d192 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -1719,6 +1719,12 @@ fn visit_mentioned_item<'tcx>( create_mono_items_for_vtable_methods(tcx, target_ty, source_ty, span, output); } } + MentionedItem::Closure(def_id, args) => { + let instance = Instance::resolve_closure(tcx, def_id, args, ty::ClosureKind::FnOnce); + if should_codegen_locally(tcx, &instance) { + output.push(create_fn_mono_item(tcx, instance, span)); + } + } } } diff --git a/tests/ui/consts/required-consts/collect-in-dead-closure.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-closure.noopt.stderr new file mode 100644 index 0000000000000..40cecaab2412d --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-closure.noopt.stderr @@ -0,0 +1,23 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-closure.rs:8:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-closure.rs:8:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-closure.rs:16:17 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn not_called::` + --> $DIR/collect-in-dead-closure.rs:23:33 + | +LL | let _closure: fn() = || not_called::(); + | ^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-closure.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-closure.opt.stderr new file mode 100644 index 0000000000000..d6298132e1ba4 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-closure.opt.stderr @@ -0,0 +1,23 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-closure.rs:8:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-closure.rs:8:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-closure.rs:16:17 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn not_called::` + --> $DIR/collect-in-dead-closure.rs:23:33 + | +LL | let _closure: fn() = || not_called::(); + | ^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-closure.rs b/tests/ui/consts/required-consts/collect-in-dead-closure.rs new file mode 100644 index 0000000000000..91f52827af4ce --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-closure.rs @@ -0,0 +1,29 @@ +//@revisions: noopt opt +//@ build-fail +//@[opt] compile-flags: -O +//! This fails without optimizations, so it should also fail with optimizations. + +struct Fail(T); +impl Fail { + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed +} + +// This function is not actually called, but it is mentioned in a closure that is coerced to a +// function pointer in dead code in a function that is called. Make sure we still find this error. +#[inline(never)] +fn not_called() { + if false { + let _ = Fail::::C; + } +} + +#[inline(never)] +fn called() { + if false { + let _closure: fn() = || not_called::(); + } +} + +pub fn main() { + called::(); +} From a7000b601753dbf3676962a6b16d0130afb63e26 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 14:07:40 +0100 Subject: [PATCH 6/6] optimization: do not add non-local reachable-non-generic items to mentioned_items --- .../src/mentioned_items.rs | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_transform/src/mentioned_items.rs b/compiler/rustc_mir_transform/src/mentioned_items.rs index 63f898630abc9..8b6902952ff96 100644 --- a/compiler/rustc_mir_transform/src/mentioned_items.rs +++ b/compiler/rustc_mir_transform/src/mentioned_items.rs @@ -1,3 +1,4 @@ +use rustc_hir::def_id::DefId; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{self, ConstOperand, Location, MentionedItem, MirPass}; use rustc_middle::ty::{self, adjustment::PointerCoercion, TyCtxt}; @@ -34,10 +35,12 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> { let const_ = constant.const_; // This is how function items get referenced: via constants of `FnDef` type. This handles // both functions that are called and those that are just turned to function pointers. - if let ty::FnDef(def_id, args) = const_.ty().kind() { + if let ty::FnDef(def_id, args) = *const_.ty().kind() + && may_codegen_locally(self.tcx, def_id) + { debug!("adding to required_items: {def_id:?}"); self.mentioned_items - .push(Spanned { node: MentionedItem::Fn(*def_id, args), span: constant.span }); + .push(Spanned { node: MentionedItem::Fn(def_id, args), span: constant.span }); } } @@ -95,3 +98,29 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> { } } } + +/// Returns `true` if we should codegen an item in the local crate, or returns `false` if we +/// can just link to the upstream crate and therefore don't need a mono item. +/// +/// This is an approximation of the collector's `should_codegen_locally`, in the sense that this +/// here may return `true` even if `should_codegen_locally` says `false`. The point is to let us +/// filter out items that definitely will not be considered by the collector. +fn may_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { + if tcx.is_foreign_item(def_id) { + // Foreign items are always linked against, there's no way of instantiating them. + return false; + } + + if def_id.is_local() { + // Local items cannot be referred to locally without monomorphizing them locally. + return true; + } + + if tcx.is_reachable_non_generic(def_id) { + // We can link to the item in question, no instance needed in this crate. + return false; + } + + // Conservative fall-back. + true +}