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 for structs and slices #78836

Merged
merged 1 commit into from
Nov 13, 2020
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
19 changes: 14 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 @@ -1218,6 +1218,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,7 +1322,7 @@ 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),

/// Variable reference, possibly containing `::` and/or type
Expand Down Expand Up @@ -1340,9 +1350,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
6 changes: 5 additions & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,11 @@ 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));
match expr {
StructRest::Base(expr) => vis.visit_expr(expr),
StructRest::Rest(_span) => {}
StructRest::None => {}
}
}
ExprKind::Paren(expr) => {
vis.visit_expr(expr);
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,11 @@ 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);
match optional_base {
StructRest::Base(expr) => visitor.visit_expr(expr),
StructRest::Rest(_span) => {}
StructRest::None => {}
}
fanzier marked this conversation as resolved.
Show resolved Hide resolved
}
ExprKind::Tup(ref subexpressions) => {
walk_list!(visitor, visit_expr, subexpressions);
Expand Down
126 changes: 119 additions & 7 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,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 @@ -198,7 +208,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 @@ -851,20 +861,22 @@ impl<'hir> LoweringContext<'_, 'hir> {
whole_span: Span,
) -> hir::ExprKind<'hir> {
// Return early in case of an ordinary assignment.
fn is_ordinary(lhs: &Expr) -> bool {
fn is_ordinary(lower_ctx: &mut LoweringContext<'_, '_>, lhs: &Expr) -> bool {
match &lhs.kind {
ExprKind::Tup(..) => false,
ExprKind::Array(..) | ExprKind::Struct(..) | ExprKind::Tup(..) => false,
// Check for tuple struct constructor.
ExprKind::Call(callee, ..) => lower_ctx.extract_tuple_struct_path(callee).is_none(),
ExprKind::Paren(e) => {
match e.kind {
// We special-case `(..)` for consistency with patterns.
ExprKind::Range(None, None, RangeLimits::HalfOpen) => false,
_ => is_ordinary(e),
_ => is_ordinary(lower_ctx, e),
}
}
_ => true,
}
}
if is_ordinary(lhs) {
if is_ordinary(self, lhs) {
return hir::ExprKind::Assign(self.lower_expr(lhs), self.lower_expr(rhs), eq_sign_span);
}
if !self.sess.features_untracked().destructuring_assignment {
Expand Down Expand Up @@ -902,6 +914,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ExprKind::Block(&self.block_all(whole_span, stmts, None), None)
}

/// If the given expression is a path to a tuple struct, returns that path.
/// It is not a complete check, but just tries to reject most paths early
/// if they are not tuple structs.
/// Type checking will take care of the full validation later.
fn extract_tuple_struct_path<'a>(&mut self, expr: &'a Expr) -> Option<&'a Path> {
// For tuple struct destructuring, it must be a non-qualified path (like in patterns).
if let ExprKind::Path(None, path) = &expr.kind {
// Does the path resolves to something disallowed in a tuple struct/variant pattern?
if let Some(partial_res) = self.resolver.get_partial_res(expr.id) {
if partial_res.unresolved_segments() == 0
&& !partial_res.base_res().expected_in_tuple_struct_pat()
{
return None;
}
}
return Some(path);
}
None
}

