Auto merge of #142293 - fmease:gci-dont-eval-the-impossible, r=<try>

GCI: Don't evaluate the initializer of free const items that have trivially unsatisfied predicates

Author: rust-bors[bot]

compiler/rustc_hir_analysis/src/lib.rs

@@ -239,12 +239,18 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
                 check::maybe_check_static_with_link_section(tcx, item_def_id);
             }
             DefKind::Const if !tcx.generics_of(item_def_id).own_requires_monomorphization() => {
-                // FIXME(generic_const_items): Passing empty instead of identity args is fishy but
-                //                             seems to be fine for now. Revisit this!
-                let instance = ty::Instance::new_raw(item_def_id.into(), ty::GenericArgs::empty());
-                let cid = GlobalId { instance, promoted: None };
-                let typing_env = ty::TypingEnv::fully_monomorphized();
-                tcx.ensure_ok().eval_to_const_value_raw(typing_env.as_query_input(cid));
+                let predicates = tcx.predicates_of(item_def_id);
+                let predicates = predicates.instantiate_identity(tcx).predicates;
+
+                if !traits::impossible_predicates(tcx, predicates) {
+                    // FIXME(generic_const_items): Passing empty instead of identity args is fishy but
+                    //                             seems to be fine for now. Revisit this!
+                    let instance =
+                        ty::Instance::new_raw(item_def_id.into(), ty::GenericArgs::empty());
+                    let cid = GlobalId { instance, promoted: None };
+                    let typing_env = ty::TypingEnv::fully_monomorphized();
+                    tcx.ensure_ok().eval_to_const_value_raw(typing_env.as_query_input(cid));
+                }
             }
             _ => (),
         }

compiler/rustc_middle/src/mir/interpret/queries.rs

