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

Suggest adding missing braces in const block pattern #78173

Closed
wants to merge 9 commits into from
Closed
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
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,8 +1074,8 @@ impl<'a> Parser<'a> {
})
} else if self.eat_keyword(kw::Unsafe) {
self.parse_block_expr(None, lo, BlockCheckMode::Unsafe(ast::UserProvided), attrs)
} else if self.check_inline_const(0) {
self.parse_const_block(lo.to(self.token.span))
} else if self.eat_keyword(kw::Const) {
Copy link
Member

Choose a reason for hiding this comment

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

I think @petrochenkov and others wanted to avoid eating the keyword before checking if it's really an inline const. My first reaction was like ... but we're eating Unsafe right above. I'm not sure if there's another way to solve this problem and we could still rely on checking or if we just need to eat the keyword.

Anyway, would leave this to @petrochenkov

Copy link
Member

Choose a reason for hiding this comment

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

The difference is that unsafe just modifies the block expression, whereas const needs to create the ExprKind::Const - but maybe it doesn't have to be too complicated shrug.

self.parse_const_block(lo.to(self.prev_token.span))
} else if self.is_do_catch_block() {
self.recover_do_catch(attrs)
} else if self.is_try_block() {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -897,10 +897,10 @@ impl<'a> Parser<'a> {
}
}

/// Parses inline const expressions.
/// Parses inline const expressions. The `const` keyword was already eaten.
fn parse_const_block(&mut self, span: Span) -> PResult<'a, P<Expr>> {
self.sess.gated_spans.gate(sym::inline_const, span);
camelid marked this conversation as resolved.
Show resolved Hide resolved
self.eat_keyword(kw::Const);

