Skip to content

Stabilize let chains in the 2024 edition #132833

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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
82 changes: 53 additions & 29 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use rustc_macros::Subdiagnostic;
use rustc_session::errors::{ExprParenthesesNeeded, report_lit_error};
use rustc_session::lint::BuiltinLintDiag;
use rustc_session::lint::builtin::BREAK_WITH_LABEL_AND_LOOP;
use rustc_span::edition::Edition;
use rustc_span::source_map::{self, Spanned};
use rustc_span::{BytePos, ErrorGuaranteed, Ident, Pos, Span, Symbol, kw, sym};
use thin_vec::{ThinVec, thin_vec};
Expand Down Expand Up @@ -2605,7 +2606,10 @@ impl<'a> Parser<'a> {
/// Parses an `if` expression (`if` token already eaten).
fn parse_expr_if(&mut self) -> PResult<'a, P<Expr>> {
let lo = self.prev_token.span;
let cond = self.parse_expr_cond()?;
// Scoping code checks the top level edition of the `if`; let's match it here.
// The CondChecker also checks the edition of the `let` itself, just to make sure.
let let_chains_policy = LetChainsPolicy::Edition(lo.edition());
let cond = self.parse_expr_cond(let_chains_policy)?;
self.parse_if_after_cond(lo, cond)
}

Expand Down Expand Up @@ -2714,18 +2718,17 @@ impl<'a> Parser<'a> {
}

/// Parses the condition of a `if` or `while` expression.
///
/// The specified `edition` in `let_chains_policy` should be that of the whole `if` construct,
/// i.e. the same span we use to later decide whether the drop behaviour should be that of
/// edition `..=2021` or that of `2024..`.
// Public because it is used in rustfmt forks such as https://github.com/tucant/rustfmt/blob/30c83df9e1db10007bdd16dafce8a86b404329b2/src/parse/macros/html.rs#L57 for custom if expressions.
pub fn parse_expr_cond(&mut self) -> PResult<'a, P<Expr>> {
pub fn parse_expr_cond(&mut self, let_chains_policy: LetChainsPolicy) -> PResult<'a, P<Expr>> {
let attrs = self.parse_outer_attributes()?;
let (mut cond, _) =
self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL | Restrictions::ALLOW_LET, attrs)?;

CondChecker::new(self).visit_expr(&mut cond);

if let ExprKind::Let(_, _, _, Recovered::No) = cond.kind {
// Remove the last feature gating of a `let` expression since it's stable.
self.psess.gated_spans.ungate_last(sym::let_chains, cond.span);
}
CondChecker::new(self, let_chains_policy).visit_expr(&mut cond);

