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

Introduce hir::ExprKind::Use and employ in for loop desugaring. #60225

Merged
merged 1 commit into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
hir::ExprKind::AddrOf(_, ref e) |
hir::ExprKind::Cast(ref e, _) |
hir::ExprKind::Type(ref e, _) |
hir::ExprKind::Use(ref e) |
hir::ExprKind::Unary(_, ref e) |
hir::ExprKind::Field(ref e, _) |
hir::ExprKind::Yield(ref e) |
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,9 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
visitor.visit_expr(subexpression);
visitor.visit_ty(typ)
}
ExprKind::Use(ref subexpression) => {
visitor.visit_expr(subexpression);
}
ExprKind::If(ref head_expression, ref if_block, ref optional_else) => {
visitor.visit_expr(head_expression);
visitor.visit_expr(if_block);
Expand Down
45 changes: 16 additions & 29 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4738,16 +4738,11 @@ impl<'a> LoweringContext<'a> {
hir::MatchSource::ForLoopDesugar,
));

// `{ let _result = ...; _result }`
// Underscore prevents an `unused_variables` lint if the head diverges.
let result_ident = self.str_to_ident("_result");
let (let_stmt, let_stmt_binding) =
self.stmt_let(e.span, false, result_ident, match_expr);

let result = P(self.expr_ident(e.span, result_ident, let_stmt_binding));
let block = P(self.block_all(e.span, hir_vec![let_stmt], Some(result)));
// Add the attributes to the outer returned expr node.
return self.expr_block(block, e.attrs.clone());
// This is effectively `{ let _result = ...; _result }`.
// The construct was introduced in #21984.
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
// FIXME(60253): Is this still necessary?
// Also, add the attributes to the outer returned expr node.
return self.expr_use(head_sp, match_expr, e.attrs.clone())
}

// Desugar `ExprKind::Try`
Expand Down Expand Up @@ -5117,6 +5112,17 @@ impl<'a> LoweringContext<'a> {
)
}

/// Wrap the given `expr` in `hir::ExprKind::Use`.
///
/// In terms of drop order, it has the same effect as
/// wrapping `expr` in `{ let _t = $expr; _t }` but
/// should provide better compile-time performance.
///
/// The drop order can be important in e.g. `if expr { .. }`.
fn expr_use(&mut self, span: Span, expr: P<hir::Expr>, attrs: ThinVec<Attribute>) -> hir::Expr {
self.expr(span, hir::ExprKind::Use(expr), attrs)
}

fn expr_match(
&mut self,
span: Span,
Expand Down Expand Up @@ -5172,25 +5178,6 @@ impl<'a> LoweringContext<'a> {
}
}

fn stmt_let(
&mut self,
sp: Span,
mutbl: bool,
ident: Ident,
ex: P<hir::Expr>,
) -> (hir::Stmt, hir::HirId) {
let (pat, pat_hid) = if mutbl {
self.pat_ident_binding_mode(sp, ident, hir::BindingAnnotation::Mutable)
} else {
self.pat_ident(sp, ident)
};

(
self.stmt_let_pat(sp, Some(ex), pat, hir::LocalSource::Normal),
pat_hid,
)
}

