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

Disallow cast with trailing braced macro in let-else #125049

Merged
merged 3 commits into from
May 22, 2024
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
95 changes: 91 additions & 4 deletions compiler/rustc_ast/src/util/classify.rs
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
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
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did other errors in this file kill these suggestions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed them intentionally in f493143 because 48% of tests/ui/parser/bad-let-else-statement.stderr was these warnings unrelated to the thing that this file is trying to test (the parser), which seemed excessive.

The "irrefutable let...else pattern" warning is already tested independently by tests/ui/let-else/let-else-irrefutable.rs.

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
Loading