Skip to content

Commit 1a64e57

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of #78365 - lcnr:const-eval-obj-safety, r=oli-obk
check object safety of generic constants As `Self` can only be effectively used in constants with `const_evaluatable_checked` this should not matter outside of it. Implements the first item of #72219 > Object safety interactions with constants r? @oli-obk for now cc @nikomatsakis
2 parents 54ea0f9 + 60bcc58 commit 1a64e57

File tree

10 files changed

+244
-52
lines changed

10 files changed

+244
-52
lines changed

‎compiler/rustc_trait_selection/src/traits/const_evaluatable.rs

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ pub fn is_const_evaluatable<'cx, 'tcx>(
8585
} else if leaf.has_param_types_or_consts() {
8686
failure_kind = cmp::min(failure_kind, FailureKind::MentionsParam);
8787
}
88+
89+
false
8890
}
89-
Node::Binop(_, _, _) | Node::UnaryOp(_, _) | Node::FunctionCall(_, _) => (),
91+
Node::Binop(_, _, _) | Node::UnaryOp(_, _) | Node::FunctionCall(_, _) => false,
9092
});
9193

9294
match failure_kind {
@@ -194,12 +196,12 @@ pub fn is_const_evaluatable<'cx, 'tcx>(
194196
///
195197
/// This is only able to represent a subset of `MIR`,
196198
/// and should not leak any information about desugarings.
197-
#[derive(Clone, Copy)]
199+
#[derive(Debug, Clone, Copy)]
198200
pub struct AbstractConst<'tcx> {
199201
// FIXME: Consider adding something like `IndexSlice`
200202
// and use this here.
201-
inner: &'tcx [Node<'tcx>],
202-
substs: SubstsRef<'tcx>,
203+
pub inner: &'tcx [Node<'tcx>],
204+
pub substs: SubstsRef<'tcx>,
203205
}
204206

205207
impl AbstractConst<'tcx> {
@@ -209,9 +211,21 @@ impl AbstractConst<'tcx> {
209211
substs: SubstsRef<'tcx>,
210212
) -> Result<Option<AbstractConst<'tcx>>, ErrorReported> {
211213
let inner = tcx.mir_abstract_const_opt_const_arg(def)?;
214+
debug!("AbstractConst::new({:?}) = {:?}", def, inner);
212215
Ok(inner.map(|inner| AbstractConst { inner, substs }))
213216
}
214217

218+
pub fn from_const(
219+
tcx: TyCtxt<'tcx>,
220+
ct: &ty::Const<'tcx>,
221+
) -> Result<Option<AbstractConst<'tcx>>, ErrorReported> {
222+
match ct.val {
223+
ty::ConstKind::Unevaluated(def, substs, None) => AbstractConst::new(tcx, def, substs),
224+
ty::ConstKind::Error(_) => Err(ErrorReported),
225+
_ => Ok(None),
226+
}
227+
}
228+
215229
#[inline]
216230
pub fn subtree(self, node: NodeId) -> AbstractConst<'tcx> {
217231
AbstractConst { inner: &self.inner[..=node.index()], substs: self.substs }
@@ -550,31 +564,32 @@ pub(super) fn try_unify_abstract_consts<'tcx>(
550564
// on `ErrorReported`.
551565
}
552566

