Skip to content

Commit

Permalink
Auto merge of rust-lang#118527 - Nadrieril:never_patterns_parse, r=co…
Browse files Browse the repository at this point in the history
…mpiler-errors

never_patterns: Parse match arms with no body

Never patterns are meant to signal unreachable cases, and thus don't take bodies:
```rust
let ptr: *const Option<!> = ...;
match *ptr {
    None => { foo(); }
    Some(!),
}
```
This PR makes rustc accept the above, and enforces that an arm has a body xor is a never pattern. This affects parsing of match arms even with the feature off, so this is delicate. (Plus this is my first non-trivial change to the parser).

~~The last commit is optional; it introduces a bit of churn to allow the new suggestions to be machine-applicable. There may be a better solution? I'm not sure.~~ EDIT: I removed that commit

r? `@compiler-errors`
  • Loading branch information
bors committed Dec 8, 2023
2 parents ae612be + 431cc4a commit 2b399b5
Show file tree
Hide file tree
Showing 47 changed files with 772 additions and 236 deletions.
22 changes: 20 additions & 2 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,24 @@ impl Pat {
pub fn is_rest(&self) -> bool {
matches!(self.kind, PatKind::Rest)
}

/// Whether this could be a never pattern, taking into account that a macro invocation can
/// return a never pattern. Used to inform errors during parsing.
pub fn could_be_never_pattern(&self) -> bool {
let mut could_be_never_pattern = false;
self.walk(&mut |pat| match &pat.kind {
PatKind::Never | PatKind::MacCall(_) => {
could_be_never_pattern = true;
false
}
PatKind::Or(s) => {
could_be_never_pattern = s.iter().all(|p| p.could_be_never_pattern());
false
}
_ => true,
});
could_be_never_pattern
}
}

