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.

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

Add LetSource to hir::ExprKind::Let #88187

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
58 changes: 35 additions & 23 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::{ImplTraitContext, LoweringContext, ParamMode, ParenthesizedGenericArgs};
use super::{
ConditionScope, ImplTraitContext, LoweringContext, ParamMode, ParenthesizedGenericArgs,
};

use rustc_ast::attr;
use rustc_ast::ptr::P as AstP;
Expand All @@ -15,6 +17,8 @@ use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{hygiene::ForLoopLoc, DUMMY_SP};

use std::mem;

impl<'hir> LoweringContext<'_, 'hir> {
fn lower_exprs(&mut self, exprs: &[AstP<Expr>]) -> &'hir [hir::Expr<'hir>] {
self.arena.alloc_from_iter(exprs.iter().map(|x| self.lower_expr_mut(x)))
Expand Down Expand Up @@ -87,7 +91,18 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ExprKind::AddrOf(k, m, ohs)
}
ExprKind::Let(ref pat, ref scrutinee, span) => {
hir::ExprKind::Let(self.lower_pat(pat), self.lower_expr(scrutinee), span)
let source = match self.condition_scope {
Some(ConditionScope::Guard) => hir::LetSource::Guard,
Some(ConditionScope::If) => hir::LetSource::If,
Some(ConditionScope::While) => hir::LetSource::While,
_ => hir::LetSource::Local,
camsteffen marked this conversation as resolved.
Show resolved Hide resolved
};
hir::ExprKind::Let(
self.lower_pat(pat),
self.lower_expr(scrutinee),
span,
source,
)
}
ExprKind::If(ref cond, ref then, ref else_opt) => {
self.lower_expr_if(cond, then, else_opt.as_deref())
Expand Down Expand Up @@ -354,14 +369,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
then: &Block,
else_opt: Option<&Expr>,
) -> hir::ExprKind<'hir> {
let lowered_cond = self.lower_expr(cond);
let lowered_cond = self.with_condition_scope(ConditionScope::If, |t| t.lower_expr(cond));
let new_cond = self.manage_let_cond(lowered_cond);
let then_expr = self.lower_block_expr(then);
if let Some(rslt) = else_opt {
hir::ExprKind::If(new_cond, self.arena.alloc(then_expr), Some(self.lower_expr(rslt)))
} else {
hir::ExprKind::If(new_cond, self.arena.alloc(then_expr), None)
}
let then_expr = self.arena.alloc(self.lower_block_expr(then));
let else_opt = else_opt.map(|e| self.lower_expr(e));
hir::ExprKind::If(new_cond, then_expr, else_opt)
}

// If `cond` kind is `let`, returns `let`. Otherwise, wraps and returns `cond`
Expand Down Expand Up @@ -400,7 +412,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
body: &Block,
opt_label: Option<Label>,
) -> hir::ExprKind<'hir> {
let lowered_cond = self.with_loop_condition_scope(|t| t.lower_expr(cond));
let lowered_cond = self.with_condition_scope(ConditionScope::While, |t| t.lower_expr(cond));
let new_cond = self.manage_let_cond(lowered_cond);
let then = self.lower_block_expr(body);
let expr_break = self.expr_break(span, ThinVec::new());
Expand Down Expand Up @@ -473,7 +485,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
if let ExprKind::Let(ref pat, ref scrutinee, _) = cond.kind {
hir::Guard::IfLet(self.lower_pat(pat), self.lower_expr(scrutinee))
} else {
hir::Guard::If(self.lower_expr(cond))
let cond = self.with_condition_scope(ConditionScope::Guard, |t| t.lower_expr(cond));
hir::Guard::If(cond)
}
});
let hir_id = self.next_id();
Expand Down Expand Up @@ -732,7 +745,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
(body_id, generator_option)
});

// Lower outside new scope to preserve `is_in_loop_condition`.
// Lower outside new scope to preserve `condition_scope`.
let fn_decl = self.lower_fn_decl(decl, None, false, None);

hir::ExprKind::Closure(capture_clause, fn_decl, body_id, fn_decl_span, generator_option)
Expand Down Expand Up @@ -1132,7 +1145,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
}