fn block_expr(&mut self, expr: P<hir::Expr>) -> hir::Block {
self.block_all(expr.span, hir::HirVec::new(), Some(expr))
}
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ impl Expr {
ExprKind::Unary(..) => ExprPrecedence::Unary,
ExprKind::Lit(_) => ExprPrecedence::Lit,
ExprKind::Type(..) | ExprKind::Cast(..) => ExprPrecedence::Cast,
ExprKind::Use(ref expr, ..) => expr.precedence(),
ExprKind::If(..) => ExprPrecedence::If,
ExprKind::While(..) => ExprPrecedence::While,
ExprKind::Loop(..) => ExprPrecedence::Loop,
Expand Down Expand Up @@ -1437,6 +1438,7 @@ impl Expr {
ExprKind::Binary(..) |
ExprKind::Yield(..) |
ExprKind::Cast(..) |
ExprKind::Use(..) |
ExprKind::Err => {
false
}
Expand Down Expand Up @@ -1486,6 +1488,10 @@ pub enum ExprKind {
Cast(P<Expr>, P<Ty>),
/// A type reference (e.g., `Foo`).
Type(P<Expr>, P<Ty>),
/// Semantically equivalent to `{ let _t = expr; _t }`.
/// Maps directly to `hair::ExprKind::Use`.
/// Only exists to tweak the drop order in HIR.
Copy link
Member

Choose a reason for hiding this comment

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

Can we have more docs on where this is introduced and why it exists? As it stands it's pretty confusing

Copy link
Contributor Author

@Centril Centril Apr 27, 2019

Choose a reason for hiding this comment

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

It is used in the for loop desugaring as seen in this PR and also in #59288 (see lowering.rs).

Copy link
Member

Choose a reason for hiding this comment

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

I'm not asking you to explain it to me, I figured it out, I'm saying that this should be in the docs.

Copy link
Contributor Author

@Centril Centril Apr 27, 2019

Choose a reason for hiding this comment

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

Sure why not. Some parts haven't been merged yet so it would be slightly weird to talk about future events in the docs. Can you file an issue for now`?

Copy link
Member

Choose a reason for hiding this comment

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

I don't find the name... useful (sorry). I'd call this SomethingScope, which for the lack of a better option would be TerminatingScope.

I believe that ExprKind::Use elsewhere has nothing to do with scopes (just like mir::Rvalue::Use doesn't), so using Use is misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eddyb Fwiw, I contemplated ExprKind::Terminating. I agree completely that ::Use sucks but I didn't have anything super better at the time.

Use(P<Expr>),
/// An `if` block, with an optional else block.
///
/// I.e., `if <expr> { <expr> } else { <expr> }`.
Expand Down
53 changes: 40 additions & 13 deletions src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,23 +995,32 @@ impl<'a> State<'a> {
self.ann.post(self, AnnNode::SubItem(ii.hir_id))
}

pub fn print_local(
&mut self,
init: Option<&hir::Expr>,
decl: impl Fn(&mut Self) -> io::Result<()>
) -> io::Result<()> {
self.space_if_not_bol()?;
self.ibox(indent_unit)?;
self.word_nbsp("let")?;

self.ibox(indent_unit)?;
decl(self)?;
self.end()?;

if let Some(ref init) = init {
self.nbsp()?;
self.word_space("=")?;
self.print_expr(&init)?;
}
self.end()
}

pub fn print_stmt(&mut self, st: &hir::Stmt) -> io::Result<()> {
self.maybe_print_comment(st.span.lo())?;
match st.node {
hir::StmtKind::Local(ref loc) => {
self.space_if_not_bol()?;
self.ibox(indent_unit)?;
self.word_nbsp("let")?;

self.ibox(indent_unit)?;
self.print_local_decl(&loc)?;
self.end()?;
if let Some(ref init) = loc.init {
self.nbsp()?;
self.word_space("=")?;
self.print_expr(&init)?;
}
self.end()?
self.print_local(loc.init.deref(), |this| this.print_local_decl(&loc))?;
}
hir::StmtKind::Item(item) => {
self.ann.nested(self, Nested::Item(item))?
Expand Down Expand Up @@ -1379,6 +1388,24 @@ impl<'a> State<'a> {
self.word_space(":")?;
self.print_type(&ty)?;
}
hir::ExprKind::Use(ref init) => {
// Print `{`:
self.cbox(indent_unit)?;
self.ibox(0)?;
self.bopen()?;

// Print `let _t = $init;`:
let temp = ast::Ident::from_str("_t");
self.print_local(Some(init), |this| this.print_ident(temp))?;
self.s.word(";")?;

// Print `_t`:
self.space_if_not_bol()?;
self.print_ident(temp)?;

// Print `}`:
self.bclose_maybe_open(expr.span, indent_unit, true)?;
}
hir::ExprKind::If(ref test, ref blk, ref elseopt) => {
self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e))?;
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#![feature(box_syntax)]
#![feature(core_intrinsics)]
#![feature(drain_filter)]
#![feature(inner_deref)]
#![cfg_attr(windows, feature(libc))]
#![feature(never_type)]
#![feature(exhaustive_patterns)]
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,10 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
self.consume_expr(&base);
}

hir::ExprKind::Use(ref expr) => {
self.consume_expr(&expr);
}

hir::ExprKind::AssignOp(_, ref lhs, ref rhs) => {
if self.mc.tables.is_method_call(expr) {
self.consume_expr(lhs);
Expand Down
8 changes: 5 additions & 3 deletions src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ fn visit_expr<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, expr: &'tcx Expr) {
hir::ExprKind::Binary(..) |
hir::ExprKind::AddrOf(..) |
hir::ExprKind::Cast(..) |
hir::ExprKind::Use(..) |
hir::ExprKind::Unary(..) |
hir::ExprKind::Break(..) |
hir::ExprKind::Continue(_) |
Expand Down Expand Up @@ -1221,6 +1222,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
hir::ExprKind::AddrOf(_, ref e) |
hir::ExprKind::Cast(ref e, _) |
hir::ExprKind::Type(ref e, _) |
hir::ExprKind::Use(ref e) |
hir::ExprKind::Unary(_, ref e) |
hir::ExprKind::Yield(ref e) |
hir::ExprKind::Repeat(ref e, _) => {
Expand Down Expand Up @@ -1524,9 +1526,9 @@ fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) {
hir::ExprKind::Match(..) | hir::ExprKind::While(..) | hir::ExprKind::Loop(..) |
hir::ExprKind::Index(..) | hir::ExprKind::Field(..) |
hir::ExprKind::Array(..) | hir::ExprKind::Tup(..) | hir::ExprKind::Binary(..) |
hir::ExprKind::Cast(..) | hir::ExprKind::Unary(..) | hir::ExprKind::Ret(..) |
hir::ExprKind::Break(..) | hir::ExprKind::Continue(..) | hir::ExprKind::Lit(_) |
hir::ExprKind::Block(..) | hir::ExprKind::AddrOf(..) |
hir::ExprKind::Cast(..) | hir::ExprKind::Use(..) | hir::ExprKind::Unary(..) |
hir::ExprKind::Ret(..) | hir::ExprKind::Break(..) | hir::ExprKind::Continue(..) |
hir::ExprKind::Lit(_) | hir::ExprKind::Block(..) | hir::ExprKind::AddrOf(..) |
hir::ExprKind::Struct(..) | hir::ExprKind::Repeat(..) |
hir::ExprKind::Closure(..) | hir::ExprKind::Path(_) | hir::ExprKind::Yield(..) |
hir::ExprKind::Box(..) | hir::ExprKind::Type(..) | hir::ExprKind::Err => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
hir::ExprKind::Assign(..) | hir::ExprKind::AssignOp(..) |
hir::ExprKind::Closure(..) | hir::ExprKind::Ret(..) |
hir::ExprKind::Unary(..) | hir::ExprKind::Yield(..) |
hir::ExprKind::MethodCall(..) | hir::ExprKind::Cast(..) |
hir::ExprKind::MethodCall(..) | hir::ExprKind::Cast(..) | hir::ExprKind::Use(..) |
hir::ExprKind::Array(..) | hir::ExprKind::Tup(..) | hir::ExprKind::If(..) |
hir::ExprKind::Binary(..) | hir::ExprKind::While(..) |
hir::ExprKind::Block(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..) |
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,12 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr:
visitor.cx.var_parent = visitor.cx.parent;
}

hir::ExprKind::Use(ref expr) => {
// `Use(expr)` does not denote a conditional scope.
// Rather, we want to achieve the same behavior as `{ let _t = expr; _t }`.
terminating(expr.hir_id.local_id);
}

hir::ExprKind::AssignOp(..) | hir::ExprKind::Index(..) |
hir::ExprKind::Unary(..) | hir::ExprKind::Call(..) | hir::ExprKind::MethodCall(..) => {
// FIXME(https://github.com/rust-lang/rfcs/issues/811) Nested method calls
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_mir/hair/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,9 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
}
}
}
hir::ExprKind::Use(ref source) => {
ExprKind::Use { source: source.to_ref() }
}
hir::ExprKind::Box(ref value) => {
ExprKind::Box {
value: value.to_ref(),
Expand Down
9 changes: 3 additions & 6 deletions src/librustc_passes/rvalue_promotion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,9 @@ fn check_expr_kind<'a, 'tcx>(
hir::ExprKind::Err => Promotable,

hir::ExprKind::AddrOf(_, ref expr) |
hir::ExprKind::Repeat(ref expr, _) => {
hir::ExprKind::Repeat(ref expr, _) |
hir::ExprKind::Type(ref expr, _) |
hir::ExprKind::Use(ref expr) => {
v.check_expr(&expr)
}

Expand Down Expand Up @@ -483,10 +485,6 @@ fn check_expr_kind<'a, 'tcx>(
array_result
}

hir::ExprKind::Type(ref expr, ref _ty) => {
v.check_expr(&expr)
}

hir::ExprKind::Tup(ref hirvec) => {
let mut tup_result = Promotable;
for index in hirvec.iter() {
Expand All @@ -495,7 +493,6 @@ fn check_expr_kind<'a, 'tcx>(
tup_result
}


// Conditional control flow (possible to implement).
hir::ExprKind::Match(ref expr, ref hirvec_arm, ref _match_source) => {
// Compute the most demanding borrow from all the arms'
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4533,6 +4533,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
self.check_expr_eq_type(&e, ty);
ty
}
ExprKind::Use(ref e) => {
self.check_expr_with_expectation(e, expected)
}
ExprKind::Array(ref args) => {
let uty = expected.to_option(self).and_then(|uty| {
match uty.sty {
Expand Down