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

Implement destructuring assignment #71156

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a8c784c
Add `destructuring_assignment` feature gate
varkor Dec 22, 2019
fc4fd10
Allow recursive tuples on the LHS of assignments
fanzier Apr 8, 2020
397e914
Handle slices and structs as well
fanzier Apr 9, 2020
7b10eec
Handle `..` except in structs
fanzier Apr 9, 2020
7823897
Fix tidy
fanzier Apr 10, 2020
475cd1e
Handle ordinary assignments as before
fanzier Apr 10, 2020
a6562bd
Only lower tuple structs if in fact a constructor
fanzier Apr 10, 2020
5e39caa
Disallow `.. rest` in struct on LHS of an assignment
fanzier Apr 10, 2020
330b181
Check for constructors in early return condition
fanzier Apr 11, 2020
27e43ae
Add tests
fanzier Apr 11, 2020
f54533b
Allow `_` as an expression
fanzier Apr 13, 2020
3631ab5
Allow `_` on the LHS of an assignment to discard a value
fanzier Apr 13, 2020
1f65f5d
Allow `..` in struct expressions on the LHS of assignments
fanzier Apr 14, 2020
2415a64
Improve error message for struct expression missing a base expression…
fanzier Apr 14, 2020
9b38750
Adjust comments and diagnostics
varkor Apr 14, 2020
1841f2d
Update feature gate issue
varkor Apr 14, 2020
76bcd89
Support `(..)`
varkor Apr 14, 2020
6b18424
Add more complex example
varkor Apr 14, 2020
ced8ce4
Fix tests
fanzier Apr 14, 2020
fb02d99
Fix handling of rest without base in LHS
varkor Apr 15, 2020
ac37e3c
Add additional tests for trailing `..`
varkor Apr 15, 2020
f015a2e
Add pre-expansion gating
varkor Apr 15, 2020
9a5d725
Make pre-expansion gate slightly less obnoxious
varkor Apr 15, 2020
fbced7d
Fix ui-fulldeps test
varkor Apr 15, 2020
60552c7
Fix rebase issues
varkor Sep 14, 2020
8614f44
Add support for `StructRest` in clippy
varkor Sep 14, 2020
a0d4c55
Prevent default match bindings in destructuring assignment
varkor Sep 14, 2020
c25779a
Fix rebase issue and test
fanzier Oct 11, 2020
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
22 changes: 17 additions & 5 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,7 @@ pub struct Expr {

// `Expr` is used a lot. Make sure it doesn't unintentionally get bigger.
#[cfg(target_arch = "x86_64")]
rustc_data_structures::static_assert_size!(Expr, 112);
rustc_data_structures::static_assert_size!(Expr, 120);

impl Expr {
/// Returns `true` if this expression would be valid somewhere that expects a value;
Expand Down Expand Up @@ -1192,6 +1192,7 @@ impl Expr {
ExprKind::Field(..) => ExprPrecedence::Field,
ExprKind::Index(..) => ExprPrecedence::Index,
ExprKind::Range(..) => ExprPrecedence::Range,
ExprKind::Underscore => ExprPrecedence::Path,
ExprKind::Path(..) => ExprPrecedence::Path,
ExprKind::AddrOf(..) => ExprPrecedence::AddrOf,
ExprKind::Break(..) => ExprPrecedence::Break,
Expand All @@ -1218,6 +1219,16 @@ pub enum RangeLimits {
Closed,
}

#[derive(Clone, Encodable, Decodable, Debug)]
pub enum StructRest {
/// `..x`.
Base(P<Expr>),
/// `..`.
Rest(Span),
/// No trailing `..` or expression.
None,
}

#[derive(Clone, Encodable, Decodable, Debug)]
pub enum ExprKind {
/// A `box x` expression.
Expand Down Expand Up @@ -1312,8 +1323,10 @@ pub enum ExprKind {
Field(P<Expr>, Ident),
/// An indexing operation (e.g., `foo[2]`).
Index(P<Expr>, P<Expr>),
/// A range (e.g., `1..2`, `1..`, `..2`, `1..=2`, `..=2`).
/// A range (e.g., `1..2`, `1..`, `..2`, `1..=2`, `..=2`; and `..` in destructuring assingment).
Range(Option<P<Expr>>, Option<P<Expr>>, RangeLimits),
/// An underscore, used in destructuring assignment to ignore a value.
Underscore,

/// Variable reference, possibly containing `::` and/or type
/// parameters (e.g., `foo::bar::<baz>`).
Expand All @@ -1340,9 +1353,8 @@ pub enum ExprKind {

/// A struct literal expression.
///
/// E.g., `Foo {x: 1, y: 2}`, or `Foo {x: 1, .. base}`,
/// where `base` is the `Option<Expr>`.
Struct(Path, Vec<Field>, Option<P<Expr>>),
/// E.g., `Foo {x: 1, y: 2}`, or `Foo {x: 1, .. rest}`.
Struct(Path, Vec<Field>, StructRest),

/// An array literal constructed from one repeated element.
///
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
visit_opt(e1, |e1| vis.visit_expr(e1));
visit_opt(e2, |e2| vis.visit_expr(e2));
}
ExprKind::Underscore => {}
ExprKind::Path(qself, path) => {
vis.visit_qself(qself);
vis.visit_path(path);
Expand Down Expand Up @@ -1274,7 +1275,9 @@ pub fn noop_visit_expr<T: MutVisitor>(
ExprKind::Struct(path, fields, expr) => {
vis.visit_path(path);
fields.flat_map_in_place(|field| vis.flat_map_field(field));
visit_opt(expr, |expr| vis.visit_expr(expr));
if let StructRest::Base(expr) = expr {
vis.visit_expr(expr);
}
}
ExprKind::Paren(expr) => {
vis.visit_expr(expr);
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,9 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
ExprKind::Struct(ref path, ref fields, ref optional_base) => {
visitor.visit_path(path, expression.id);
walk_list!(visitor, visit_field, fields);
walk_list!(visitor, visit_expr, optional_base);
if let StructRest::Base(expr) = optional_base {
visitor.visit_expr(expr);
}
}
ExprKind::Tup(ref subexpressions) => {
walk_list!(visitor, visit_expr, subexpressions);
Expand Down Expand Up @@ -807,6 +809,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
walk_list!(visitor, visit_expr, start);
walk_list!(visitor, visit_expr, end);
}
ExprKind::Underscore => {}
ExprKind::Path(ref maybe_qself, ref path) => {
if let Some(ref qself) = *maybe_qself {
visitor.visit_ty(&qself.ty);
Expand Down
260 changes: 255 additions & 5 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::thin_vec::ThinVec;
use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::def::{DefKind, Res};
use rustc_span::hygiene::ForLoopLoc;
use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned};
use rustc_span::symbol::{sym, Ident, Symbol};
Expand Down Expand Up @@ -146,7 +146,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ExprKind::Block(self.lower_block(blk, opt_label.is_some()), opt_label)
}
ExprKind::Assign(ref el, ref er, span) => {
hir::ExprKind::Assign(self.lower_expr(el), self.lower_expr(er), span)
self.lower_expr_assign(el, er, span, e.span)
}
ExprKind::AssignOp(op, ref el, ref er) => hir::ExprKind::AssignOp(
self.lower_binop(op),
Expand All @@ -163,6 +163,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
ExprKind::Range(ref e1, ref e2, lims) => {
self.lower_expr_range(e.span, e1.as_deref(), e2.as_deref(), lims)
}
ExprKind::Underscore => {
self.sess
.struct_span_err(
e.span,
"expected expression, found reserved identifier `_`",
)
.span_label(e.span, "expected expression")
.emit();
hir::ExprKind::Err
}
ExprKind::Path(ref qself, ref path) => {
let qpath = self.lower_qpath(
e.id,
Expand All @@ -186,8 +196,18 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
ExprKind::InlineAsm(ref asm) => self.lower_expr_asm(e.span, asm),
ExprKind::LlvmInlineAsm(ref asm) => self.lower_expr_llvm_asm(asm),
ExprKind::Struct(ref path, ref fields, ref maybe_expr) => {
let maybe_expr = maybe_expr.as_ref().map(|x| self.lower_expr(x));
ExprKind::Struct(ref path, ref fields, ref rest) => {
let rest = match rest {
StructRest::Base(e) => Some(self.lower_expr(e)),
StructRest::Rest(sp) => {
self.sess
.struct_span_err(*sp, "base expression required after `..`")
.span_label(*sp, "add a base expression here")
.emit();
Some(&*self.arena.alloc(self.expr_err(*sp)))
}
StructRest::None => None,
};
hir::ExprKind::Struct(
self.arena.alloc(self.lower_qpath(
e.id,
Expand All @@ -197,7 +217,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
ImplTraitContext::disallowed(),
)),
self.arena.alloc_from_iter(fields.iter().map(|x| self.lower_field(x))),
maybe_expr,
rest,
)
}
ExprKind::Yield(ref opt_expr) => self.lower_expr_yield(e.span, opt_expr.as_deref()),
Expand Down Expand Up @@ -840,6 +860,236 @@ impl<'hir> LoweringContext<'_, 'hir> {
})
}

/// Destructure the LHS of complex assignments.
/// For instance, lower `(a, b) = t` to `{ let (lhs1, lhs2) = t; a = lhs1; b = lhs2; }`.
fn lower_expr_assign(
&mut self,
lhs: &Expr,
rhs: &Expr,
eq_sign_span: Span,
whole_span: Span,
) -> hir::ExprKind<'hir> {
// Return early in case of an ordinary assignment.
let is_ordinary = match &lhs.kind {
ExprKind::Array(..)
| ExprKind::Struct(..)
| ExprKind::Tup(..)
| ExprKind::Underscore => false,
ExprKind::Call(callee, ..) => {
// Check for tuple struct constructor.
if let ExprKind::Path(qself, path) = &callee.kind {
let qpath = self.lower_qpath(
callee.id,
qself,
path,
ParamMode::Optional,
ImplTraitContext::disallowed(),
);
match qpath {
hir::QPath::Resolved(
_,
hir::Path { res: Res::Def(DefKind::Ctor(..), _), .. },
) => false,
_ => true,
}
} else {
true
}
}
// `(..)`.
ExprKind::Paren(e) => {
if let ExprKind::Range(None, None, RangeLimits::HalfOpen) = e.kind {
false
} else {
true
}
}
_ => true,
};
if is_ordinary {
return hir::ExprKind::Assign(self.lower_expr(lhs), self.lower_expr(rhs), eq_sign_span);
}

let mut assignments = vec![];

// The LHS becomes a pattern: `(lhs1, lhs2)`.
let pat = self.destructure_assign(lhs, eq_sign_span, &mut assignments);
let rhs = self.lower_expr(rhs);

// Introduce a `let` for destructuring: `let (lhs1, lhs2) = t`.
let destructure_let = self.stmt_let_pat(
ThinVec::new(),
whole_span,
Some(rhs),
pat,
hir::LocalSource::AssignDesugar(eq_sign_span),
);

// `a = lhs1; b = lhs2;`.
let stmts = self
.arena
.alloc_from_iter(std::iter::once(destructure_let).chain(assignments.into_iter()));

// Wrap everything in a block.
hir::ExprKind::Block(&self.block_all(whole_span, stmts, None), None)
}

/// Convert the LHS of a destructuring assignment to a pattern.
/// Each sub-assignment is recorded in `assignments`.
fn destructure_assign(
&mut self,
lhs: &Expr,
eq_sign_span: Span,
assignments: &mut Vec<hir::Stmt<'hir>>,
) -> &'hir hir::Pat<'hir> {
match &lhs.kind {
// Underscore pattern.
ExprKind::Underscore => {
return self.pat_without_dbm(lhs.span, hir::PatKind::Wild);
}
// Slice patterns.
ExprKind::Array(elements) => {
let (pats, rest) =
self.destructure_sequence(elements, "slice", eq_sign_span, assignments);
let slice_pat = if let Some((i, span)) = rest {
let (before, after) = pats.split_at(i);
hir::PatKind::Slice(
before,
Some(self.pat_without_dbm(span, hir::PatKind::Wild)),
after,
)
} else {
hir::PatKind::Slice(pats, None, &[])
};
return self.pat_without_dbm(lhs.span, slice_pat);
}
// Tuple structs.
ExprKind::Call(callee, args) => {
if let ExprKind::Path(qself, path) = &callee.kind {
let (pats, rest) =
self.destructure_sequence(args, "tuple struct", eq_sign_span, assignments);
let qpath = self.lower_qpath(
callee.id,
qself,
path,
ParamMode::Optional,
ImplTraitContext::disallowed(),
);
match qpath {
hir::QPath::Resolved(
_,
hir::Path { res: Res::Def(DefKind::Ctor(..), _), .. },
) => {
// Destructure like a tuple struct since the path is in fact a constructor.
let tuple_struct_pat =
hir::PatKind::TupleStruct(qpath, pats, rest.map(|r| r.0));
return self.pat_without_dbm(lhs.span, tuple_struct_pat);
}
_ => {
// If the path is not a constructor, lower as an ordinary LHS.
// Typecheck will report an error later.
}
}
}
}
// Structs.
ExprKind::Struct(path, fields, rest) => {
let field_pats = self.arena.alloc_from_iter(fields.iter().map(|f| {
let pat = self.destructure_assign(&f.expr, eq_sign_span, assignments);
hir::FieldPat {
hir_id: self.next_id(),
ident: f.ident,
pat,
is_shorthand: f.is_shorthand,
span: f.span,
}
}));
let qpath = self.lower_qpath(
lhs.id,
&None,
path,
ParamMode::Optional,
ImplTraitContext::disallowed(),
);
let fields_omitted = match rest {
StructRest::Base(e) => {
self.sess
.struct_span_err(
e.span,
"functional record updates are not allowed in destructuring \
assignments",
)
.span_suggestion(
e.span,
"consider removing the trailing pattern",
String::new(),
rustc_errors::Applicability::MachineApplicable,
)
.emit();
true
}
StructRest::Rest(_) => true,
StructRest::None => false,
};
let struct_pat = hir::PatKind::Struct(qpath, field_pats, fields_omitted);
return self.pat_without_dbm(lhs.span, struct_pat);
}
// Tuples.
ExprKind::Tup(elements) => {
let (pats, rest) =
self.destructure_sequence(elements, "tuple", eq_sign_span, assignments);
let tuple_pat = hir::PatKind::Tuple(pats, rest.map(|r| r.0));
return self.pat_without_dbm(lhs.span, tuple_pat);
}
// `(..)`. We special-case this for consistency with declarations.
ExprKind::Paren(e) => {
if let ExprKind::Range(None, None, RangeLimits::HalfOpen) = e.kind {
let tuple_pat = hir::PatKind::Tuple(&[], Some(0));
return self.pat_without_dbm(lhs.span, tuple_pat);
}
}
_ => {}
}
// Treat all other cases as normal lvalue.
let ident = Ident::new(sym::lhs, lhs.span);
let (pat, binding) = self.pat_ident(lhs.span, ident);
let ident = self.expr_ident(lhs.span, ident, binding);
let assign = hir::ExprKind::Assign(self.lower_expr(lhs), ident, eq_sign_span);
let expr = self.expr(lhs.span, assign, ThinVec::new());
assignments.push(self.stmt_expr(lhs.span, expr));
pat
}

/// Destructure a sequence of expressions occurring on the LHS of an assignment.
/// Such a sequence occurs in a tuple (struct)/slice.
/// Return a sequence of corresponding patterns, and the index and the span of `..` if it
/// exists.
/// Each sub-assignment is recorded in `assignments`.
fn destructure_sequence(
&mut self,
elements: &[AstP<Expr>],
ctx: &str,
eq_sign_span: Span,
assignments: &mut Vec<hir::Stmt<'hir>>,
) -> (&'hir [&'hir hir::Pat<'hir>], Option<(usize, Span)>) {
let mut rest = None;
let elements =
self.arena.alloc_from_iter(elements.iter().enumerate().filter_map(|(i, e)| {
// Check for `..` pattern.
if let ExprKind::Range(None, None, RangeLimits::HalfOpen) = e.kind {
if let Some((_, prev_span)) = rest {
self.ban_extra_rest_pat(e.span, prev_span, ctx);
} else {
rest = Some((i, e.span));
}
None
} else {
Some(self.destructure_assign(e, eq_sign_span, assignments))
}
}));
(elements, rest)
}

/// Desugar `<start>..=<end>` into `std::ops::RangeInclusive::new(<start>, <end>)`.
fn lower_expr_range_closed(&mut self, span: Span, e1: &Expr, e2: &Expr) -> hir::ExprKind<'hir> {
let e1 = self.lower_expr_mut(e1);
Expand Down
Loading