From 85d6029c136f155a2b994be8ef7b2ed0876428bf Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Sat, 4 Sep 2021 15:44:26 +0100 Subject: [PATCH 1/7] `AbstractConst::root`: Always run `subst` when `Node` is `Leaf` --- compiler/rustc_privacy/src/lib.rs | 9 +++---- .../src/traits/const_evaluatable.rs | 25 +++++++++---------- .../src/traits/object_safety.rs | 18 +++++++------ 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 9c376c6c93e2e..96a398ddf0507 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -23,7 +23,7 @@ use rustc_middle::mir::abstract_const::Node as ACNode; use rustc_middle::span_bug; use rustc_middle::ty::fold::TypeVisitor; use rustc_middle::ty::query::Providers; -use rustc_middle::ty::subst::{InternalSubsts, Subst}; +use rustc_middle::ty::subst::InternalSubsts; use rustc_middle::ty::{self, Const, GenericParamDefKind, TraitRef, Ty, TyCtxt, TypeFoldable}; use rustc_session::lint; use rustc_span::hygiene::Transparency; @@ -153,11 +153,8 @@ where tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, ) -> ControlFlow { - const_evaluatable::walk_abstract_const(tcx, ct, |node| match node.root() { - ACNode::Leaf(leaf) => { - let leaf = leaf.subst(tcx, ct.substs); - self.visit_const(leaf) - } + const_evaluatable::walk_abstract_const(tcx, ct, |node| match node.root(tcx, ct.substs) { + ACNode::Leaf(leaf) => self.visit_const(leaf), ACNode::Cast(_, _, ty) => self.visit_ty(ty), ACNode::Binop(..) | ACNode::UnaryOp(..) | ACNode::FunctionCall(_, _) => { ControlFlow::CONTINUE diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index ddabe5967d79c..f070e9dd5e5d5 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -8,6 +8,7 @@ //! In this case we try to build an abstract representation of this constant using //! `mir_abstract_const` which can then be checked for structural equality with other //! generic constants mentioned in the `caller_bounds` of the current environment. +use crate::traits::ty::subst::GenericArg; use rustc_errors::ErrorReported; use rustc_hir::def::DefKind; use rustc_index::bit_set::BitSet; @@ -80,9 +81,8 @@ pub fn is_const_evaluatable<'cx, 'tcx>( Concrete, } let mut failure_kind = FailureKind::Concrete; - walk_abstract_const::(tcx, ct, |node| match node.root() { + walk_abstract_const::(tcx, ct, |node| match node.root(tcx, ct.substs) { Node::Leaf(leaf) => { - let leaf = leaf.subst(tcx, ct.substs); if leaf.has_infer_types_or_consts() { failure_kind = FailureKind::MentionsInfer; } else if leaf.definitely_has_param_types_or_consts(tcx) { @@ -92,7 +92,6 @@ pub fn is_const_evaluatable<'cx, 'tcx>( ControlFlow::CONTINUE } Node::Cast(_, _, ty) => { - let ty = ty.subst(tcx, ct.substs); if ty.has_infer_types_or_consts() { failure_kind = FailureKind::MentionsInfer; } else if ty.definitely_has_param_types_or_consts(tcx) { @@ -218,8 +217,12 @@ impl<'tcx> AbstractConst<'tcx> { } #[inline] - pub fn root(self) -> Node<'tcx> { - self.inner.last().copied().unwrap() + pub fn root(self, tcx: TyCtxt<'tcx>, substs: &[GenericArg<'tcx>]) -> Node<'tcx> { + let mut node = self.inner.last().copied().unwrap(); + if let Node::Leaf(leaf) = node { + node = Node::Leaf(leaf.subst(tcx, substs)); + } + node } } @@ -587,7 +590,7 @@ where f: &mut dyn FnMut(AbstractConst<'tcx>) -> ControlFlow, ) -> ControlFlow { f(ct)?; - let root = ct.root(); + let root = ct.root(tcx, ct.substs); match root { Node::Leaf(_) => ControlFlow::CONTINUE, Node::Binop(_, l, r) => { @@ -615,16 +618,14 @@ pub(super) fn try_unify<'tcx>( // We substitute generics repeatedly to allow AbstractConsts to unify where a // ConstKind::Unevalated could be turned into an AbstractConst that would unify e.g. // Param(N) should unify with Param(T), substs: [Unevaluated("T2", [Unevaluated("T3", [Param(N)])])] - while let Node::Leaf(a_ct) = a.root() { - let a_ct = a_ct.subst(tcx, a.substs); + while let Node::Leaf(a_ct) = a.root(tcx, a.substs) { match AbstractConst::from_const(tcx, a_ct) { Ok(Some(a_act)) => a = a_act, Ok(None) => break, Err(_) => return true, } } - while let Node::Leaf(b_ct) = b.root() { - let b_ct = b_ct.subst(tcx, b.substs); + while let Node::Leaf(b_ct) = b.root(tcx, b.substs) { match AbstractConst::from_const(tcx, b_ct) { Ok(Some(b_act)) => b = b_act, Ok(None) => break, @@ -632,10 +633,8 @@ pub(super) fn try_unify<'tcx>( } } - match (a.root(), b.root()) { + match (a.root(tcx, a.substs), b.root(tcx, b.substs)) { (Node::Leaf(a_ct), Node::Leaf(b_ct)) => { - let a_ct = a_ct.subst(tcx, a.substs); - let b_ct = b_ct.subst(tcx, b.substs); if a_ct.ty != b_ct.ty { return false; } diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 57b8a84300ff9..dee95a3be386c 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -838,14 +838,16 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeFoldable<'tcx>>( // constants which are not considered const evaluatable. use rustc_middle::mir::abstract_const::Node; if let Ok(Some(ct)) = AbstractConst::new(self.tcx, uv.shrink()) { - const_evaluatable::walk_abstract_const(self.tcx, ct, |node| match node.root() { - Node::Leaf(leaf) => { - let leaf = leaf.subst(self.tcx, ct.substs); - self.visit_const(leaf) - } - Node::Cast(_, _, ty) => self.visit_ty(ty), - Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => { - ControlFlow::CONTINUE + const_evaluatable::walk_abstract_const(self.tcx, ct, |node| { + match node.root(self.tcx, ct.substs) { + Node::Leaf(leaf) => { + // let leaf = leaf.subst(self.tcx, ct.substs); + self.visit_const(leaf) + } + Node::Cast(_, _, ty) => self.visit_ty(ty), + Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => { + ControlFlow::CONTINUE + } } }) } else { From 22c27385734808a69c5520a8417dd3e45f01b536 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Sat, 4 Sep 2021 15:47:00 +0100 Subject: [PATCH 2/7] Minor cleanup: make imports more consistent --- compiler/rustc_trait_selection/src/traits/const_evaluatable.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index f070e9dd5e5d5..5f7a9a17e03aa 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -8,7 +8,6 @@ //! In this case we try to build an abstract representation of this constant using //! `mir_abstract_const` which can then be checked for structural equality with other //! generic constants mentioned in the `caller_bounds` of the current environment. -use crate::traits::ty::subst::GenericArg; use rustc_errors::ErrorReported; use rustc_hir::def::DefKind; use rustc_index::bit_set::BitSet; @@ -17,7 +16,7 @@ use rustc_infer::infer::InferCtxt; use rustc_middle::mir::abstract_const::{Node, NodeId, NotConstEvaluatable}; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::mir::{self, Rvalue, StatementKind, TerminatorKind}; -use rustc_middle::ty::subst::{Subst, SubstsRef}; +use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef}; use rustc_middle::ty::{self, TyCtxt, TypeFoldable}; use rustc_session::lint; use rustc_span::def_id::LocalDefId; From 6e4061819fa00c74c704507a72d4ee3ca60d60e5 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Sat, 4 Sep 2021 15:49:02 +0100 Subject: [PATCH 3/7] Remove left over comment --- compiler/rustc_trait_selection/src/traits/object_safety.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index dee95a3be386c..6c60394116b93 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -840,10 +840,7 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeFoldable<'tcx>>( if let Ok(Some(ct)) = AbstractConst::new(self.tcx, uv.shrink()) { const_evaluatable::walk_abstract_const(self.tcx, ct, |node| { match node.root(self.tcx, ct.substs) { - Node::Leaf(leaf) => { - // let leaf = leaf.subst(self.tcx, ct.substs); - self.visit_const(leaf) - } + Node::Leaf(leaf) => self.visit_const(leaf), Node::Cast(_, _, ty) => self.visit_ty(ty), Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => { ControlFlow::CONTINUE From fc5633fed6905b1dbf307cbce795e2545817e023 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Sat, 4 Sep 2021 16:28:55 +0100 Subject: [PATCH 4/7] Make fields on `AbstractConst` private --- compiler/rustc_privacy/src/lib.rs | 2 +- .../src/traits/const_evaluatable.rs | 22 +++++++++---------- .../src/traits/object_safety.rs | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 96a398ddf0507..e424a51d01c97 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -153,7 +153,7 @@ where tcx: TyCtxt<'tcx>, ct: AbstractConst<'tcx>, ) -> ControlFlow { - const_evaluatable::walk_abstract_const(tcx, ct, |node| match node.root(tcx, ct.substs) { + const_evaluatable::walk_abstract_const(tcx, ct, |node| match node.root(tcx) { ACNode::Leaf(leaf) => self.visit_const(leaf), ACNode::Cast(_, _, ty) => self.visit_ty(ty), ACNode::Binop(..) | ACNode::UnaryOp(..) | ACNode::FunctionCall(_, _) => { diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index 5f7a9a17e03aa..1adcda41102d2 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -16,7 +16,7 @@ use rustc_infer::infer::InferCtxt; use rustc_middle::mir::abstract_const::{Node, NodeId, NotConstEvaluatable}; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::mir::{self, Rvalue, StatementKind, TerminatorKind}; -use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef}; +use rustc_middle::ty::subst::{Subst, SubstsRef}; use rustc_middle::ty::{self, TyCtxt, TypeFoldable}; use rustc_session::lint; use rustc_span::def_id::LocalDefId; @@ -80,7 +80,7 @@ pub fn is_const_evaluatable<'cx, 'tcx>( Concrete, } let mut failure_kind = FailureKind::Concrete; - walk_abstract_const::(tcx, ct, |node| match node.root(tcx, ct.substs) { + walk_abstract_const::(tcx, ct, |node| match node.root(tcx) { Node::Leaf(leaf) => { if leaf.has_infer_types_or_consts() { failure_kind = FailureKind::MentionsInfer; @@ -185,8 +185,8 @@ pub fn is_const_evaluatable<'cx, 'tcx>( pub struct AbstractConst<'tcx> { // FIXME: Consider adding something like `IndexSlice` // and use this here. - pub inner: &'tcx [Node<'tcx>], - pub substs: SubstsRef<'tcx>, + inner: &'tcx [Node<'tcx>], + substs: SubstsRef<'tcx>, } impl<'tcx> AbstractConst<'tcx> { @@ -216,10 +216,10 @@ impl<'tcx> AbstractConst<'tcx> { } #[inline] - pub fn root(self, tcx: TyCtxt<'tcx>, substs: &[GenericArg<'tcx>]) -> Node<'tcx> { - let mut node = self.inner.last().copied().unwrap(); + pub fn root(self, tcx: TyCtxt<'tcx>) -> Node<'tcx> { + let node = self.inner.last().copied().unwrap(); if let Node::Leaf(leaf) = node { - node = Node::Leaf(leaf.subst(tcx, substs)); + return Node::Leaf(leaf.subst(tcx, self.substs)); } node } @@ -589,7 +589,7 @@ where f: &mut dyn FnMut(AbstractConst<'tcx>) -> ControlFlow, ) -> ControlFlow { f(ct)?; - let root = ct.root(tcx, ct.substs); + let root = ct.root(tcx); match root { Node::Leaf(_) => ControlFlow::CONTINUE, Node::Binop(_, l, r) => { @@ -617,14 +617,14 @@ pub(super) fn try_unify<'tcx>( // We substitute generics repeatedly to allow AbstractConsts to unify where a // ConstKind::Unevalated could be turned into an AbstractConst that would unify e.g. // Param(N) should unify with Param(T), substs: [Unevaluated("T2", [Unevaluated("T3", [Param(N)])])] - while let Node::Leaf(a_ct) = a.root(tcx, a.substs) { + while let Node::Leaf(a_ct) = a.root(tcx) { match AbstractConst::from_const(tcx, a_ct) { Ok(Some(a_act)) => a = a_act, Ok(None) => break, Err(_) => return true, } } - while let Node::Leaf(b_ct) = b.root(tcx, b.substs) { + while let Node::Leaf(b_ct) = b.root(tcx) { match AbstractConst::from_const(tcx, b_ct) { Ok(Some(b_act)) => b = b_act, Ok(None) => break, @@ -632,7 +632,7 @@ pub(super) fn try_unify<'tcx>( } } - match (a.root(tcx, a.substs), b.root(tcx, b.substs)) { + match (a.root(tcx), b.root(tcx)) { (Node::Leaf(a_ct), Node::Leaf(b_ct)) => { if a_ct.ty != b_ct.ty { return false; diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 6c60394116b93..4eaa3ea47325f 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -839,7 +839,7 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeFoldable<'tcx>>( use rustc_middle::mir::abstract_const::Node; if let Ok(Some(ct)) = AbstractConst::new(self.tcx, uv.shrink()) { const_evaluatable::walk_abstract_const(self.tcx, ct, |node| { - match node.root(self.tcx, ct.substs) { + match node.root(self.tcx) { Node::Leaf(leaf) => self.visit_const(leaf), Node::Cast(_, _, ty) => self.visit_ty(ty), Node::Binop(..) | Node::UnaryOp(..) | Node::FunctionCall(_, _) => { From b7d99983f879696522a5c770241a2fb171071eab Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Sat, 4 Sep 2021 23:13:15 +0100 Subject: [PATCH 5/7] Add line that was unintentionally removed --- compiler/rustc_trait_selection/src/traits/const_evaluatable.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index 1adcda41102d2..4acde1b216535 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -91,6 +91,7 @@ pub fn is_const_evaluatable<'cx, 'tcx>( ControlFlow::CONTINUE } Node::Cast(_, _, ty) => { + let ty = ty.subst(tcx, ct.substs); if ty.has_infer_types_or_consts() { failure_kind = FailureKind::MentionsInfer; } else if ty.definitely_has_param_types_or_consts(tcx) { From 99b8c016cea9b1f1917d287401b498abd4eabd29 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Tue, 19 Oct 2021 22:18:13 +0100 Subject: [PATCH 6/7] Address lcnr review --- .../rustc_trait_selection/src/traits/const_evaluatable.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index 4acde1b216535..2cf7b6b1d9393 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -219,10 +219,12 @@ impl<'tcx> AbstractConst<'tcx> { #[inline] pub fn root(self, tcx: TyCtxt<'tcx>) -> Node<'tcx> { let node = self.inner.last().copied().unwrap(); - if let Node::Leaf(leaf) = node { - return Node::Leaf(leaf.subst(tcx, self.substs)); + match node { + Node::Leaf(leaf) => Node::Leaf(leaf.subst(tcx, self.substs)), + Node::Cast(kind, operand, ty) => Node::Cast(kind, operand, ty.subst(tcx, self.substs)), + // Don't perform substitution on the following as they can't directly contain generic params + Node::Binop(_, _, _) | Node::UnaryOp(_, _) | Node::FunctionCall(_, _) => node, } - node } } From be30e602969cefee2785dc25f17c34ebc4f007b3 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Wed, 20 Oct 2021 10:21:06 +0100 Subject: [PATCH 7/7] remove duplicate subst --- compiler/rustc_trait_selection/src/traits/const_evaluatable.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index 2cf7b6b1d9393..076597587f510 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -91,7 +91,6 @@ pub fn is_const_evaluatable<'cx, 'tcx>( ControlFlow::CONTINUE } Node::Cast(_, _, ty) => { - let ty = ty.subst(tcx, ct.substs); if ty.has_infer_types_or_consts() { failure_kind = FailureKind::MentionsInfer; } else if ty.definitely_has_param_types_or_consts(tcx) {