let blk = self.parse_block()?;
let anon_const = AnonConst {
id: DUMMY_NODE_ID,
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,9 @@ impl<'a> Parser<'a> {
let pat = self.parse_pat_with_range_pat(false, None)?;
self.sess.gated_spans.gate(sym::box_patterns, lo.to(self.prev_token.span));
PatKind::Box(pat)
} else if self.check_inline_const(0) {
// Parse `const pat`
let const_expr = self.parse_const_block(lo.to(self.token.span))?;
} else if self.eat_keyword(kw::Const) {
// Parse `const { pat }`
let const_expr = self.parse_const_block(lo.to(self.prev_token.span))?;

if let Some(re) = self.parse_range_end() {
self.parse_pat_range_begin_with(const_expr, re)?
Expand Down Expand Up @@ -754,7 +754,8 @@ impl<'a> Parser<'a> {

fn parse_pat_range_end(&mut self) -> PResult<'a, P<Expr>> {
if self.check_inline_const(0) {
self.parse_const_block(self.token.span)
self.eat_keyword(kw::Const);
self.parse_const_block(self.prev_token.span)
} else if self.check_path() {
let lo = self.token.span;
let (qself, path) = if self.eat_lt() {
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,14 @@ impl<'a> Parser<'a> {
match self.parse_stmt_without_recovery() {
// If the next token is an open brace (e.g., `if a b {`), the place-
// inside-a-block suggestion would be more likely wrong than right.
//
// But we don't want to trigger this if we just parsed a pattern,
camelid marked this conversation as resolved.
Show resolved Hide resolved
// so this only triggers if the current token is neither `=>` nor `=`.
Ok(Some(_))
if self.look_ahead(1, |t| t == &token::OpenDelim(token::Brace))
|| do_not_suggest_help => {}
if do_not_suggest_help
|| (self.token != token::FatArrow
&& self.token != token::Eq
&& self.look_ahead(1, |t| t == &token::OpenDelim(token::Brace))) => {}
Ok(Some(stmt)) => {
let stmt_own_line = self.sess.source_map().is_line_before_span_empty(sp);
let stmt_span = if stmt_own_line && self.eat(&token::Semi) {
Expand All @@ -319,7 +324,7 @@ impl<'a> Parser<'a> {
stmt_span,
"try placing this code inside a block",
format!("{{ {} }}", snippet),
// Speculative; has been misleading in the past (#46836).
camelid marked this conversation as resolved.
Show resolved Hide resolved
// Speculative; has been misleading in the past (see #46836).
Applicability::MaybeIncorrect,
);
}
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/parser/inline-const-pat-let.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// run-pass

#![allow(incomplete_features)]
#![feature(inline_const)]

fn if_let_1() -> i32 {
let x = 2;
const Y: i32 = 3;

if let const { (Y + 1) / 2 } = x {
x
} else {
0
}
}

fn if_let_2() -> i32 {
let x = 2;

if let const { 1 + 2 } = x {
const { 1 + 2 }
} else {
0
}
}

fn main() {
assert_eq!(if_let_1(), 2);
assert_eq!(if_let_2(), 0);
}
61 changes: 61 additions & 0 deletions src/test/ui/parser/inline-const-without-block.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// run-rustfix

// See issue #78168.

#![allow(incomplete_features)]
#![feature(inline_const)]

// FIXME(#78171): the lint has to be allowed because of a bug
#[allow(dead_code)]
const fn one() -> i32 {
1
}

fn foo() -> i32 {
let x = 2;

match x {
const { 2 } => {}
//~^ ERROR expected `{`, found `2`
//~| HELP try placing this code inside a block
_ => {}
}

match x {
const { 1 + 2 * 3 / 4 } => {}
//~^ ERROR expected `{`, found `1`
//~| HELP try placing this code inside a block
_ => {}
}

match x {
const { one() } => {}
//~^ ERROR expected `{`, found `one`
//~| HELP try placing this code inside a block
_ => {}
}

x
}

fn bar() -> i32 {
let x = const { 2 };
//~^ ERROR expected `{`, found `2`
//~| HELP try placing this code inside a block

x
}

fn baz() -> i32 {
let y = const { 1 + 2 * 3 / 4 };
//~^ ERROR expected `{`, found `1`
//~| HELP try placing this code inside a block

y
}

fn main() {
foo();
bar();
baz();
}
61 changes: 61 additions & 0 deletions src/test/ui/parser/inline-const-without-block.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// run-rustfix

// See issue #78168.

#![allow(incomplete_features)]
#![feature(inline_const)]

// FIXME(#78171): the lint has to be allowed because of a bug
#[allow(dead_code)]
const fn one() -> i32 {
1
}

fn foo() -> i32 {
let x = 2;

match x {
const 2 => {}
//~^ ERROR expected `{`, found `2`
//~| HELP try placing this code inside a block
_ => {}
}

match x {
const 1 + 2 * 3 / 4 => {}
//~^ ERROR expected `{`, found `1`
//~| HELP try placing this code inside a block
_ => {}
}

match x {
const one() => {}
//~^ ERROR expected `{`, found `one`
//~| HELP try placing this code inside a block
_ => {}
}

x
}

fn bar() -> i32 {
let x = const 2;
//~^ ERROR expected `{`, found `2`
//~| HELP try placing this code inside a block

x
}

fn baz() -> i32 {
let y = const 1 + 2 * 3 / 4;
//~^ ERROR expected `{`, found `1`
//~| HELP try placing this code inside a block

y
}

fn main() {
foo();
bar();
baz();
}
47 changes: 47 additions & 0 deletions src/test/ui/parser/inline-const-without-block.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error: expected `{`, found `2`
--> $DIR/inline-const-without-block.rs:18:15
|
LL | const 2 => {}
| ^
| |
| expected `{`
| help: try placing this code inside a block: `{ 2 }`

error: expected `{`, found `1`
--> $DIR/inline-const-without-block.rs:25:15
|
LL | const 1 + 2 * 3 / 4 => {}
| ^------------
| |
| expected `{`
| help: try placing this code inside a block: `{ 1 + 2 * 3 / 4 }`

error: expected `{`, found `one`
--> $DIR/inline-const-without-block.rs:32:15
|
LL | const one() => {}
| ^^^--
| |
| expected `{`
| help: try placing this code inside a block: `{ one() }`

error: expected `{`, found `2`
--> $DIR/inline-const-without-block.rs:42:19
|
LL | let x = const 2;
| ^
| |
| expected `{`
| help: try placing this code inside a block: `{ 2 }`

error: expected `{`, found `1`
--> $DIR/inline-const-without-block.rs:50:19
|
LL | let y = const 1 + 2 * 3 / 4;
| ^------------
| |
| expected `{`
| help: try placing this code inside a block: `{ 1 + 2 * 3 / 4 }`

error: aborting due to 5 previous errors

2 changes: 1 addition & 1 deletion src/test/ui/parser/issue-66357-unexpected-unreachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@

fn f() { |[](* }
//~^ ERROR expected one of `,` or `:`, found `(`
//~| ERROR expected one of `&`, `(`, `)`, `-`, `...`, `..=`, `..`, `[`, `_`, `box`, `mut`, `ref`, `|`, identifier, or path, found `*`
//~| ERROR expected one of `&`, `(`, `)`, `-`, `...`, `..=`, `..`, `[`, `_`, `box`, `const`, `mut`, `ref`, `|`, identifier, or path, found `*`
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: expected one of `,` or `:`, found `(`
LL | fn f() { |[](* }
| ^ expected one of `,` or `:`

error: expected one of `&`, `(`, `)`, `-`, `...`, `..=`, `..`, `[`, `_`, `box`, `mut`, `ref`, `|`, identifier, or path, found `*`
error: expected one of `&`, `(`, `)`, `-`, `...`, `..=`, `..`, `[`, `_`, `box`, `const`, `mut`, `ref`, `|`, identifier, or path, found `*`
--> $DIR/issue-66357-unexpected-unreachable.rs:14:14
|
LL | fn f() { |[](* }
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/parser/keyword-const-as-identifier.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This file was auto-generated using 'src/etc/generate-keyword-tests.py const'

fn main() {
let const = "foo"; //~ error: expected identifier, found keyword `const`
let const = "foo";
//~^ ERROR expected `{`, found `=`
//~| ERROR inline-const is experimental [E0658]
}
19 changes: 12 additions & 7 deletions src/test/ui/parser/keyword-const-as-identifier.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
error: expected identifier, found keyword `const`
--> $DIR/keyword-const-as-identifier.rs:4:9
error: expected `{`, found `=`
--> $DIR/keyword-const-as-identifier.rs:2:15
|
LL | let const = "foo";
| ^^^^^ expected identifier, found keyword
Copy link
Member

Choose a reason for hiding this comment

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

This has regressed a bit now.
cc @oli-obk

Copy link
Member Author

Choose a reason for hiding this comment

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

The thing is that this:

let const { "foo" } = "foo";

is a totally valid statement (although it ICEs; see #78174). Perhaps we could give the old warning if it's just const and then = immediately after?

Copy link
Member

Choose a reason for hiding this comment

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

I see, but, what do you expect that code to do?.
I'd expect an error anyway something like error[E0005]: refutable pattern in local binding: ``&_`` not covered.

Copy link
Member Author

@camelid camelid Oct 22, 2020

Choose a reason for hiding this comment

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

It's valid syntactically, not semantically. (And either way it shouldn't ICE.)

And yes, the error you suggested is correct; it's the one that occurs for let "foo" = "foo";: error[E0005]: refutable pattern in local binding: `&_` not covered

Copy link
Member

Choose a reason for hiding this comment

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

exactly, 👍

| ^ expected `{`

error[E0658]: inline-const is experimental
--> $DIR/keyword-const-as-identifier.rs:2:9
|
help: you can escape reserved keywords to use them as identifiers
LL | let const = "foo";
| ^^^^^
|
LL | let r#const = "foo";
| ^^^^^^^
= note: see issue #76001 <https://github.com/rust-lang/rust/issues/76001> for more information
= help: add `#![feature(inline_const)]` to the crate attributes to enable

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.