Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Jul 6, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 23 additions & 38 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -1837,15 +1834,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {

pub struct Holds<'tcx> {
ty: Ty<'tcx>,
holds: bool,
}

impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for Holds<'tcx> {
type Result = std::ops::ControlFlow<()>;

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)
}
Expand All @@ -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(
Expand Down Expand Up @@ -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(()),
}
}
}
Expand Down Expand Up @@ -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!(
Expand All @@ -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) {
Expand Down Expand Up @@ -4437,11 +4426,7 @@ impl<'b, 'v, 'tcx> Visitor<'v> for ConditionVisitor<'b, 'tcx> {
// arms might be missing an initialization.
let results: Vec<bool> = 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) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/default.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}
Expand Down
74 changes: 32 additions & 42 deletions compiler/rustc_hir_analysis/src/collect/generics_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::ControlFlow;

use crate::middle::resolve_bound_vars as rbv;
use hir::{
intravisit::{self, Visitor},
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -460,50 +457,45 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
struct LateBoundRegionsDetector<'tcx> {
tcx: TyCtxt<'tcx>,
outer_index: ty::DebruijnIndex,
has_late_bound_regions: Option<Span>,
}

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<Span>;
fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) -> ControlFlow<Span> {
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<Span> {
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<Span> {
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),
}
}
}
Expand All @@ -513,20 +505,15 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
generics: &'tcx hir::Generics<'tcx>,
decl: &'tcx hir::FnDecl<'tcx>,
) -> Option<Span> {
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) {
return Some(param.span);
}
}
}
visitor.visit_fn_decl(decl);
visitor.has_late_bound_regions
visitor.visit_fn_decl(decl).break_value()
}

let decl = node.fn_decl()?;
Expand All @@ -536,26 +523,29 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S

struct AnonConstInParamTyDetector {
in_param_ty: bool,
found_anon_const_in_param_ty: bool,
ct: HirId,
}

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
{
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)
}
}
2 changes: 0 additions & 2 deletions compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_trait_selection/src/traits/object_safety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,10 +805,11 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable<TyCtxt<'tcx>>>(
.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
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_trait_selection/src/traits/wf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,6 @@ impl<'a, 'tcx> 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 {
debug!("wf bounds for t={:?} t.kind={:#?}", t, t.kind());

Expand Down
Loading