From 7dca61b68b50b7e38bf3cec027eec2002755b2a4 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 5 Jul 2024 15:00:40 +0000 Subject: [PATCH] Use `ControlFlow` results for visitors that are only looking for a single value --- .../src/diagnostics/conflict_errors.rs | 61 ++++++--------- .../src/deriving/default.rs | 2 +- .../src/collect/generics_of.rs | 74 ++++++++----------- .../src/fn_ctxt/inspect_obligations.rs | 2 - .../src/traits/object_safety.rs | 5 +- .../rustc_trait_selection/src/traits/wf.rs | 2 - 6 files changed, 59 insertions(+), 87 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index e291ebd9bb8..c2c3f5bc79e 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -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::{Obligation, ObligationCause, ObligationCtxt}; use std::iter; +use std::ops::ControlFlow; use crate::borrow_set::TwoPhaseActivation; use crate::borrowck_errors; @@ -784,20 +785,20 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { /// binding declaration within every scope we inspect. struct Finder { hir_id: hir::HirId, - found: bool, } 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 { - 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 { - 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. @@ -822,9 +823,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { _ => continue, }; if let Some(&hir_id) = local_hir_id { - let mut finder = Finder { hir_id, found: false }; - finder.visit_expr(e); - if finder.found { + if (Finder { hir_id }).visit_expr(e).is_break() { // The current scope includes the declaration of the binding we're accessing, we // can't look up any further for loops. break; @@ -839,9 +838,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { hir::Node::Expr(hir::Expr { kind: hir::ExprKind::If(cond, ..), .. }) => { - let mut finder = Finder { hir_id: expr.hir_id, found: false }; - finder.visit_expr(cond); - if finder.found { + if (Finder { hir_id: expr.hir_id }).visit_expr(cond).is_break() { // The expression where the move error happened is in a `while let` // condition Don't suggest clone as it will likely end in an // infinite loop. @@ -1837,7 +1834,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { pub struct Holds<'tcx> { ty: Ty<'tcx>, - holds: bool, } impl<'tcx> TypeVisitor> for Holds<'tcx> { @@ -1845,7 +1841,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { if t == self.ty { - self.holds = true; + return ControlFlow::Break(()); } t.super_visit_with(self) } @@ -1863,9 +1859,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { && rcvr_ty == ty && let ty::Ref(_, inner, _) = rcvr_ty.kind() && let inner = inner.peel_refs() - && let mut v = (Holds { ty: inner, holds: false }) - && let _ = v.visit_ty(local_ty) - && v.holds + && (Holds { ty: inner }).visit_ty(local_ty).is_break() && let None = self.infcx.type_implements_trait_shallow(clone, inner, self.param_env) { 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 -struct ReferencedStatementsVisitor<'a>(&'a [Span], bool); +struct ReferencedStatementsVisitor<'a>(&'a [Span]); -impl<'a, 'v> Visitor<'v> for ReferencedStatementsVisitor<'a> { - fn visit_stmt(&mut self, s: &'v hir::Stmt<'v>) { +impl<'v> Visitor<'v> for ReferencedStatementsVisitor<'_> { + type Result = ControlFlow<()>; + fn visit_stmt(&mut self, s: &'v hir::Stmt<'v>) -> Self::Result { match s.kind { - hir::StmtKind::Semi(expr) if self.0.contains(&expr.span) => { - self.1 = true; - } - _ => {} + hir::StmtKind::Semi(expr) if self.0.contains(&expr.span) => ControlFlow::Break(()), + _ => ControlFlow::Continue(()), } } } @@ -4375,9 +4368,7 @@ impl<'b, 'v, 'tcx> Visitor<'v> for ConditionVisitor<'b, 'tcx> { hir::ExprKind::If(cond, body, None) => { // `if` expressions with no `else` that initialize the binding might be missing an // `else` arm. - let mut v = ReferencedStatementsVisitor(self.spans, false); - v.visit_expr(body); - if v.1 { + if ReferencedStatementsVisitor(self.spans).visit_expr(body).is_break() { self.errors.push(( cond.span, format!( @@ -4394,11 +4385,9 @@ impl<'b, 'v, 'tcx> Visitor<'v> for ConditionVisitor<'b, 'tcx> { hir::ExprKind::If(cond, body, Some(other)) => { // `if` expressions where the binding is only initialized in one of the two arms // might be missing a binding initialization. - let mut a = ReferencedStatementsVisitor(self.spans, false); - a.visit_expr(body); - let mut b = ReferencedStatementsVisitor(self.spans, false); - b.visit_expr(other); - match (a.1, b.1) { + let a = ReferencedStatementsVisitor(self.spans).visit_expr(body).is_break(); + let b = ReferencedStatementsVisitor(self.spans).visit_expr(other).is_break(); + match (a, b) { (true, true) | (false, false) => {} (true, false) => { 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. let results: Vec = arms .iter() - .map(|arm| { - let mut v = ReferencedStatementsVisitor(self.spans, false); - v.visit_arm(arm); - v.1 - }) + .map(|arm| ReferencedStatementsVisitor(self.spans).visit_arm(arm).is_break()) .collect(); if results.iter().any(|x| *x) && !results.iter().all(|x| *x) { for (arm, seen) in arms.iter().zip(results) { diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index 577523a1d5a..7a65ed97f00 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -240,7 +240,7 @@ fn has_a_default_variant(item: &Annotatable) -> bool { if v.attrs.iter().any(|attr| attr.has_name(kw::Default)) { ControlFlow::Break(()) } else { - // no need to subrecurse. + // no need to walk the variant, we are only looking for top level variants ControlFlow::Continue(()) } } diff --git a/compiler/rustc_hir_analysis/src/collect/generics_of.rs b/compiler/rustc_hir_analysis/src/collect/generics_of.rs index 9b02c1a61fa..22d465c8e62 100644 --- a/compiler/rustc_hir_analysis/src/collect/generics_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/generics_of.rs @@ -1,3 +1,5 @@ +use std::ops::ControlFlow; + use crate::middle::resolve_bound_vars as rbv; use hir::{ 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; for (_parent, node) in tcx.hir().parent_iter(hir_id) { if let Some(generics) = node.generics() { - let mut visitor = AnonConstInParamTyDetector { - in_param_ty: false, - found_anon_const_in_param_ty: false, - ct: hir_id, - }; + let mut visitor = AnonConstInParamTyDetector { in_param_ty: false, ct: hir_id }; - visitor.visit_generics(generics); - in_param_ty = visitor.found_anon_const_in_param_ty; + in_param_ty = visitor.visit_generics(generics).is_break(); break; } } @@ -460,50 +457,45 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option { tcx: TyCtxt<'tcx>, outer_index: ty::DebruijnIndex, - has_late_bound_regions: Option, } impl<'tcx> Visitor<'tcx> for LateBoundRegionsDetector<'tcx> { - fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) { - if self.has_late_bound_regions.is_some() { - return; - } + type Result = ControlFlow; + fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) -> ControlFlow { match ty.kind { hir::TyKind::BareFn(..) => { self.outer_index.shift_in(1); - intravisit::walk_ty(self, ty); + let res = intravisit::walk_ty(self, ty); self.outer_index.shift_out(1); + res } _ => intravisit::walk_ty(self, ty), } } - fn visit_poly_trait_ref(&mut self, tr: &'tcx hir::PolyTraitRef<'tcx>) { - if self.has_late_bound_regions.is_some() { - return; - } + fn visit_poly_trait_ref(&mut self, tr: &'tcx hir::PolyTraitRef<'tcx>) -> ControlFlow { 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); + res } - fn visit_lifetime(&mut self, lt: &'tcx hir::Lifetime) { - if self.has_late_bound_regions.is_some() { - return; - } - + fn visit_lifetime(&mut self, lt: &'tcx hir::Lifetime) -> ControlFlow { 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, _, _)) - if debruijn < self.outer_index => {} + if debruijn < self.outer_index => + { + ControlFlow::Continue(()) + } Some( rbv::ResolvedArg::LateBound(..) | rbv::ResolvedArg::Free(..) | rbv::ResolvedArg::Error(_), ) - | None => { - self.has_late_bound_regions = Some(lt.ident.span); - } + | None => ControlFlow::Break(lt.ident.span), } } } @@ -513,11 +505,7 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option, decl: &'tcx hir::FnDecl<'tcx>, ) -> Option { - let mut visitor = LateBoundRegionsDetector { - tcx, - outer_index: ty::INNERMOST, - has_late_bound_regions: None, - }; + let mut visitor = LateBoundRegionsDetector { tcx, outer_index: ty::INNERMOST }; for param in generics.params { if let GenericParamKind::Lifetime { .. } = param.kind { 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(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option 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 { let prev = self.in_param_ty; self.in_param_ty = true; - self.visit_ty(ty); + let res = self.visit_ty(ty); 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 { - self.found_anon_const_in_param_ty = true; - } else { - intravisit::walk_anon_const(self, c) + return ControlFlow::Break(()); } + intravisit::walk_anon_const(self, c) } } diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs index fab7eb7495c..90dd5f73586 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs @@ -104,8 +104,6 @@ struct NestedObligationsForSelfTy<'a, 'tcx> { } impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> { - type Result = (); - fn span(&self) -> Span { self.root_cause.span } diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index f1611bd049d..5b2c8fb1950 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -805,10 +805,11 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable>>( .unwrap() .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 { - ControlFlow::Continue(()) // do not walk contained types, do not report error, do collect $200 + ControlFlow::Continue(()) } 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 diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index e3952679f96..f071dc6c784 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -640,8 +640,6 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { } impl<'a, 'tcx> TypeVisitor> for WfPredicates<'a, 'tcx> { - type Result = (); - fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { debug!("wf bounds for t={:?} t.kind={:#?}", t, t.kind());