Skip to content

Commit

Permalink
Rollup merge of rust-lang#125049 - dtolnay:castbrace, r=compiler-errors
Browse files Browse the repository at this point in the history 
Disallow cast with trailing braced macro in let-else

This fixes an edge case I noticed while porting rust-lang#118880 and rust-lang#119062 to syn.

Previously, rustc incorrectly accepted code such as:

```rust
let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! {
    8
} else {
    return;
};
```

even though a right curl brace `}` directly before `else` in a `let...else` statement is not supposed to be valid syntax.
  • Loading branch information
@fmease
fmease authored May 22, 2024
2 parents b3604de + a36b94d commit 5b485f0
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 297 deletions.
95 changes: 91 additions & 4 deletions compiler/rustc_ast/src/util/classify.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,17 @@ pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool {
}
}

pub enum TrailingBrace<'a> {
/// Trailing brace in a macro call, like the one in `x as *const brace! {}`.
/// We will suggest changing the macro call to a different delimiter.
MacCall(&'a ast::MacCall),
/// Trailing brace in any other expression, such as `a + B {}`. We will
/// suggest wrapping the innermost expression in parentheses: `a + (B {})`.
Expr(&'a ast::Expr),
}

/// If an expression ends with `}`, returns the innermost expression ending in the `}`
pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> {
pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<TrailingBrace<'_>> {
loop {
match &expr.kind {
AddrOf(_, _, e)
Expand Down Expand Up @@ -111,10 +120,14 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> {
| Struct(..)
| TryBlock(..)
| While(..)
| ConstBlock(_) => break Some(expr),
| ConstBlock(_) => break Some(TrailingBrace::Expr(expr)),

Cast(_, ty) => {
break type_trailing_braced_mac_call(ty).map(TrailingBrace::MacCall);
}

MacCall(mac) => {
break (mac.args.delim == Delimiter::Brace).then_some(expr);
break (mac.args.delim == Delimiter::Brace).then_some(TrailingBrace::MacCall(mac));
}

InlineAsm(_) | OffsetOf(_, _) | IncludedBytes(_) | FormatArgs(_) => {
Expand All @@ -131,7 +144,6 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> {
| MethodCall(_)
| Tup(_)
| Lit(_)
| Cast(_, _)
| Type(_, _)
| Await(_, _)
| Field(_, _)
Expand All @@ -148,3 +160,78 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> {
}
}
}

/// If the type's last token is `}`, it must be due to a braced macro call, such
/// as in `*const brace! { ... }`. Returns that trailing macro call.
fn type_trailing_braced_mac_call(mut ty: &ast::Ty) -> Option<&ast::MacCall> {
loop {
match &ty.kind {
ast::TyKind::MacCall(mac) => {
break (mac.args.delim == Delimiter::Brace).then_some(mac);
}

ast::TyKind::Ptr(mut_ty) | ast::TyKind::Ref(_, mut_ty) => {
ty = &mut_ty.ty;
}

ast::TyKind::BareFn(fn_ty) => match &fn_ty.decl.output {
ast::FnRetTy::Default(_) => break None,
ast::FnRetTy::Ty(ret) => ty = ret,
},

ast::TyKind::Path(_, path) => match path_return_type(path) {
Some(trailing_ty) => ty = trailing_ty,
None => break None,
},

ast::TyKind::TraitObject(bounds, _) | ast::TyKind::ImplTrait(_, bounds, _) => {
match bounds.last() {
Some(ast::GenericBound::Trait(bound, _)) => {
match path_return_type(&bound.trait_ref.path) {
Some(trailing_ty) => ty = trailing_ty,
None => break None,
}
}
Some(ast::GenericBound::Outlives(_)) | None => break None,
}
}

ast::TyKind::Slice(..)
| ast::TyKind::Array(..)
| ast::TyKind::Never
| ast::TyKind::Tup(..)
| ast::TyKind::Paren(..)
| ast::TyKind::Typeof(..)
| ast::TyKind::Infer
| ast::TyKind::ImplicitSelf
| ast::TyKind::CVarArgs
| ast::TyKind::Pat(..)
| ast::TyKind::Dummy
| ast::TyKind::Err(..) => break None,

// These end in brace, but cannot occur in a let-else statement.
// They are only parsed as fields of a data structure. For the
// purpose of denying trailing braces in the expression of a
// let-else, we can disregard these.
ast::TyKind::AnonStruct(..) | ast::TyKind::AnonUnion(..) => break None,
}
}
}

/// Returns the trailing return type in the given path, if it has one.
///
/// ```ignore (illustrative)
/// ::std::ops::FnOnce(&str) -> fn() -> *const c_void
/// ^^^^^^^^^^^^^^^^^^^^^
/// ```
fn path_return_type(path: &ast::Path) -> Option<&ast::Ty> {
let last_segment = path.segments.last()?;
let args = last_segment.args.as_ref()?;
match &**args {
ast::GenericArgs::Parenthesized(args) => match &args.output {
ast::FnRetTy::Default(_) => None,
ast::FnRetTy::Ty(ret) => Some(ret),
},
ast::GenericArgs::AngleBracketed(_) => None,
}
}
28 changes: 17 additions & 11 deletions compiler/rustc_parse/src/parser/stmt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use ast::Label;
use rustc_ast as ast;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, TokenKind};
use rustc_ast::util::classify;
use rustc_ast::util::classify::{self, TrailingBrace};
use rustc_ast::{AttrStyle, AttrVec, LocalKind, MacCall, MacCallStmt, MacStmtStyle};
use rustc_ast::{Block, BlockCheckMode, Expr, ExprKind, HasAttrs, Local, Recovered, Stmt};
use rustc_ast::{StmtKind, DUMMY_NODE_ID};
Expand Down Expand Up @@ -407,18 +407,24 @@ impl<'a> Parser<'a> {

fn check_let_else_init_trailing_brace(&self, init: &ast::Expr) {
if let Some(trailing) = classify::expr_trailing_brace(init) {
let sugg = match &trailing.kind {
ExprKind::MacCall(mac) => errors::WrapInParentheses::MacroArgs {
left: mac.args.dspan.open,
right: mac.args.dspan.close,
},
_ => errors::WrapInParentheses::Expression {
left: trailing.span.shrink_to_lo(),
right: trailing.span.shrink_to_hi(),
},
let (span, sugg) = match trailing {
TrailingBrace::MacCall(mac) => (
mac.span(),
errors::WrapInParentheses::MacroArgs {
left: mac.args.dspan.open,
right: mac.args.dspan.close,
},
),
TrailingBrace::Expr(expr) => (
expr.span,
errors::WrapInParentheses::Expression {
left: expr.span.shrink_to_lo(),
right: expr.span.shrink_to_hi(),
},
),
};
self.dcx().emit_err(errors::InvalidCurlyInLetElse {
span: trailing.span.with_lo(trailing.span.hi() - BytePos(1)),
span: span.with_lo(span.hi() - BytePos(1)),
sugg,
});
}
Expand Down
73 changes: 40 additions & 33 deletions tests/ui/parser/bad-let-else-statement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
#![feature(explicit_tail_calls)]

fn a() {
let foo = {
//~^ WARN irrefutable `let...else` pattern
let 0 = {
1
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -22,8 +21,7 @@ fn b() {
}

fn c() {
let foo = if true {
//~^ WARN irrefutable `let...else` pattern
let 0 = if true {
1
} else {
0
Expand All @@ -43,8 +41,7 @@ fn d() {
}

fn e() {
let foo = match true {
//~^ WARN irrefutable `let...else` pattern
let 0 = match true {
true => 1,
false => 0
} else {
Expand All @@ -53,10 +50,12 @@ fn e() {
};
}

struct X {a: i32}
fn f() {
let foo = X {
//~^ WARN irrefutable `let...else` pattern
struct X {
a: i32,
}

let X { a: 0 } = X {
a: 1
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -74,8 +73,7 @@ fn g() {
}

fn h() {
let foo = const {
//~^ WARN irrefutable `let...else` pattern
let 0 = const {
1
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -84,8 +82,7 @@ fn h() {
}

fn i() {
let foo = &{
//~^ WARN irrefutable `let...else` pattern
let 0 = &{
1
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -94,8 +91,8 @@ fn i() {
}

fn j() {
let bar = 0;
let foo = bar = { //~ ERROR: cannot assign twice
let mut bar = 0;
let foo = bar = {
//~^ WARN irrefutable `let...else` pattern
1
} else {
Expand All @@ -105,8 +102,7 @@ fn j() {
}

fn k() {
let foo = 1 + {
//~^ WARN irrefutable `let...else` pattern
let 0 = 1 + {
1
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -115,8 +111,8 @@ fn k() {
}

fn l() {
let foo = 1..{
//~^ WARN irrefutable `let...else` pattern
const RANGE: std::ops::Range<u8> = 0..0;
let RANGE = 1..{
1
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -125,8 +121,7 @@ fn l() {
}

fn m() {
let foo = return {
//~^ WARN irrefutable `let...else` pattern
let 0 = return {
()
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -135,8 +130,7 @@ fn m() {
}

fn n() {
let foo = -{
//~^ WARN irrefutable `let...else` pattern
let 0 = -{
1
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -145,8 +139,7 @@ fn n() {
}

fn o() -> Result<(), ()> {
let foo = do yeet {
//~^ WARN irrefutable `let...else` pattern
let 0 = do yeet {
()
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand All @@ -155,8 +148,7 @@ fn o() -> Result<(), ()> {
}

fn p() {
let foo = become {
//~^ WARN irrefutable `let...else` pattern
let 0 = become {
()
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
Expand Down Expand Up @@ -185,22 +177,37 @@ fn r() {

fn s() {
macro_rules! a {
() => { {} }
//~^ WARN irrefutable `let...else` pattern
//~| WARN irrefutable `let...else` pattern
() => {
{ 1 }
};
}

macro_rules! b {
(1) => {
let x = a!() else { return; };
let 0 = a!() else { return; };
};
(2) => {
let x = a! {} else { return; };
let 0 = a! {} else { return; };
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
};
}

b!(1); b!(2);
b!(1);
b!(2);
}

fn t() {
macro_rules! primitive {
(8) => { u8 };
}

let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! {
//~^ WARN irrefutable `let...else` pattern
8
} else {
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
return;
};
}

fn main() {}
Loading

0 comments on commit 5b485f0

Please sign in to comment.