Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace #[default_method_body_is_const] with #[const_trait] #96964

Merged
merged 5 commits into from
May 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -277,8 +277,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// sensitive check here. But we can at least rule out functions that are not const
// at all.
if !ecx.tcx.is_const_fn_raw(def.did) {
// allow calling functions marked with #[default_method_body_is_const].
if !ecx.tcx.has_attr(def.did, sym::default_method_body_is_const) {
// allow calling functions inside a trait marked with #[const_trait].
if !ecx.tcx.is_const_default_method(def.did) {
// We certainly do *not* want to actually call the fn
// though, so be sure we return here.
throw_unsup_format!("calling non-const function `{}`", instance)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Rust MIR: a lowered representation of Rust.
#![feature(control_flow_enum)]
#![feature(decl_macro)]
#![feature(exact_size_is_empty)]
#![feature(let_chains)]
#![feature(let_else)]
#![feature(map_try_insert)]
#![feature(min_specialization)]
Expand Down
29 changes: 10 additions & 19 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,8 +711,6 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}
};

let mut nonconst_call_permission = false;

// Attempting to call a trait method?
if let Some(trait_id) = tcx.trait_of_item(callee) {
trace!("attempting to call a trait method");
Expand Down Expand Up @@ -774,13 +772,12 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}
}
_ if !tcx.is_const_fn_raw(callee) => {
// At this point, it is only legal when the caller is marked with
// #[default_method_body_is_const], and the callee is in the same
// trait.
let callee_trait = tcx.trait_of_item(callee);
if callee_trait.is_some()
&& tcx.has_attr(caller.to_def_id(), sym::default_method_body_is_const)
&& callee_trait == tcx.trait_of_item(caller)
// At this point, it is only legal when the caller is in a trait
// marked with #[const_trait], and the callee is in the same trait.
let mut nonconst_call_permission = false;
if let Some(callee_trait) = tcx.trait_of_item(callee)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish if chains could be booleans, sigh

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this means :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, meant let chains. just don't like how this has to be a mutable assignment lol. not an actionable comment.

&& tcx.has_attr(callee_trait, sym::const_trait)
&& Some(callee_trait) == tcx.trait_of_item(caller)
// Can only call methods when it's `<Self as TheTrait>::f`.
&& tcx.types.self_param == substs.type_at(0)
{
Expand Down Expand Up @@ -874,16 +871,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
let is_intrinsic = tcx.is_intrinsic(callee);

if !tcx.is_const_fn_raw(callee) {
if tcx.trait_of_item(callee).is_some() {
if tcx.has_attr(callee, sym::default_method_body_is_const) {
// To get to here we must have already found a const impl for the
// trait, but for it to still be non-const can be that the impl is
// using default method bodies.
nonconst_call_permission = true;
}
}

if !nonconst_call_permission {
if !tcx.is_const_default_method(callee) {
// To get to here we must have already found a const impl for the
// trait, but for it to still be non-const can be that the impl is
// using default method bodies.
self.check_op(ops::FnCallNonConst {
caller,
callee,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::mir;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::{sym, Symbol};
use rustc_span::Symbol;

pub use self::qualifs::Qualif;

Expand Down Expand Up @@ -84,10 +84,10 @@ pub fn rustc_allow_const_fn_unstable(
// functions are subject to more stringent restrictions than "const-unstable" functions: They
// cannot use unstable features and can only call other "const-stable" functions.
pub fn is_const_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
// A default body marked const is not const-stable because const
// A default body in a `#[const_trait]` is not const-stable because const
// trait fns currently cannot be const-stable. We shouldn't
// restrict default bodies to only call const-stable functions.
if tcx.has_attr(def_id, sym::default_method_body_is_const) {
if tcx.is_const_default_method(def_id) {
return false;
}

Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
),
// RFC 2632
gated!(
default_method_body_is_const, Normal, template!(Word), WarnFollowing, const_trait_impl,
"`default_method_body_is_const` is a temporary placeholder for declaring default bodies \
as `const`, which may be removed or renamed in the future."
const_trait, Normal, template!(Word), WarnFollowing, const_trait_impl,
"`const` is a temporary placeholder for marking a trait that is suitable for `const` \
`impls` and all default bodies as `const`, which may be removed or renamed in the \
future."
),
// lang-team MCP 147
gated!(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub enum DefKind {
/// [RFC 2593]: https://github.com/rust-lang/rfcs/pull/2593
Ctor(CtorOf, CtorKind),
/// Associated function: `impl MyStruct { fn associated() {} }`
/// or `trait Foo { fn associated() {} }`
AssocFn,
/// Associated constant: `trait MyTrait { const ASSOC: usize; }`
AssocConst,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -891,9 +891,9 @@ fn should_encode_mir(tcx: TyCtxt<'_>, def_id: LocalDefId) -> (bool, bool) {
let needs_inline = (generics.requires_monomorphization(tcx)
|| tcx.codegen_fn_attrs(def_id).requests_inline())
&& tcx.sess.opts.output_types.should_codegen();
// The function has a `const` modifier or is annotated with `default_method_body_is_const`.
// 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())
|| tcx.has_attr(def_id.to_def_id(), sym::default_method_body_is_const);
|| tcx.is_const_default_method(def_id.to_def_id());
let always_encode_mir = tcx.sess.opts.debugging_opts.always_encode_mir;
(is_const_fn, needs_inline || always_encode_mir)
}
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,9 +494,7 @@ impl<'hir> Map<'hir> {
BodyOwnerKind::Fn if self.tcx.is_const_fn_raw(def_id.to_def_id()) => {
ConstContext::ConstFn
}
BodyOwnerKind::Fn
if self.tcx.has_attr(def_id.to_def_id(), sym::default_method_body_is_const) =>
{
BodyOwnerKind::Fn if self.tcx.is_const_default_method(def_id.to_def_id()) => {
ConstContext::ConstFn
}
BodyOwnerKind::Fn | BodyOwnerKind::Closure => return None,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2301,6 +2301,11 @@ impl<'tcx> TyCtxt<'tcx> {
matches!(self.def_kind(def_id), DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..))
&& self.impl_constness(def_id) == hir::Constness::Const
}

#[inline]
pub fn is_const_default_method(self, def_id: DefId) -> bool {
matches!(self.trait_of_item(def_id), Some(trait_id) if self.has_attr(trait_id, sym::const_trait))
}
}

/// Yields the parent function's `LocalDefId` if `def_id` is an `impl Trait` definition.
Expand Down
23 changes: 7 additions & 16 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,7 @@ impl CheckAttrVisitor<'_> {
| sym::rustc_if_this_changed
| sym::rustc_then_this_would_need => self.check_rustc_dirty_clean(&attr),
sym::cmse_nonsecure_entry => self.check_cmse_nonsecure_entry(attr, span, target),
sym::default_method_body_is_const => {
self.check_default_method_body_is_const(attr, span, target)
}
sym::const_trait => self.check_const_trait(attr, span, target),
sym::must_not_suspend => self.check_must_not_suspend(&attr, span, target),
sym::must_use => self.check_must_use(hir_id, &attr, span, target),
sym::rustc_pass_by_value => self.check_pass_by_value(&attr, span, target),
Expand Down Expand Up @@ -2081,23 +2079,14 @@ impl CheckAttrVisitor<'_> {
}
}

/// default_method_body_is_const should only be applied to trait methods with default bodies.
fn check_default_method_body_is_const(
&self,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
/// `#[const_trait]` only applies to traits.
fn check_const_trait(&self, attr: &Attribute, _span: Span, target: Target) -> bool {
match target {
Target::Method(MethodKind::Trait { body: true }) => true,
Target::Trait => true,
_ => {
self.tcx
.sess
.struct_span_err(
attr.span,
"attribute should be applied to a trait method with body",
)
.span_label(span, "not a trait method or missing a body")
.struct_span_err(attr.span, "attribute should be applied to a trait")
.emit();
false
}
Expand Down Expand Up @@ -2191,6 +2180,8 @@ impl CheckAttrVisitor<'_> {
"attribute `{}` without any lints has no effect",
attr.name_or_empty()
)
} else if attr.name_or_empty() == sym::default_method_body_is_const {
format!("`default_method_body_is_const` has been replaced with `#[const_trait]` on traits")
} else {
return;
};
Expand Down
62 changes: 0 additions & 62 deletions compiler/rustc_passes/src/check_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{self, Visitor};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_session::parse::feature_err;
Expand Down Expand Up @@ -64,66 +63,6 @@ pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { check_mod_const_bodies, ..*providers };
}

fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) {
let _: Option<_> = try {
if let hir::ItemKind::Impl(ref imp) = item.kind && let hir::Constness::Const = imp.constness {
let trait_def_id = imp.of_trait.as_ref()?.trait_def_id()?;
let ancestors = tcx
.trait_def(trait_def_id)
.ancestors(tcx, item.def_id.to_def_id())
.ok()?;
let mut to_implement = Vec::new();

for trait_item in tcx.associated_items(trait_def_id).in_definition_order()
{
if let ty::AssocItem {
kind: ty::AssocKind::Fn,
defaultness,
def_id: trait_item_id,
..
} = *trait_item
{
// we can ignore functions that do not have default bodies:
// if those are unimplemented it will be caught by typeck.
if !defaultness.has_value()
|| tcx
.has_attr(trait_item_id, sym::default_method_body_is_const)
{
continue;
}

let is_implemented = ancestors
.leaf_def(tcx, trait_item_id)
.map(|node_item| !node_item.defining_node.is_from_trait())
.unwrap_or(false);

if !is_implemented {
to_implement.push(trait_item_id);
}
}
}

// all nonconst trait functions (not marked with #[default_method_body_is_const])
// must be implemented
if !to_implement.is_empty() {
let not_implemented = to_implement
.into_iter()
.map(|did| tcx.item_name(did).to_string())
.collect::<Vec<_>>()
.join("`, `");
tcx
.sess
.struct_span_err(
item.span,
"const trait implementations may not use non-const default functions",
)
.note(&format!("`{}` not implemented", not_implemented))
.emit();
}
}
Comment on lines -70 to -123
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we forgot to actually forbid impl const on non #[const_trait] which would be here

};
}

#[derive(Copy, Clone)]
struct CheckConstVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -254,7 +193,6 @@ impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> {

fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
intravisit::walk_item(self, item);
check_item(self.tcx, item);
}

fn visit_anon_const(&mut self, anon: &'tcx hir::AnonConst) {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ symbols! {
const_raw_ptr_deref,
const_raw_ptr_to_usize_cast,
const_refs_to_cell,
const_trait,
const_trait_bound_opt_out,
const_trait_impl,
const_transmute,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{
self, Binder, EarlyBinder, Predicate, PredicateKind, ToPredicate, Ty, TyCtxt,
};
use rustc_span::{sym, Span};
use rustc_span::Span;
use rustc_trait_selection::traits;

fn sized_constraint_for_ty<'tcx>(
Expand Down Expand Up @@ -153,7 +153,7 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {
let constness = match hir_id {
Some(hir_id) => match tcx.hir().get(hir_id) {
hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. })
if tcx.has_attr(def_id, sym::default_method_body_is_const) =>
if tcx.is_const_default_method(def_id) =>
{
hir::Constness::Const
}
Expand Down
3 changes: 2 additions & 1 deletion library/core/src/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ use crate::marker::Destruct;
#[lang = "clone"]
#[rustc_diagnostic_item = "Clone"]
#[rustc_trivial_field_reads]
#[cfg_attr(not(bootstrap), const_trait)]
pub trait Clone: Sized {
/// Returns a copy of the value.
///
Expand All @@ -129,7 +130,7 @@ pub trait Clone: Sized {
/// allocations.
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[default_method_body_is_const]
#[cfg_attr(bootstrap, default_method_body_is_const)]
fn clone_from(&mut self, source: &Self)
where
Self: ~const Destruct,
Expand Down
12 changes: 7 additions & 5 deletions library/core/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ use self::Ordering::*;
append_const_msg,
)
)]
#[cfg_attr(not(bootstrap), const_trait)]
#[rustc_diagnostic_item = "PartialEq"]
pub trait PartialEq<Rhs: ?Sized = Self> {
/// This method tests for `self` and `other` values to be equal, and is used
Expand All @@ -226,7 +227,7 @@ pub trait PartialEq<Rhs: ?Sized = Self> {
#[inline]
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
#[default_method_body_is_const]
#[cfg_attr(bootstrap, default_method_body_is_const)]
fn ne(&self, other: &Rhs) -> bool {
!self.eq(other)
}
Expand Down Expand Up @@ -1053,6 +1054,7 @@ impl PartialOrd for Ordering {
append_const_msg,
)
)]
#[cfg_attr(not(bootstrap), const_trait)]
#[rustc_diagnostic_item = "PartialOrd"]
pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
/// This method returns an ordering between `self` and `other` values if one exists.
Expand Down Expand Up @@ -1096,7 +1098,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
#[inline]
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
#[default_method_body_is_const]
#[cfg_attr(bootstrap, default_method_body_is_const)]
fn lt(&self, other: &Rhs) -> bool {
matches!(self.partial_cmp(other), Some(Less))
}
Expand All @@ -1116,7 +1118,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
#[inline]
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
#[default_method_body_is_const]
#[cfg_attr(bootstrap, default_method_body_is_const)]
fn le(&self, other: &Rhs) -> bool {
// Pattern `Some(Less | Eq)` optimizes worse than negating `None | Some(Greater)`.
// FIXME: The root cause was fixed upstream in LLVM with:
Expand All @@ -1139,7 +1141,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
#[inline]
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
#[default_method_body_is_const]
#[cfg_attr(bootstrap, default_method_body_is_const)]
fn gt(&self, other: &Rhs) -> bool {
matches!(self.partial_cmp(other), Some(Greater))
}
Expand All @@ -1159,7 +1161,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
#[inline]
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
#[default_method_body_is_const]
#[cfg_attr(bootstrap, default_method_body_is_const)]
fn ge(&self, other: &Rhs) -> bool {
matches!(self.partial_cmp(other), Some(Greater | Equal))
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/rfc-2632-const-trait-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ pub struct S<T>(T);
// @has - '//pre[@class="rust trait"]/code/a[@class="trait"]' 'Clone'
// @!has - '//pre[@class="rust trait"]/code/span[@class="where"]' '~const'
// @has - '//pre[@class="rust trait"]/code/span[@class="where"]' ': Clone'
#[const_trait]
pub trait Tr<T> {
// @!has - '//div[@id="method.a"]/h4[@class="code-header"]' '~const'
// @has - '//div[@id="method.a"]/h4[@class="code-header"]/a[@class="trait"]' 'Clone'
// @!has - '//div[@id="method.a"]/h4[@class="code-header"]/span[@class="where"]' '~const'
// @has - '//div[@id="method.a"]/h4[@class="code-header"]/span[@class="where fmt-newline"]' ': Clone'
#[default_method_body_is_const]
fn a<A: ~const Clone + ~const Destruct>()
where
Option<A>: ~const Clone + ~const Destruct,
Expand Down
Loading