-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Introduce move expressions (move($expr))
#155023
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
base: main
Are you sure you want to change the base?
Changes from all commits
17517fd
9f0f8c8
9953aa0
0dcce73
1203e52
71d9fed
cac3426
5aba0a6
c72426a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ use std::mem; | |
| use std::ops::ControlFlow; | ||
| use std::sync::Arc; | ||
|
|
||
| use rustc_ast::node_id::NodeMap; | ||
| use rustc_ast::*; | ||
| use rustc_ast_pretty::pprust::expr_to_string; | ||
| use rustc_data_structures::stack::ensure_sufficient_stack; | ||
|
|
@@ -20,15 +21,50 @@ use visit::{Visitor, walk_expr}; | |
| use super::errors::{ | ||
| AsyncCoroutinesNotSupported, AwaitOnlyInAsyncFnAndBlocks, ClosureCannotBeStatic, | ||
| CoroutineTooManyParameters, FunctionalRecordUpdateDestructuringAssignment, | ||
| InclusiveRangeWithNoEnd, MatchArmWithNoBody, NeverPatternWithBody, NeverPatternWithGuard, | ||
| UnderscoreExprLhsAssign, | ||
| InclusiveRangeWithNoEnd, MatchArmWithNoBody, MoveExprOnlyInPlainClosures, NeverPatternWithBody, | ||
| NeverPatternWithGuard, UnderscoreExprLhsAssign, | ||
| }; | ||
| use super::{ | ||
| GenericArgsMode, ImplTraitContext, LoweringContext, ParamMode, ResolverAstLoweringExt, | ||
| }; | ||
| use crate::errors::{InvalidLegacyConstGenericArg, UseConstGenericArg, YieldInClosure}; | ||
| use crate::{AllowReturnTypeNotation, FnDeclKind, ImplTraitPosition, TryBlockScope}; | ||
|
|
||
| struct MoveExprOccurrence<'a> { | ||
| id: NodeId, | ||
| move_kw_span: Span, | ||
| expr: &'a Expr, | ||
| } | ||
|
|
||
| struct MoveExprCollector<'a> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment please: what does this struct do?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my preference is to have comments that walk through an example, e.g., // Given an expression like this
//
// ```
// || move(foo.bar).clone()
// ```
//
// this will pass walk the AST for the closure and collect `move(foo.bar)` expressions into the vector. |
||
| occurrences: Vec<MoveExprOccurrence<'a>>, | ||
| } | ||
|
|
||
| impl<'a> MoveExprCollector<'a> { | ||
| fn collect(expr: &'a Expr) -> Vec<MoveExprOccurrence<'a>> { | ||
| let mut this = Self { occurrences: Vec::new() }; | ||
| this.visit_expr(expr); | ||
| this.occurrences | ||
| } | ||
| } | ||
|
|
||
| impl<'a> Visitor<'a> for MoveExprCollector<'a> { | ||
| fn visit_expr(&mut self, expr: &'a Expr) { | ||
| match &expr.kind { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...then here you can refer back to the example... // If this is `move` expression (`move(foo.bar)` in the example above) push its id into the vector.
// Else recurse.or something lik ethat. |
||
| ExprKind::Move(inner, move_kw_span) => { | ||
| self.visit_expr(inner); | ||
| self.occurrences.push(MoveExprOccurrence { | ||
| id: expr.id, | ||
| move_kw_span: *move_kw_span, | ||
| expr: inner, | ||
| }); | ||
| } | ||
| ExprKind::Closure(..) | ExprKind::Gen(..) | ExprKind::ConstBlock(..) => {} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth calling this behavior out too: does not recurse into nested closures or const blocks. What about things like... || {
fn bar() {
move(x)
}
}...I guess that'd be an error, but still, seems like we should stop recursing at items. |
||
| _ => walk_expr(self, expr), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| struct WillCreateDefIdsVisitor {} | ||
|
|
||
| impl<'v> rustc_ast::visit::Visitor<'v> for WillCreateDefIdsVisitor { | ||
|
|
@@ -95,11 +131,12 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
| ExprKind::ForLoop { pat, iter, body, label, kind } => { | ||
| return self.lower_expr_for(e, pat, iter, body, *label, *kind); | ||
| } | ||
| ExprKind::Closure(box closure) => return self.lower_expr_closure_expr(e, closure), | ||
| _ => (), | ||
| } | ||
|
|
||
| let expr_hir_id = self.lower_node_id(e.id); | ||
| let attrs = self.lower_attrs(expr_hir_id, &e.attrs, e.span, Target::from_expr(e)); | ||
| self.lower_attrs(expr_hir_id, &e.attrs, e.span, Target::from_expr(e)); | ||
|
|
||
| let kind = match &e.kind { | ||
| ExprKind::Array(exprs) => hir::ExprKind::Array(self.lower_exprs(exprs)), | ||
|
|
@@ -212,43 +249,38 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
| }, | ||
| ), | ||
| ExprKind::Await(expr, await_kw_span) => self.lower_expr_await(*await_kw_span, expr), | ||
| ExprKind::Move(_, move_kw_span) => { | ||
| if !self.tcx.features().move_expr() { | ||
| return self.expr_err(*move_kw_span, self.dcx().has_errors().unwrap()); | ||
| } | ||
| if let Some((ident, binding)) = self | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment here, I don't understand what is going on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. OK, I get it a bit better now (still want a comment), but I don't undersatnd why last is the right thing to do -- I would expect this to be a map, what if there are multiple |
||
| .move_expr_bindings | ||
| .last() | ||
| .and_then(|bindings| bindings.get(&e.id).copied()) | ||
| { | ||
| hir::ExprKind::Path(hir::QPath::Resolved( | ||
| None, | ||
| self.arena.alloc(hir::Path { | ||
| span: self.lower_span(e.span), | ||
| res: Res::Local(binding), | ||
| segments: arena_vec![ | ||
| self; | ||
| hir::PathSegment::new( | ||
| self.lower_ident(ident), | ||
| self.next_id(), | ||
| Res::Local(binding), | ||
| ) | ||
| ], | ||
| }), | ||
| )) | ||
| } else { | ||
| let guar = self | ||
| .dcx() | ||
| .emit_err(MoveExprOnlyInPlainClosures { span: *move_kw_span }); | ||
| hir::ExprKind::Err(guar) | ||
| } | ||
| } | ||
| ExprKind::Use(expr, use_kw_span) => self.lower_expr_use(*use_kw_span, expr), | ||
| ExprKind::Closure(box Closure { | ||
| binder, | ||
| capture_clause, | ||
| constness, | ||
| coroutine_kind, | ||
| movability, | ||
| fn_decl, | ||
| body, | ||
| fn_decl_span, | ||
| fn_arg_span, | ||
| }) => match coroutine_kind { | ||
| Some(coroutine_kind) => self.lower_expr_coroutine_closure( | ||
| binder, | ||
| *capture_clause, | ||
| e.id, | ||
| expr_hir_id, | ||
| *coroutine_kind, | ||
| *constness, | ||
| fn_decl, | ||
| body, | ||
| *fn_decl_span, | ||
| *fn_arg_span, | ||
| ), | ||
| None => self.lower_expr_closure( | ||
| attrs, | ||
| binder, | ||
| *capture_clause, | ||
| e.id, | ||
| *constness, | ||
| *movability, | ||
| fn_decl, | ||
| body, | ||
| *fn_decl_span, | ||
| *fn_arg_span, | ||
| ), | ||
| }, | ||
| ExprKind::Gen(capture_clause, block, genblock_kind, decl_span) => { | ||
| let desugaring_kind = match genblock_kind { | ||
| GenBlockKind::Async => hir::CoroutineDesugaring::Async, | ||
|
|
@@ -383,7 +415,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
|
|
||
| ExprKind::Try(sub_expr) => self.lower_expr_try(e.span, sub_expr), | ||
|
|
||
| ExprKind::Paren(_) | ExprKind::ForLoop { .. } => { | ||
| ExprKind::Paren(_) | ExprKind::ForLoop { .. } | ExprKind::Closure(..) => { | ||
| unreachable!("already handled") | ||
| } | ||
|
|
||
|
|
@@ -792,6 +824,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
| fn_arg_span: None, | ||
| kind: hir::ClosureKind::Coroutine(coroutine_kind), | ||
| constness: hir::Constness::NotConst, | ||
| explicit_captures: &[], | ||
| })) | ||
| } | ||
|
|
||
|
|
@@ -1055,6 +1088,135 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
| hir::ExprKind::Use(self.lower_expr(expr), self.lower_span(use_kw_span)) | ||
| } | ||
|
|
||
| fn lower_expr_closure_expr(&mut self, e: &Expr, closure: &Closure) -> hir::Expr<'hir> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have a short comment calling out the key points "manages lowering closure expressions and desugaring |
||
| let expr_hir_id = self.lower_node_id(e.id); | ||
| let attrs = self.lower_attrs(expr_hir_id, &e.attrs, e.span, Target::from_expr(e)); | ||
|
|
||
| match closure.coroutine_kind { | ||
| // FIXME(TaKO8Ki): Support `move(expr)` in coroutine closures too. | ||
| // For the first step, we only support plain closures. | ||
| Some(coroutine_kind) => hir::Expr { | ||
| hir_id: expr_hir_id, | ||
| kind: self.lower_expr_coroutine_closure( | ||
| &closure.binder, | ||
| closure.capture_clause, | ||
| e.id, | ||
| expr_hir_id, | ||
| coroutine_kind, | ||
| closure.constness, | ||
| &closure.fn_decl, | ||
| &closure.body, | ||
| closure.fn_decl_span, | ||
| closure.fn_arg_span, | ||
| ), | ||
| span: self.lower_span(e.span), | ||
| }, | ||
| None => self.lower_expr_plain_closure_with_move_exprs( | ||
| expr_hir_id, | ||
| attrs, | ||
| &closure.binder, | ||
| closure.capture_clause, | ||
| e.id, | ||
| closure.constness, | ||
| closure.movability, | ||
| &closure.fn_decl, | ||
| &closure.body, | ||
| closure.fn_decl_span, | ||
| closure.fn_arg_span, | ||
| e.span, | ||
| ), | ||
| } | ||
| } | ||
|
|
||
| fn lower_expr_plain_closure_with_move_exprs( | ||
| &mut self, | ||
| expr_hir_id: HirId, | ||
| attrs: &[rustc_hir::Attribute], | ||
| binder: &ClosureBinder, | ||
| capture_clause: CaptureBy, | ||
| closure_id: NodeId, | ||
| constness: Const, | ||
| movability: Movability, | ||
| decl: &FnDecl, | ||
| body: &Expr, | ||
| fn_decl_span: Span, | ||
| fn_arg_span: Span, | ||
| whole_span: Span, | ||
| ) -> hir::Expr<'hir> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment. Give a code example that you can use to walk through the process. |
||
| let occurrences = MoveExprCollector::collect(body); | ||
| if occurrences.is_empty() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment. What does it mean if this is empty in terms of the Rust source code? |
||
| return hir::Expr { | ||
| hir_id: expr_hir_id, | ||
| kind: self.lower_expr_closure( | ||
| attrs, | ||
| binder, | ||
| capture_clause, | ||
| closure_id, | ||
| constness, | ||
| movability, | ||
| decl, | ||
| body, | ||
| fn_decl_span, | ||
| fn_arg_span, | ||
| &[], | ||
| ), | ||
| span: self.lower_span(whole_span), | ||
| }; | ||
| } | ||
|
|
||
| let mut bindings = NodeMap::default(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment this "paragraph". I think that this code is, for each move-expresion, creating a new variable. This would be great to show in terms of the example. |
||
| let mut lowered_occurrences = Vec::with_capacity(occurrences.len()); | ||
| for (index, occurrence) in occurrences.iter().enumerate() { | ||
| let ident = | ||
| Ident::from_str_and_span(&format!("__move_expr_{index}"), occurrence.move_kw_span); | ||
| let (pat, binding) = self.pat_ident(occurrence.expr.span, ident); | ||
| bindings.insert(occurrence.id, (ident, binding)); | ||
| lowered_occurrences.push((occurrence, pat, binding)); | ||
| } | ||
|
|
||
| self.move_expr_bindings.push(bindings); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment this "paragraph". I think that this code is, for each move-expresion, creating the |
||
| let mut stmts = Vec::with_capacity(lowered_occurrences.len()); | ||
| for (occurrence, pat, _) in &lowered_occurrences { | ||
| let init = self.lower_expr(occurrence.expr); | ||
| stmts.push(self.stmt_let_pat( | ||
| None, | ||
| occurrence.expr.span, | ||
| Some(init), | ||
| *pat, | ||
| hir::LocalSource::Normal, | ||
| )); | ||
| } | ||
|
|
||
| let explicit_captures = self.arena.alloc_from_iter( | ||
| lowered_occurrences | ||
| .iter() | ||
| .map(|(_, _, binding)| hir::ExplicitCapture { var_hir_id: *binding }), | ||
| ); | ||
|
|
||
| let closure_expr = self.arena.alloc(hir::Expr { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment: lower the closure itself. The |
||
| hir_id: expr_hir_id, | ||
| kind: self.lower_expr_closure( | ||
| attrs, | ||
| binder, | ||
| capture_clause, | ||
| closure_id, | ||
| constness, | ||
| movability, | ||
| decl, | ||
| body, | ||
| fn_decl_span, | ||
| fn_arg_span, | ||
| explicit_captures, | ||
| ), | ||
| span: self.lower_span(whole_span), | ||
| }); | ||
| self.move_expr_bindings.pop(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd separate this out and put a comment that this restores the original state. |
||
|
|
||
| let stmts = self.arena.alloc_from_iter(stmts); | ||
| let block = self.block_all(whole_span, stmts, Some(closure_expr)); | ||
| self.expr(whole_span, hir::ExprKind::Block(block, None)) | ||
| } | ||
|
|
||
| fn lower_expr_closure( | ||
| &mut self, | ||
| attrs: &[rustc_hir::Attribute], | ||
|
|
@@ -1067,6 +1229,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
| body: &Expr, | ||
| fn_decl_span: Span, | ||
| fn_arg_span: Span, | ||
| explicit_captures: &'hir [hir::ExplicitCapture], | ||
| ) -> hir::ExprKind<'hir> { | ||
| let closure_def_id = self.local_def_id(closure_id); | ||
| let (binder_clause, generic_params) = self.lower_closure_binder(binder); | ||
|
|
@@ -1108,6 +1271,7 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
| fn_arg_span: Some(self.lower_span(fn_arg_span)), | ||
| kind: closure_kind, | ||
| constness: self.lower_constness(constness), | ||
| explicit_captures, | ||
| }); | ||
|
|
||
| hir::ExprKind::Closure(c) | ||
|
|
@@ -1230,7 +1394,9 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> { | |
| // "coroutine that returns &str", rather than directly returning a `&str`. | ||
| kind: hir::ClosureKind::CoroutineClosure(coroutine_desugaring), | ||
| constness: self.lower_constness(constness), | ||
| explicit_captures: &[], | ||
| }); | ||
|
|
||
| hir::ExprKind::Closure(c) | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment please -- what is this struct for? what are the roles of its fields?
View changes since the review