fn lower_jump_destination(&mut self, id: NodeId, opt_label: Option<Label>) -> hir::Destination {
if self.is_in_loop_condition && opt_label.is_none() {
if opt_label.is_none() && self.condition_scope == Some(ConditionScope::While) {
hir::Destination {
label: None,
target_id: Err(hir::LoopIdError::UnlabeledCfInWhileCondition),
Expand Down Expand Up @@ -1160,8 +1173,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

fn with_loop_scope<T>(&mut self, loop_id: NodeId, f: impl FnOnce(&mut Self) -> T) -> T {
// We're no longer in the base loop's condition; we're in another loop.
let was_in_loop_condition = self.is_in_loop_condition;
self.is_in_loop_condition = false;
let condition_scope = mem::take(&mut self.condition_scope);

let len = self.loop_scopes.len();
self.loop_scopes.push(loop_id);
Expand All @@ -1175,19 +1187,19 @@ impl<'hir> LoweringContext<'_, 'hir> {

self.loop_scopes.pop().unwrap();

self.is_in_loop_condition = was_in_loop_condition;
self.condition_scope = condition_scope;

result
}

fn with_loop_condition_scope<T>(&mut self, f: impl FnOnce(&mut Self) -> T) -> T {
let was_in_loop_condition = self.is_in_loop_condition;
self.is_in_loop_condition = true;

fn with_condition_scope<T>(
&mut self,
condition_scope: ConditionScope,
f: impl FnOnce(&mut Self) -> T,
) -> T {
let condition_scope = mem::replace(&mut self.condition_scope, Some(condition_scope));
let result = f(self);

self.is_in_loop_condition = was_in_loop_condition;

self.condition_scope = condition_scope;
result
}

Expand Down
19 changes: 11 additions & 8 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ struct LoweringContext<'a, 'hir: 'a> {
current_item: Option<Span>,

catch_scopes: Vec<NodeId>,
condition_scope: Option<ConditionScope>,
loop_scopes: Vec<NodeId>,
is_in_loop_condition: bool,
is_in_trait_impl: bool,
is_in_dyn_type: bool,

Expand Down Expand Up @@ -332,8 +332,8 @@ pub fn lower_crate<'a, 'hir>(
attrs: BTreeMap::default(),
non_exported_macro_attrs: Vec::new(),
catch_scopes: Vec::new(),
condition_scope: None,
loop_scopes: Vec::new(),
is_in_loop_condition: false,
is_in_trait_impl: false,
is_in_dyn_type: false,
anonymous_lifetime_mode: AnonymousLifetimeMode::PassThrough,
Expand Down Expand Up @@ -419,6 +419,13 @@ enum AnonymousLifetimeMode {
PassThrough,
}

#[derive(Copy, Clone, Debug, PartialEq)]
enum ConditionScope {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have separate LetSource and ConditionScope types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I guess they are interchangeable. It's just that LetSource makes more sense in ExprKind::Let and Option<ConditionScope> makes more sense in LoweringContext. Should I just use LetSource?

Guard,
If,
While,
}

impl<'a, 'hir> LoweringContext<'a, 'hir> {
fn lower_crate(mut self, c: &Crate) -> &'hir hir::Crate<'hir> {
/// Full-crate AST visitor that inserts into a fresh
Expand Down Expand Up @@ -957,17 +964,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}

fn with_new_scopes<T>(&mut self, f: impl FnOnce(&mut Self) -> T) -> T {
let was_in_loop_condition = self.is_in_loop_condition;
self.is_in_loop_condition = false;

let catch_scopes = mem::take(&mut self.catch_scopes);
let condition_scope = mem::take(&mut self.condition_scope);
let loop_scopes = mem::take(&mut self.loop_scopes);
let ret = f(self);
self.catch_scopes = catch_scopes;
self.condition_scope = condition_scope;
self.loop_scopes = loop_scopes;

self.is_in_loop_condition = was_in_loop_condition;

ret
}

Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,7 @@ pub enum ExprKind<'hir> {
///
/// These are not `Local` and only occur as expressions.
/// The `let Some(x) = foo()` in `if let Some(x) = foo()` is an example of `Let(..)`.
Let(&'hir Pat<'hir>, &'hir Expr<'hir>, Span),
Let(&'hir Pat<'hir>, &'hir Expr<'hir>, Span, LetSource),
/// An `if` block, with an optional else block.
///
/// I.e., `if <expr> { <expr> } else { <expr> }`.
Expand Down Expand Up @@ -1886,6 +1886,15 @@ pub enum LocalSource {
AssignDesugar(Span),
}

#[derive(Copy, Clone, PartialEq, Eq, Encodable, Hash, Debug)]
#[derive(HashStable_Generic)]
pub enum LetSource {
Guard,
If,
Local,
While,
}

/// Hints at the original code for a `match _ { .. }`.
#[derive(Copy, Clone, PartialEq, Eq, Encodable, Hash, Debug)]
#[derive(HashStable_Generic)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
ExprKind::DropTemps(ref subexpression) => {
visitor.visit_expr(subexpression);
}
ExprKind::Let(ref pat, ref expr, _) => {
ExprKind::Let(ref pat, ref expr, ..) => {
visitor.visit_expr(expr);
visitor.visit_pat(pat);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ impl<'a> State<'a> {
// Print `}`:
self.bclose_maybe_open(expr.span, true);
}
hir::ExprKind::Let(ref pat, ref scrutinee, _) => {
hir::ExprKind::Let(ref pat, ref scrutinee, ..) => {
self.print_let(pat, scrutinee);
}
hir::ExprKind::If(ref test, ref blk, ref elseopt) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ impl<'tcx> Cx<'tcx> {
},
Err(err) => bug!("invalid loop id for continue: {}", err),
},
hir::ExprKind::Let(ref pat, ref expr, _) => {
hir::ExprKind::Let(ref pat, ref expr, ..) => {
ExprKind::Let { expr: self.mirror_expr(expr), pat: self.pattern_from_hir(pat) }
}
hir::ExprKind::If(cond, then, else_opt) => ExprKind::If {
Expand Down
Loading