Be better at enforcing that const_conditions is only called on const items

compiler-errors · compiler-errors · commit 0f5a47d0884d · 2024-10-24T09:46:36.000Z
diff --git a/compiler/rustc_hir_analysis/src/bounds.rs b/compiler/rustc_hir_analysis/src/bounds.rs
@@ -89,7 +89,11 @@ impl<'tcx> Bounds<'tcx> {
         host: ty::HostPolarity,
         span: Span,
     ) {
-        self.clauses.push((bound_trait_ref.to_host_effect_clause(tcx, host), span));
+        if tcx.is_const_trait(bound_trait_ref.def_id()) {
+            self.clauses.push((bound_trait_ref.to_host_effect_clause(tcx, host), span));
+        } else {
+            tcx.dcx().span_delayed_bug(span, "tried to lower {host:?} bound for non-const trait");
+        }
     }
 
     pub(crate) fn clauses(
diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
@@ -206,8 +206,8 @@ fn compare_method_predicate_entailment<'tcx>(
     );
 
     // FIXME(effects): This should be replaced with a more dedicated method.
-    let check_const_if_const = tcx.constness(impl_def_id) == hir::Constness::Const;
-    if check_const_if_const {
+    let is_conditionally_const = tcx.is_conditionally_const(impl_def_id);
+    if is_conditionally_const {
         // Augment the hybrid param-env with the const conditions
         // of the impl header and the trait method.
         hybrid_preds.extend(
@@ -255,7 +255,7 @@ fn compare_method_predicate_entailment<'tcx>(
     // This registers the `~const` bounds of the impl method, which we will prove
     // using the hybrid param-env that we earlier augmented with the const conditions
     // from the impl header and trait method declaration.
-    if check_const_if_const {
+    if is_conditionally_const {
         for (const_condition, span) in
             tcx.const_conditions(impl_m.def_id).instantiate_own_identity()
         {
@@ -1904,9 +1904,8 @@ fn compare_type_predicate_entailment<'tcx>(
     let trait_ty_predicates = tcx.predicates_of(trait_ty.def_id);
 
     let impl_ty_own_bounds = impl_ty_predicates.instantiate_own_identity();
-    let impl_ty_own_const_conditions =
-        tcx.const_conditions(impl_ty.def_id).instantiate_own_identity();
-    if impl_ty_own_bounds.len() == 0 && impl_ty_own_const_conditions.len() == 0 {
+    // If there are no bounds, then there are no const conditions, so no need to check that here.
+    if impl_ty_own_bounds.len() == 0 {
         // Nothing to check.
         return Ok(());
     }
@@ -1931,8 +1930,8 @@ fn compare_type_predicate_entailment<'tcx>(
     let impl_ty_span = tcx.def_span(impl_ty_def_id);
     let normalize_cause = ObligationCause::misc(impl_ty_span, impl_ty_def_id);
 
-    let check_const_if_const = tcx.constness(impl_def_id) == hir::Constness::Const;
-    if check_const_if_const {
+    let is_conditionally_const = tcx.is_conditionally_const(impl_ty.def_id);
+    if is_conditionally_const {
         // Augment the hybrid param-env with the const conditions
         // of the impl header and the trait assoc type.
         hybrid_preds.extend(
@@ -1968,8 +1967,10 @@ fn compare_type_predicate_entailment<'tcx>(
         ocx.register_obligation(traits::Obligation::new(tcx, cause, param_env, predicate));
     }
 
-    if check_const_if_const {
+    if is_conditionally_const {
         // Validate the const conditions of the impl associated type.
+        let impl_ty_own_const_conditions =
+            tcx.const_conditions(impl_ty.def_id).instantiate_own_identity();
         for (const_condition, span) in impl_ty_own_const_conditions {
             let normalize_cause = traits::ObligationCause::misc(span, impl_ty_def_id);
             let const_condition = ocx.normalize(&normalize_cause, param_env, const_condition);
@@ -2081,7 +2082,7 @@ pub(super) fn check_type_bounds<'tcx>(
         .collect();
 
     // Only in a const implementation do we need to check that the `~const` item bounds hold.
-    if tcx.constness(container_id) == hir::Constness::Const {
+    if tcx.is_conditionally_const(impl_ty_def_id) {
         obligations.extend(
             tcx.implied_const_bounds(trait_ty.def_id)
                 .iter_instantiated_copied(tcx, rebased_args)
diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs
@@ -1372,7 +1372,7 @@ fn check_impl<'tcx>(
                 }
 
                 // Ensure that the `~const` where clauses of the trait hold for the impl.
-                if tcx.constness(item.owner_id.def_id) == hir::Constness::Const {
+                if tcx.is_conditionally_const(item.owner_id.def_id) {
                     for (bound, _) in
                         tcx.const_conditions(trait_ref.def_id).instantiate(tcx, trait_ref.args)
                     {
diff --git a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs
@@ -888,48 +888,8 @@ pub(super) fn const_conditions<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
 ) -> ty::ConstConditions<'tcx> {
-    // This logic is spaghetti, and should be cleaned up. The current methods that are
-    // defined to deal with constness are very unintuitive.
-    if tcx.is_const_fn_raw(def_id.to_def_id()) {
-        // Ok, const fn or method in const trait.
-    } else {
-        match tcx.def_kind(def_id) {
-            DefKind::Trait => {
-                if !tcx.is_const_trait(def_id.to_def_id()) {
-                    return Default::default();
-                }
-            }
-            DefKind::Impl { .. } => {
-                // FIXME(effects): Should be using a dedicated function to
-                // test if this is a const trait impl.
-                if tcx.constness(def_id) != hir::Constness::Const {
-                    return Default::default();
-                }
-            }
-            DefKind::AssocTy | DefKind::AssocFn => {
-                let parent_def_id = tcx.local_parent(def_id).to_def_id();
-                match tcx.associated_item(def_id).container {
-                    ty::AssocItemContainer::TraitContainer => {
-                        if !tcx.is_const_trait(parent_def_id) {
-                            return Default::default();
-                        }
-                    }
-                    ty::AssocItemContainer::ImplContainer => {
-                        // FIXME(effects): Should be using a dedicated function to
-                        // test if this is a const trait impl.
-                        if tcx.constness(parent_def_id) != hir::Constness::Const {
-                            return Default::default();
-                        }
-                    }
-                }
-            }
-            DefKind::Closure | DefKind::OpaqueTy => {
-                // Closures and RPITs will eventually have const conditions
-                // for `~const` bounds.
-                return Default::default();
-            }
-            _ => return Default::default(),
-        }
+    if !tcx.is_conditionally_const(def_id) {
+        bug!("const_conditions invoked for item that is not conditionally const: {def_id:?}");
     }
 
     let (generics, trait_def_id_and_supertraits, has_parent) = match tcx.hir_node_by_def_id(def_id)
@@ -940,7 +900,7 @@ pub(super) fn const_conditions<'tcx>(
             hir::ItemKind::Trait(_, _, generics, supertraits, _) => {
                 (generics, Some((item.owner_id.def_id, supertraits)), false)
             }
-            _ => return Default::default(),
+            _ => bug!("const_conditions called on wrong item: {def_id:?}"),
         },
         // While associated types are not really const, we do allow them to have `~const`
         // bounds and where clauses. `const_conditions` is responsible for gathering
@@ -950,13 +910,21 @@ pub(super) fn const_conditions<'tcx>(
             hir::TraitItemKind::Fn(_, _) | hir::TraitItemKind::Type(_, _) => {
                 (item.generics, None, true)
             }
-            _ => return Default::default(),
+            _ => bug!("const_conditions called on wrong item: {def_id:?}"),
         },
         Node::ImplItem(item) => match item.kind {
-            hir::ImplItemKind::Fn(_, _) | hir::ImplItemKind::Type(_) => (item.generics, None, true),
-            _ => return Default::default(),
+            hir::ImplItemKind::Fn(_, _) | hir::ImplItemKind::Type(_) => {
+                (item.generics, None, tcx.is_conditionally_const(tcx.local_parent(def_id)))
+            }
+            _ => bug!("const_conditions called on wrong item: {def_id:?}"),
         },
-        _ => return Default::default(),
+        Node::ForeignItem(item) => match item.kind {
+            hir::ForeignItemKind::Fn(_, _, generics) => (generics, None, false),
+            _ => bug!("const_conditions called on wrong item: {def_id:?}"),
+        },
+        // N.B. Tuple ctors are unconditionally constant.
+        Node::Ctor(hir::VariantData::Tuple { .. }) => return Default::default(),
+        _ => bug!("const_conditions called on wrong item: {def_id:?}"),
     };
 
     let icx = ItemCtxt::new(tcx, def_id);
@@ -1017,30 +985,22 @@ pub(super) fn implied_const_bounds<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
 ) -> ty::EarlyBinder<'tcx, &'tcx [(ty::PolyTraitRef<'tcx>, Span)]> {
+    if !tcx.is_conditionally_const(def_id) {
+        bug!("const_conditions invoked for item that is not conditionally const: {def_id:?}");
+    }
+
     let bounds = match tcx.hir_node_by_def_id(def_id) {
         Node::Item(hir::Item { kind: hir::ItemKind::Trait(..), .. }) => {
-            if !tcx.is_const_trait(def_id.to_def_id()) {
-                return ty::EarlyBinder::bind(&[]);
-            }
-
             implied_predicates_with_filter(
                 tcx,
                 def_id.to_def_id(),
                 PredicateFilter::SelfConstIfConst,
             )
         }
         Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Type(..), .. }) => {
-            if !tcx.is_const_trait(tcx.local_parent(def_id).to_def_id()) {
-                return ty::EarlyBinder::bind(&[]);
-            }
-
             explicit_item_bounds_with_filter(tcx, def_id, PredicateFilter::ConstIfConst)
         }
-        Node::OpaqueTy(..) => {
-            // We should eventually collect the `~const` bounds on opaques.
-            return ty::EarlyBinder::bind(&[]);
-        }
-        _ => return ty::EarlyBinder::bind(&[]),
+        _ => bug!("implied_const_bounds called on wrong item: {def_id:?}"),
     };
 
     bounds.map_bound(|bounds| {
diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs
@@ -861,12 +861,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // FIXME(effects): Should this be `is_const_fn_raw`? It depends on if we move
         // const stability checking here too, I guess.
-        if self.tcx.is_const_fn(callee_did)
-            || self
-                .tcx
-                .trait_of_item(callee_did)
-                .is_some_and(|def_id| self.tcx.is_const_trait(def_id))
-        {
+        if self.tcx.is_conditionally_const(callee_did) {
             let q = self.tcx.const_conditions(callee_did);
             // FIXME(effects): Use this span with a better cause code.
             for (cond, _) in q.instantiate(self.tcx, callee_args) {
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -1423,7 +1423,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                 let g = tcx.generics_of(def_id);
                 record!(self.tables.generics_of[def_id] <- g);
                 record!(self.tables.explicit_predicates_of[def_id] <- self.tcx.explicit_predicates_of(def_id));
-                record!(self.tables.const_conditions[def_id] <- self.tcx.const_conditions(def_id));
                 let inferred_outlives = self.tcx.inferred_outlives_of(def_id);
                 record_defaulted_array!(self.tables.inferred_outlives_of[def_id] <- inferred_outlives);
 
@@ -1434,6 +1433,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                     }
                 }
             }
+            if tcx.is_conditionally_const(def_id) {
+                record!(self.tables.const_conditions[def_id] <- self.tcx.const_conditions(def_id));
+            }
             if should_encode_type(tcx, local_id, def_kind) {
                 record!(self.tables.type_of[def_id] <- self.tcx.type_of(def_id));
             }
@@ -1457,20 +1459,20 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                     self.tcx.explicit_super_predicates_of(def_id).skip_binder());
                 record_defaulted_array!(self.tables.explicit_implied_predicates_of[def_id] <-
                     self.tcx.explicit_implied_predicates_of(def_id).skip_binder());
-                record_defaulted_array!(self.tables.implied_const_bounds[def_id]
-                    <- self.tcx.implied_const_bounds(def_id).skip_binder());
                 let module_children = self.tcx.module_children_local(local_id);
                 record_array!(self.tables.module_children_non_reexports[def_id] <-
                     module_children.iter().map(|child| child.res.def_id().index));
+                if self.tcx.is_const_trait(def_id) {
+                    record_defaulted_array!(self.tables.implied_const_bounds[def_id]
+                        <- self.tcx.implied_const_bounds(def_id).skip_binder());
+                }
             }
             if let DefKind::TraitAlias = def_kind {
                 record!(self.tables.trait_def[def_id] <- self.tcx.trait_def(def_id));
                 record_defaulted_array!(self.tables.explicit_super_predicates_of[def_id] <-
                     self.tcx.explicit_super_predicates_of(def_id).skip_binder());
                 record_defaulted_array!(self.tables.explicit_implied_predicates_of[def_id] <-
                     self.tcx.explicit_implied_predicates_of(def_id).skip_binder());
-                record_defaulted_array!(self.tables.implied_const_bounds[def_id]
-                    <- self.tcx.implied_const_bounds(def_id).skip_binder());
             }
             if let DefKind::Trait | DefKind::Impl { .. } = def_kind {
                 let associated_item_def_ids = self.tcx.associated_item_def_ids(def_id);
@@ -1653,8 +1655,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                 if let ty::AssocKind::Type = item.kind {
                     self.encode_explicit_item_bounds(def_id);
                     self.encode_explicit_item_super_predicates(def_id);
-                    record_defaulted_array!(self.tables.implied_const_bounds[def_id]
-                        <- self.tcx.implied_const_bounds(def_id).skip_binder());
+                    if tcx.is_conditionally_const(def_id) {
+                        record_defaulted_array!(self.tables.implied_const_bounds[def_id]
+                            <- self.tcx.implied_const_bounds(def_id).skip_binder());
+                    }
                 }
             }
             AssocItemContainer::ImplContainer => {
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
@@ -384,7 +384,7 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
     }
 
     fn is_const_impl(self, def_id: DefId) -> bool {
-        self.constness(def_id) == hir::Constness::Const
+        self.is_conditionally_const(def_id)
     }
 
     fn const_conditions(
@@ -3140,6 +3140,7 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
+    // FIXME(effects): Please remove this. It's a footgun.
     /// Whether the trait impl is marked const. This does not consider stability or feature gates.
     pub fn is_const_trait_impl_raw(self, def_id: DefId) -> bool {
         let Some(local_def_id) = def_id.as_local() else { return false };
diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
@@ -1999,10 +1999,75 @@ impl<'tcx> TyCtxt<'tcx> {
     pub fn is_const_fn_raw(self, def_id: DefId) -> bool {
         matches!(
             self.def_kind(def_id),
-            DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) | DefKind::Closure
+            DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::Closure
         ) && self.constness(def_id) == hir::Constness::Const
     }
 
+    /// Whether this item is conditionally constant for the purposes of the
+    /// effects implementation.
+    ///
+    /// This roughly corresponds to all const functions and other callable
+    /// items, along with const impls and traits, and associated types within
+    /// those impls and traits.
+    pub fn is_conditionally_const(self, def_id: impl Into<DefId>) -> bool {
+        let def_id: DefId = def_id.into();
+        match self.def_kind(def_id) {
+            DefKind::Impl { of_trait: true } => {
+                self.constness(def_id) == hir::Constness::Const
+                    && self.is_const_trait(
+                        self.trait_id_of_impl(def_id)
+                            .expect("expected trait for trait implementation"),
+                    )
+            }
+            DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) => {
+                self.constness(def_id) == hir::Constness::Const
+            }
+            DefKind::Trait => self.is_const_trait(def_id),
+            DefKind::AssocTy | DefKind::AssocFn => {
+                let parent_def_id = self.parent(def_id);
+                match self.def_kind(parent_def_id) {
+                    DefKind::Impl { of_trait: false } => {
+                        self.constness(def_id) == hir::Constness::Const
+                    }
+                    DefKind::Impl { of_trait: true } | DefKind::Trait => {
+                        self.is_conditionally_const(parent_def_id)
+                    }
+                    _ => bug!("unexpected parent item of associated item: {parent_def_id:?}"),
+                }
+            }
+            DefKind::Closure | DefKind::OpaqueTy => {
+                // Closures and RPITs will eventually have const conditions
+                // for `~const` bounds.
+                false
+            }
+            DefKind::Ctor(_, CtorKind::Const)
+            | DefKind::Impl { of_trait: false }
+            | DefKind::Mod
+            | DefKind::Struct
+            | DefKind::Union
+            | DefKind::Enum
+            | DefKind::Variant
+            | DefKind::TyAlias
+            | DefKind::ForeignTy
+            | DefKind::TraitAlias
+            | DefKind::TyParam
+            | DefKind::Const
+            | DefKind::ConstParam
+            | DefKind::Static { .. }
+            | DefKind::AssocConst
+            | DefKind::Macro(_)
+            | DefKind::ExternCrate
+            | DefKind::Use
+            | DefKind::ForeignMod
+            | DefKind::AnonConst
+            | DefKind::InlineConst
+            | DefKind::Field
+            | DefKind::LifetimeParam
+            | DefKind::GlobalAsm
+            | DefKind::SyntheticCoroutineBody => false,
+        }
+    }
+
     #[inline]
     pub fn is_const_trait(self, def_id: DefId) -> bool {
         self.trait_def(def_id).constness == hir::Constness::Const
diff --git a/compiler/rustc_ty_utils/src/ty.rs b/compiler/rustc_ty_utils/src/ty.rs
@@ -152,12 +152,13 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {
 
     // We extend the param-env of our item with the const conditions of the item,
     // since we're allowed to assume `~const` bounds hold within the item itself.
-    predicates.extend(
-        tcx.const_conditions(def_id)
-            .instantiate_identity(tcx)
-            .into_iter()
-            .map(|(trait_ref, _)| trait_ref.to_host_effect_clause(tcx, ty::HostPolarity::Maybe)),
-    );
+    if tcx.is_conditionally_const(def_id) {
+        predicates.extend(
+            tcx.const_conditions(def_id).instantiate_identity(tcx).into_iter().map(
+                |(trait_ref, _)| trait_ref.to_host_effect_clause(tcx, ty::HostPolarity::Maybe),
+            ),
+        );
+    }
 
     let local_did = def_id.as_local();
 
diff --git a/tests/ui/traits/const-traits/const_derives/derive-const-with-params.stderr b/tests/ui/traits/const-traits/const_derives/derive-const-with-params.stderr
diff --git a/tests/ui/traits/const-traits/effects/spec-effectvar-ice.rs b/tests/ui/traits/const-traits/effects/spec-effectvar-ice.rs
diff --git a/tests/ui/traits/const-traits/effects/spec-effectvar-ice.stderr b/tests/ui/traits/const-traits/effects/spec-effectvar-ice.stderr

Original file line number	Diff line number	Diff line change
`@@ -1372,7 +1372,7 @@ fn check_impl<'tcx>(`
`1372`	`1372`	`}`
`1373`	`1373`
`1374`	`1374`	// Ensure that the `~const` where clauses of the trait hold for the impl.
`1375`		`- if tcx.constness(item.owner_id.def_id) == hir::Constness::Const {`
	`1375`	`+ if tcx.is_conditionally_const(item.owner_id.def_id) {`
`1376`	`1376`	`for (bound, _) in`
`1377`	`1377`	`tcx.const_conditions(trait_ref.def_id).instantiate(tcx, trait_ref.args)`
`1378`	`1378`	`{`
Original file line number	Diff line number	Diff line change
`@@ -384,7 +384,7 @@ impl<'tcx> Interner for TyCtxt<'tcx> {`
`384`	`384`	`}`
`385`	`385`
`386`	`386`	`fn is_const_impl(self, def_id: DefId) -> bool {`
`387`		`- self.constness(def_id) == hir::Constness::Const`
	`387`	`+ self.is_conditionally_const(def_id)`
`388`	`388`	`}`
`389`	`389`
`390`	`390`	`fn const_conditions(`
`@@ -3140,6 +3140,7 @@ impl<'tcx> TyCtxt<'tcx> {`
`3140`	`3140`	`}`
`3141`	`3141`	`}`
`3142`	`3142`
	`3143`	`+ // FIXME(effects): Please remove this. It's a footgun.`
`3143`	`3144`	`/// Whether the trait impl is marked const. This does not consider stability or feature gates.`
`3144`	`3145`	`pub fn is_const_trait_impl_raw(self, def_id: DefId) -> bool {`
`3145`	`3146`	`let Some(local_def_id) = def_id.as_local() else { return false };`