@@ -13,7 +13,7 @@ use crate::ty::{self, GenericArgs, TyCtxt, TypeVisitableExt};
 
 impl<'tcx> TyCtxt<'tcx> {
     /// Evaluates a constant without providing any generic parameters. This is useful to evaluate consts
-    /// that can't take any generic arguments like const items or enum discriminants. If a
+    /// that can't take any generic arguments like enum discriminants. If a
     /// generic parameter is used within the constant `ErrorHandled::TooGeneric` will be returned.
     #[instrument(skip(self), level = "debug")]
     pub fn const_eval_poly(self, def_id: DefId) -> EvalToConstValueResult<'tcx> {
@@ -29,7 +29,7 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 
     /// Evaluates a constant without providing any generic parameters. This is useful to evaluate consts
-    /// that can't take any generic arguments like const items or enum discriminants. If a
+    /// that can't take any generic arguments like enum discriminants. If a
     /// generic parameter is used within the constant `ErrorHandled::TooGeneric` will be returned.
     #[instrument(skip(self), level = "debug")]
     pub fn const_eval_poly_to_alloc(self, def_id: DefId) -> EvalToAllocationRawResult<'tcx> {

compiler/rustc_monomorphize/src/collector.rs

@@ -1434,72 +1434,62 @@ struct RootCollector<'a, 'tcx> {
     entry_fn: Option<(DefId, EntryFnType)>,
 }
 
-impl<'v> RootCollector<'_, 'v> {
+impl<'tcx> RootCollector<'_, 'tcx> {
     fn process_item(&mut self, id: hir::ItemId) {
-        match self.tcx.def_kind(id.owner_id) {
+        let tcx = self.tcx;
+        let def_id = id.owner_id.to_def_id();
+
+        match tcx.def_kind(def_id) {
             DefKind::Enum | DefKind::Struct | DefKind::Union => {
-                if self.strategy == MonoItemCollectionStrategy::Eager
-                    && !self.tcx.generics_of(id.owner_id).requires_monomorphization(self.tcx)
-                {
-                    debug!("RootCollector: ADT drop-glue for `{id:?}`",);
-                    let id_args =
-                        ty::GenericArgs::for_item(self.tcx, id.owner_id.to_def_id(), |param, _| {
-                            match param.kind {
-                                GenericParamDefKind::Lifetime => {
-                                    self.tcx.lifetimes.re_erased.into()
-                                }
-                                GenericParamDefKind::Type { .. }
-                                | GenericParamDefKind::Const { .. } => {
-                                    unreachable!(
-                                        "`own_requires_monomorphization` check means that \
-                                we should have no type/const params"
-                                    )
-                                }
-                            }
-                        });
-
-                    // This type is impossible to instantiate, so we should not try to
-                    // generate a `drop_in_place` instance for it.
-                    if self.tcx.instantiate_and_check_impossible_predicates((
-                        id.owner_id.to_def_id(),
-                        id_args,
-                    )) {
-                        return;
-                    }
+                if self.strategy != MonoItemCollectionStrategy::Eager {
+                    return;
+                }
 
-                    let ty =
-                        self.tcx.type_of(id.owner_id.to_def_id()).instantiate(self.tcx, id_args);
-                    assert!(!ty.has_non_region_param());
-                    visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.output);
+                if tcx.generics_of(def_id).requires_monomorphization(tcx) {
+                    return;
+                }
+                let args = ty::GenericArgs::for_item(tcx, def_id, |param, _| {
+                    expect_and_erase_regions(tcx, param)
+                });
+                if tcx.instantiate_and_check_impossible_predicates((def_id, args)) {
+                    return;
                 }
+
+                let ty = tcx.type_of(def_id).instantiate(tcx, args);
+                debug_assert!(!ty.has_non_region_param());
+
+                debug!("RootCollector: ADT drop-glue for `{id:?}`");
+                visit_drop_use(tcx, ty, true, DUMMY_SP, self.output);
             }
             DefKind::GlobalAsm => {
-                debug!(
-                    "RootCollector: ItemKind::GlobalAsm({})",
-                    self.tcx.def_path_str(id.owner_id)
-                );
+                debug!("RootCollector: ItemKind::GlobalAsm({})", tcx.def_path_str(def_id));
                 self.output.push(dummy_spanned(MonoItem::GlobalAsm(id)));
             }
             DefKind::Static { .. } => {
-                let def_id = id.owner_id.to_def_id();
-                debug!("RootCollector: ItemKind::Static({})", self.tcx.def_path_str(def_id));
+                debug!("RootCollector: ItemKind::Static({})", tcx.def_path_str(def_id));
                 self.output.push(dummy_spanned(MonoItem::Static(def_id)));
             }
             DefKind::Const => {
-                // Const items only generate mono items if they are actually used somewhere.
-                // Just declaring them is insufficient.
+                if tcx.generics_of(def_id).requires_monomorphization(tcx) {
+                    return;
+                }
 
-                // But even just declaring them must collect the items they refer to
-                // unless their generics require monomorphization.
-                if !self.tcx.generics_of(id.owner_id).own_requires_monomorphization()
-                    && let Ok(val) = self.tcx.const_eval_poly(id.owner_id.to_def_id())
-                {
-                    collect_const_value(self.tcx, val, self.output);
+                let args = ty::GenericArgs::for_item(tcx, def_id, |param, _| {
+                    expect_and_erase_regions(tcx, param)
+                });
+                if tcx.instantiate_and_check_impossible_predicates((def_id, args)) {
+                    return;
                 }
+
+                let Ok(val) = tcx.const_eval_poly(def_id) else { return };
+
+                // Const items only generate mono items if they are actually used somewhere.
+                // Just declaring them is insufficient.
+                collect_const_value(tcx, val, self.output);
             }
             DefKind::Impl { .. } => {
                 if self.strategy == MonoItemCollectionStrategy::Eager {
-                    create_mono_items_for_default_impls(self.tcx, id, self.output);
+                    create_mono_items_for_default_impls(tcx, id, self.output);
                 }
             }
             DefKind::Fn => {
@@ -1622,35 +1612,23 @@ fn create_mono_items_for_default_impls<'tcx>(
     item: hir::ItemId,
     output: &mut MonoItems<'tcx>,
 ) {
-    let Some(impl_) = tcx.impl_trait_header(item.owner_id) else {
-        return;
-    };
-
+    let impl_def_id = item.owner_id.to_def_id();
+    let Some(impl_) = tcx.impl_trait_header(impl_def_id) else { return };
     if matches!(impl_.polarity, ty::ImplPolarity::Negative) {
         return;
     }
 
-    if tcx.generics_of(item.owner_id).own_requires_monomorphization() {
-        return;
-    }
-
     // Lifetimes never affect trait selection, so we are allowed to eagerly
     // instantiate an instance of an impl method if the impl (and method,
     // which we check below) is only parameterized over lifetime. In that case,
     // we use the ReErased, which has no lifetime information associated with
     // it, to validate whether or not the impl is legal to instantiate at all.
-    let only_region_params = |param: &ty::GenericParamDef, _: &_| match param.kind {
-        GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(),
-        GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
-            unreachable!(
-                "`own_requires_monomorphization` check means that \
-                we should have no type/const params"
-            )
-        }
-    };
-    let impl_args = GenericArgs::for_item(tcx, item.owner_id.to_def_id(), only_region_params);
-    let trait_ref = impl_.trait_ref.instantiate(tcx, impl_args);
-
+    if tcx.generics_of(impl_def_id).requires_monomorphization(tcx) {
+        return;
+    }
+    let impl_args = ty::GenericArgs::for_item(tcx, impl_def_id, |param, _| {
+        expect_and_erase_regions(tcx, param)
+    });
     // Unlike 'lazy' monomorphization that begins by collecting items transitively
     // called by `main` or other global items, when eagerly monomorphizing impl
     // items, we never actually check that the predicates of this impl are satisfied
@@ -1660,13 +1638,15 @@ fn create_mono_items_for_default_impls<'tcx>(
     // consider higher-ranked predicates such as `for<'a> &'a mut [u8]: Copy` to
     // be trivially false. We must now check that the impl has no impossible-to-satisfy
     // predicates.
-    if tcx.instantiate_and_check_impossible_predicates((item.owner_id.to_def_id(), impl_args)) {
+    if tcx.instantiate_and_check_impossible_predicates((impl_def_id, impl_args)) {
         return;
     }
 
+    let trait_ref = impl_.trait_ref.instantiate(tcx, impl_args);
+
     let typing_env = ty::TypingEnv::fully_monomorphized();
     let trait_ref = tcx.normalize_erasing_regions(typing_env, trait_ref);
-    let overridden_methods = tcx.impl_item_implementor_ids(item.owner_id);
+    let overridden_methods = tcx.impl_item_implementor_ids(impl_def_id);
     for method in tcx.provided_trait_methods(trait_ref.def_id) {
         if overridden_methods.contains_key(&method.def_id) {
             continue;
@@ -1679,7 +1659,9 @@ fn create_mono_items_for_default_impls<'tcx>(
         // As mentioned above, the method is legal to eagerly instantiate if it
         // only has lifetime generic parameters. This is validated by calling
         // `own_requires_monomorphization` on both the impl and method.
-        let args = trait_ref.args.extend_to(tcx, method.def_id, only_region_params);
+        let args = trait_ref
+            .args
+            .extend_to(tcx, method.def_id, |param, _| expect_and_erase_regions(tcx, param));
         let instance = ty::Instance::expect_resolve(tcx, typing_env, method.def_id, args, DUMMY_SP);
 
         let mono_item = create_fn_mono_item(tcx, instance, DUMMY_SP);
@@ -1689,6 +1671,18 @@ fn create_mono_items_for_default_impls<'tcx>(
     }
 }
 
+fn expect_and_erase_regions<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    param: &ty::GenericParamDef,
+) -> ty::GenericArg<'tcx> {
+    match param.kind {
+        GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(),
+        GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
+            bug!("unexpected non-region param")
+        }
+    }
+}
+
 //=-----------------------------------------------------------------------------
 // Top-level entry point, tying it all together
 //=-----------------------------------------------------------------------------

compiler/rustc_passes/src/reachable.rs

@@ -37,6 +37,7 @@ use rustc_middle::query::Providers;
 use rustc_middle::ty::{self, ExistentialTraitRef, TyCtxt};
 use rustc_privacy::DefIdVisitor;
 use rustc_session::config::CrateType;
+use rustc_trait_selection::traits;
 use tracing::debug;
 
 /// Determines whether this item is recursive for reachability. See `is_recursively_reachable_local`
@@ -205,19 +206,34 @@ impl<'tcx> ReachableContext<'tcx> {
                     }
 
                     hir::ItemKind::Const(_, _, _, init) => {
-                        // Only things actually ending up in the final constant value are reachable
-                        // for codegen. Everything else is only needed during const-eval, so even if
-                        // const-eval happens in a downstream crate, all they need is
-                        // `mir_for_ctfe`.
+                        if self.tcx.generics_of(item.owner_id).own_requires_monomorphization() {
+                            // In this case, we don't want to evaluate the const initializer.
+                            // In lieu of that, we have to consider everything mentioned in it
+                            // as reachable, since it *may* end up in the final value.
+                            self.visit_nested_body(init);
+                            return;
+                        }
+
+                        let predicates = self.tcx.predicates_of(item.owner_id);
+                        let predicates = predicates.instantiate_identity(self.tcx).predicates;
+                        if traits::impossible_predicates(self.tcx, predicates) {
+                            // The constant is impossible to reference.
+                            // Therefore nothing can be reachable via it.
+                            return;
+                        }
+
                         match self.tcx.const_eval_poly_to_alloc(item.owner_id.def_id.into()) {
                             Ok(alloc) => {
+                                // Only things actually ending up in the final constant value are
+                                // reachable for codegen. Everything else is only needed during
+                                // const-eval, so even if const-eval happens in a downstream crate,
+                                // all they need is `mir_for_ctfe`.
                                 let alloc = self.tcx.global_alloc(alloc.alloc_id).unwrap_memory();
                                 self.propagate_from_alloc(alloc);
                             }
-                            // We can't figure out which value the constant will evaluate to. In
-                            // lieu of that, we have to consider everything mentioned in the const
-                            // initializer reachable, since it *may* end up in the final value.
-                            Err(ErrorHandled::TooGeneric(_)) => self.visit_nested_body(init),
+                            // We've checked at the start that there aren't any non-lifetime params
+                            // in scope. The const initializer can't possibly be too generic.
+                            Err(ErrorHandled::TooGeneric(_)) => bug!(),
                             // If there was an error evaluating the const, nothing can be reachable
                             // via it, and anyway compilation will fail.
                             Err(ErrorHandled::Reported(..)) => {}

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -719,8 +719,13 @@ fn replace_param_and_infer_args_with_placeholder<'tcx>(
 /// returns true, then either normalize encountered an error or one of the predicates did not
 /// hold. Used when creating vtables to check for unsatisfiable methods. This should not be
 /// used during analysis.
+#[instrument(level = "debug", skip(tcx))]
 pub fn impossible_predicates<'tcx>(tcx: TyCtxt<'tcx>, predicates: Vec<ty::Clause<'tcx>>) -> bool {
-    debug!("impossible_predicates(predicates={:?})", predicates);
+    debug_assert!(!predicates.has_non_region_param());
+    if predicates.is_empty() {
+        return false;
+    }
+
     let (infcx, param_env) = tcx
         .infer_ctxt()
         .with_next_trait_solver(true)
@@ -747,26 +752,23 @@ pub fn impossible_predicates<'tcx>(tcx: TyCtxt<'tcx>, predicates: Vec<ty::Clause
     false
 }
 
+#[instrument(level = "debug", skip(tcx), ret)]
 fn instantiate_and_check_impossible_predicates<'tcx>(
     tcx: TyCtxt<'tcx>,
-    key: (DefId, GenericArgsRef<'tcx>),
+    (def_id, args): (DefId, GenericArgsRef<'tcx>),
 ) -> bool {
-    debug!("instantiate_and_check_impossible_predicates(key={:?})", key);
-
-    let mut predicates = tcx.predicates_of(key.0).instantiate(tcx, key.1).predicates;
+    let mut predicates = tcx.predicates_of(def_id).instantiate(tcx, args).predicates;
 
     // Specifically check trait fulfillment to avoid an error when trying to resolve
     // associated items.
-    if let Some(trait_def_id) = tcx.trait_of_item(key.0) {
-        let trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, key.1);
+    if let Some(trait_def_id) = tcx.trait_of_item(def_id) {
+        let trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, args);
         predicates.push(trait_ref.upcast(tcx));
     }
 
     predicates.retain(|predicate| !predicate.has_param());
-    let result = impossible_predicates(tcx, predicates);
 
-    debug!("instantiate_and_check_impossible_predicates(key={:?}) = {:?}", key, result);
-    result
+    impossible_predicates(tcx, predicates)
 }
 
 /// Checks whether a trait's associated item is impossible to reference on a given impl.