553-
fn walk_abstract_const<'tcx, F>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, mut f: F)
567+
// FIXME: Use `std::ops::ControlFlow` instead of `bool` here.
568+
pub fn walk_abstract_const<'tcx, F>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, mut f: F) -> bool
554569
where
555-
F: FnMut(Node<'tcx>),
570+
F: FnMut(Node<'tcx>) -> bool,
556571
{
557-
recurse(tcx, ct, &mut f);
558-
fn recurse<'tcx>(tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, f: &mut dyn FnMut(Node<'tcx>)) {
572+
fn recurse<'tcx>(
573+
tcx: TyCtxt<'tcx>,
574+
ct: AbstractConst<'tcx>,
575+
f: &mut dyn FnMut(Node<'tcx>) -> bool,
576+
) -> bool {
559577
let root = ct.root();
560-
f(root);
561-
match root {
562-
Node::Leaf(_) => (),
563-
Node::Binop(_, l, r) => {
564-
recurse(tcx, ct.subtree(l), f);
565-
recurse(tcx, ct.subtree(r), f);
566-
}
567-
Node::UnaryOp(_, v) => {
568-
recurse(tcx, ct.subtree(v), f);
569-
}
570-
Node::FunctionCall(func, args) => {
571-
recurse(tcx, ct.subtree(func), f);
572-
for &arg in args {
573-
recurse(tcx, ct.subtree(arg), f);
578+
f(root)
579+
|| match root {
580+
Node::Leaf(_) => false,
581+
Node::Binop(_, l, r) => {
582+
recurse(tcx, ct.subtree(l), f) || recurse(tcx, ct.subtree(r), f)
583+
}
584+
Node::UnaryOp(_, v) => recurse(tcx, ct.subtree(v), f),
585+
Node::FunctionCall(func, args) => {
586+
recurse(tcx, ct.subtree(func), f)
587+
|| args.iter().any(|&arg| recurse(tcx, ct.subtree(arg), f))
574588
}
575589
}
576-
}
577590
}
591+
592+
recurse(tcx, ct, &mut f)
578593
}
579594

580595
/// Tries to unify two abstract constants using structural equality.

‎compiler/rustc_trait_selection/src/traits/object_safety.rs

Lines changed: 61 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use super::elaborate_predicates;
1212

1313
use crate::infer::TyCtxtInferExt;
14+
use crate::traits::const_evaluatable::{self, AbstractConst};
1415
use crate::traits::query::evaluate_obligation::InferCtxtExt;
1516
use crate::traits::{self, Obligation, ObligationCause};
1617
use rustc_errors::FatalError;
@@ -249,7 +250,7 @@ fn predicates_reference_self(
249250
predicates
250251
.predicates
251252
.iter()
252-
.map(|(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), *sp))
253+
.map(|&(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), sp))
253254
.filter_map(|predicate| predicate_references_self(tcx, predicate))
254255
.collect()
255256
}
@@ -260,7 +261,7 @@ fn bounds_reference_self(tcx: TyCtxt<'_>, trait_def_id: DefId) -> SmallVec<[Span
260261
.in_definition_order()
261262
.filter(|item| item.kind == ty::AssocKind::Type)
262263
.flat_map(|item| tcx.explicit_item_bounds(item.def_id))
263-
.map(|(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), *sp))
264+
.map(|&(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), sp))
264265
.filter_map(|predicate| predicate_references_self(tcx, predicate))
265266
.collect()
266267
}
@@ -415,7 +416,7 @@ fn virtual_call_violation_for_method<'tcx>(
415416
));
416417
}
417418

418-
for (i, input_ty) in sig.skip_binder().inputs()[1..].iter().enumerate() {
419+
for (i, &input_ty) in sig.skip_binder().inputs()[1..].iter().enumerate() {
419420
if contains_illegal_self_type_reference(tcx, trait_def_id, input_ty) {
420421
return Some(MethodViolationCode::ReferencesSelfInput(i));
421422
}
@@ -438,10 +439,7 @@ fn virtual_call_violation_for_method<'tcx>(
438439
// so outlives predicates will always hold.
439440
.cloned()
440441
.filter(|(p, _)| p.to_opt_type_outlives().is_none())
441-
.collect::<Vec<_>>()
442-
// Do a shallow visit so that `contains_illegal_self_type_reference`
443-
// may apply it's custom visiting.
444-
.visit_tys_shallow(|t| contains_illegal_self_type_reference(tcx, trait_def_id, t))
442+
.any(|pred| contains_illegal_self_type_reference(tcx, trait_def_id, pred))
445443
{
446444
return Some(MethodViolationCode::WhereClauseReferencesSelf);
447445
}
@@ -715,10 +713,10 @@ fn receiver_is_dispatchable<'tcx>(
715713
})
716714
}
717715

718-
fn contains_illegal_self_type_reference<'tcx>(
716+
fn contains_illegal_self_type_reference<'tcx, T: TypeFoldable<'tcx>>(
719717
tcx: TyCtxt<'tcx>,
720718
trait_def_id: DefId,
721-
ty: Ty<'tcx>,
719+
value: T,
722720
) -> bool {
723721
// This is somewhat subtle. In general, we want to forbid
724722
// references to `Self` in the argument and return types,
@@ -761,15 +759,14 @@ fn contains_illegal_self_type_reference<'tcx>(
761759

762760
struct IllegalSelfTypeVisitor<'tcx> {
763761
tcx: TyCtxt<'tcx>,
764-
self_ty: Ty<'tcx>,
765762
trait_def_id: DefId,
766763
supertraits: Option<Vec<ty::PolyTraitRef<'tcx>>>,
767764
}
768765

769766
impl<'tcx> TypeVisitor<'tcx> for IllegalSelfTypeVisitor<'tcx> {
770767
fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
771768
match t.kind() {
772-
ty::Param(_) => t == self.self_ty,
769+
ty::Param(_) => t == self.tcx.types.self_param,
773770
ty::Projection(ref data) => {
774771
// This is a projected type `<Foo as SomeTrait>::X`.
775772

@@ -802,22 +799,62 @@ fn contains_illegal_self_type_reference<'tcx>(
802799
}
803800
}
804801

