Skip to content

Commit

Permalink
Rollup merge of #119554 - matthewjasper:remove-guard-distinction, r=c…
Browse files Browse the repository at this point in the history
…ompiler-errors

Fix scoping for let chains in match guards

If let guards were previously represented as a different type of guard in HIR and THIR. This meant that let chains in match guards were not handled correctly because they were treated exactly like normal guards.

- Remove `hir::Guard` and `thir::Guard`.
- Make the scoping different between normal guards and if let guards also check for let chains.

closes #118593
  • Loading branch information
matthiaskrgr authored Jan 5, 2024
2 parents 8309063 + 44bba54 commit 958417f
Show file tree
Hide file tree
Showing 34 changed files with 272 additions and 292 deletions.
15 changes: 1 addition & 14 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,20 +546,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

fn lower_arm(&mut self, arm: &Arm) -> hir::Arm<'hir> {
let pat = self.lower_pat(&arm.pat);
let guard = arm.guard.as_ref().map(|cond| {
if let ExprKind::Let(pat, scrutinee, span, is_recovered) = &cond.kind {
hir::Guard::IfLet(self.arena.alloc(hir::Let {
hir_id: self.next_id(),
span: self.lower_span(*span),
pat: self.lower_pat(pat),
ty: None,
init: self.lower_expr(scrutinee),
is_recovered: *is_recovered,
}))
} else {
hir::Guard::If(self.lower_expr(cond))
}
});
let guard = arm.guard.as_ref().map(|cond| self.lower_expr(cond));
let hir_id = self.next_id();
let span = self.lower_span(arm.span);
self.lower_attrs(hir_id, &arm.attrs);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3590,7 +3590,7 @@ impl<'b, 'v> Visitor<'v> for ConditionVisitor<'b> {
));
} else if let Some(guard) = &arm.guard {
self.errors.push((
arm.pat.span.to(guard.body().span),
arm.pat.span.to(guard.span),
format!(
"if this pattern and condition are matched, {} is not \
initialized",
Expand Down
22 changes: 1 addition & 21 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ pub struct Arm<'hir> {
/// If this pattern and the optional guard matches, then `body` is evaluated.
pub pat: &'hir Pat<'hir>,
/// Optional guard clause.
pub guard: Option<Guard<'hir>>,
pub guard: Option<&'hir Expr<'hir>>,
/// The expression the arm evaluates to if this arm matches.
pub body: &'hir Expr<'hir>,
}
Expand All @@ -1280,26 +1280,6 @@ pub struct Let<'hir> {
pub is_recovered: Option<ErrorGuaranteed>,
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub enum Guard<'hir> {
If(&'hir Expr<'hir>),
IfLet(&'hir Let<'hir>),
}

impl<'hir> Guard<'hir> {
/// Returns the body of the guard
///
/// In other words, returns the e in either of the following:
///
/// - `if e`
/// - `if let x = e`
pub fn body(&self) -> &'hir Expr<'hir> {
match self {
Guard::If(e) | Guard::IfLet(Let { init: e, .. }) => e,
}
}
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub struct ExprField<'hir> {
#[stable_hasher(ignore)]
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,13 +619,8 @@ pub fn walk_stmt<'v, V: Visitor<'v>>(visitor: &mut V, statement: &'v Stmt<'v>) {
pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm<'v>) {
visitor.visit_id(arm.hir_id);
visitor.visit_pat(arm.pat);
if let Some(ref g) = arm.guard {
match g {
Guard::If(ref e) => visitor.visit_expr(e),
Guard::IfLet(ref l) => {
visitor.visit_let_expr(l);
}
}
if let Some(ref e) = arm.guard {
visitor.visit_expr(e);
}
visitor.visit_expr(arm.body);
}
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,24 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
}

fn resolve_arm<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, arm: &'tcx hir::Arm<'tcx>) {
fn has_let_expr(expr: &Expr<'_>) -> bool {
match &expr.kind {
hir::ExprKind::Binary(_, lhs, rhs) => has_let_expr(lhs) || has_let_expr(rhs),
hir::ExprKind::Let(..) => true,
_ => false,
}
}

let prev_cx = visitor.cx;

visitor.terminating_scopes.insert(arm.hir_id.local_id);

visitor.enter_node_scope_with_dtor(arm.hir_id.local_id);
visitor.cx.var_parent = visitor.cx.parent;

if let Some(hir::Guard::If(expr)) = arm.guard {
if let Some(expr) = arm.guard
&& !has_let_expr(expr)
{
visitor.terminating_scopes.insert(expr.hir_id.local_id);
}

Expand Down
14 changes: 3 additions & 11 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1874,17 +1874,9 @@ impl<'a> State<'a> {
self.print_pat(arm.pat);
self.space();
if let Some(ref g) = arm.guard {
match *g {
hir::Guard::If(e) => {
self.word_space("if");
self.print_expr(e);
self.space();
}
hir::Guard::IfLet(&hir::Let { pat, ty, init, .. }) => {
self.word_nbsp("if");
self.print_let(pat, ty, init);
}
}
self.word_space("if");
self.print_expr(g);
self.space();
}
self.word_space("=>");

Expand Down
11 changes: 2 additions & 9 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut other_arms = vec![]; // Used only for diagnostics.
let mut prior_arm = None;
for arm in arms {
if let Some(g) = &arm.guard {
if let Some(e) = &arm.guard {
self.diverges.set(Diverges::Maybe);
match g {
hir::Guard::If(e) => {
self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {});
}
hir::Guard::IfLet(l) => {
self.check_expr_let(l);
}
};
self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {});
}

self.diverges.set(Diverges::Maybe);
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_hir_typeck/src/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,12 +669,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
);
self.walk_pat(discr_place, arm.pat, arm.guard.is_some());

match arm.guard {
Some(hir::Guard::If(e)) => self.consume_expr(e),
Some(hir::Guard::IfLet(l)) => {
self.walk_local(l.init, l.pat, None, |t| t.borrow_expr(l.init, ty::ImmBorrow))
}
None => {}
if let Some(ref e) = arm.guard {
self.consume_expr(e)
}

self.consume_expr(arm.body);
Expand Down
9 changes: 1 addition & 8 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,20 +519,13 @@ pub struct FruInfo<'tcx> {
#[derive(Clone, Debug, HashStable)]
pub struct Arm<'tcx> {
pub pattern: Box<Pat<'tcx>>,
pub guard: Option<Guard<'tcx>>,
pub guard: Option<ExprId>,
pub body: ExprId,
pub lint_level: LintLevel,
pub scope: region::Scope,
pub span: Span,
}

/// A `match` guard.
#[derive(Clone, Debug, HashStable)]
pub enum Guard<'tcx> {
If(ExprId),
IfLet(Box<Pat<'tcx>>, ExprId),
}

#[derive(Copy, Clone, Debug, HashStable)]
pub enum LogicalOp {
/// The `&&` operator.
Expand Down
11 changes: 3 additions & 8 deletions compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{
AdtExpr, Arm, Block, ClosureExpr, Expr, ExprKind, Guard, InlineAsmExpr, InlineAsmOperand, Pat,
AdtExpr, Arm, Block, ClosureExpr, Expr, ExprKind, InlineAsmExpr, InlineAsmOperand, Pat,
PatKind, Stmt, StmtKind, Thir,
};

Expand Down Expand Up @@ -213,13 +213,8 @@ pub fn walk_arm<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
visitor: &mut V,
arm: &'thir Arm<'tcx>,
) {
match arm.guard {
Some(Guard::If(expr)) => visitor.visit_expr(&visitor.thir()[expr]),
Some(Guard::IfLet(ref pat, expr)) => {
visitor.visit_pat(pat);
visitor.visit_expr(&visitor.thir()[expr]);
}
None => {}
if let Some(expr) = arm.guard {
visitor.visit_expr(&visitor.thir()[expr])
}
visitor.visit_pat(&arm.pattern);
visitor.visit_expr(&visitor.thir()[arm.body]);
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
cond,
Some(condition_scope),
condition_scope,
source_info
source_info,
true,
));

this.expr_into_dest(destination, then_blk, then)
Expand Down Expand Up @@ -173,6 +174,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Some(condition_scope),
condition_scope,
source_info,
true,
)
});
let (short_circuit, continuation, constant) = match op {
Expand Down
Loading

0 comments on commit 958417f

Please sign in to comment.