1
Fork 0

Auto merge of #134122 - oli-obk:push-zqnyznxtpnll, r=petrochenkov

Move impl constness into impl trait header

This PR is kind of the opposite of the rejected https://github.com/rust-lang/rust/pull/134114

Instead of moving more things into the `constness` query, we want to keep them where their corresponding hir nodes are lowered. So I gave this a spin for impls, which have an obvious place to be (the impl trait header). And surprisingly it's also a perf improvement (likely just slightly better query & cache usage).

The issue was that removing anything from the `constness` query makes it just return `NotConst`, which is wrong. So I had to change it to `bug!` out if used wrongly, and only then remove the impl blocks from the `constness` query. I think this change is good in general, because it makes using `constness` more robust (as can be seen by how few sites that had to be changed, so it was almost solely used specifically for the purpose of asking for functions' constness). The main thing where this change was not great was in clippy, which was using the `constness` query as a general DefId -> constness map. I added a `DefKind` filter in front of that. If it becomes a more common pattern we can always move that helper into rustc.
This commit is contained in:
bors 2024-12-13 16:17:34 +00:00
commit e217f94917
10 changed files with 99 additions and 44 deletions

View file

@ -4,27 +4,29 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::query::Providers; use rustc_middle::query::Providers;
use rustc_middle::ty::TyCtxt; use rustc_middle::ty::TyCtxt;
pub fn is_parent_const_impl_raw(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { fn parent_impl_constness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> hir::Constness {
let parent_id = tcx.local_parent(def_id); let parent_id = tcx.local_parent(def_id);
matches!(tcx.def_kind(parent_id), DefKind::Impl { .. }) if matches!(tcx.def_kind(parent_id), DefKind::Impl { .. })
&& tcx.constness(parent_id) == hir::Constness::Const && let Some(header) = tcx.impl_trait_header(parent_id)
{
header.constness
} else {
hir::Constness::NotConst
}
} }
/// Checks whether an item is considered to be `const`. If it is a constructor, anonymous const, /// Checks whether an item is considered to be `const`. If it is a constructor, it is const.
/// const block, const item or associated const, it is const. If it is a trait impl/function, /// If it is an assoc method or function,
/// return if it has a `const` modifier. If it is an intrinsic, report whether said intrinsic /// return if it has a `const` modifier. If it is an intrinsic, report whether said intrinsic
/// has a `rustc_const_{un,}stable` attribute. Otherwise, return `Constness::NotConst`. /// has a `rustc_const_{un,}stable` attribute. Otherwise, panic.
fn constness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> hir::Constness { fn constness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> hir::Constness {
let node = tcx.hir_node_by_def_id(def_id); let node = tcx.hir_node_by_def_id(def_id);
match node { match node {
hir::Node::Ctor(_) hir::Node::Ctor(hir::VariantData::Tuple(..))
| hir::Node::AnonConst(_)
| hir::Node::ConstBlock(_)
| hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) => { | hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) => {
hir::Constness::Const hir::Constness::Const
} }
hir::Node::Item(hir::Item { kind: hir::ItemKind::Impl(impl_), .. }) => impl_.constness,
hir::Node::ForeignItem(_) => { hir::Node::ForeignItem(_) => {
// Foreign items cannot be evaluated at compile-time. // Foreign items cannot be evaluated at compile-time.
hir::Constness::NotConst hir::Constness::NotConst
@ -38,10 +40,12 @@ fn constness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> hir::Constness {
// If the function itself is not annotated with `const`, it may still be a `const fn` // If the function itself is not annotated with `const`, it may still be a `const fn`
// if it resides in a const trait impl. // if it resides in a const trait impl.
let is_const = is_parent_const_impl_raw(tcx, def_id); parent_impl_constness(tcx, def_id)
if is_const { hir::Constness::Const } else { hir::Constness::NotConst }
} else { } else {
hir::Constness::NotConst tcx.dcx().span_bug(
tcx.def_span(def_id),
format!("should not be requesting the constness of items that can't be const: {node:#?}: {:?}", tcx.def_kind(def_id))
)
} }
} }
} }

View file

@ -4042,9 +4042,7 @@ impl<'hir> Node<'hir> {
_ => None, _ => None,
}, },
Node::TraitItem(ti) => match ti.kind { Node::TraitItem(ti) => match ti.kind {
TraitItemKind::Fn(ref sig, TraitFn::Provided(_)) => { TraitItemKind::Fn(ref sig, _) => Some(FnKind::Method(ti.ident, sig)),
Some(FnKind::Method(ti.ident, sig))
}
_ => None, _ => None,
}, },
Node::ImplItem(ii) => match ii.kind { Node::ImplItem(ii) => match ii.kind {

View file

@ -1611,7 +1611,7 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::ImplTrai
impl_.of_trait.as_ref().map(|ast_trait_ref| { impl_.of_trait.as_ref().map(|ast_trait_ref| {
let selfty = tcx.type_of(def_id).instantiate_identity(); let selfty = tcx.type_of(def_id).instantiate_identity();
check_impl_constness(tcx, tcx.is_const_trait_impl(def_id.to_def_id()), ast_trait_ref); check_impl_constness(tcx, impl_.constness, ast_trait_ref);
let trait_ref = icx.lowerer().lower_impl_trait_ref(ast_trait_ref, selfty); let trait_ref = icx.lowerer().lower_impl_trait_ref(ast_trait_ref, selfty);
@ -1619,22 +1619,23 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::ImplTrai
trait_ref: ty::EarlyBinder::bind(trait_ref), trait_ref: ty::EarlyBinder::bind(trait_ref),
safety: impl_.safety, safety: impl_.safety,
polarity: polarity_of_impl(tcx, def_id, impl_, item.span), polarity: polarity_of_impl(tcx, def_id, impl_, item.span),
constness: impl_.constness,
} }
}) })
} }
fn check_impl_constness( fn check_impl_constness(
tcx: TyCtxt<'_>, tcx: TyCtxt<'_>,
is_const: bool, constness: hir::Constness,
hir_trait_ref: &hir::TraitRef<'_>, hir_trait_ref: &hir::TraitRef<'_>,
) -> Option<ErrorGuaranteed> { ) {
if !is_const { if let hir::Constness::NotConst = constness {
return None; return;
} }
let trait_def_id = hir_trait_ref.trait_def_id()?; let Some(trait_def_id) = hir_trait_ref.trait_def_id() else { return };
if tcx.is_const_trait(trait_def_id) { if tcx.is_const_trait(trait_def_id) {
return None; return;
} }
let trait_name = tcx.item_name(trait_def_id).to_string(); let trait_name = tcx.item_name(trait_def_id).to_string();
@ -1650,14 +1651,14 @@ fn check_impl_constness(
), ),
(false, _) | (_, false) => (None, ""), (false, _) | (_, false) => (None, ""),
}; };
Some(tcx.dcx().emit_err(errors::ConstImplForNonConstTrait { tcx.dcx().emit_err(errors::ConstImplForNonConstTrait {
trait_ref_span: hir_trait_ref.path.span, trait_ref_span: hir_trait_ref.path.span,
trait_name, trait_name,
local_trait_span, local_trait_span,
suggestion_pre, suggestion_pre,
marking: (), marking: (),
adding: (), adding: (),
})) });
} }
fn polarity_of_impl( fn polarity_of_impl(

View file

@ -1264,12 +1264,7 @@ fn should_encode_fn_sig(def_kind: DefKind) -> bool {
fn should_encode_constness(def_kind: DefKind) -> bool { fn should_encode_constness(def_kind: DefKind) -> bool {
match def_kind { match def_kind {
DefKind::Fn DefKind::Fn | DefKind::AssocFn | DefKind::Closure | DefKind::Ctor(_, CtorKind::Fn) => true,
| DefKind::AssocFn
| DefKind::Closure
| DefKind::Impl { of_trait: true }
| DefKind::Variant
| DefKind::Ctor(..) => true,
DefKind::Struct DefKind::Struct
| DefKind::Union | DefKind::Union
@ -1281,7 +1276,7 @@ fn should_encode_constness(def_kind: DefKind) -> bool {
| DefKind::Static { .. } | DefKind::Static { .. }
| DefKind::TyAlias | DefKind::TyAlias
| DefKind::OpaqueTy | DefKind::OpaqueTy
| DefKind::Impl { of_trait: false } | DefKind::Impl { .. }
| DefKind::ForeignTy | DefKind::ForeignTy
| DefKind::ConstParam | DefKind::ConstParam
| DefKind::InlineConst | DefKind::InlineConst
@ -1296,6 +1291,8 @@ fn should_encode_constness(def_kind: DefKind) -> bool {
| DefKind::LifetimeParam | DefKind::LifetimeParam
| DefKind::GlobalAsm | DefKind::GlobalAsm
| DefKind::ExternCrate | DefKind::ExternCrate
| DefKind::Ctor(_, CtorKind::Const)
| DefKind::Variant
| DefKind::SyntheticCoroutineBody => false, | DefKind::SyntheticCoroutineBody => false,
} }
} }

View file

@ -746,7 +746,10 @@ rustc_queries! {
desc { |tcx| "computing drop-check constraints for `{}`", tcx.def_path_str(key) } desc { |tcx| "computing drop-check constraints for `{}`", tcx.def_path_str(key) }
} }
/// Returns `true` if this is a const fn / const impl. /// Returns the constness of function-like things (tuple struct/variant constructors, functions,
/// methods)
///
/// Will ICE if used on things that are always const or never const.
/// ///
/// **Do not call this function manually.** It is only meant to cache the base data for the /// **Do not call this function manually.** It is only meant to cache the base data for the
/// higher-level functions. Consider using `is_const_fn` or `is_const_trait_impl` instead. /// higher-level functions. Consider using `is_const_fn` or `is_const_trait_impl` instead.

View file

@ -3141,7 +3141,7 @@ impl<'tcx> TyCtxt<'tcx> {
/// Whether the trait impl is marked const. This does not consider stability or feature gates. /// Whether the trait impl is marked const. This does not consider stability or feature gates.
pub fn is_const_trait_impl(self, def_id: DefId) -> bool { pub fn is_const_trait_impl(self, def_id: DefId) -> bool {
self.def_kind(def_id) == DefKind::Impl { of_trait: true } self.def_kind(def_id) == DefKind::Impl { of_trait: true }
&& self.constness(def_id) == hir::Constness::Const && self.impl_trait_header(def_id).unwrap().constness == hir::Constness::Const
} }
pub fn intrinsic(self, def_id: impl IntoQueryParam<DefId> + Copy) -> Option<ty::IntrinsicDef> { pub fn intrinsic(self, def_id: impl IntoQueryParam<DefId> + Copy) -> Option<ty::IntrinsicDef> {

View file

@ -254,6 +254,7 @@ pub struct ImplTraitHeader<'tcx> {
pub trait_ref: ty::EarlyBinder<'tcx, ty::TraitRef<'tcx>>, pub trait_ref: ty::EarlyBinder<'tcx, ty::TraitRef<'tcx>>,
pub polarity: ImplPolarity, pub polarity: ImplPolarity,
pub safety: hir::Safety, pub safety: hir::Safety,
pub constness: hir::Constness,
} }
#[derive(Copy, Clone, PartialEq, Eq, Debug, TypeFoldable, TypeVisitable)] #[derive(Copy, Clone, PartialEq, Eq, Debug, TypeFoldable, TypeVisitable)]
@ -2005,17 +2006,25 @@ impl<'tcx> TyCtxt<'tcx> {
let def_id: DefId = def_id.into(); let def_id: DefId = def_id.into();
match self.def_kind(def_id) { match self.def_kind(def_id) {
DefKind::Impl { of_trait: true } => { DefKind::Impl { of_trait: true } => {
self.constness(def_id) == hir::Constness::Const let header = self.impl_trait_header(def_id).unwrap();
&& self.is_const_trait( header.constness == hir::Constness::Const
self.trait_id_of_impl(def_id) && self.is_const_trait(header.trait_ref.skip_binder().def_id)
.expect("expected trait for trait implementation"),
)
} }
DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) => { DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) => {
self.constness(def_id) == hir::Constness::Const self.constness(def_id) == hir::Constness::Const
} }
DefKind::Trait => self.is_const_trait(def_id), DefKind::Trait => self.is_const_trait(def_id),
DefKind::AssocTy | DefKind::AssocFn => { DefKind::AssocTy => {
let parent_def_id = self.parent(def_id);
match self.def_kind(parent_def_id) {
DefKind::Impl { of_trait: false } => false,
DefKind::Impl { of_trait: true } | DefKind::Trait => {
self.is_conditionally_const(parent_def_id)
}
_ => bug!("unexpected parent item of associated type: {parent_def_id:?}"),
}
}
DefKind::AssocFn => {
let parent_def_id = self.parent(def_id); let parent_def_id = self.parent(def_id);
match self.def_kind(parent_def_id) { match self.def_kind(parent_def_id) {
DefKind::Impl { of_trait: false } => { DefKind::Impl { of_trait: false } => {
@ -2024,7 +2033,7 @@ impl<'tcx> TyCtxt<'tcx> {
DefKind::Impl { of_trait: true } | DefKind::Trait => { DefKind::Impl { of_trait: true } | DefKind::Trait => {
self.is_conditionally_const(parent_def_id) self.is_conditionally_const(parent_def_id)
} }
_ => bug!("unexpected parent item of associated item: {parent_def_id:?}"), _ => bug!("unexpected parent item of associated fn: {parent_def_id:?}"),
} }
} }
DefKind::OpaqueTy => match self.opaque_ty_origin(def_id) { DefKind::OpaqueTy => match self.opaque_ty_origin(def_id) {

View file

@ -389,7 +389,7 @@ impl<'tcx> TyCtxt<'tcx> {
.delay_as_bug(); .delay_as_bug();
} }
dtor_candidate = Some((*item_id, self.constness(impl_did))); dtor_candidate = Some((*item_id, self.impl_trait_header(impl_did).unwrap().constness));
}); });
let (did, constness) = dtor_candidate?; let (did, constness) = dtor_candidate?;

View file

@ -128,7 +128,9 @@ fn evaluate_host_effect_from_selection_candiate<'tcx>(
Err(_) => Err(EvaluationFailure::NoSolution), Err(_) => Err(EvaluationFailure::NoSolution),
Ok(Some(source)) => match source { Ok(Some(source)) => match source {
ImplSource::UserDefined(impl_) => { ImplSource::UserDefined(impl_) => {
if tcx.constness(impl_.impl_def_id) != hir::Constness::Const { if tcx.impl_trait_header(impl_.impl_def_id).unwrap().constness
!= hir::Constness::Const
{
return Err(EvaluationFailure::NoSolution); return Err(EvaluationFailure::NoSolution);
} }

View file

@ -6,8 +6,11 @@ use clippy_utils::source::SpanRangeExt;
use clippy_utils::{is_from_proc_macro, path_to_local}; use clippy_utils::{is_from_proc_macro, path_to_local};
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Constness, Expr, ExprKind}; use rustc_hir::{BinOpKind, Constness, Expr, ExprKind};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_lint::{LateContext, LateLintPass, Lint, LintContext}; use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::TyCtxt;
use rustc_session::impl_lint_pass; use rustc_session::impl_lint_pass;
declare_clippy_lint! { declare_clippy_lint! {
@ -94,6 +97,44 @@ impl ManualFloatMethods {
} }
} }
fn is_not_const(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
match tcx.def_kind(def_id) {
DefKind::Mod
| DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::Variant
| DefKind::Trait
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TraitAlias
| DefKind::AssocTy
| DefKind::Macro(..)
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::ExternCrate
| DefKind::Use
| DefKind::ForeignMod
| DefKind::GlobalAsm
| DefKind::Impl { .. }
| DefKind::OpaqueTy
| DefKind::SyntheticCoroutineBody
| DefKind::TyParam => true,
DefKind::AnonConst
| DefKind::InlineConst
| DefKind::Const
| DefKind::ConstParam
| DefKind::Static { .. }
| DefKind::Ctor(..)
| DefKind::AssocConst => false,
DefKind::Fn
| DefKind::AssocFn
| DefKind::Closure => tcx.constness(def_id) == Constness::NotConst,
}
}
impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let ExprKind::Binary(kind, lhs, rhs) = expr.kind if let ExprKind::Binary(kind, lhs, rhs) = expr.kind
@ -105,7 +146,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods {
&& exprs.iter_mut().partition_in_place(|i| path_to_local(i).is_some()) == 2 && exprs.iter_mut().partition_in_place(|i| path_to_local(i).is_some()) == 2
&& !in_external_macro(cx.sess(), expr.span) && !in_external_macro(cx.sess(), expr.span)
&& ( && (
matches!(cx.tcx.constness(cx.tcx.hir().enclosing_body_owner(expr.hir_id)), Constness::NotConst) is_not_const(cx.tcx, cx.tcx.hir().enclosing_body_owner(expr.hir_id).into())
|| self.msrv.meets(msrvs::CONST_FLOAT_CLASSIFY) || self.msrv.meets(msrvs::CONST_FLOAT_CLASSIFY)
) )
&& let [first, second, const_1, const_2] = exprs && let [first, second, const_1, const_2] = exprs