805-
fn visit_const(&mut self, _c: &ty::Const<'tcx>) -> bool {
806-
// FIXME(#72219) Look into the unevaluated constants for object safety violations.
807-
// Do not walk substitutions of unevaluated consts, as they contain `Self`, even
808-
// though the const expression doesn't necessary use it. Currently type variables
809-
// inside array length expressions are forbidden, so they can't break the above
810-
// rules.
811-
false
802+
fn visit_const(&mut self, ct: &ty::Const<'tcx>) -> bool {
803+
// First check if the type of this constant references `Self`.
804+
if self.visit_ty(ct.ty) {
805+
return true;
806+
}
807+
808+
// Constants can only influence object safety if they reference `Self`.
809+
// This is only possible for unevaluated constants, so we walk these here.
810+
//
811+
// If `AbstractConst::new` returned an error we already failed compilation
812+
// so we don't have to emit an additional error here.
813+
//
814+
// We currently recurse into abstract consts here but do not recurse in
815+
// `is_const_evaluatable`. This means that the object safety check is more
816+
// liberal than the const eval check.
817+
//
818+
// This shouldn't really matter though as we can't really use any
819+
// constants which are not considered const evaluatable.
820+
use rustc_middle::mir::abstract_const::Node;
821+
if let Ok(Some(ct)) = AbstractConst::from_const(self.tcx, ct) {
822+
const_evaluatable::walk_abstract_const(self.tcx, ct, |node| match node {
823+
Node::Leaf(leaf) => {
824+
let leaf = leaf.subst(self.tcx, ct.substs);
825+
self.visit_const(leaf)
826+
}
827+
Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => false,
828+
})
829+
} else {
830+
false
831+
}
832+
}
833+
834+
fn visit_predicate(&mut self, pred: ty::Predicate<'tcx>) -> bool {
835+
if let ty::PredicateAtom::ConstEvaluatable(def, substs) = pred.skip_binders() {
836+
// FIXME(const_evaluatable_checked): We should probably deduplicate the logic for
837+
// `AbstractConst`s here, it might make sense to change `ConstEvaluatable` to
838+
// take a `ty::Const` instead.
839+
use rustc_middle::mir::abstract_const::Node;
840+
if let Ok(Some(ct)) = AbstractConst::new(self.tcx, def, substs) {
841+
const_evaluatable::walk_abstract_const(self.tcx, ct, |node| match node {
842+
Node::Leaf(leaf) => {
843+
let leaf = leaf.subst(self.tcx, ct.substs);
844+
self.visit_const(leaf)
845+
}
846+
Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => false,
847+
})
848+
} else {
849+
false
850+
}
851+
} else {
852+
pred.super_visit_with(self)
853+
}
812854
}
813855
}
814856

815-
ty.visit_with(&mut IllegalSelfTypeVisitor {
816-
tcx,
817-
self_ty: tcx.types.self_param,
818-
trait_def_id,
819-
supertraits: None,
820-
})
857+
value.visit_with(&mut IllegalSelfTypeVisitor { tcx, trait_def_id, supertraits: None })
821858
}
822859

823860
pub fn provide(providers: &mut ty::query::Providers) {

‎compiler/rustc_typeck/src/collect.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,25 +2090,25 @@ fn const_evaluatable_predicates_of<'tcx>(
20902090
if let hir::Node::Item(item) = node {
20912091
if let hir::ItemKind::Impl { ref of_trait, ref self_ty, .. } = item.kind {
20922092
if let Some(of_trait) = of_trait {
2093-
warn!("const_evaluatable_predicates_of({:?}): visit impl trait_ref", def_id);
2093+
debug!("const_evaluatable_predicates_of({:?}): visit impl trait_ref", def_id);
20942094
collector.visit_trait_ref(of_trait);
20952095
}
20962096

2097-
warn!("const_evaluatable_predicates_of({:?}): visit_self_ty", def_id);
2097+
debug!("const_evaluatable_predicates_of({:?}): visit_self_ty", def_id);
20982098
collector.visit_ty(self_ty);
20992099
}
21002100
}
21012101

21022102
if let Some(generics) = node.generics() {
2103-
warn!("const_evaluatable_predicates_of({:?}): visit_generics", def_id);
2103+
debug!("const_evaluatable_predicates_of({:?}): visit_generics", def_id);
21042104
collector.visit_generics(generics);
21052105
}
21062106

21072107
if let Some(fn_sig) = tcx.hir().fn_sig_by_hir_id(hir_id) {
2108-
warn!("const_evaluatable_predicates_of({:?}): visit_fn_decl", def_id);
2108+
debug!("const_evaluatable_predicates_of({:?}): visit_fn_decl", def_id);
21092109
collector.visit_fn_decl(fn_sig.decl);
21102110
}
2111-
warn!("const_evaluatable_predicates_of({:?}) = {:?}", def_id, collector.preds);
2111+
debug!("const_evaluatable_predicates_of({:?}) = {:?}", def_id, collector.preds);
21122112

21132113
collector.preds
21142114
}