/// Convert the LHS of a destructuring assignment to a pattern.
/// Each sub-assignment is recorded in `assignments`.
fn destructure_assign(
Expand All @@ -911,6 +943,86 @@ impl<'hir> LoweringContext<'_, 'hir> {
assignments: &mut Vec<hir::Stmt<'hir>>,
) -> &'hir hir::Pat<'hir> {
match &lhs.kind {
// 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 Some(path) = self.extract_tuple_struct_path(callee) {
let (pats, rest) = self.destructure_sequence(
args,
"tuple struct or variant",
eq_sign_span,
assignments,
);
let qpath = self.lower_qpath(
callee.id,
&None,
path,
ParamMode::Optional,
ImplTraitContext::disallowed(),
);
// Destructure like a tuple struct.
let tuple_struct_pat =
hir::PatKind::TupleStruct(qpath, pats, rest.map(|r| r.0));
return self.pat_without_dbm(lhs.span, tuple_struct_pat);
}
}
// 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) =
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session) {
gate_all!(const_trait_impl, "const trait impls are experimental");
gate_all!(half_open_range_patterns, "half-open range patterns are unstable");
gate_all!(inline_const, "inline-const is experimental");
gate_all!(destructuring_assignment, "destructuring assignments are unstable");

// All uses of `gate_all!` below this point were added in #65742,
// and subsequently disabled (with the non-early gating readded).
Expand Down
21 changes: 10 additions & 11 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1729,7 +1729,7 @@ impl<'a> State<'a> {
&mut self,
path: &ast::Path,
fields: &[ast::Field],
wth: &Option<P<ast::Expr>>,
rest: &ast::StructRest,
attrs: &[ast::Attribute],
) {
self.print_path(path, true, 0);
Expand All @@ -1750,22 +1750,21 @@ impl<'a> State<'a> {
},
|f| f.span,
);
match *wth {
Some(ref expr) => {
match rest {
ast::StructRest::Base(_) | ast::StructRest::Rest(_) => {
self.ibox(INDENT_UNIT);
if !fields.is_empty() {
self.s.word(",");
self.s.space();
}
self.s.word("..");
self.print_expr(expr);
self.end();
}
_ => {
if !fields.is_empty() {
self.s.word(",")
if let ast::StructRest::Base(ref expr) = *rest {
self.print_expr(expr);
}
self.end();
}
ast::StructRest::None if !fields.is_empty() => self.s.word(","),
_ => {}
}
self.s.word("}");
}
Expand Down Expand Up @@ -1891,8 +1890,8 @@ impl<'a> State<'a> {
ast::ExprKind::Repeat(ref element, ref count) => {
self.print_expr_repeat(element, count, attrs);
}
ast::ExprKind::Struct(ref path, ref fields, ref wth) => {
self.print_expr_struct(path, &fields[..], wth, attrs);
ast::ExprKind::Struct(ref path, ref fields, ref rest) => {
self.print_expr_struct(path, &fields[..], rest, attrs);
}
ast::ExprKind::Tup(ref exprs) => {
self.print_expr_tup(&exprs[..], attrs);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ impl<'a> ExtCtxt<'a> {
path: ast::Path,
fields: Vec<ast::Field>,
) -> P<ast::Expr> {
self.expr(span, ast::ExprKind::Struct(path, fields, None))
self.expr(span, ast::ExprKind::Struct(path, fields, ast::StructRest::None))
}
pub fn expr_struct_ident(
&self,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,4 +484,9 @@ impl<Id> Res<Id> {
pub fn matches_ns(&self, ns: Namespace) -> bool {
self.ns().map_or(true, |actual_ns| actual_ns == ns)
}

/// Returns whether such a resolved path can occur in a tuple struct/variant pattern
pub fn expected_in_tuple_struct_pat(&self) -> bool {
matches!(self, Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(..))
}
}
10 changes: 8 additions & 2 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2087,7 +2087,7 @@ impl<'a> Parser<'a> {
recover: bool,
) -> PResult<'a, P<Expr>> {
let mut fields = Vec::new();
let mut base = None;
let mut base = ast::StructRest::None;
let mut recover_async = false;

attrs.extend(self.parse_inner_attributes()?);
Expand All @@ -2102,8 +2102,14 @@ impl<'a> Parser<'a> {
while self.token != token::CloseDelim(token::Brace) {
if self.eat(&token::DotDot) {
let exp_span = self.prev_token.span;
// We permit `.. }` on the left-hand side of a destructuring assignment.
if self.check(&token::CloseDelim(token::Brace)) {
self.sess.gated_spans.gate(sym::destructuring_assignment, self.prev_token.span);
base = ast::StructRest::Rest(self.prev_token.span.shrink_to_hi());
break;
}
match self.parse_expr() {
Ok(e) => base = Some(e),
Ok(e) => base = ast::StructRest::Base(e),
Err(mut e) if recover => {
e.emit();
self.recover_stmt();
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,7 @@ impl<'a> PathSource<'a> {
_,
)
| Res::SelfCtor(..)),
PathSource::TupleStruct(..) => {
matches!(res, Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(..))
}
PathSource::TupleStruct(..) => res.expected_in_tuple_struct_pat(),
PathSource::Struct => matches!(res, Res::Def(
DefKind::Struct
| DefKind::Union
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_save_analysis/src/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ impl<'tcx> DumpVisitor<'tcx> {
path: &'tcx hir::QPath<'tcx>,
fields: &'tcx [hir::Field<'tcx>],
variant: &'tcx ty::VariantDef,
base: Option<&'tcx hir::Expr<'tcx>>,
rest: Option<&'tcx hir::Expr<'tcx>>,
) {
if let Some(struct_lit_data) = self.save_ctxt.get_expr_data(ex) {
if let hir::QPath::Resolved(_, path) = path {
Expand All @@ -836,7 +836,9 @@ impl<'tcx> DumpVisitor<'tcx> {
}
}

walk_list!(self, visit_expr, base);
if let Some(base) = rest {
self.visit_expr(&base);
}
}

fn process_method_call(
Expand Down Expand Up @@ -1399,7 +1401,7 @@ impl<'tcx> Visitor<'tcx> for DumpVisitor<'tcx> {
debug!("visit_expr {:?}", ex.kind);
self.process_macro_use(ex.span);
match ex.kind {
hir::ExprKind::Struct(ref path, ref fields, ref base) => {
hir::ExprKind::Struct(ref path, ref fields, ref rest) => {
let hir_expr = self.save_ctxt.tcx.hir().expect_expr(ex.hir_id);
let adt = match self.save_ctxt.typeck_results().expr_ty_opt(&hir_expr) {
Some(ty) if ty.ty_adt_def().is_some() => ty.ty_adt_def().unwrap(),
Expand All @@ -1409,7 +1411,7 @@ impl<'tcx> Visitor<'tcx> for DumpVisitor<'tcx> {
}
};
let res = self.save_ctxt.get_path_res(hir_expr.hir_id);
self.process_struct_lit(ex, path, fields, adt.variant_of_res(res), *base)
self.process_struct_lit(ex, path, fields, adt.variant_of_res(res), *rest)
}
hir::ExprKind::MethodCall(ref seg, _, args, _) => {
self.process_method_call(ex, seg, args)
Expand Down
Loading