Skip to content

Commit adce36b

Browse files
CentrilCentril
authored
Rollup merge of rust-lang#60225 - Centril:hir-exprkind-use-in-for-loops, r=oli-obk
Introduce hir::ExprKind::Use and employ in for loop desugaring. In the `for $pat in $expr $block` desugaring we end with a `{ let _result = $match_expr; _result }` construct which makes `for` loops into a terminating scope and affects drop order. The construct was introduced in year 2015 by @pnkfelix in rust-lang#21984. This PR replaces the construct with `hir::ExprKind::Use(P<hir::Expr>)` which is equivalent semantically but should hopefully be less costly in terms of compile time performance (to be determined). This is extracted out of rust-lang@91b0abd from rust-lang#59288 for easier review and so that the perf implications wrt. `for`-loops can be measured. r? @oli-obk
2 parents 7cdb49d + 4bd36ab commit adce36b

File tree

13 files changed

+92
-52
lines changed

13 files changed

+92
-52
lines changed

‎src/librustc/cfg/construct.rs

+1
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
369369
hir::ExprKind::AddrOf(_, ref e) |
370370
hir::ExprKind::Cast(ref e, _) |
371371
hir::ExprKind::Type(ref e, _) |
372+
hir::ExprKind::Use(ref e) |
372373
hir::ExprKind::Unary(_, ref e) |
373374
hir::ExprKind::Field(ref e, _) |
374375
hir::ExprKind::Yield(ref e) |

‎src/librustc/hir/intravisit.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1029,6 +1029,9 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
10291029
visitor.visit_expr(subexpression);
10301030
visitor.visit_ty(typ)
10311031
}
1032+
ExprKind::Use(ref subexpression) => {
1033+
visitor.visit_expr(subexpression);
1034+
}
10321035
ExprKind::If(ref head_expression, ref if_block, ref optional_else) => {
10331036
visitor.visit_expr(head_expression);
10341037
visitor.visit_expr(if_block);

‎src/librustc/hir/lowering.rs

+16-29
Original file line numberDiff line numberDiff line change
@@ -4738,16 +4738,11 @@ impl<'a> LoweringContext<'a> {
47384738
hir::MatchSource::ForLoopDesugar,
47394739
));
47404740

4741-
// `{ let _result = ...; _result }`
4742-
// Underscore prevents an `unused_variables` lint if the head diverges.
4743-
let result_ident = self.str_to_ident("_result");
4744-
let (let_stmt, let_stmt_binding) =
4745-
self.stmt_let(e.span, false, result_ident, match_expr);
4746-
4747-
let result = P(self.expr_ident(e.span, result_ident, let_stmt_binding));
4748-
let block = P(self.block_all(e.span, hir_vec![let_stmt], Some(result)));
4749-
// Add the attributes to the outer returned expr node.
4750-
return self.expr_block(block, e.attrs.clone());
4741+
// This is effectively `{ let _result = ...; _result }`.
4742+
// The construct was introduced in #21984.
4743+
// FIXME(60253): Is this still necessary?
4744+
// Also, add the attributes to the outer returned expr node.
4745+
return self.expr_use(head_sp, match_expr, e.attrs.clone())
47514746
}
47524747

47534748
// Desugar `ExprKind::Try`
@@ -5117,6 +5112,17 @@ impl<'a> LoweringContext<'a> {
51175112
)
51185113
}
51195114

5115+
/// Wrap the given `expr` in `hir::ExprKind::Use`.
5116+
///
5117+
/// In terms of drop order, it has the same effect as
5118+
/// wrapping `expr` in `{ let _t = $expr; _t }` but
5119+
/// should provide better compile-time performance.
5120+
///
5121+
/// The drop order can be important in e.g. `if expr { .. }`.
5122+
fn expr_use(&mut self, span: Span, expr: P<hir::Expr>, attrs: ThinVec<Attribute>) -> hir::Expr {
5123+
self.expr(span, hir::ExprKind::Use(expr), attrs)
5124+
}
5125+
51205126
fn expr_match(
51215127
&mut self,
51225128
span: Span,
@@ -5172,25 +5178,6 @@ impl<'a> LoweringContext<'a> {
51725178
}
51735179
}
51745180

