Skip to content

Properly handle postfix inc/dec in standalone and subexpr scenarios #104875

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

Merged
merged 3 commits into from
Dec 14, 2022
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
50 changes: 15 additions & 35 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,6 @@ enum IsStandalone {
Standalone,
/// It's a subexpression, i.e., *not* standalone.
Subexpr,
/// It's maybe standalone; we're not sure.
Maybe,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -213,14 +211,8 @@ impl MultiSugg {
err.multipart_suggestion(&self.msg, self.patches, self.applicability);
}

/// Overrides individual messages and applicabilities.
fn emit_many(
err: &mut Diagnostic,
msg: &str,
applicability: Applicability,
suggestions: impl Iterator<Item = Self>,
) {
err.multipart_suggestions(msg, suggestions.map(|s| s.patches), applicability);
fn emit_verbose(self, err: &mut Diagnostic) {
err.multipart_suggestion_verbose(&self.msg, self.patches, self.applicability);
}
}

Expand Down Expand Up @@ -1267,26 +1259,24 @@ impl<'a> Parser<'a> {
&mut self,
operand_expr: P<Expr>,
op_span: Span,
prev_is_semi: bool,
start_stmt: bool,
) -> PResult<'a, P<Expr>> {
let standalone =
if prev_is_semi { IsStandalone::Standalone } else { IsStandalone::Subexpr };
let standalone = if start_stmt { IsStandalone::Standalone } else { IsStandalone::Subexpr };
let kind = IncDecRecovery { standalone, op: IncOrDec::Inc, fixity: UnaryFixity::Pre };

self.recover_from_inc_dec(operand_expr, kind, op_span)
}

pub(super) fn recover_from_postfix_increment(
&mut self,
operand_expr: P<Expr>,
op_span: Span,
start_stmt: bool,
) -> PResult<'a, P<Expr>> {
let kind = IncDecRecovery {
standalone: IsStandalone::Maybe,
standalone: if start_stmt { IsStandalone::Standalone } else { IsStandalone::Subexpr },
op: IncOrDec::Inc,
fixity: UnaryFixity::Post,
};

self.recover_from_inc_dec(operand_expr, kind, op_span)
}

Expand Down Expand Up @@ -1315,34 +1305,25 @@ impl<'a> Parser<'a> {
};

match kind.standalone {
IsStandalone::Standalone => self.inc_dec_standalone_suggest(kind, spans).emit(&mut err),
IsStandalone::Standalone => {
self.inc_dec_standalone_suggest(kind, spans).emit_verbose(&mut err)
}
IsStandalone::Subexpr => {
let Ok(base_src) = self.span_to_snippet(base.span)
else { return help_base_case(err, base) };
else { return help_base_case(err, base) };
match kind.fixity {
UnaryFixity::Pre => {
self.prefix_inc_dec_suggest(base_src, kind, spans).emit(&mut err)
}
UnaryFixity::Post => {
self.postfix_inc_dec_suggest(base_src, kind, spans).emit(&mut err)
// won't suggest since we can not handle the precedences
// for example: `a + b++` has been parsed (a + b)++ and we can not suggest here
if !matches!(base.kind, ExprKind::Binary(_, _, _)) {
self.postfix_inc_dec_suggest(base_src, kind, spans).emit(&mut err)
}
}
}
}
IsStandalone::Maybe => {
let Ok(base_src) = self.span_to_snippet(base.span)
else { return help_base_case(err, base) };
let sugg1 = match kind.fixity {
UnaryFixity::Pre => self.prefix_inc_dec_suggest(base_src, kind, spans),
UnaryFixity::Post => self.postfix_inc_dec_suggest(base_src, kind, spans),
};
let sugg2 = self.inc_dec_standalone_suggest(kind, spans);
MultiSugg::emit_many(
&mut err,
"use `+= 1` instead",
Applicability::Unspecified,
[sugg1, sugg2].into_iter(),
)
}
}
Err(err)
}
Expand Down Expand Up @@ -1392,7 +1373,6 @@ impl<'a> Parser<'a> {
}

patches.push((post_span, format!(" {}= 1", kind.op.chr())));

