Skip to content

Commit

Permalink
tcx.is_const_fn doesn't work the way it is described, remove it
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
RalfJung committed Oct 25, 2024
1 parent 36dda45 commit 8849ac6
Show file tree
Hide file tree
Showing 21 changed files with 55 additions and 64 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}

// 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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'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 });
}
Expand Down Expand Up @@ -174,7 +174,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
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");
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,8 +431,8 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
// 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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,7 +1597,7 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::ImplTrai
impl_.of_trait.as_ref().map(|ast_trait_ref| {
let selfty = tcx.type_of(def_id).instantiate_identity();

check_impl_constness(tcx, tcx.is_const_trait_impl_raw(def_id.to_def_id()), ast_trait_ref);
check_impl_constness(tcx, tcx.is_const_trait_impl(def_id.to_def_id()), ast_trait_ref);

let trait_ref = icx.lowerer().lower_impl_trait_ref(ast_trait_ref, selfty);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
//
// This check is here because there is currently no way to express a trait bound for `FnDef` types only.
if let ty::FnDef(def_id, _args) = *arg_ty.kind() {
if idx == 0 && !self.tcx.is_const_fn_raw(def_id) {
if idx == 0 && !self.tcx.is_const_fn(def_id) {
self.dcx().emit_err(errors::ConstSelectMustBeConst { span });
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1751,7 +1751,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// to tell them that in the diagnostic. Does not affect typeck.
let is_constable = match element.kind {
hir::ExprKind::Call(func, _args) => 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) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ impl<'hir> Map<'hir> {
BodyOwnerKind::Static(mutability) => 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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/graphviz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ where
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))]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,12 +741,11 @@ rustc_queries! {
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
Expand Down
41 changes: 13 additions & 28 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3120,39 +3120,24 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

/// 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<DefId> + Copy) -> Option<ty::IntrinsicDef> {
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1995,8 +1995,11 @@ impl<'tcx> TyCtxt<'tcx> {
(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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ impl<'tcx> Validator<'_, 'tcx> {
}
// 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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1997,7 +1997,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
) {
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
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
// 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 });
}
Expand Down Expand Up @@ -597,8 +597,8 @@ impl<'tcx> MissingStabilityAnnotations<'tcx> {
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 =
Expand Down Expand Up @@ -820,7 +820,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'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 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
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) => {
Expand All @@ -413,7 +413,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
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()
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ impl Item {
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
Expand All @@ -662,7 +662,7 @@ impl Item {
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
Expand Down
16 changes: 10 additions & 6 deletions src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_utils/src/visitors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,13 +346,13 @@ pub fn is_const_evaluatable<'tcx>(cx: &LateContext<'tcx>, 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() => {},
Expand Down

0 comments on commit 8849ac6

Please sign in to comment.