From 8849ac6042770500db91aca860ec8d86af2c74a3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 12 Oct 2024 20:37:35 +0200 Subject: [PATCH] tcx.is_const_fn doesn't work the way it is described, remove it Then we can rename the _raw functions to drop their suffix, and instead explicitly use is_stable_const_fn for the few cases where that is really what you want. --- .../src/check_consts/check.rs | 2 +- .../rustc_const_eval/src/check_consts/mod.rs | 2 +- .../rustc_const_eval/src/check_consts/ops.rs | 4 +- .../src/const_eval/machine.rs | 4 +- compiler/rustc_hir_analysis/src/collect.rs | 2 +- compiler/rustc_hir_typeck/src/callee.rs | 2 +- compiler/rustc_hir_typeck/src/expr.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_middle/src/hir/map/mod.rs | 2 +- compiler/rustc_middle/src/mir/graphviz.rs | 2 +- compiler/rustc_middle/src/mir/pretty.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 7 ++-- compiler/rustc_middle/src/ty/context.rs | 41 ++++++------------- compiler/rustc_middle/src/ty/mod.rs | 5 ++- .../rustc_mir_transform/src/promote_consts.rs | 2 +- compiler/rustc_passes/src/check_attr.rs | 2 +- compiler/rustc_passes/src/stability.rs | 8 ++-- .../src/traits/select/candidate_assembly.rs | 4 +- src/librustdoc/clean/types.rs | 4 +- .../clippy_utils/src/qualify_min_const_fn.rs | 16 +++++--- src/tools/clippy/clippy_utils/src/visitors.rs | 4 +- 21 files changed, 55 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_const_eval/src/check_consts/check.rs b/compiler/rustc_const_eval/src/check_consts/check.rs index ef01fe88979..8f1a887a961 100644 --- a/compiler/rustc_const_eval/src/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/check_consts/check.rs @@ -731,7 +731,7 @@ fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location } // Trait functions are not `const fn` so we have to skip them here. - if !tcx.is_const_fn_raw(callee) && !is_trait { + if !tcx.is_const_fn(callee) && !is_trait { self.check_op(ops::FnCallNonConst { caller, callee, diff --git a/compiler/rustc_const_eval/src/check_consts/mod.rs b/compiler/rustc_const_eval/src/check_consts/mod.rs index 146b559f2d7..56da6791847 100644 --- a/compiler/rustc_const_eval/src/check_consts/mod.rs +++ b/compiler/rustc_const_eval/src/check_consts/mod.rs @@ -112,7 +112,7 @@ pub fn is_safe_to_expose_on_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> b } // Const-stability is only relevant for `const fn`. - assert!(tcx.is_const_fn_raw(def_id)); + assert!(tcx.is_const_fn(def_id)); match tcx.lookup_const_stability(def_id) { None => { diff --git a/compiler/rustc_const_eval/src/check_consts/ops.rs b/compiler/rustc_const_eval/src/check_consts/ops.rs index 0f151dcdb03..3ac06ae6491 100644 --- a/compiler/rustc_const_eval/src/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/check_consts/ops.rs @@ -122,7 +122,7 @@ fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, _: Span) -> Diag<'tcx> { if let Ok(Some(ImplSource::UserDefined(data))) = implsrc { // FIXME(effects) revisit this - if !tcx.is_const_trait_impl_raw(data.impl_def_id) { + if !tcx.is_const_trait_impl(data.impl_def_id) { let span = tcx.def_span(data.impl_def_id); err.subdiagnostic(errors::NonConstImplNote { span }); } @@ -174,7 +174,7 @@ macro_rules! error { let note = match self_ty.kind() { FnDef(def_id, ..) => { let span = tcx.def_span(*def_id); - if ccx.tcx.is_const_fn_raw(*def_id) { + if ccx.tcx.is_const_fn(*def_id) { span_bug!(span, "calling const FnDef errored when it shouldn't"); } diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 7793de9c933..d54c5b750f0 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -431,8 +431,8 @@ fn find_mir_or_eval_fn( // sensitive check here. But we can at least rule out functions that are not const at // all. That said, we have to allow calling functions inside a trait marked with // #[const_trait]. These *are* const-checked! - // FIXME: why does `is_const_fn_raw` not classify them as const? - if (!ecx.tcx.is_const_fn_raw(def) && !ecx.tcx.is_const_default_method(def)) + // FIXME(effects): why does `is_const_fn` not classify them as const? + if (!ecx.tcx.is_const_fn(def) && !ecx.tcx.is_const_default_method(def)) || ecx.tcx.has_attr(def, sym::rustc_do_not_const_check) { // We certainly do *not* want to actually call the fn diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index f46b7a8bc9c..3add801cf56 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1597,7 +1597,7 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option match *self.node_ty(func.hir_id).kind() { - ty::FnDef(def_id, _) if tcx.is_const_fn(def_id) => traits::IsConstable::Fn, + ty::FnDef(def_id, _) if tcx.is_stable_const_fn(def_id) => traits::IsConstable::Fn, _ => traits::IsConstable::No, }, hir::ExprKind::Path(qpath) => { diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index e06c86ae4c0..47f7a8b7c20 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1081,7 +1081,7 @@ fn should_encode_mir( && (generics.requires_monomorphization(tcx) || tcx.cross_crate_inlinable(def_id))); // The function has a `const` modifier or is in a `#[const_trait]`. - let is_const_fn = tcx.is_const_fn_raw(def_id.to_def_id()) + let is_const_fn = tcx.is_const_fn(def_id.to_def_id()) || tcx.is_const_default_method(def_id.to_def_id()); (is_const_fn, opt) } diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 8fd5ff1f369..926691013dd 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -329,7 +329,7 @@ pub fn body_const_context(self, def_id: impl Into) -> Option ConstContext::Static(mutability), BodyOwnerKind::Fn if self.tcx.is_constructor(def_id) => return None, - BodyOwnerKind::Fn | BodyOwnerKind::Closure if self.tcx.is_const_fn_raw(def_id) => { + BodyOwnerKind::Fn | BodyOwnerKind::Closure if self.tcx.is_const_fn(def_id) => { ConstContext::ConstFn } BodyOwnerKind::Fn if self.tcx.is_const_default_method(def_id) => ConstContext::ConstFn, diff --git a/compiler/rustc_middle/src/mir/graphviz.rs b/compiler/rustc_middle/src/mir/graphviz.rs index a3fe8f9cffa..7bb41193d5c 100644 --- a/compiler/rustc_middle/src/mir/graphviz.rs +++ b/compiler/rustc_middle/src/mir/graphviz.rs @@ -17,7 +17,7 @@ pub fn write_mir_graphviz(tcx: TyCtxt<'_>, single: Option, w: &mut W) let mirs = def_ids .iter() .flat_map(|def_id| { - if tcx.is_const_fn_raw(*def_id) { + if tcx.is_const_fn(*def_id) { vec![tcx.optimized_mir(*def_id), tcx.mir_for_ctfe(*def_id)] } else { vec![tcx.instance_mir(ty::InstanceKind::Item(*def_id))] diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index faa022b50ef..e690bf74b6b 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -317,7 +317,7 @@ pub fn write_mir_pretty<'tcx>( }; // For `const fn` we want to render both the optimized MIR and the MIR for ctfe. - if tcx.is_const_fn_raw(def_id) { + if tcx.is_const_fn(def_id) { render_body(w, tcx.optimized_mir(def_id))?; writeln!(w)?; writeln!(w, "// MIR FOR CTFE")?; diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index d03fc39c9ad..94bdb913528 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -741,12 +741,11 @@ desc { |tcx| "computing drop-check constraints for `{}`", tcx.def_path_str(key) } } - /// Returns `true` if this is a const fn, use the `is_const_fn` to know whether your crate - /// actually sees it as const fn (e.g., the const-fn-ness might be unstable and you might - /// not have the feature gate active). + /// Returns `true` if this is a const fn / const impl. /// /// **Do not call this function manually.** It is only meant to cache the base data for the - /// `is_const_fn` function. Consider using `is_const_fn` or `is_const_fn_raw` instead. + /// higher-level functions. Consider using `is_const_fn` or `is_const_trait_impl` instead. + /// Also note that neither of them takes into account feature gates and stability. query constness(key: DefId) -> hir::Constness { desc { |tcx| "checking if item is const: `{}`", tcx.def_path_str(key) } separate_provide_extern diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 1c6a5e9011f..a6a0a6dc222 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -3120,39 +3120,24 @@ pub fn map_opaque_lifetime_to_parent_lifetime( } } - /// Whether the `def_id` counts as const fn in the current crate, considering all active - /// feature gates - pub fn is_const_fn(self, def_id: DefId) -> bool { - if self.is_const_fn_raw(def_id) { - match self.lookup_const_stability(def_id) { - Some(stability) if stability.is_const_unstable() => { - // has a `rustc_const_unstable` attribute, check whether the user enabled the - // corresponding feature gate. - stability.feature.is_some_and(|f| self.features().enabled(f)) - } - // functions without const stability are either stable user written - // const fn or the user is using feature gates and we thus don't - // care what they do - _ => true, + /// Whether `def_id` is a stable const fn (i.e., doesn't need any feature gates to be called). + /// + /// When this is `false`, the function may still be callable as a `const fn` due to features + /// being enabled! + pub fn is_stable_const_fn(self, def_id: DefId) -> bool { + self.is_const_fn(def_id) + && match self.lookup_const_stability(def_id) { + None => true, // a fn in a non-staged_api crate + Some(stability) if stability.is_const_stable() => true, + _ => false, } - } else { - false - } } // 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 }; - let node = self.hir_node_by_def_id(local_def_id); - - matches!( - node, - hir::Node::Item(hir::Item { - kind: hir::ItemKind::Impl(hir::Impl { constness, .. }), - .. - }) if matches!(constness, hir::Constness::Const) - ) + pub fn is_const_trait_impl(self, def_id: DefId) -> bool { + self.def_kind(def_id) == DefKind::Impl { of_trait: true } + && self.constness(def_id) == hir::Constness::Const } pub fn intrinsic(self, def_id: impl IntoQueryParam + Copy) -> Option { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 85414764817..b92fc864b49 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1995,8 +1995,11 @@ pub fn adjust_ident_and_get_scope( (ident, scope) } + /// Checks whether this is a `const fn`. Returns `false` for non-functions. + /// + /// Even if this returns `true`, constness may still be unstable! #[inline] - pub fn is_const_fn_raw(self, def_id: DefId) -> bool { + pub fn is_const_fn(self, def_id: DefId) -> bool { matches!( self.def_kind(def_id), DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::Closure diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index 86c4b241a2b..fa9a6bfcf7c 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -673,7 +673,7 @@ fn validate_call( } // Make sure the callee is a `const fn`. let is_const_fn = match *fn_ty.kind() { - ty::FnDef(def_id, _) => self.tcx.is_const_fn_raw(def_id), + ty::FnDef(def_id, _) => self.tcx.is_const_fn(def_id), _ => false, }; if !is_const_fn { diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 62c502f9524..ed0d7ed8acc 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -1997,7 +1997,7 @@ fn check_rustc_allow_const_fn_unstable( ) { match target { Target::Fn | Target::Method(_) - if self.tcx.is_const_fn_raw(hir_id.expect_owner().to_def_id()) => {} + if self.tcx.is_const_fn(hir_id.expect_owner().to_def_id()) => {} // FIXME(#80564): We permit struct fields and match arms to have an // `#[allow_internal_unstable]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index e24e6a486f4..67e7496771d 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -179,7 +179,7 @@ fn annotate( // their ABI; `fn_sig.abi` is *not* correct for foreign functions. && !is_foreign_item && const_stab.is_some() - && (!self.in_trait_impl || !self.tcx.is_const_fn_raw(def_id.to_def_id())) + && (!self.in_trait_impl || !self.tcx.is_const_fn(def_id.to_def_id())) { self.tcx.dcx().emit_err(errors::MissingConstErr { fn_sig_span: fn_sig.span }); } @@ -597,8 +597,8 @@ fn check_missing_or_wrong_const_stability(&self, def_id: LocalDefId, span: Span) return; } - let is_const = self.tcx.is_const_fn_raw(def_id.to_def_id()) - || self.tcx.is_const_trait_impl_raw(def_id.to_def_id()); + let is_const = self.tcx.is_const_fn(def_id.to_def_id()) + || self.tcx.is_const_trait_impl(def_id.to_def_id()); let is_stable = self.tcx.lookup_stability(def_id).is_some_and(|stability| stability.level.is_stable()); let missing_const_stability_attribute = @@ -820,7 +820,7 @@ fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { // `#![feature(const_trait_impl)]` is unstable, so any impl declared stable // needs to have an error emitted. if features.const_trait_impl() - && self.tcx.is_const_trait_impl_raw(item.owner_id.to_def_id()) + && self.tcx.is_const_trait_impl(item.owner_id.to_def_id()) && const_stab.is_some_and(|(stab, _)| stab.is_const_stable()) { self.tcx.dcx().emit_err(errors::TraitImplConstStable { span: item.span }); diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 47601b0c18d..e027586563e 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -393,7 +393,7 @@ fn assemble_closure_candidates( let self_ty = obligation.self_ty().skip_binder(); match *self_ty.kind() { ty::Closure(def_id, _) => { - let is_const = self.tcx().is_const_fn_raw(def_id); + let is_const = self.tcx().is_const_fn(def_id); debug!(?kind, ?obligation, "assemble_unboxed_candidates"); match self.infcx.closure_kind(self_ty) { Some(closure_kind) => { @@ -413,7 +413,7 @@ fn assemble_closure_candidates( } ty::CoroutineClosure(def_id, args) => { let args = args.as_coroutine_closure(); - let is_const = self.tcx().is_const_fn_raw(def_id); + let is_const = self.tcx().is_const_fn(def_id); if let Some(closure_kind) = self.infcx.closure_kind(self_ty) // Ambiguity if upvars haven't been constrained yet && !args.tupled_upvars_ty().is_ty_var() diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 1c060fa0150..c62144be3da 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -640,7 +640,7 @@ fn build_fn_header( asyncness: ty::Asyncness, ) -> hir::FnHeader { let sig = tcx.fn_sig(def_id).skip_binder(); - let constness = if tcx.is_const_fn_raw(def_id) { + let constness = if tcx.is_const_fn(def_id) { hir::Constness::Const } else { hir::Constness::NotConst @@ -662,7 +662,7 @@ fn build_fn_header( safety }, abi, - constness: if tcx.is_const_fn_raw(def_id) { + constness: if tcx.is_const_fn(def_id) { hir::Constness::Const } else { hir::Constness::NotConst diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 5f12b6bf99e..46739862de6 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -334,7 +334,7 @@ fn check_terminator<'tcx>( | TerminatorKind::TailCall { func, args, fn_span: _ } => { let fn_ty = func.ty(body, tcx); if let ty::FnDef(fn_def_id, _) = *fn_ty.kind() { - if !is_const_fn(tcx, fn_def_id, msrv) { + if !is_stable_const_fn(tcx, fn_def_id, msrv) { return Err(( span, format!( @@ -377,12 +377,12 @@ fn check_terminator<'tcx>( } } -fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool { +fn is_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool { tcx.is_const_fn(def_id) - && tcx.lookup_const_stability(def_id).map_or(true, |const_stab| { + && tcx.lookup_const_stability(def_id).is_none_or(|const_stab| { if let rustc_attr::StabilityLevel::Stable { since, .. } = const_stab.level { // Checking MSRV is manually necessary because `rustc` has no such concept. This entire - // function could be removed if `rustc` provided a MSRV-aware version of `is_const_fn`. + // function could be removed if `rustc` provided a MSRV-aware version of `is_stable_const_fn`. // as a part of an unimplemented MSRV check https://github.com/rust-lang/rust/issues/65262. let const_stab_rust_version = match since { @@ -393,8 +393,12 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool { msrv.meets(const_stab_rust_version) } else { - // Unstable const fn with the feature enabled. - msrv.current().is_none() + // Unstable const fn, check if the feature is enabled. We need both the regular stability + // feature and (if set) the const stability feature to const-call this function. + let stab = tcx.lookup_stability(def_id); + let is_enabled = stab.is_some_and(|s| s.is_stable() || tcx.features().enabled(s.feature)) + && const_stab.feature.is_none_or(|f| tcx.features().enabled(f)); + is_enabled && msrv.current().is_none() } }) } diff --git a/src/tools/clippy/clippy_utils/src/visitors.rs b/src/tools/clippy/clippy_utils/src/visitors.rs index 02931306f16..8db6502dbfb 100644 --- a/src/tools/clippy/clippy_utils/src/visitors.rs +++ b/src/tools/clippy/clippy_utils/src/visitors.rs @@ -346,13 +346,13 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) { .cx .qpath_res(p, hir_id) .opt_def_id() - .map_or(false, |id| self.cx.tcx.is_const_fn_raw(id)) => {}, + .map_or(false, |id| self.cx.tcx.is_const_fn(id)) => {}, ExprKind::MethodCall(..) if self .cx .typeck_results() .type_dependent_def_id(e.hir_id) - .map_or(false, |id| self.cx.tcx.is_const_fn_raw(id)) => {}, + .map_or(false, |id| self.cx.tcx.is_const_fn(id)) => {}, ExprKind::Binary(_, lhs, rhs) if self.cx.typeck_results().expr_ty(lhs).peel_refs().is_primitive_ty() && self.cx.typeck_results().expr_ty(rhs).peel_refs().is_primitive_ty() => {},