MultiSugg {
msg: format!("use `{}= 1` instead", kind.op.chr()),
patches,
Expand Down
15 changes: 9 additions & 6 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ macro_rules! maybe_whole_expr {
pub(super) enum LhsExpr {
NotYetParsed,
AttributesParsed(AttrWrapper),
AlreadyParsed(P<Expr>),
AlreadyParsed(P<Expr>, bool), // (expr, starts_statement)
}

impl From<Option<AttrWrapper>> for LhsExpr {
Expand All @@ -101,7 +101,7 @@ impl From<P<Expr>> for LhsExpr {
///
/// This conversion does not allocate.
fn from(expr: P<Expr>) -> Self {
LhsExpr::AlreadyParsed(expr)
LhsExpr::AlreadyParsed(expr, false)
}
}

Expand Down Expand Up @@ -173,7 +173,9 @@ impl<'a> Parser<'a> {
min_prec: usize,
lhs: LhsExpr,
) -> PResult<'a, P<Expr>> {
let mut lhs = if let LhsExpr::AlreadyParsed(expr) = lhs {
let mut starts_stmt = false;
let mut lhs = if let LhsExpr::AlreadyParsed(expr, starts_statement) = lhs {
starts_stmt = starts_statement;
expr
} else {
let attrs = match lhs {
Expand Down Expand Up @@ -292,7 +294,7 @@ impl<'a> Parser<'a> {
let op_span = self.prev_token.span.to(self.token.span);
// Eat the second `+`
self.bump();
lhs = self.recover_from_postfix_increment(lhs, op_span)?;
lhs = self.recover_from_postfix_increment(lhs, op_span, starts_stmt)?;
continue;
}

Expand Down Expand Up @@ -599,14 +601,15 @@ impl<'a> Parser<'a> {
token::BinOp(token::Plus)
if this.look_ahead(1, |t| *t == token::BinOp(token::Plus)) =>
{
let prev_is_semi = this.prev_token == token::Semi;
let starts_stmt = this.prev_token == token::Semi
|| this.prev_token == token::CloseDelim(Delimiter::Brace);
let pre_span = this.token.span.to(this.look_ahead(1, |t| t.span));
// Eat both `+`s.
this.bump();
this.bump();

let operand_expr = this.parse_dot_or_call_expr(Default::default())?;
this.recover_from_prefix_increment(operand_expr, pre_span, prev_is_semi)
this.recover_from_prefix_increment(operand_expr, pre_span, starts_stmt)
}
token::Ident(..) if this.token.is_keyword(kw::Box) => {
make_it!(this, attrs, |this, _| this.parse_box_expr(lo))
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl<'a> Parser<'a> {
// Perform this outside of the `collect_tokens_trailing_token` closure,
// since our outer attributes do not apply to this part of the expression
let expr = self.with_res(Restrictions::STMT_EXPR, |this| {
this.parse_assoc_expr_with(0, LhsExpr::AlreadyParsed(expr))
this.parse_assoc_expr_with(0, LhsExpr::AlreadyParsed(expr, true))
})?;
Ok(self.mk_stmt(lo.to(self.prev_token.span), StmtKind::Expr(expr)))
} else {
Expand Down Expand Up @@ -190,7 +190,7 @@ impl<'a> Parser<'a> {
let e = self.mk_expr(lo.to(hi), ExprKind::MacCall(mac));
let e = self.maybe_recover_from_bad_qpath(e)?;
let e = self.parse_dot_or_call_expr_with(e, lo, attrs)?;
let e = self.parse_assoc_expr_with(0, LhsExpr::AlreadyParsed(e))?;
let e = self.parse_assoc_expr_with(0, LhsExpr::AlreadyParsed(e, false))?;
StmtKind::Expr(e)
};
Ok(self.mk_stmt(lo.to(hi), kind))
Expand Down
63 changes: 63 additions & 0 deletions src/test/ui/parser/increment-autofix-2.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// run-rustfix

struct Foo {
bar: Bar,
}

struct Bar {
qux: i32,
}

pub fn post_regular() {
let mut i = 0;
i += 1; //~ ERROR Rust has no postfix increment operator
println!("{}", i);
}

pub fn post_while() {
let mut i = 0;
while { let tmp = i; i += 1; tmp } < 5 {
//~^ ERROR Rust has no postfix increment operator
println!("{}", i);
}
}

pub fn post_regular_tmp() {
let mut tmp = 0;
tmp += 1; //~ ERROR Rust has no postfix increment operator
println!("{}", tmp);
}

pub fn post_while_tmp() {
let mut tmp = 0;
while { let tmp_ = tmp; tmp += 1; tmp_ } < 5 {
//~^ ERROR Rust has no postfix increment operator
println!("{}", tmp);
}
}

pub fn post_field() {
let mut foo = Foo { bar: Bar { qux: 0 } };
foo.bar.qux += 1;
//~^ ERROR Rust has no postfix increment operator
println!("{}", foo.bar.qux);
}

pub fn post_field_tmp() {
struct S {
tmp: i32
}
let mut s = S { tmp: 0 };
s.tmp += 1;
//~^ ERROR Rust has no postfix increment operator
println!("{}", s.tmp);
}

pub fn pre_field() {
let mut foo = Foo { bar: Bar { qux: 0 } };
foo.bar.qux += 1;
//~^ ERROR Rust has no prefix increment operator
println!("{}", foo.bar.qux);
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// run-rustfix

struct Foo {
bar: Bar,
}
Expand Down Expand Up @@ -35,7 +37,7 @@ pub fn post_while_tmp() {
}

pub fn post_field() {
let foo = Foo { bar: Bar { qux: 0 } };
let mut foo = Foo { bar: Bar { qux: 0 } };
foo.bar.qux++;
//~^ ERROR Rust has no postfix increment operator
println!("{}", foo.bar.qux);
Expand All @@ -45,14 +47,14 @@ pub fn post_field_tmp() {
struct S {
tmp: i32
}
let s = S { tmp: 0 };
let mut s = S { tmp: 0 };
s.tmp++;
//~^ ERROR Rust has no postfix increment operator
println!("{}", s.tmp);
}

pub fn pre_field() {
let foo = Foo { bar: Bar { qux: 0 } };
let mut foo = Foo { bar: Bar { qux: 0 } };
++foo.bar.qux;
//~^ ERROR Rust has no prefix increment operator
println!("{}", foo.bar.qux);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
error: Rust has no postfix increment operator
--> $DIR/increment-notfixed.rs:11:6
--> $DIR/increment-autofix-2.rs:13:6
|
LL | i++;
| ^^ not a valid postfix operator
|
help: use `+= 1` instead
|
LL | { let tmp = i; i += 1; tmp };
| +++++++++++ ~~~~~~~~~~~~~~~
LL | i += 1;
| ~~~~

error: Rust has no postfix increment operator
--> $DIR/increment-notfixed.rs:17:12
--> $DIR/increment-autofix-2.rs:19:12
|
LL | while i++ < 5 {
| ----- ^^ not a valid postfix operator
Expand All @@ -23,24 +21,20 @@ help: use `+= 1` instead
|
LL | while { let tmp = i; i += 1; tmp } < 5 {
| +++++++++++ ~~~~~~~~~~~~~~~
LL | while i += 1 < 5 {
| ~~~~

error: Rust has no postfix increment operator
--> $DIR/increment-notfixed.rs:25:8
--> $DIR/increment-autofix-2.rs:27:8
|
LL | tmp++;
| ^^ not a valid postfix operator
|
help: use `+= 1` instead
|
LL | { let tmp_ = tmp; tmp += 1; tmp_ };
| ++++++++++++ ~~~~~~~~~~~~~~~~~~
LL | tmp += 1;
| ~~~~

error: Rust has no postfix increment operator
--> $DIR/increment-notfixed.rs:31:14
--> $DIR/increment-autofix-2.rs:33:14
|
LL | while tmp++ < 5 {
| ----- ^^ not a valid postfix operator
Expand All @@ -51,37 +45,31 @@ help: use `+= 1` instead
|
LL | while { let tmp_ = tmp; tmp += 1; tmp_ } < 5 {
| ++++++++++++ ~~~~~~~~~~~~~~~~~~
LL | while tmp += 1 < 5 {
| ~~~~

error: Rust has no postfix increment operator
--> $DIR/increment-notfixed.rs:39:16
--> $DIR/increment-autofix-2.rs:41:16
|
LL | foo.bar.qux++;
| ^^ not a valid postfix operator
|
help: use `+= 1` instead
|
LL | { let tmp = foo.bar.qux; foo.bar.qux += 1; tmp };
| +++++++++++ ~~~~~~~~~~~~~~~~~~~~~~~~~
LL | foo.bar.qux += 1;
| ~~~~

error: Rust has no postfix increment operator
--> $DIR/increment-notfixed.rs:49:10
--> $DIR/increment-autofix-2.rs:51:10
|
LL | s.tmp++;
| ^^ not a valid postfix operator
|
help: use `+= 1` instead
|
LL | { let tmp = s.tmp; s.tmp += 1; tmp };
| +++++++++++ ~~~~~~~~~~~~~~~~~~~
LL | s.tmp += 1;
| ~~~~

error: Rust has no prefix increment operator
--> $DIR/increment-notfixed.rs:56:5
--> $DIR/increment-autofix-2.rs:58:5
|
LL | ++foo.bar.qux;
| ^^ not a valid prefix operator
Expand Down
Loading