diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index c79b2624f8cb0..5f6d8ac751e26 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -85,8 +85,10 @@ pub fn is_const_evaluatable<'cx, 'tcx>( } else if leaf.has_param_types_or_consts() { failure_kind = cmp::min(failure_kind, FailureKind::MentionsParam); } + + false } - Node::Binop(_, _, _) | Node::UnaryOp(_, _) | Node::FunctionCall(_, _) => (), + Node::Binop(_, _, _) | Node::UnaryOp(_, _) | Node::FunctionCall(_, _) => false, }); match failure_kind { @@ -194,12 +196,12 @@ pub fn is_const_evaluatable<'cx, 'tcx>( /// /// This is only able to represent a subset of `MIR`, /// and should not leak any information about desugarings. -#[derive(Clone, Copy)] +#[derive(Debug, Clone, Copy)] pub struct AbstractConst<'tcx> { // FIXME: Consider adding something like `IndexSlice` // and use this here. - inner: &'tcx [Node<'tcx>], - substs: SubstsRef<'tcx>, + pub inner: &'tcx [Node<'tcx>], + pub substs: SubstsRef<'tcx>, } impl AbstractConst<'tcx> { @@ -209,9 +211,21 @@ impl AbstractConst<'tcx> { substs: SubstsRef<'tcx>, ) -> Result>, ErrorReported> { let inner = tcx.mir_abstract_const_opt_const_arg(def)?; + debug!("AbstractConst::new({:?}) = {:?}", def, inner); Ok(inner.map(|inner| AbstractConst { inner, substs })) } + pub fn from_const( + tcx: TyCtxt<'tcx>, + ct: &ty::Const<'tcx>, + ) -> Result>, ErrorReported> { + match ct.val { + ty::ConstKind::Unevaluated(def, substs, None) => AbstractConst::new(tcx, def, substs), + ty::ConstKind::Error(_) => Err(ErrorReported), + _ => Ok(None), + } + } + #[inline] pub fn subtree(self, node: NodeId) -> AbstractConst<'tcx> { AbstractConst { inner: &self.inner[..=node.index()], substs: self.substs } @@ -550,31 +564,32 @@ pub(super) fn try_unify_abstract_consts<'tcx>( // on `ErrorReported`. } -fn walk_abstract_const<'tcx, F>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, mut f: F) +// FIXME: Use `std::ops::ControlFlow` instead of `bool` here. +pub fn walk_abstract_const<'tcx, F>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, mut f: F) -> bool where - F: FnMut(Node<'tcx>), + F: FnMut(Node<'tcx>) -> bool, { - recurse(tcx, ct, &mut f); - fn recurse<'tcx>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, f: &mut dyn FnMut(Node<'tcx>)) { + fn recurse<'tcx>( + tcx: TyCtxt<'tcx>, + ct: AbstractConst<'tcx>, + f: &mut dyn FnMut(Node<'tcx>) -> bool, + ) -> bool { let root = ct.root(); - f(root); - match root { - Node::Leaf(_) => (), - Node::Binop(_, l, r) => { - recurse(tcx, ct.subtree(l), f); - recurse(tcx, ct.subtree(r), f); - } - Node::UnaryOp(_, v) => { - recurse(tcx, ct.subtree(v), f); - } - Node::FunctionCall(func, args) => { - recurse(tcx, ct.subtree(func), f); - for &arg in args { - recurse(tcx, ct.subtree(arg), f); + f(root) + || match root { + Node::Leaf(_) => false, + Node::Binop(_, l, r) => { + recurse(tcx, ct.subtree(l), f) || recurse(tcx, ct.subtree(r), f) + } + Node::UnaryOp(_, v) => recurse(tcx, ct.subtree(v), f), + Node::FunctionCall(func, args) => { + recurse(tcx, ct.subtree(func), f) + || args.iter().any(|&arg| recurse(tcx, ct.subtree(arg), f)) } } - } } + + recurse(tcx, ct, &mut f) } /// Tries to unify two abstract constants using structural equality. diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index d1647e686a84f..d2ac24b6100da 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -11,6 +11,7 @@ use super::elaborate_predicates; use crate::infer::TyCtxtInferExt; +use crate::traits::const_evaluatable::{self, AbstractConst}; use crate::traits::query::evaluate_obligation::InferCtxtExt; use crate::traits::{self, Obligation, ObligationCause}; use rustc_errors::FatalError; @@ -249,7 +250,7 @@ fn predicates_reference_self( predicates .predicates .iter() - .map(|(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), *sp)) + .map(|&(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), sp)) .filter_map(|predicate| predicate_references_self(tcx, predicate)) .collect() } @@ -260,7 +261,7 @@ fn bounds_reference_self(tcx: TyCtxt<'_>, trait_def_id: DefId) -> SmallVec<[Span .in_definition_order() .filter(|item| item.kind == ty::AssocKind::Type) .flat_map(|item| tcx.explicit_item_bounds(item.def_id)) - .map(|(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), *sp)) + .map(|&(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), sp)) .filter_map(|predicate| predicate_references_self(tcx, predicate)) .collect() } @@ -415,7 +416,7 @@ fn virtual_call_violation_for_method<'tcx>( )); } - for (i, input_ty) in sig.skip_binder().inputs()[1..].iter().enumerate() { + for (i, &input_ty) in sig.skip_binder().inputs()[1..].iter().enumerate() { if contains_illegal_self_type_reference(tcx, trait_def_id, input_ty) { return Some(MethodViolationCode::ReferencesSelfInput(i)); } @@ -438,10 +439,7 @@ fn virtual_call_violation_for_method<'tcx>( // so outlives predicates will always hold. .cloned() .filter(|(p, _)| p.to_opt_type_outlives().is_none()) - .collect::>() - // Do a shallow visit so that `contains_illegal_self_type_reference` - // may apply it's custom visiting. - .visit_tys_shallow(|t| contains_illegal_self_type_reference(tcx, trait_def_id, t)) + .any(|pred| contains_illegal_self_type_reference(tcx, trait_def_id, pred)) { return Some(MethodViolationCode::WhereClauseReferencesSelf); } @@ -715,10 +713,10 @@ fn receiver_is_dispatchable<'tcx>( }) } -fn contains_illegal_self_type_reference<'tcx>( +fn contains_illegal_self_type_reference<'tcx, T: TypeFoldable<'tcx>>( tcx: TyCtxt<'tcx>, trait_def_id: DefId, - ty: Ty<'tcx>, + value: T, ) -> bool { // This is somewhat subtle. In general, we want to forbid // references to `Self` in the argument and return types, @@ -761,7 +759,6 @@ fn contains_illegal_self_type_reference<'tcx>( struct IllegalSelfTypeVisitor<'tcx> { tcx: TyCtxt<'tcx>, - self_ty: Ty<'tcx>, trait_def_id: DefId, supertraits: Option>>, } @@ -769,7 +766,7 @@ fn contains_illegal_self_type_reference<'tcx>( impl<'tcx> TypeVisitor<'tcx> for IllegalSelfTypeVisitor<'tcx> { fn visit_ty(&mut self, t: Ty<'tcx>) -> bool { match t.kind() { - ty::Param(_) => t == self.self_ty, + ty::Param(_) => t == self.tcx.types.self_param, ty::Projection(ref data) => { // This is a projected type `::X`. @@ -802,22 +799,62 @@ fn contains_illegal_self_type_reference<'tcx>( } } - fn visit_const(&mut self, _c: &ty::Const<'tcx>) -> bool { - // FIXME(#72219) Look into the unevaluated constants for object safety violations. - // Do not walk substitutions of unevaluated consts, as they contain `Self`, even - // though the const expression doesn't necessary use it. Currently type variables - // inside array length expressions are forbidden, so they can't break the above - // rules. - false + fn visit_const(&mut self, ct: &ty::Const<'tcx>) -> bool { + // First check if the type of this constant references `Self`. + if self.visit_ty(ct.ty) { + return true; + } + + // Constants can only influence object safety if they reference `Self`. + // This is only possible for unevaluated constants, so we walk these here. + // + // If `AbstractConst::new` returned an error we already failed compilation + // so we don't have to emit an additional error here. + // + // We currently recurse into abstract consts here but do not recurse in + // `is_const_evaluatable`. This means that the object safety check is more + // liberal than the const eval check. + // + // This shouldn't really matter though as we can't really use any + // constants which are not considered const evaluatable. + use rustc_middle::mir::abstract_const::Node; + if let Ok(Some(ct)) = AbstractConst::from_const(self.tcx, ct) { + const_evaluatable::walk_abstract_const(self.tcx, ct, |node| match node { + Node::Leaf(leaf) => { + let leaf = leaf.subst(self.tcx, ct.substs); + self.visit_const(leaf) + } + Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => false, + }) + } else { + false + } + } + + fn visit_predicate(&mut self, pred: ty::Predicate<'tcx>) -> bool { + if let ty::PredicateAtom::ConstEvaluatable(def, substs) = pred.skip_binders() { + // FIXME(const_evaluatable_checked): We should probably deduplicate the logic for + // `AbstractConst`s here, it might make sense to change `ConstEvaluatable` to + // take a `ty::Const` instead. + use rustc_middle::mir::abstract_const::Node; + if let Ok(Some(ct)) = AbstractConst::new(self.tcx, def, substs) { + const_evaluatable::walk_abstract_const(self.tcx, ct, |node| match node { + Node::Leaf(leaf) => { + let leaf = leaf.subst(self.tcx, ct.substs); + self.visit_const(leaf) + } + Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => false, + }) + } else { + false + } + } else { + pred.super_visit_with(self) + } } } - ty.visit_with(&mut IllegalSelfTypeVisitor { - tcx, - self_ty: tcx.types.self_param, - trait_def_id, - supertraits: None, - }) + value.visit_with(&mut IllegalSelfTypeVisitor { tcx, trait_def_id, supertraits: None }) } pub fn provide(providers: &mut ty::query::Providers) { diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index b30fb7be273f1..cde0faa8caca7 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2090,25 +2090,25 @@ fn const_evaluatable_predicates_of<'tcx>( if let hir::Node::Item(item) = node { if let hir::ItemKind::Impl { ref of_trait, ref self_ty, .. } = item.kind { if let Some(of_trait) = of_trait { - warn!("const_evaluatable_predicates_of({:?}): visit impl trait_ref", def_id); + debug!("const_evaluatable_predicates_of({:?}): visit impl trait_ref", def_id); collector.visit_trait_ref(of_trait); } - warn!("const_evaluatable_predicates_of({:?}): visit_self_ty", def_id); + debug!("const_evaluatable_predicates_of({:?}): visit_self_ty", def_id); collector.visit_ty(self_ty); } } if let Some(generics) = node.generics() { - warn!("const_evaluatable_predicates_of({:?}): visit_generics", def_id); + debug!("const_evaluatable_predicates_of({:?}): visit_generics", def_id); collector.visit_generics(generics); } if let Some(fn_sig) = tcx.hir().fn_sig_by_hir_id(hir_id) { - warn!("const_evaluatable_predicates_of({:?}): visit_fn_decl", def_id); + debug!("const_evaluatable_predicates_of({:?}): visit_fn_decl", def_id); collector.visit_fn_decl(fn_sig.decl); } - warn!("const_evaluatable_predicates_of({:?}) = {:?}", def_id, collector.preds); + debug!("const_evaluatable_predicates_of({:?}) = {:?}", def_id, collector.preds); collector.preds } diff --git a/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.rs b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.rs new file mode 100644 index 0000000000000..5be4b41784c27 --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.rs @@ -0,0 +1,21 @@ +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] + + +const fn bar() -> usize { 7 } + +trait Foo { + fn test(&self) -> [u8; bar::()]; +} + +impl Foo for () { + fn test(&self) -> [u8; bar::()] { + [0; bar::()] + } +} + +fn use_dyn(v: &dyn Foo) { //~ERROR the trait `Foo` cannot be made into an object + v.test(); +} + +fn main() {} diff --git a/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.stderr b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.stderr new file mode 100644 index 0000000000000..e0e6029252c00 --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.stderr @@ -0,0 +1,18 @@ +error[E0038]: the trait `Foo` cannot be made into an object + --> $DIR/object-safety-err-ret.rs:17:15 + | +LL | fn use_dyn(v: &dyn Foo) { + | ^^^^^^^^ `Foo` cannot be made into an object + | + = help: consider moving `test` to another trait +note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit + --> $DIR/object-safety-err-ret.rs:8:23 + | +LL | trait Foo { + | --- this trait cannot be made into an object... +LL | fn test(&self) -> [u8; bar::()]; + | ^^^^^^^^^^^^^^^^^^^ ...because method `test` references the `Self` type in its return type + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0038`. diff --git a/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.rs b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.rs new file mode 100644 index 0000000000000..5fbd4a5fa2e64 --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.rs @@ -0,0 +1,22 @@ +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] +#![deny(where_clauses_object_safety)] + + +const fn bar() -> usize { 7 } + +trait Foo { + fn test(&self) where [u8; bar::()]: Sized; + //~^ ERROR the trait `Foo` cannot be made into an object + //~| WARN this was previously accepted by the compiler but is being phased out +} + +impl Foo for () { + fn test(&self) where [u8; bar::()]: Sized {} +} + +fn use_dyn(v: &dyn Foo) { + v.test(); +} + +fn main() {} diff --git a/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.stderr b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.stderr new file mode 100644 index 0000000000000..45c7d835f339a --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.stderr @@ -0,0 +1,24 @@ +error: the trait `Foo` cannot be made into an object + --> $DIR/object-safety-err-where-bounds.rs:9:8 + | +LL | fn test(&self) where [u8; bar::()]: Sized; + | ^^^^ + | +note: the lint level is defined here + --> $DIR/object-safety-err-where-bounds.rs:3:9 + | +LL | #![deny(where_clauses_object_safety)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #51443 +note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit + --> $DIR/object-safety-err-where-bounds.rs:9:8 + | +LL | trait Foo { + | --- this trait cannot be made into an object... +LL | fn test(&self) where [u8; bar::()]: Sized; + | ^^^^ ...because method `test` references the `Self` type in its `where` clause + = help: consider moving `test` to another trait + +error: aborting due to previous error + diff --git a/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok-infer-err.rs b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok-infer-err.rs new file mode 100644 index 0000000000000..9a95908d59d0d --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok-infer-err.rs @@ -0,0 +1,22 @@ +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] + +trait Foo { + fn test(&self) -> [u8; N + 1]; +} + +impl Foo for () { + fn test(&self) -> [u8; N + 1] { + [0; N + 1] + } +} + +fn use_dyn(v: &dyn Foo) where [u8; N + 1]: Sized { + assert_eq!(v.test(), [0; N + 1]); +} + +fn main() { + // FIXME(const_evaluatable_checked): Improve the error message here. + use_dyn(&()); + //~^ ERROR type annotations needed +} diff --git a/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok-infer-err.stderr b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok-infer-err.stderr new file mode 100644 index 0000000000000..dd2c11e42c5d2 --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok-infer-err.stderr @@ -0,0 +1,12 @@ +error[E0284]: type annotations needed: cannot satisfy `the constant `use_dyn::<{_: usize}>::{constant#0}` can be evaluated` + --> $DIR/object-safety-ok-infer-err.rs:20:5 + | +LL | fn use_dyn(v: &dyn Foo) where [u8; N + 1]: Sized { + | ----- required by this bound in `use_dyn` +... +LL | use_dyn(&()); + | ^^^^^^^ cannot satisfy `the constant `use_dyn::<{_: usize}>::{constant#0}` can be evaluated` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0284`. diff --git a/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok.rs b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok.rs new file mode 100644 index 0000000000000..ae78b7936a289 --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok.rs @@ -0,0 +1,21 @@ +// run-pass +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] + +trait Foo { + fn test(&self) -> [u8; N + 1]; +} + +impl Foo for () { + fn test(&self) -> [u8; N + 1] { + [0; N + 1] + } +} + +fn use_dyn(v: &dyn Foo) where [u8; N + 1]: Sized { + assert_eq!(v.test(), [0; N + 1]); +} + +fn main() { + use_dyn::<3>(&()); +}