‎src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![feature(const_generics, const_evaluatable_checked)]
2+
#![allow(incomplete_features)]
3+
4+
5+
const fn bar<T: ?Sized>() -> usize { 7 }
6+
7+
trait Foo {
8+
fn test(&self) -> [u8; bar::<Self>()];
9+
}
10+
11+
impl Foo for () {
12+
fn test(&self) -> [u8; bar::<Self>()] {
13+
[0; bar::<Self>()]
14+
}
15+
}
16+
17+
fn use_dyn(v: &dyn Foo) { //~ERROR the trait `Foo` cannot be made into an object
18+
v.test();
19+
}
20+
21+
fn main() {}

‎src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-ret.stderr

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error[E0038]: the trait `Foo` cannot be made into an object
2+
--> $DIR/object-safety-err-ret.rs:17:15
3+
|
4+
LL | fn use_dyn(v: &dyn Foo) {
5+
| ^^^^^^^^ `Foo` cannot be made into an object
6+
|
7+
= help: consider moving `test` to another trait
8+
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 <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
9+
--> $DIR/object-safety-err-ret.rs:8:23
10+
|
11+
LL | trait Foo {
12+
| --- this trait cannot be made into an object...
13+
LL | fn test(&self) -> [u8; bar::<Self>()];
14+
| ^^^^^^^^^^^^^^^^^^^ ...because method `test` references the `Self` type in its return type
15+
16+
error: aborting due to previous error
17+
18+
For more information about this error, try `rustc --explain E0038`.

‎src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#![feature(const_generics, const_evaluatable_checked)]
2+
#![allow(incomplete_features)]
3+
#![deny(where_clauses_object_safety)]
4+
5+
6+
const fn bar<T: ?Sized>() -> usize { 7 }
7+
8+
trait Foo {
9+
fn test(&self) where [u8; bar::<Self>()]: Sized;
10+
//~^ ERROR the trait `Foo` cannot be made into an object
11+
//~| WARN this was previously accepted by the compiler but is being phased out
12+
}
13+
14+
impl Foo for () {
15+
fn test(&self) where [u8; bar::<Self>()]: Sized {}
16+
}
17+
18+
fn use_dyn(v: &dyn Foo) {
19+
v.test();
20+
}
21+
22+
fn main() {}

‎src/test/ui/const-generics/const_evaluatable_checked/object-safety-err-where-bounds.stderr

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
error: the trait `Foo` cannot be made into an object
2+
--> $DIR/object-safety-err-where-bounds.rs:9:8
3+
|
4+
LL | fn test(&self) where [u8; bar::<Self>()]: Sized;
5+
| ^^^^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/object-safety-err-where-bounds.rs:3:9
9+
|
10+
LL | #![deny(where_clauses_object_safety)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
13+
= note: for more information, see issue #51443 <https://github.com/rust-lang/rust/issues/51443>
14+
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 <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
15+
--> $DIR/object-safety-err-where-bounds.rs:9:8
16+
|
17+
LL | trait Foo {
18+
| --- this trait cannot be made into an object...
19+
LL | fn test(&self) where [u8; bar::<Self>()]: Sized;
20+
| ^^^^ ...because method `test` references the `Self` type in its `where` clause
21+
= help: consider moving `test` to another trait
22+
23+
error: aborting due to previous error
24+

‎src/test/ui/const-generics/const_evaluatable_checked/object-safety-ok-infer-err.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#![feature(const_generics, const_evaluatable_checked)]
2+
#![allow(incomplete_features)]
3+
4+
trait Foo<const N: usize> {
5+
fn test(&self) -> [u8; N + 1];
6+
}
7+
8+
impl<const N: usize> Foo<N> for () {
9+
fn test(&self) -> [u8; N + 1] {
10+
[0; N + 1]
11+
}
12+
}
13+
14+
fn use_dyn<const N: usize>(v: &dyn Foo<N>) where [u8; N + 1]: Sized {
15+
assert_eq!(v.test(), [0; N + 1]);
16+
}
17+
18+
fn main() {
19+
// FIXME(const_evaluatable_checked): Improve the error message here.
20+
use_dyn(&());
21+
//~^ ERROR type annotations needed
22+
}

0 commit comments

Comments
 (0)