Ok(cond)
}
Expand Down Expand Up @@ -3020,7 +3023,7 @@ impl<'a> Parser<'a> {

/// Parses a `while` or `while let` expression (`while` token already eaten).
fn parse_expr_while(&mut self, opt_label: Option<Label>, lo: Span) -> PResult<'a, P<Expr>> {
let cond = self.parse_expr_cond().map_err(|mut err| {
let cond = self.parse_expr_cond(LetChainsPolicy::AlwaysAllowed).map_err(|mut err| {
err.span_label(lo, "while parsing the condition of this `while` expression");
err
})?;
Expand Down Expand Up @@ -3404,17 +3407,17 @@ impl<'a> Parser<'a> {
}

fn parse_match_arm_guard(&mut self) -> PResult<'a, Option<P<Expr>>> {
// Used to check the `let_chains` and `if_let_guard` features mostly by scanning
// Used to check the `if_let_guard` feature mostly by scanning
// `&&` tokens.
fn check_let_expr(expr: &Expr) -> (bool, bool) {
fn has_let_expr(expr: &Expr) -> bool {
match &expr.kind {
ExprKind::Binary(BinOp { node: BinOpKind::And, .. }, lhs, rhs) => {
let lhs_rslt = check_let_expr(lhs);
let rhs_rslt = check_let_expr(rhs);
(lhs_rslt.0 || rhs_rslt.0, false)
let lhs_rslt = has_let_expr(lhs);
let rhs_rslt = has_let_expr(rhs);
lhs_rslt || rhs_rslt
}
ExprKind::Let(..) => (true, true),
_ => (false, true),
ExprKind::Let(..) => true,
_ => false,
}
}
if !self.eat_keyword(exp!(If)) {
Expand All @@ -3425,14 +3428,9 @@ impl<'a> Parser<'a> {
let if_span = self.prev_token.span;
let mut cond = self.parse_match_guard_condition()?;

CondChecker::new(self).visit_expr(&mut cond);
CondChecker::new(self, LetChainsPolicy::AlwaysAllowed).visit_expr(&mut cond);

let (has_let_expr, does_not_have_bin_op) = check_let_expr(&cond);
if has_let_expr {
if does_not_have_bin_op {
// Remove the last feature gating of a `let` expression since it's stable.
self.psess.gated_spans.ungate_last(sym::let_chains, cond.span);
}
if has_let_expr(&cond) {
let span = if_span.to(cond.span);
self.psess.gated_spans.gate(sym::if_let_guard, span);
}
Expand All @@ -3459,7 +3457,7 @@ impl<'a> Parser<'a> {
unreachable!()
};
self.psess.gated_spans.ungate_last(sym::guard_patterns, cond.span);
CondChecker::new(self).visit_expr(&mut cond);
CondChecker::new(self, LetChainsPolicy::AlwaysAllowed).visit_expr(&mut cond);
let right = self.prev_token.span;
self.dcx().emit_err(errors::ParenthesesInMatchPat {
span: vec![left, right],
Expand Down Expand Up @@ -4038,7 +4036,14 @@ pub(crate) enum ForbiddenLetReason {
NotSupportedParentheses(#[primary_span] Span),
}

/// Visitor to check for invalid/unstable use of `ExprKind::Let` that can't
/// Whether let chains are allowed on all editions, or it's edition dependent.
/// In case of edition dependence, specify the currently present edition.
pub enum LetChainsPolicy {
AlwaysAllowed,
Edition(Edition),
}

/// Visitor to check for invalid use of `ExprKind::Let` that can't
/// easily be caught in parsing. For example:
///
/// ```rust,ignore (example)
Expand All @@ -4049,19 +4054,29 @@ pub(crate) enum ForbiddenLetReason {
/// ```
struct CondChecker<'a> {
parser: &'a Parser<'a>,
let_chains_policy: LetChainsPolicy,
depth: u32,
forbid_let_reason: Option<ForbiddenLetReason>,
missing_let: Option<errors::MaybeMissingLet>,
comparison: Option<errors::MaybeComparison>,
}

impl<'a> CondChecker<'a> {
fn new(parser: &'a Parser<'a>) -> Self {
CondChecker { parser, forbid_let_reason: None, missing_let: None, comparison: None }
fn new(parser: &'a Parser<'a>, let_chains_policy: LetChainsPolicy) -> Self {
CondChecker {
parser,
forbid_let_reason: None,
missing_let: None,
comparison: None,
let_chains_policy,
depth: 0,
}
}
}

impl MutVisitor for CondChecker<'_> {
fn visit_expr(&mut self, e: &mut P<Expr>) {
self.depth += 1;
use ForbiddenLetReason::*;

let span = e.span;
Expand All @@ -4076,8 +4091,16 @@ impl MutVisitor for CondChecker<'_> {
comparison: self.comparison,
},
));
} else {
self.parser.psess.gated_spans.gate(sym::let_chains, span);
} else if self.depth > 1 {
// Top level let is always allowed, only gate chains
match self.let_chains_policy {
LetChainsPolicy::AlwaysAllowed => (),
LetChainsPolicy::Edition(edition) => {
if !edition.at_least_rust_2024() || !span.at_least_rust_2024() {
self.parser.psess.gated_spans.gate(sym::let_chains, span);
}
}
}
}
}
ExprKind::Binary(Spanned { node: BinOpKind::And, .. }, _, _) => {
Expand Down Expand Up @@ -4179,5 +4202,6 @@ impl MutVisitor for CondChecker<'_> {
// These would forbid any let expressions they contain already.
}
}
self.depth -= 1;
}
}
2 changes: 1 addition & 1 deletion tests/mir-opt/building/logical_or_in_conditional.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// skip-filecheck
//@ compile-flags: -Z validate-mir
#![feature(let_chains)]
//@ edition: 2024
struct Droppy(u8);
impl Drop for Droppy {
fn drop(&mut self) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn test_complex() -> () {
bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = E::f() -> [return: bb1, unwind: bb34];
_2 = E::f() -> [return: bb1, unwind: bb35];
}

bb1: {
Expand All @@ -42,7 +42,7 @@ fn test_complex() -> () {

bb5: {
StorageLive(_4);
_4 = always_true() -> [return: bb6, unwind: bb34];
_4 = always_true() -> [return: bb6, unwind: bb35];
}

bb6: {
Expand All @@ -64,7 +64,7 @@ fn test_complex() -> () {
}

bb9: {
drop(_7) -> [return: bb11, unwind: bb34];
drop(_7) -> [return: bb11, unwind: bb35];
}

bb10: {
Expand All @@ -78,7 +78,7 @@ fn test_complex() -> () {
}

bb12: {
drop(_7) -> [return: bb13, unwind: bb34];
drop(_7) -> [return: bb13, unwind: bb35];
}

bb13: {
Expand All @@ -98,7 +98,7 @@ fn test_complex() -> () {
}

bb15: {
drop(_10) -> [return: bb17, unwind: bb34];
drop(_10) -> [return: bb17, unwind: bb35];
}

bb16: {
Expand All @@ -113,11 +113,12 @@ fn test_complex() -> () {

bb18: {
_1 = const ();
StorageDead(_2);
goto -> bb22;
}

bb19: {
drop(_10) -> [return: bb20, unwind: bb34];
drop(_10) -> [return: bb20, unwind: bb35];
}

bb20: {
Expand All @@ -127,6 +128,7 @@ fn test_complex() -> () {
}

bb21: {
StorageDead(_2);
_1 = const ();
goto -> bb22;
}
Expand All @@ -135,18 +137,17 @@ fn test_complex() -> () {
StorageDead(_8);
StorageDead(_5);
StorageDead(_4);
StorageDead(_2);
StorageDead(_1);
StorageLive(_11);
_11 = always_true() -> [return: bb23, unwind: bb34];
_11 = always_true() -> [return: bb23, unwind: bb35];
}

bb23: {
switchInt(move _11) -> [0: bb25, otherwise: bb24];
}

bb24: {
goto -> bb32;
goto -> bb33;
}

bb25: {
Expand All @@ -155,7 +156,7 @@ fn test_complex() -> () {

bb26: {
StorageLive(_12);
_12 = E::f() -> [return: bb27, unwind: bb34];
_12 = E::f() -> [return: bb27, unwind: bb35];
}

bb27: {
Expand All @@ -178,21 +179,26 @@ fn test_complex() -> () {

bb31: {
_0 = const ();
goto -> bb33;
StorageDead(_12);
goto -> bb34;
}

bb32: {
_0 = const ();
StorageDead(_12);
goto -> bb33;
}

bb33: {
_0 = const ();
goto -> bb34;
}

bb34: {
StorageDead(_11);
StorageDead(_12);
return;
}

bb34 (cleanup): {
bb35 (cleanup): {
resume;
}
}
2 changes: 1 addition & 1 deletion tests/ui/deriving/auxiliary/malicious-macro.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(let_chains)]
//@ edition: 2024

extern crate proc_macro;

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/drop/drop-order-comparisons.e2021.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
//@ [e2024] edition: 2024
//@ run-pass

#![feature(let_chains)]
#![cfg_attr(e2021, feature(let_chains))]
#![cfg_attr(e2021, warn(rust_2024_compatibility))]

fn t_bindings() {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/drop/drop-order-comparisons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
//@ [e2024] edition: 2024
//@ run-pass

#![feature(let_chains)]
#![cfg_attr(e2021, feature(let_chains))]
#![cfg_attr(e2021, warn(rust_2024_compatibility))]

fn t_bindings() {
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/drop/drop_order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
//@ compile-flags: -Z validate-mir
//@ revisions: edition2021 edition2024
//@ [edition2021] edition: 2021
//@ [edition2024] compile-flags: -Z lint-mir
//@ [edition2024] edition: 2024

#![feature(let_chains)]
#![cfg_attr(edition2021, feature(let_chains))]

use std::cell::RefCell;
use std::convert::TryInto;
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/drop/drop_order_if_let_rescope.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
//@ run-pass
//@ edition:2024
//@ compile-flags: -Z validate-mir

#![feature(let_chains)]
//@ compile-flags: -Z validate-mir -Z lint-mir

use std::cell::RefCell;
use std::convert::TryInto;
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/drop/issue-100276.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
//@ check-pass
//@ compile-flags: -Z validate-mir
#![feature(let_chains)]
//@ revisions: edition2021 edition2024
//@ [edition2021] edition: 2021
//@ [edition2024] compile-flags: -Z lint-mir
//@ [edition2024] edition: 2024

#![cfg_attr(edition2021, feature(let_chains))]

fn let_chains(entry: std::io::Result<std::fs::DirEntry>) {
if let Ok(entry) = entry
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/expr/if/attrs/let-chains-attr.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//@ check-pass

#![feature(let_chains)]
//@ edition:2024

#[cfg(false)]
fn foo() {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/lint/issue-121070-let-range.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ check-pass
//@ edition:2024

#![feature(let_chains)]
#![allow(irrefutable_let_patterns)]
fn main() {
let _a = 0..1;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/mir/issue-99852.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ check-pass
//@ compile-flags: -Z validate-mir
#![feature(let_chains)]
//@ edition: 2024

fn lambda<T, U>() -> U
where
Expand Down
Loading
Loading