5175-
fn stmt_let(
5176-
&mut self,
5177-
sp: Span,
5178-
mutbl: bool,
5179-
ident: Ident,
5180-
ex: P<hir::Expr>,
5181-
) -> (hir::Stmt, hir::HirId) {
5182-
let (pat, pat_hid) = if mutbl {
5183-
self.pat_ident_binding_mode(sp, ident, hir::BindingAnnotation::Mutable)
5184-
} else {
5185-
self.pat_ident(sp, ident)
5186-
};
5187-
5188-
(
5189-
self.stmt_let_pat(sp, Some(ex), pat, hir::LocalSource::Normal),
5190-
pat_hid,
5191-
)
5192-
}
5193-
51945181
fn block_expr(&mut self, expr: P<hir::Expr>) -> hir::Block {
51955182
self.block_all(expr.span, hir::HirVec::new(), Some(expr))
51965183
}

‎src/librustc/hir/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -1366,6 +1366,7 @@ impl Expr {
13661366
ExprKind::Unary(..) => ExprPrecedence::Unary,
13671367
ExprKind::Lit(_) => ExprPrecedence::Lit,
13681368
ExprKind::Type(..) | ExprKind::Cast(..) => ExprPrecedence::Cast,
1369+
ExprKind::Use(ref expr, ..) => expr.precedence(),
13691370
ExprKind::If(..) => ExprPrecedence::If,
13701371
ExprKind::While(..) => ExprPrecedence::While,
13711372
ExprKind::Loop(..) => ExprPrecedence::Loop,
@@ -1437,6 +1438,7 @@ impl Expr {
14371438
ExprKind::Binary(..) |
14381439
ExprKind::Yield(..) |
14391440
ExprKind::Cast(..) |
1441+
ExprKind::Use(..) |
14401442
ExprKind::Err => {
14411443
false
14421444
}
@@ -1486,6 +1488,10 @@ pub enum ExprKind {
14861488
Cast(P<Expr>, P<Ty>),
14871489
/// A type reference (e.g., `Foo`).
14881490
Type(P<Expr>, P<Ty>),
1491+
/// Semantically equivalent to `{ let _t = expr; _t }`.
1492+
/// Maps directly to `hair::ExprKind::Use`.
1493+
/// Only exists to tweak the drop order in HIR.
1494+
Use(P<Expr>),
14891495
/// An `if` block, with an optional else block.
14901496
///
14911497
/// I.e., `if <expr> { <expr> } else { <expr> }`.

‎src/librustc/hir/print.rs

+40-13
Original file line numberDiff line numberDiff line change
@@ -995,23 +995,32 @@ impl<'a> State<'a> {
995995
self.ann.post(self, AnnNode::SubItem(ii.hir_id))
996996
}
997997

998+
pub fn print_local(
999+
&mut self,
1000+
init: Option<&hir::Expr>,
1001+
decl: impl Fn(&mut Self) -> io::Result<()>
1002+
) -> io::Result<()> {
1003+
self.space_if_not_bol()?;
1004+
self.ibox(indent_unit)?;
1005+
self.word_nbsp("let")?;
1006+
1007+
self.ibox(indent_unit)?;
1008+
decl(self)?;
1009+
self.end()?;
1010+
1011+
if let Some(ref init) = init {
1012+
self.nbsp()?;
1013+
self.word_space("=")?;
1014+
self.print_expr(&init)?;
1015+
}
1016+
self.end()
1017+
}
1018+
9981019
pub fn print_stmt(&mut self, st: &hir::Stmt) -> io::Result<()> {
9991020
self.maybe_print_comment(st.span.lo())?;
10001021
match st.node {
10011022
hir::StmtKind::Local(ref loc) => {
1002-
self.space_if_not_bol()?;
1003-
self.ibox(indent_unit)?;
1004-
self.word_nbsp("let")?;
1005-
1006-
self.ibox(indent_unit)?;
1007-
self.print_local_decl(&loc)?;
1008-
self.end()?;
1009-
if let Some(ref init) = loc.init {
1010-
self.nbsp()?;
1011-
self.word_space("=")?;
1012-
self.print_expr(&init)?;
1013-
}
1014-
self.end()?
1023+
self.print_local(loc.init.deref(), |this| this.print_local_decl(&loc))?;
10151024
}
10161025
hir::StmtKind::Item(item) => {
10171026
self.ann.nested(self, Nested::Item(item))?
@@ -1379,6 +1388,24 @@ impl<'a> State<'a> {
13791388
self.word_space(":")?;
13801389
self.print_type(&ty)?;
13811390
}
1391+
hir::ExprKind::Use(ref init) => {
1392+
// Print `{`:
1393+
self.cbox(indent_unit)?;
1394+
self.ibox(0)?;
1395+
self.bopen()?;
1396+
1397+
// Print `let _t = $init;`:
1398+
let temp = ast::Ident::from_str("_t");
1399+
self.print_local(Some(init), |this| this.print_ident(temp))?;
1400+
self.s.word(";")?;
1401+
1402+
// Print `_t`:
1403+
self.space_if_not_bol()?;
1404+
self.print_ident(temp)?;
1405+
1406+
// Print `}`:
1407+
self.bclose_maybe_open(expr.span, indent_unit, true)?;
1408+
}
13821409
hir::ExprKind::If(ref test, ref blk, ref elseopt) => {
13831410
self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e))?;
13841411
}

‎src/librustc/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#![feature(box_syntax)]
3838
#![feature(core_intrinsics)]
3939
#![feature(drain_filter)]
40+
#![feature(inner_deref)]
4041
#![cfg_attr(windows, feature(libc))]
4142
#![feature(never_type)]
4243
#![feature(exhaustive_patterns)]

‎src/librustc/middle/expr_use_visitor.rs

+4
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,10 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
521521
self.consume_expr(&base);
522522
}
523523

524+
hir::ExprKind::Use(ref expr) => {
525+
self.consume_expr(&expr);
526+
}
527+
524528
hir::ExprKind::AssignOp(_, ref lhs, ref rhs) => {
525529
if self.mc.tables.is_method_call(expr) {
526530
self.consume_expr(lhs);

‎src/librustc/middle/liveness.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,7 @@ fn visit_expr<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, expr: &'tcx Expr) {
521521
hir::ExprKind::Binary(..) |
522522
hir::ExprKind::AddrOf(..) |
523523
hir::ExprKind::Cast(..) |
524+
hir::ExprKind::Use(..) |
524525
hir::ExprKind::Unary(..) |
525526
hir::ExprKind::Break(..) |
526527
hir::ExprKind::Continue(_) |
@@ -1221,6 +1222,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
12211222
hir::ExprKind::AddrOf(_, ref e) |
12221223
hir::ExprKind::Cast(ref e, _) |
12231224
hir::ExprKind::Type(ref e, _) |
1225+
hir::ExprKind::Use(ref e) |
12241226
hir::ExprKind::Unary(_, ref e) |
12251227
hir::ExprKind::Yield(ref e) |
12261228
hir::ExprKind::Repeat(ref e, _) => {
@@ -1524,9 +1526,9 @@ fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) {
15241526
hir::ExprKind::Match(..) | hir::ExprKind::While(..) | hir::ExprKind::Loop(..) |
15251527
hir::ExprKind::Index(..) | hir::ExprKind::Field(..) |
15261528
hir::ExprKind::Array(..) | hir::ExprKind::Tup(..) | hir::ExprKind::Binary(..) |
1527-
hir::ExprKind::Cast(..) | hir::ExprKind::Unary(..) | hir::ExprKind::Ret(..) |
1528-
hir::ExprKind::Break(..) | hir::ExprKind::Continue(..) | hir::ExprKind::Lit(_) |
1529-
hir::ExprKind::Block(..) | hir::ExprKind::AddrOf(..) |
1529+
hir::ExprKind::Cast(..) | hir::ExprKind::Use(..) | hir::ExprKind::Unary(..) |
1530+
hir::ExprKind::Ret(..) | hir::ExprKind::Break(..) | hir::ExprKind::Continue(..) |
1531+
hir::ExprKind::Lit(_) | hir::ExprKind::Block(..) | hir::ExprKind::AddrOf(..) |
15301532
hir::ExprKind::Struct(..) | hir::ExprKind::Repeat(..) |
15311533
hir::ExprKind::Closure(..) | hir::ExprKind::Path(_) | hir::ExprKind::Yield(..) |
15321534
hir::ExprKind::Box(..) | hir::ExprKind::Type(..) | hir::ExprKind::Err => {

‎src/librustc/middle/mem_categorization.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
678678
hir::ExprKind::Assign(..) | hir::ExprKind::AssignOp(..) |
679679
hir::ExprKind::Closure(..) | hir::ExprKind::Ret(..) |
680680
hir::ExprKind::Unary(..) | hir::ExprKind::Yield(..) |
681-
hir::ExprKind::MethodCall(..) | hir::ExprKind::Cast(..) |
681+
hir::ExprKind::MethodCall(..) | hir::ExprKind::Cast(..) | hir::ExprKind::Use(..) |
682682
hir::ExprKind::Array(..) | hir::ExprKind::Tup(..) | hir::ExprKind::If(..) |
683683
hir::ExprKind::Binary(..) | hir::ExprKind::While(..) |
684684
hir::ExprKind::Block(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..) |

‎src/librustc/middle/region.rs

+6
Original file line numberDiff line numberDiff line change
@@ -909,6 +909,12 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr:
909909
visitor.cx.var_parent = visitor.cx.parent;
910910
}
911911

912+
hir::ExprKind::Use(ref expr) => {
913+
// `Use(expr)` does not denote a conditional scope.
914+
// Rather, we want to achieve the same behavior as `{ let _t = expr; _t }`.
915+
terminating(expr.hir_id.local_id);
916+
}
917+
912918
hir::ExprKind::AssignOp(..) | hir::ExprKind::Index(..) |
913919
hir::ExprKind::Unary(..) | hir::ExprKind::Call(..) | hir::ExprKind::MethodCall(..) => {
914920
// FIXME(https://github.com/rust-lang/rfcs/issues/811) Nested method calls

‎src/librustc_mir/hair/cx/expr.rs

+3
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,9 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
759759
}
760760
}
761761
}
762+
hir::ExprKind::Use(ref source) => {
763+
ExprKind::Use { source: source.to_ref() }
764+
}
762765
hir::ExprKind::Box(ref value) => {
763766
ExprKind::Box {
764767
value: value.to_ref(),

‎src/librustc_passes/rvalue_promotion.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,9 @@ fn check_expr_kind<'a, 'tcx>(
436436
hir::ExprKind::Err => Promotable,
437437

438438
hir::ExprKind::AddrOf(_, ref expr) |
439-
hir::ExprKind::Repeat(ref expr, _) => {
439+
hir::ExprKind::Repeat(ref expr, _) |
440+
hir::ExprKind::Type(ref expr, _) |
441+
hir::ExprKind::Use(ref expr) => {
440442
v.check_expr(&expr)
441443
}
442444

@@ -483,10 +485,6 @@ fn check_expr_kind<'a, 'tcx>(
483485
array_result
484486
}
485487

486-
hir::ExprKind::Type(ref expr, ref _ty) => {
487-
v.check_expr(&expr)
488-
}
489-
490488
hir::ExprKind::Tup(ref hirvec) => {
491489
let mut tup_result = Promotable;
492490
for index in hirvec.iter() {
@@ -495,7 +493,6 @@ fn check_expr_kind<'a, 'tcx>(
495493
tup_result
496494
}
497495

498-
499496
// Conditional control flow (possible to implement).
500497
hir::ExprKind::Match(ref expr, ref hirvec_arm, ref _match_source) => {
501498
// Compute the most demanding borrow from all the arms'

‎src/librustc_typeck/check/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -4533,6 +4533,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
45334533
self.check_expr_eq_type(&e, ty);
45344534
ty
45354535
}
4536+
ExprKind::Use(ref e) => {
4537+
self.check_expr_with_expectation(e, expected)
4538+
}
45364539
ExprKind::Array(ref args) => {
45374540
let uty = expected.to_option(self).and_then(|uty| {
45384541
match uty.sty {

0 commit comments

Comments
 (0)