/// A single field in a struct pattern.
Expand Down Expand Up @@ -1080,8 +1098,8 @@ pub struct Arm {
pub pat: P<Pat>,
/// Match arm guard, e.g. `n > 10` in `match foo { n if n > 10 => {}, _ => {} }`
pub guard: Option<P<Expr>>,
/// Match arm body.
pub body: P<Expr>,
/// Match arm body. Omitted if the pattern is a never pattern.
pub body: Option<P<Expr>>,
pub span: Span,
pub id: NodeId,
pub is_placeholder: bool,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ pub fn noop_flat_map_arm<T: MutVisitor>(mut arm: Arm, vis: &mut T) -> SmallVec<[
vis.visit_id(id);
vis.visit_pat(pat);
visit_opt(guard, |guard| vis.visit_expr(guard));
vis.visit_expr(body);
visit_opt(body, |body| vis.visit_expr(body));
vis.visit_span(span);
smallvec![arm]
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ pub fn walk_param<'a, V: Visitor<'a>>(visitor: &mut V, param: &'a Param) {
pub fn walk_arm<'a, V: Visitor<'a>>(visitor: &mut V, arm: &'a Arm) {
visitor.visit_pat(&arm.pat);
walk_list!(visitor, visit_expr, &arm.guard);
visitor.visit_expr(&arm.body);
walk_list!(visitor, visit_expr, &arm.body);
walk_list!(visitor, visit_attribute, &arm.attrs);
}

Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_ast_lowering/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ ast_lowering_invalid_register =
ast_lowering_invalid_register_class =
invalid register class `{$reg_class}`: {$error}
ast_lowering_match_arm_with_no_body =
`match` arm with no body
.suggestion = add a body after the pattern
ast_lowering_misplaced_assoc_ty_binding =
associated type bounds are only allowed in where clauses and function signatures, not in {$position}
Expand All @@ -104,6 +108,15 @@ ast_lowering_misplaced_impl_trait =
ast_lowering_misplaced_relax_trait_bound =
`?Trait` bounds are only permitted at the point where a type parameter is declared
ast_lowering_never_pattern_with_body =
a never pattern is always unreachable
.label = this will never be executed
.suggestion = remove this expression
ast_lowering_never_pattern_with_guard =
a guard on a never pattern will never be run
.suggestion = remove this guard
ast_lowering_not_supported_for_lifetime_binder_async_closure =
`for<...>` binders on `async` closures are not currently supported
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,32 @@ pub struct NotSupportedForLifetimeBinderAsyncClosure {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_lowering_match_arm_with_no_body)]
pub struct MatchArmWithNoBody {
#[primary_span]
pub span: Span,
#[suggestion(code = " => todo!(),", applicability = "has-placeholders")]
pub suggestion: Span,
}

#[derive(Diagnostic)]
#[diag(ast_lowering_never_pattern_with_body)]
pub struct NeverPatternWithBody {
#[primary_span]
#[label]
#[suggestion(code = "", applicability = "maybe-incorrect")]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_lowering_never_pattern_with_guard)]
pub struct NeverPatternWithGuard {
#[primary_span]
#[suggestion(code = "", applicability = "maybe-incorrect")]
pub span: Span,
}

#[derive(Diagnostic, Clone, Copy)]
#[diag(ast_lowering_arbitrary_expression_in_pattern)]
pub struct ArbitraryExpressionInPattern {
Expand Down
50 changes: 40 additions & 10 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use super::errors::{
AsyncCoroutinesNotSupported, AsyncNonMoveClosureNotSupported, AwaitOnlyInAsyncFnAndBlocks,
BaseExpressionDoubleDot, ClosureCannotBeStatic, CoroutineTooManyParameters,
FunctionalRecordUpdateDestructuringAssignment, InclusiveRangeWithNoEnd,
NotSupportedForLifetimeBinderAsyncClosure, UnderscoreExprLhsAssign,
FunctionalRecordUpdateDestructuringAssignment, InclusiveRangeWithNoEnd, MatchArmWithNoBody,
NeverPatternWithBody, NeverPatternWithGuard, NotSupportedForLifetimeBinderAsyncClosure,
UnderscoreExprLhsAssign,
};
use super::ResolverAstLoweringExt;
use super::{ImplTraitContext, LoweringContext, ParamMode, ParenthesizedGenericArgs};
Expand Down Expand Up @@ -549,7 +550,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

fn lower_arm(&mut self, arm: &Arm) -> hir::Arm<'hir> {
let pat = self.lower_pat(&arm.pat);
let guard = arm.guard.as_ref().map(|cond| {
let mut guard = arm.guard.as_ref().map(|cond| {
if let ExprKind::Let(pat, scrutinee, span, is_recovered) = &cond.kind {
hir::Guard::IfLet(self.arena.alloc(hir::Let {
hir_id: self.next_id(),
Expand All @@ -564,14 +565,43 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
});
let hir_id = self.next_id();
let span = self.lower_span(arm.span);
self.lower_attrs(hir_id, &arm.attrs);
hir::Arm {
hir_id,
pat,
guard,
body: self.lower_expr(&arm.body),
span: self.lower_span(arm.span),
}
let is_never_pattern = pat.is_never_pattern();
let body = if let Some(body) = &arm.body
&& !is_never_pattern
{
self.lower_expr(body)
} else {
// Either `body.is_none()` or `is_never_pattern` here.
if !is_never_pattern {
let suggestion = span.shrink_to_hi();
self.tcx.sess.emit_err(MatchArmWithNoBody { span, suggestion });
} else if let Some(body) = &arm.body {
self.tcx.sess.emit_err(NeverPatternWithBody { span: body.span });
guard = None;
} else if let Some(g) = &arm.guard {
self.tcx.sess.emit_err(NeverPatternWithGuard { span: g.span });
guard = None;
}

// We add a fake `loop {}` arm body so that it typecks to `!`.
// FIXME(never_patterns): Desugar into a call to `unreachable_unchecked`.
let block = self.arena.alloc(hir::Block {
stmts: &[],
expr: None,
hir_id: self.next_id(),
rules: hir::BlockCheckMode::DefaultBlock,
span,
targeted_by_break: false,
});
self.arena.alloc(hir::Expr {
hir_id: self.next_id(),
kind: hir::ExprKind::Loop(block, None, hir::LoopSource::Loop, span),
span,
})
};
hir::Arm { hir_id, pat, guard, body, span }
}

/// Lower an `async` construct to a coroutine that implements `Future`.
Expand Down
37 changes: 21 additions & 16 deletions compiler/rustc_ast_pretty/src/pprust/state/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,28 +631,33 @@ impl<'a> State<'a> {
self.print_expr(e);
self.space();
}
self.word_space("=>");

match &arm.body.kind {
ast::ExprKind::Block(blk, opt_label) => {
if let Some(label) = opt_label {
self.print_ident(label.ident);
self.word_space(":");
}
if let Some(body) = &arm.body {
self.word_space("=>");

match &body.kind {
ast::ExprKind::Block(blk, opt_label) => {
if let Some(label) = opt_label {
self.print_ident(label.ident);
self.word_space(":");
}

// The block will close the pattern's ibox.
self.print_block_unclosed_indent(blk);
// The block will close the pattern's ibox.
self.print_block_unclosed_indent(blk);

// If it is a user-provided unsafe block, print a comma after it.
if let BlockCheckMode::Unsafe(ast::UserProvided) = blk.rules {
// If it is a user-provided unsafe block, print a comma after it.
if let BlockCheckMode::Unsafe(ast::UserProvided) = blk.rules {
self.word(",");
}
}
_ => {
self.end(); // Close the ibox for the pattern.
self.print_expr(body);
self.word(",");
}
}
_ => {
self.end(); // Close the ibox for the pattern.
self.print_expr(&arm.body);
self.word(",");
}
} else {
self.word(",");
}
self.end(); // Close enclosing cbox.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ fn cs_partial_cmp(
&& let Some(last) = arms.last_mut()
&& let PatKind::Wild = last.pat.kind
{
last.body = expr2;
last.body = Some(expr2);
expr1
} else {
let eq_arm = cx.arm(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_expand/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ expand_macro_const_stability =
.label = invalid const stability attribute
.label2 = const stability attribute affects this macro
expand_macro_expands_to_match_arm = macros cannot expand to match arms
expand_malformed_feature_attribute =
malformed `feature` attribute input
.expected = expected just one word
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 @@ -505,7 +505,7 @@ impl<'a> ExtCtxt<'a> {
attrs: AttrVec::new(),
pat,
guard: None,
body: expr,
body: Some(expr),
span,
id: ast::DUMMY_NODE_ID,
is_placeholder: false,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_expand/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ pub(crate) struct IncompleteParse<'a> {
pub label_span: Span,
pub macro_path: &'a ast::Path,
pub kind_name: &'a str,
#[note(expand_macro_expands_to_match_arm)]
pub expands_to_match_arm: Option<()>,

#[suggestion(
expand_suggestion_add_semi,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,12 +955,15 @@ pub fn ensure_complete_parse<'a>(
_ => None,
};

let expands_to_match_arm = kind_name == "pattern" && parser.token == token::FatArrow;

parser.sess.emit_err(IncompleteParse {
span: def_site_span,
token,
label_span: span,
macro_path,
kind_name,
expands_to_match_arm: expands_to_match_arm.then_some(()),
add_semicolon,
});
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/placeholders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub fn placeholder(
}]),
AstFragmentKind::Arms => AstFragment::Arms(smallvec![ast::Arm {
attrs: Default::default(),
body: expr_placeholder(),
body: Some(expr_placeholder()),
guard: None,
id,
pat: pat(),
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,23 @@ impl<'hir> Pat<'hir> {
true
})
}

/// Whether this a never pattern.
pub fn is_never_pattern(&self) -> bool {
let mut is_never_pattern = false;
self.walk(|pat| match &pat.kind {
PatKind::Never => {
is_never_pattern = true;
false
}
PatKind::Or(s) => {
is_never_pattern = s.iter().all(|p| p.is_never_pattern());
false
}
_ => true,
});
is_never_pattern
}
}

/// A single field in a struct pattern.
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,8 +1000,10 @@ impl EarlyLintPass for UnusedDocComment {
}

fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
let arm_span = arm.pat.span.with_hi(arm.body.span.hi());
warn_if_doc(cx, arm_span, "match arms", &arm.attrs);
if let Some(body) = &arm.body {
let arm_span = arm.pat.span.with_hi(body.span.hi());
warn_if_doc(cx, arm_span, "match arms", &arm.attrs);
}
}

fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &ast::Pat) {
Expand Down
20 changes: 11 additions & 9 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1113,15 +1113,17 @@ impl EarlyLintPass for UnusedParens {
}
ExprKind::Match(ref _expr, ref arm) => {
for a in arm {
self.check_unused_delims_expr(
cx,
&a.body,
UnusedDelimsCtx::MatchArmExpr,
false,
None,
None,
true,
);
if let Some(body) = &a.body {
self.check_unused_delims_expr(
cx,
body,
UnusedDelimsCtx::MatchArmExpr,
false,
None,
None,
true,
);
}
}
}
_ => {}
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,6 @@ parse_macro_expands_to_adt_field = macros cannot expand to {$adt_ty} fields
parse_macro_expands_to_enum_variant = macros cannot expand to enum variants
parse_macro_expands_to_match_arm = macros cannot expand to match arms
parse_macro_invocation_visibility = can't qualify macro invocation with `pub`
.suggestion = remove the visibility
.help = try adjusting the macro to put `{$vis}` inside the invocation
Expand Down
Loading

0 comments on commit 2b399b5

Please sign in to comment.