Use `ControlFlow` results for visitors that are only looking for a single value

This commit is contained in:
Oli Scherer 2024-07-05 15:00:40 +00:00
parent d2e6cf7fa7
commit 7dca61b68b
6 changed files with 59 additions and 87 deletions

View File

@ -39,6 +39,7 @@ use rustc_trait_selection::traits::error_reporting::suggestions::TypeErrCtxtExt;
use rustc_trait_selection::traits::error_reporting::FindExprBySpan; use rustc_trait_selection::traits::error_reporting::FindExprBySpan;
use rustc_trait_selection::traits::{Obligation, ObligationCause, ObligationCtxt}; use rustc_trait_selection::traits::{Obligation, ObligationCause, ObligationCtxt};
use std::iter; use std::iter;
use std::ops::ControlFlow;
use crate::borrow_set::TwoPhaseActivation; use crate::borrow_set::TwoPhaseActivation;
use crate::borrowck_errors; use crate::borrowck_errors;
@ -784,20 +785,20 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
/// binding declaration within every scope we inspect. /// binding declaration within every scope we inspect.
struct Finder { struct Finder {
hir_id: hir::HirId, hir_id: hir::HirId,
found: bool,
} }
impl<'hir> Visitor<'hir> for Finder { impl<'hir> Visitor<'hir> for Finder {
fn visit_pat(&mut self, pat: &'hir hir::Pat<'hir>) { type Result = ControlFlow<()>;
fn visit_pat(&mut self, pat: &'hir hir::Pat<'hir>) -> Self::Result {
if pat.hir_id == self.hir_id { if pat.hir_id == self.hir_id {
self.found = true; return ControlFlow::Break(());
} }
hir::intravisit::walk_pat(self, pat); hir::intravisit::walk_pat(self, pat)
} }
fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) { fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) -> Self::Result {
if ex.hir_id == self.hir_id { if ex.hir_id == self.hir_id {
self.found = true; return ControlFlow::Break(());
} }
hir::intravisit::walk_expr(self, ex); hir::intravisit::walk_expr(self, ex)
} }
} }
// The immediate HIR parent of the moved expression. We'll look for it to be a call. // The immediate HIR parent of the moved expression. We'll look for it to be a call.
@ -822,9 +823,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
_ => continue, _ => continue,
}; };
if let Some(&hir_id) = local_hir_id { if let Some(&hir_id) = local_hir_id {
let mut finder = Finder { hir_id, found: false }; if (Finder { hir_id }).visit_expr(e).is_break() {
finder.visit_expr(e);
if finder.found {
// The current scope includes the declaration of the binding we're accessing, we // The current scope includes the declaration of the binding we're accessing, we
// can't look up any further for loops. // can't look up any further for loops.
break; break;
@ -839,9 +838,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
hir::Node::Expr(hir::Expr { hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::If(cond, ..), .. kind: hir::ExprKind::If(cond, ..), ..
}) => { }) => {
let mut finder = Finder { hir_id: expr.hir_id, found: false }; if (Finder { hir_id: expr.hir_id }).visit_expr(cond).is_break() {
finder.visit_expr(cond);
if finder.found {
// The expression where the move error happened is in a `while let` // The expression where the move error happened is in a `while let`
// condition Don't suggest clone as it will likely end in an // condition Don't suggest clone as it will likely end in an
// infinite loop. // infinite loop.
@ -1837,7 +1834,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
pub struct Holds<'tcx> { pub struct Holds<'tcx> {
ty: Ty<'tcx>, ty: Ty<'tcx>,
holds: bool,
} }
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for Holds<'tcx> { impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for Holds<'tcx> {
@ -1845,7 +1841,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
if t == self.ty { if t == self.ty {
self.holds = true; return ControlFlow::Break(());
} }
t.super_visit_with(self) t.super_visit_with(self)
} }
@ -1863,9 +1859,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
&& rcvr_ty == ty && rcvr_ty == ty
&& let ty::Ref(_, inner, _) = rcvr_ty.kind() && let ty::Ref(_, inner, _) = rcvr_ty.kind()
&& let inner = inner.peel_refs() && let inner = inner.peel_refs()
&& let mut v = (Holds { ty: inner, holds: false }) && (Holds { ty: inner }).visit_ty(local_ty).is_break()
&& let _ = v.visit_ty(local_ty)
&& v.holds
&& let None = self.infcx.type_implements_trait_shallow(clone, inner, self.param_env) && let None = self.infcx.type_implements_trait_shallow(clone, inner, self.param_env)
{ {
err.span_label( err.span_label(
@ -4325,15 +4319,14 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> {
} }
/// Detect whether one of the provided spans is a statement nested within the top-most visited expr /// Detect whether one of the provided spans is a statement nested within the top-most visited expr
struct ReferencedStatementsVisitor<'a>(&'a [Span], bool); struct ReferencedStatementsVisitor<'a>(&'a [Span]);
impl<'a, 'v> Visitor<'v> for ReferencedStatementsVisitor<'a> { impl<'v> Visitor<'v> for ReferencedStatementsVisitor<'_> {
fn visit_stmt(&mut self, s: &'v hir::Stmt<'v>) { type Result = ControlFlow<()>;
fn visit_stmt(&mut self, s: &'v hir::Stmt<'v>) -> Self::Result {
match s.kind { match s.kind {
hir::StmtKind::Semi(expr) if self.0.contains(&expr.span) => { hir::StmtKind::Semi(expr) if self.0.contains(&expr.span) => ControlFlow::Break(()),
self.1 = true; _ => ControlFlow::Continue(()),
}
_ => {}
} }
} }
} }
@ -4375,9 +4368,7 @@ impl<'b, 'v, 'tcx> Visitor<'v> for ConditionVisitor<'b, 'tcx> {
hir::ExprKind::If(cond, body, None) => { hir::ExprKind::If(cond, body, None) => {
// `if` expressions with no `else` that initialize the binding might be missing an // `if` expressions with no `else` that initialize the binding might be missing an
// `else` arm. // `else` arm.
let mut v = ReferencedStatementsVisitor(self.spans, false); if ReferencedStatementsVisitor(self.spans).visit_expr(body).is_break() {
v.visit_expr(body);
if v.1 {
self.errors.push(( self.errors.push((
cond.span, cond.span,
format!( format!(
@ -4394,11 +4385,9 @@ impl<'b, 'v, 'tcx> Visitor<'v> for ConditionVisitor<'b, 'tcx> {
hir::ExprKind::If(cond, body, Some(other)) => { hir::ExprKind::If(cond, body, Some(other)) => {
// `if` expressions where the binding is only initialized in one of the two arms // `if` expressions where the binding is only initialized in one of the two arms
// might be missing a binding initialization. // might be missing a binding initialization.
let mut a = ReferencedStatementsVisitor(self.spans, false); let a = ReferencedStatementsVisitor(self.spans).visit_expr(body).is_break();
a.visit_expr(body); let b = ReferencedStatementsVisitor(self.spans).visit_expr(other).is_break();
let mut b = ReferencedStatementsVisitor(self.spans, false); match (a, b) {
b.visit_expr(other);
match (a.1, b.1) {
(true, true) | (false, false) => {} (true, true) | (false, false) => {}
(true, false) => { (true, false) => {
if other.span.is_desugaring(DesugaringKind::WhileLoop) { if other.span.is_desugaring(DesugaringKind::WhileLoop) {
@ -4437,11 +4426,7 @@ impl<'b, 'v, 'tcx> Visitor<'v> for ConditionVisitor<'b, 'tcx> {
// arms might be missing an initialization. // arms might be missing an initialization.
let results: Vec<bool> = arms let results: Vec<bool> = arms
.iter() .iter()
.map(|arm| { .map(|arm| ReferencedStatementsVisitor(self.spans).visit_arm(arm).is_break())
let mut v = ReferencedStatementsVisitor(self.spans, false);
v.visit_arm(arm);
v.1
})
.collect(); .collect();
if results.iter().any(|x| *x) && !results.iter().all(|x| *x) { if results.iter().any(|x| *x) && !results.iter().all(|x| *x) {
for (arm, seen) in arms.iter().zip(results) { for (arm, seen) in arms.iter().zip(results) {

View File

@ -240,7 +240,7 @@ fn has_a_default_variant(item: &Annotatable) -> bool {
if v.attrs.iter().any(|attr| attr.has_name(kw::Default)) { if v.attrs.iter().any(|attr| attr.has_name(kw::Default)) {
ControlFlow::Break(()) ControlFlow::Break(())
} else { } else {
// no need to subrecurse. // no need to walk the variant, we are only looking for top level variants
ControlFlow::Continue(()) ControlFlow::Continue(())
} }
} }

View File

@ -1,3 +1,5 @@
use std::ops::ControlFlow;
use crate::middle::resolve_bound_vars as rbv; use crate::middle::resolve_bound_vars as rbv;
use hir::{ use hir::{
intravisit::{self, Visitor}, intravisit::{self, Visitor},
@ -87,14 +89,9 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
let mut in_param_ty = false; let mut in_param_ty = false;
for (_parent, node) in tcx.hir().parent_iter(hir_id) { for (_parent, node) in tcx.hir().parent_iter(hir_id) {
if let Some(generics) = node.generics() { if let Some(generics) = node.generics() {
let mut visitor = AnonConstInParamTyDetector { let mut visitor = AnonConstInParamTyDetector { in_param_ty: false, ct: hir_id };
in_param_ty: false,
found_anon_const_in_param_ty: false,
ct: hir_id,
};
visitor.visit_generics(generics); in_param_ty = visitor.visit_generics(generics).is_break();
in_param_ty = visitor.found_anon_const_in_param_ty;
break; break;
} }
} }
@ -460,50 +457,45 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
struct LateBoundRegionsDetector<'tcx> { struct LateBoundRegionsDetector<'tcx> {
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,
outer_index: ty::DebruijnIndex, outer_index: ty::DebruijnIndex,
has_late_bound_regions: Option<Span>,
} }
impl<'tcx> Visitor<'tcx> for LateBoundRegionsDetector<'tcx> { impl<'tcx> Visitor<'tcx> for LateBoundRegionsDetector<'tcx> {
fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) { type Result = ControlFlow<Span>;
if self.has_late_bound_regions.is_some() { fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) -> ControlFlow<Span> {
return;
}
match ty.kind { match ty.kind {
hir::TyKind::BareFn(..) => { hir::TyKind::BareFn(..) => {
self.outer_index.shift_in(1); self.outer_index.shift_in(1);
intravisit::walk_ty(self, ty); let res = intravisit::walk_ty(self, ty);
self.outer_index.shift_out(1); self.outer_index.shift_out(1);
res
} }
_ => intravisit::walk_ty(self, ty), _ => intravisit::walk_ty(self, ty),
} }
} }
fn visit_poly_trait_ref(&mut self, tr: &'tcx hir::PolyTraitRef<'tcx>) { fn visit_poly_trait_ref(&mut self, tr: &'tcx hir::PolyTraitRef<'tcx>) -> ControlFlow<Span> {
if self.has_late_bound_regions.is_some() {
return;
}
self.outer_index.shift_in(1); self.outer_index.shift_in(1);
intravisit::walk_poly_trait_ref(self, tr); let res = intravisit::walk_poly_trait_ref(self, tr);
self.outer_index.shift_out(1); self.outer_index.shift_out(1);
res
} }
fn visit_lifetime(&mut self, lt: &'tcx hir::Lifetime) { fn visit_lifetime(&mut self, lt: &'tcx hir::Lifetime) -> ControlFlow<Span> {
if self.has_late_bound_regions.is_some() {
return;
}
match self.tcx.named_bound_var(lt.hir_id) { match self.tcx.named_bound_var(lt.hir_id) {
Some(rbv::ResolvedArg::StaticLifetime | rbv::ResolvedArg::EarlyBound(..)) => {} Some(rbv::ResolvedArg::StaticLifetime | rbv::ResolvedArg::EarlyBound(..)) => {
ControlFlow::Continue(())
}
Some(rbv::ResolvedArg::LateBound(debruijn, _, _)) Some(rbv::ResolvedArg::LateBound(debruijn, _, _))
if debruijn < self.outer_index => {} if debruijn < self.outer_index =>
{
ControlFlow::Continue(())
}
Some( Some(
rbv::ResolvedArg::LateBound(..) rbv::ResolvedArg::LateBound(..)
| rbv::ResolvedArg::Free(..) | rbv::ResolvedArg::Free(..)
| rbv::ResolvedArg::Error(_), | rbv::ResolvedArg::Error(_),
) )
| None => { | None => ControlFlow::Break(lt.ident.span),
self.has_late_bound_regions = Some(lt.ident.span);
}
} }
} }
} }
@ -513,11 +505,7 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
generics: &'tcx hir::Generics<'tcx>, generics: &'tcx hir::Generics<'tcx>,
decl: &'tcx hir::FnDecl<'tcx>, decl: &'tcx hir::FnDecl<'tcx>,
) -> Option<Span> { ) -> Option<Span> {
let mut visitor = LateBoundRegionsDetector { let mut visitor = LateBoundRegionsDetector { tcx, outer_index: ty::INNERMOST };
tcx,
outer_index: ty::INNERMOST,
has_late_bound_regions: None,
};
for param in generics.params { for param in generics.params {
if let GenericParamKind::Lifetime { .. } = param.kind { if let GenericParamKind::Lifetime { .. } = param.kind {
if tcx.is_late_bound(param.hir_id) { if tcx.is_late_bound(param.hir_id) {
@ -525,8 +513,7 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
} }
} }
} }
visitor.visit_fn_decl(decl); visitor.visit_fn_decl(decl).break_value()
visitor.has_late_bound_regions
} }
let decl = node.fn_decl()?; let decl = node.fn_decl()?;
@ -536,26 +523,29 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
struct AnonConstInParamTyDetector { struct AnonConstInParamTyDetector {
in_param_ty: bool, in_param_ty: bool,
found_anon_const_in_param_ty: bool,
ct: HirId, ct: HirId,
} }
impl<'v> Visitor<'v> for AnonConstInParamTyDetector { impl<'v> Visitor<'v> for AnonConstInParamTyDetector {
fn visit_generic_param(&mut self, p: &'v hir::GenericParam<'v>) { type Result = ControlFlow<()>;
fn visit_generic_param(&mut self, p: &'v hir::GenericParam<'v>) -> Self::Result {
if let GenericParamKind::Const { ty, default: _, is_host_effect: _, synthetic: _ } = p.kind if let GenericParamKind::Const { ty, default: _, is_host_effect: _, synthetic: _ } = p.kind
{ {
let prev = self.in_param_ty; let prev = self.in_param_ty;
self.in_param_ty = true; self.in_param_ty = true;
self.visit_ty(ty); let res = self.visit_ty(ty);
self.in_param_ty = prev; self.in_param_ty = prev;
res
} else {
ControlFlow::Continue(())
} }
} }
fn visit_anon_const(&mut self, c: &'v hir::AnonConst) { fn visit_anon_const(&mut self, c: &'v hir::AnonConst) -> Self::Result {
if self.in_param_ty && self.ct == c.hir_id { if self.in_param_ty && self.ct == c.hir_id {
self.found_anon_const_in_param_ty = true; return ControlFlow::Break(());
} else {
intravisit::walk_anon_const(self, c)
} }
intravisit::walk_anon_const(self, c)
} }
} }

View File

@ -104,8 +104,6 @@ struct NestedObligationsForSelfTy<'a, 'tcx> {
} }
impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> { impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> {
type Result = ();
fn span(&self) -> Span { fn span(&self) -> Span {
self.root_cause.span self.root_cause.span
} }

View File

@ -805,10 +805,11 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable<TyCtxt<'tcx>>>(
.unwrap() .unwrap()
.contains(&data.trait_ref(self.tcx).def_id); .contains(&data.trait_ref(self.tcx).def_id);
// only walk contained types if it's not a super trait
if is_supertrait_of_current_trait { if is_supertrait_of_current_trait {
ControlFlow::Continue(()) // do not walk contained types, do not report error, do collect $200 ControlFlow::Continue(())
} else { } else {
t.super_visit_with(self) // DO walk contained types, POSSIBLY reporting an error t.super_visit_with(self) // POSSIBLY reporting an error
} }
} }
_ => t.super_visit_with(self), // walk contained types, if any _ => t.super_visit_with(self), // walk contained types, if any

View File

@ -640,8 +640,6 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
} }
impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> { impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
type Result = ();
fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
debug!("wf bounds for t={:?} t.kind={:#?}", t, t.kind()); debug!("wf bounds for t={:?} t.kind={:#?}", t, t.kind());