From d4fe9553f65df51a18999e956fd507e26271e74e Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Thu, 7 May 2020 03:57:31 +0200 Subject: [PATCH 01/12] Implement partial error recovery for `let` with `BinOpEq` When parsing `let x: i8 += 1` the compiler interprets `i8` as a trait which makes it more complicated to do error recovery. More advanced error recovery is not implemented in this commit. --- src/librustc_parse/parser/stmt.rs | 29 ++++++++++++++++++++++-- src/test/ui/parser/let-binop-plus.rs | 7 ++++++ src/test/ui/parser/let-binop-plus.stderr | 9 ++++++++ src/test/ui/parser/let-binop.rs | 8 +++++++ src/test/ui/parser/let-binop.stderr | 21 +++++++++++++++++ 5 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/parser/let-binop-plus.rs create mode 100644 src/test/ui/parser/let-binop-plus.stderr create mode 100644 src/test/ui/parser/let-binop.rs create mode 100644 src/test/ui/parser/let-binop.stderr diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs index 849193151c335..049aa7447f4db 100644 --- a/src/librustc_parse/parser/stmt.rs +++ b/src/librustc_parse/parser/stmt.rs @@ -12,7 +12,7 @@ use rustc_ast::ast::{Block, BlockCheckMode, Expr, ExprKind, Local, Stmt, StmtKin use rustc_ast::ptr::P; use rustc_ast::token::{self, TokenKind}; use rustc_ast::util::classify; -use rustc_errors::{Applicability, PResult}; +use rustc_errors::{struct_span_err, Applicability, PResult}; use rustc_span::source_map::{BytePos, Span}; use rustc_span::symbol::{kw, sym}; @@ -217,7 +217,32 @@ impl<'a> Parser<'a> { /// Parses the RHS of a local variable declaration (e.g., '= 14;'). fn parse_initializer(&mut self, skip_eq: bool) -> PResult<'a, Option>> { - if self.eat(&token::Eq) || skip_eq { Ok(Some(self.parse_expr()?)) } else { Ok(None) } + let parse = if !self.eat(&token::Eq) && !skip_eq { + // Error recovery for `let x += 1` + if matches!(self.token.kind, TokenKind::BinOpEq(_)) { + struct_span_err!( + self.sess.span_diagnostic, + self.token.span, + E0067, + "can't reassign to a uninitialized variable" + ) + .span_suggestion_short( + self.token.span, + "replace with `=` to initialize the variable", + "=".to_string(), + Applicability::MaybeIncorrect, + ) + .emit(); + self.bump(); + true + } else { + false + } + } else { + true + }; + + if parse { Ok(Some(self.parse_expr()?)) } else { Ok(None) } } /// Parses a block. No inner attributes are allowed. diff --git a/src/test/ui/parser/let-binop-plus.rs b/src/test/ui/parser/let-binop-plus.rs new file mode 100644 index 0000000000000..8d883d6e24894 --- /dev/null +++ b/src/test/ui/parser/let-binop-plus.rs @@ -0,0 +1,7 @@ +#![allow(bare_trait_objects)] + +fn main() { + let a: i8 += 1; + //~^ ERROR expected trait, found builtin type `i8` + let _ = a; +} diff --git a/src/test/ui/parser/let-binop-plus.stderr b/src/test/ui/parser/let-binop-plus.stderr new file mode 100644 index 0000000000000..baa935aff713c --- /dev/null +++ b/src/test/ui/parser/let-binop-plus.stderr @@ -0,0 +1,9 @@ +error[E0404]: expected trait, found builtin type `i8` + --> $DIR/let-binop-plus.rs:4:12 + | +LL | let a: i8 += 1; + | ^^ not a trait + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0404`. diff --git a/src/test/ui/parser/let-binop.rs b/src/test/ui/parser/let-binop.rs new file mode 100644 index 0000000000000..d445ab6bb8a1f --- /dev/null +++ b/src/test/ui/parser/let-binop.rs @@ -0,0 +1,8 @@ +fn main() { + let a: i8 *= 1; //~ ERROR can't reassign to a uninitialized variable + let _ = a; + let b += 1; //~ ERROR can't reassign to a uninitialized variable + let _ = b; + let c *= 1; //~ ERROR can't reassign to a uninitialized variable + let _ = c; +} diff --git a/src/test/ui/parser/let-binop.stderr b/src/test/ui/parser/let-binop.stderr new file mode 100644 index 0000000000000..3e9d4a80a70ef --- /dev/null +++ b/src/test/ui/parser/let-binop.stderr @@ -0,0 +1,21 @@ +error[E0067]: can't reassign to a uninitialized variable + --> $DIR/let-binop.rs:2:15 + | +LL | let a: i8 *= 1; + | ^^ help: replace with `=` to initialize the variable + +error[E0067]: can't reassign to a uninitialized variable + --> $DIR/let-binop.rs:4:11 + | +LL | let b += 1; + | ^^ help: replace with `=` to initialize the variable + +error[E0067]: can't reassign to a uninitialized variable + --> $DIR/let-binop.rs:6:11 + | +LL | let c *= 1; + | ^^ help: replace with `=` to initialize the variable + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0067`. From 48ff12acb184672393692e087927a66ff7907d71 Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Thu, 7 May 2020 04:09:57 +0200 Subject: [PATCH 02/12] Expand partial error recovery for `let` with `BinOpEq` --- src/librustc_parse/parser/stmt.rs | 40 +++++++++++++++++++++-------- src/test/ui/parser/let-binop.stderr | 22 ++++++++++++++-- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs index 049aa7447f4db..aceee81432896 100644 --- a/src/librustc_parse/parser/stmt.rs +++ b/src/librustc_parse/parser/stmt.rs @@ -145,12 +145,12 @@ impl<'a> Parser<'a> { } fn parse_local_mk(&mut self, lo: Span, attrs: AttrVec) -> PResult<'a, Stmt> { - let local = self.parse_local(attrs)?; + let local = self.parse_local(lo, attrs)?; Ok(self.mk_stmt(lo.to(self.prev_token.span), StmtKind::Local(local))) } /// Parses a local variable declaration. - fn parse_local(&mut self, attrs: AttrVec) -> PResult<'a, P> { + fn parse_local(&mut self, let_span: Span, attrs: AttrVec) -> PResult<'a, P> { let lo = self.prev_token.span; let pat = self.parse_top_pat(GateOr::Yes)?; @@ -174,7 +174,7 @@ impl<'a> Parser<'a> { } else { (None, None) }; - let init = match (self.parse_initializer(err.is_some()), err) { + let init = match (self.parse_initializer(let_span, ty.is_some(), err.is_some()), err) { (Ok(init), None) => { // init parsed, ty parsed init @@ -216,23 +216,43 @@ impl<'a> Parser<'a> { } /// Parses the RHS of a local variable declaration (e.g., '= 14;'). - fn parse_initializer(&mut self, skip_eq: bool) -> PResult<'a, Option>> { + fn parse_initializer( + &mut self, + let_span: Span, + has_ty: bool, + skip_eq: bool, + ) -> PResult<'a, Option>> { let parse = if !self.eat(&token::Eq) && !skip_eq { // Error recovery for `let x += 1` if matches!(self.token.kind, TokenKind::BinOpEq(_)) { - struct_span_err!( + let mut err = struct_span_err!( self.sess.span_diagnostic, self.token.span, E0067, "can't reassign to a uninitialized variable" - ) - .span_suggestion_short( + ); + err.span_suggestion_short( self.token.span, "replace with `=` to initialize the variable", "=".to_string(), - Applicability::MaybeIncorrect, - ) - .emit(); + if has_ty { + // for `let x: i8 += 1` it's highly likely that the `+` is a typo + Applicability::MachineApplicable + } else { + // for `let x += 1` it's a bit less likely that the `+` is a typo + Applicability::MaybeIncorrect + }, + ); + // In case of code like `let x += 1` it's possible the user may have meant to write `x += 1` + if !has_ty { + err.span_suggestion_short( + let_span, + "remove to reassign to a previously initialized variable", + "".to_string(), + Applicability::MaybeIncorrect, + ); + } + err.emit(); self.bump(); true } else { diff --git a/src/test/ui/parser/let-binop.stderr b/src/test/ui/parser/let-binop.stderr index 3e9d4a80a70ef..c37612430cef1 100644 --- a/src/test/ui/parser/let-binop.stderr +++ b/src/test/ui/parser/let-binop.stderr @@ -8,13 +8,31 @@ error[E0067]: can't reassign to a uninitialized variable --> $DIR/let-binop.rs:4:11 | LL | let b += 1; - | ^^ help: replace with `=` to initialize the variable + | ^^ + | +help: replace with `=` to initialize the variable + | +LL | let b = 1; + | ^ +help: remove to reassign to a previously initialized variable + | +LL | b += 1; + | -- error[E0067]: can't reassign to a uninitialized variable --> $DIR/let-binop.rs:6:11 | LL | let c *= 1; - | ^^ help: replace with `=` to initialize the variable + | ^^ + | +help: replace with `=` to initialize the variable + | +LL | let c = 1; + | ^ +help: remove to reassign to a previously initialized variable + | +LL | c *= 1; + | -- error: aborting due to 3 previous errors From 05d653199871955eba90abdd3b176603f030ab60 Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Thu, 7 May 2020 05:00:59 +0200 Subject: [PATCH 03/12] Error recovery for `let` with `+=` --- src/librustc_parse/parser/stmt.rs | 65 ++++++++++++------------ src/test/ui/parser/let-binop-plus.rs | 1 + src/test/ui/parser/let-binop-plus.stderr | 11 +++- 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs index aceee81432896..224f4ebf53828 100644 --- a/src/librustc_parse/parser/stmt.rs +++ b/src/librustc_parse/parser/stmt.rs @@ -222,44 +222,43 @@ impl<'a> Parser<'a> { has_ty: bool, skip_eq: bool, ) -> PResult<'a, Option>> { - let parse = if !self.eat(&token::Eq) && !skip_eq { + // In case of code like `let x: i8 += 1`, `i8` is interpreted as a trait consuming the `+` + // from `+=`. + let ate_plus = self.prev_token.is_like_plus() && has_ty; + let parse = if !skip_eq && (ate_plus || matches!(self.token.kind, TokenKind::BinOpEq(_))) { // Error recovery for `let x += 1` - if matches!(self.token.kind, TokenKind::BinOpEq(_)) { - let mut err = struct_span_err!( - self.sess.span_diagnostic, - self.token.span, - E0067, - "can't reassign to a uninitialized variable" - ); + let mut err = struct_span_err!( + self.sess.span_diagnostic, + self.token.span, + E0067, + "can't reassign to a uninitialized variable" + ); + err.span_suggestion_short( + self.token.span, + "replace with `=` to initialize the variable", + "=".to_string(), + if has_ty { + // for `let x: i8 += 1` it's highly likely that the `+` is a typo + Applicability::MachineApplicable + } else { + // for `let x += 1` it's a bit less likely that the `+` is a typo + Applicability::MaybeIncorrect + }, + ); + // In case of code like `let x += 1` it's possible the user may have meant to write `x += 1` + if !has_ty { err.span_suggestion_short( - self.token.span, - "replace with `=` to initialize the variable", - "=".to_string(), - if has_ty { - // for `let x: i8 += 1` it's highly likely that the `+` is a typo - Applicability::MachineApplicable - } else { - // for `let x += 1` it's a bit less likely that the `+` is a typo - Applicability::MaybeIncorrect - }, + let_span, + "remove to reassign to a previously initialized variable", + "".to_string(), + Applicability::MaybeIncorrect, ); - // In case of code like `let x += 1` it's possible the user may have meant to write `x += 1` - if !has_ty { - err.span_suggestion_short( - let_span, - "remove to reassign to a previously initialized variable", - "".to_string(), - Applicability::MaybeIncorrect, - ); - } - err.emit(); - self.bump(); - true - } else { - false } - } else { + err.emit(); + self.bump(); true + } else { + self.eat(&token::Eq) || skip_eq }; if parse { Ok(Some(self.parse_expr()?)) } else { Ok(None) } diff --git a/src/test/ui/parser/let-binop-plus.rs b/src/test/ui/parser/let-binop-plus.rs index 8d883d6e24894..98473e9f096d8 100644 --- a/src/test/ui/parser/let-binop-plus.rs +++ b/src/test/ui/parser/let-binop-plus.rs @@ -3,5 +3,6 @@ fn main() { let a: i8 += 1; //~^ ERROR expected trait, found builtin type `i8` + //~| ERROR can't reassign to a uninitialized variable let _ = a; } diff --git a/src/test/ui/parser/let-binop-plus.stderr b/src/test/ui/parser/let-binop-plus.stderr index baa935aff713c..d7d84ff16a0a1 100644 --- a/src/test/ui/parser/let-binop-plus.stderr +++ b/src/test/ui/parser/let-binop-plus.stderr @@ -1,9 +1,16 @@ +error[E0067]: can't reassign to a uninitialized variable + --> $DIR/let-binop-plus.rs:4:16 + | +LL | let a: i8 += 1; + | ^ help: replace with `=` to initialize the variable + error[E0404]: expected trait, found builtin type `i8` --> $DIR/let-binop-plus.rs:4:12 | LL | let a: i8 += 1; | ^^ not a trait -error: aborting due to previous error +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0404`. +Some errors have detailed explanations: E0067, E0404. +For more information about an error, try `rustc --explain E0067`. From 6ad24baf06c687517f188e8c6e6ce848924d001c Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Thu, 7 May 2020 23:45:51 +0200 Subject: [PATCH 04/12] Adjust according to estebank's review comments --- src/librustc_parse/parser/stmt.rs | 19 ++++++++----------- src/test/ui/parser/let-binop-plus.rs | 2 +- src/test/ui/parser/let-binop-plus.stderr | 4 ++-- src/test/ui/parser/let-binop.rs | 6 +++--- src/test/ui/parser/let-binop.stderr | 20 ++++++++++---------- 5 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs index 224f4ebf53828..bec810fde081d 100644 --- a/src/librustc_parse/parser/stmt.rs +++ b/src/librustc_parse/parser/stmt.rs @@ -174,7 +174,10 @@ impl<'a> Parser<'a> { } else { (None, None) }; - let init = match (self.parse_initializer(let_span, ty.is_some(), err.is_some()), err) { + let init = match ( + self.parse_initializer(let_span.until(pat.span), ty.is_some(), err.is_some()), + err, + ) { (Ok(init), None) => { // init parsed, ty parsed init @@ -231,25 +234,19 @@ impl<'a> Parser<'a> { self.sess.span_diagnostic, self.token.span, E0067, - "can't reassign to a uninitialized variable" + "can't reassign to an uninitialized variable" ); err.span_suggestion_short( self.token.span, - "replace with `=` to initialize the variable", + "initialize the variable", "=".to_string(), - if has_ty { - // for `let x: i8 += 1` it's highly likely that the `+` is a typo - Applicability::MachineApplicable - } else { - // for `let x += 1` it's a bit less likely that the `+` is a typo - Applicability::MaybeIncorrect - }, + Applicability::MaybeIncorrect, ); // In case of code like `let x += 1` it's possible the user may have meant to write `x += 1` if !has_ty { err.span_suggestion_short( let_span, - "remove to reassign to a previously initialized variable", + "otherwise, reassign to a previously initialized variable", "".to_string(), Applicability::MaybeIncorrect, ); diff --git a/src/test/ui/parser/let-binop-plus.rs b/src/test/ui/parser/let-binop-plus.rs index 98473e9f096d8..4d6d9b5c8d37f 100644 --- a/src/test/ui/parser/let-binop-plus.rs +++ b/src/test/ui/parser/let-binop-plus.rs @@ -3,6 +3,6 @@ fn main() { let a: i8 += 1; //~^ ERROR expected trait, found builtin type `i8` - //~| ERROR can't reassign to a uninitialized variable + //~| ERROR can't reassign to an uninitialized variable let _ = a; } diff --git a/src/test/ui/parser/let-binop-plus.stderr b/src/test/ui/parser/let-binop-plus.stderr index d7d84ff16a0a1..91a59fe24fedc 100644 --- a/src/test/ui/parser/let-binop-plus.stderr +++ b/src/test/ui/parser/let-binop-plus.stderr @@ -1,8 +1,8 @@ -error[E0067]: can't reassign to a uninitialized variable +error[E0067]: can't reassign to an uninitialized variable --> $DIR/let-binop-plus.rs:4:16 | LL | let a: i8 += 1; - | ^ help: replace with `=` to initialize the variable + | ^ help: initialize the variable error[E0404]: expected trait, found builtin type `i8` --> $DIR/let-binop-plus.rs:4:12 diff --git a/src/test/ui/parser/let-binop.rs b/src/test/ui/parser/let-binop.rs index d445ab6bb8a1f..7f58f5df2d412 100644 --- a/src/test/ui/parser/let-binop.rs +++ b/src/test/ui/parser/let-binop.rs @@ -1,8 +1,8 @@ fn main() { - let a: i8 *= 1; //~ ERROR can't reassign to a uninitialized variable + let a: i8 *= 1; //~ ERROR can't reassign to an uninitialized variable let _ = a; - let b += 1; //~ ERROR can't reassign to a uninitialized variable + let b += 1; //~ ERROR can't reassign to an uninitialized variable let _ = b; - let c *= 1; //~ ERROR can't reassign to a uninitialized variable + let c *= 1; //~ ERROR can't reassign to an uninitialized variable let _ = c; } diff --git a/src/test/ui/parser/let-binop.stderr b/src/test/ui/parser/let-binop.stderr index c37612430cef1..8a90b7cf74a4a 100644 --- a/src/test/ui/parser/let-binop.stderr +++ b/src/test/ui/parser/let-binop.stderr @@ -1,37 +1,37 @@ -error[E0067]: can't reassign to a uninitialized variable +error[E0067]: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:2:15 | LL | let a: i8 *= 1; - | ^^ help: replace with `=` to initialize the variable + | ^^ help: initialize the variable -error[E0067]: can't reassign to a uninitialized variable +error[E0067]: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:4:11 | LL | let b += 1; | ^^ | -help: replace with `=` to initialize the variable +help: initialize the variable | LL | let b = 1; | ^ -help: remove to reassign to a previously initialized variable +help: otherwise, reassign to a previously initialized variable | -LL | b += 1; +LL | b += 1; | -- -error[E0067]: can't reassign to a uninitialized variable +error[E0067]: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:6:11 | LL | let c *= 1; | ^^ | -help: replace with `=` to initialize the variable +help: initialize the variable | LL | let c = 1; | ^ -help: remove to reassign to a previously initialized variable +help: otherwise, reassign to a previously initialized variable | -LL | c *= 1; +LL | c *= 1; | -- error: aborting due to 3 previous errors From 98532a30901d7544c49fe82f499db53699645de0 Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Wed, 20 May 2020 22:09:03 +0200 Subject: [PATCH 05/12] Adjust according to petrochenkov's review comments --- src/librustc_parse/parser/stmt.rs | 65 ++++++++---------------- src/test/ui/parser/let-binop-plus.rs | 8 --- src/test/ui/parser/let-binop-plus.stderr | 16 ------ src/test/ui/parser/let-binop.stderr | 29 ++--------- 4 files changed, 27 insertions(+), 91 deletions(-) delete mode 100644 src/test/ui/parser/let-binop-plus.rs delete mode 100644 src/test/ui/parser/let-binop-plus.stderr diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs index bec810fde081d..53f32b7c800bd 100644 --- a/src/librustc_parse/parser/stmt.rs +++ b/src/librustc_parse/parser/stmt.rs @@ -12,7 +12,7 @@ use rustc_ast::ast::{Block, BlockCheckMode, Expr, ExprKind, Local, Stmt, StmtKin use rustc_ast::ptr::P; use rustc_ast::token::{self, TokenKind}; use rustc_ast::util::classify; -use rustc_errors::{struct_span_err, Applicability, PResult}; +use rustc_errors::{Applicability, PResult}; use rustc_span::source_map::{BytePos, Span}; use rustc_span::symbol::{kw, sym}; @@ -145,12 +145,12 @@ impl<'a> Parser<'a> { } fn parse_local_mk(&mut self, lo: Span, attrs: AttrVec) -> PResult<'a, Stmt> { - let local = self.parse_local(lo, attrs)?; + let local = self.parse_local(attrs)?; Ok(self.mk_stmt(lo.to(self.prev_token.span), StmtKind::Local(local))) } /// Parses a local variable declaration. - fn parse_local(&mut self, let_span: Span, attrs: AttrVec) -> PResult<'a, P> { + fn parse_local(&mut self, attrs: AttrVec) -> PResult<'a, P> { let lo = self.prev_token.span; let pat = self.parse_top_pat(GateOr::Yes)?; @@ -174,10 +174,7 @@ impl<'a> Parser<'a> { } else { (None, None) }; - let init = match ( - self.parse_initializer(let_span.until(pat.span), ty.is_some(), err.is_some()), - err, - ) { + let init = match (self.parse_initializer(err.is_some()), err) { (Ok(init), None) => { // init parsed, ty parsed init @@ -219,46 +216,28 @@ impl<'a> Parser<'a> { } /// Parses the RHS of a local variable declaration (e.g., '= 14;'). - fn parse_initializer( - &mut self, - let_span: Span, - has_ty: bool, - skip_eq: bool, - ) -> PResult<'a, Option>> { - // In case of code like `let x: i8 += 1`, `i8` is interpreted as a trait consuming the `+` - // from `+=`. - let ate_plus = self.prev_token.is_like_plus() && has_ty; - let parse = if !skip_eq && (ate_plus || matches!(self.token.kind, TokenKind::BinOpEq(_))) { - // Error recovery for `let x += 1` - let mut err = struct_span_err!( - self.sess.span_diagnostic, - self.token.span, - E0067, - "can't reassign to an uninitialized variable" - ); - err.span_suggestion_short( - self.token.span, - "initialize the variable", - "=".to_string(), - Applicability::MaybeIncorrect, - ); - // In case of code like `let x += 1` it's possible the user may have meant to write `x += 1` - if !has_ty { - err.span_suggestion_short( - let_span, - "otherwise, reassign to a previously initialized variable", - "".to_string(), + fn parse_initializer(&mut self, eq_optional: bool) -> PResult<'a, Option>> { + let eq_consumed = match self.token.kind { + token::BinOpEq(..) => { + // Recover `let x = 1` as `let x = 1` + self.struct_span_err( + self.token.span, + "can't reassign to an uninitialized variable", + ) + .span_suggestion_short( + self.token.span, + "initialize the variable", + "=".to_string(), Applicability::MaybeIncorrect, - ); + ) + .emit(); + self.bump(); + true } - err.emit(); - self.bump(); - true - } else { - self.eat(&token::Eq) || skip_eq + _ => self.eat(&token::Eq), }; - if parse { Ok(Some(self.parse_expr()?)) } else { Ok(None) } + Ok(if eq_consumed || eq_optional { Some(self.parse_expr()?) } else { None }) } /// Parses a block. No inner attributes are allowed. diff --git a/src/test/ui/parser/let-binop-plus.rs b/src/test/ui/parser/let-binop-plus.rs deleted file mode 100644 index 4d6d9b5c8d37f..0000000000000 --- a/src/test/ui/parser/let-binop-plus.rs +++ /dev/null @@ -1,8 +0,0 @@ -#![allow(bare_trait_objects)] - -fn main() { - let a: i8 += 1; - //~^ ERROR expected trait, found builtin type `i8` - //~| ERROR can't reassign to an uninitialized variable - let _ = a; -} diff --git a/src/test/ui/parser/let-binop-plus.stderr b/src/test/ui/parser/let-binop-plus.stderr deleted file mode 100644 index 91a59fe24fedc..0000000000000 --- a/src/test/ui/parser/let-binop-plus.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error[E0067]: can't reassign to an uninitialized variable - --> $DIR/let-binop-plus.rs:4:16 - | -LL | let a: i8 += 1; - | ^ help: initialize the variable - -error[E0404]: expected trait, found builtin type `i8` - --> $DIR/let-binop-plus.rs:4:12 - | -LL | let a: i8 += 1; - | ^^ not a trait - -error: aborting due to 2 previous errors - -Some errors have detailed explanations: E0067, E0404. -For more information about an error, try `rustc --explain E0067`. diff --git a/src/test/ui/parser/let-binop.stderr b/src/test/ui/parser/let-binop.stderr index 8a90b7cf74a4a..71431499ac70b 100644 --- a/src/test/ui/parser/let-binop.stderr +++ b/src/test/ui/parser/let-binop.stderr @@ -1,39 +1,20 @@ -error[E0067]: can't reassign to an uninitialized variable +error: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:2:15 | LL | let a: i8 *= 1; | ^^ help: initialize the variable -error[E0067]: can't reassign to an uninitialized variable +error: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:4:11 | LL | let b += 1; - | ^^ - | -help: initialize the variable - | -LL | let b = 1; - | ^ -help: otherwise, reassign to a previously initialized variable - | -LL | b += 1; - | -- + | ^^ help: initialize the variable -error[E0067]: can't reassign to an uninitialized variable +error: can't reassign to an uninitialized variable --> $DIR/let-binop.rs:6:11 | LL | let c *= 1; - | ^^ - | -help: initialize the variable - | -LL | let c = 1; - | ^ -help: otherwise, reassign to a previously initialized variable - | -LL | c *= 1; - | -- + | ^^ help: initialize the variable error: aborting due to 3 previous errors -For more information about this error, try `rustc --explain E0067`. From 3da3d15f9f150dafd7d635859795c4c714e7b7ad Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Sat, 30 May 2020 18:24:19 +0100 Subject: [PATCH 06/12] [RISC-V] Do not force frame pointers We have been seeing some very inefficient code that went away when using `-Cforce-frame-pointers=no`. For instance `core::ptr::drop_in_place` at `-Oz` was compiled into a function which consisted entirely of saving registers to the stack, then using the frame pointer to restore the same registers (without any instructions between the prolog and epilog). The RISC-V LLVM backend supports frame pointer elimination, so it makes sense to allow this to happen when using Rust. It's not clear to me that frame pointers have ever been required in the general case. In rust-lang/rust#61675 it was pointed out that this made reassembling stack traces easier, which is true, but there is a code generation option for forcing frame pointers, and I feel the default should not be to require frame pointers, given it demonstrably makes code size worse (around 10% in some embedded applications). The kinds of targets mentioned in rust-lang/rust#61675 are popular, but should not dictate that code generation should be worse for all RISC-V targets, especially as there is a way to use CFI information to reconstruct the stack when the frame pointer is eliminated. It is also a misconception that `fp` is always used for the frame pointer. `fp` is an ABI name for `x8` (aka `s0`), and if no frame pointer is required, `x8` may be used for other callee-saved values. This commit does ensure that the standard library is built with unwind tables, so that users do not need to rebuild the standard library in order to get a backtrace that includes standard library calls (which is the original reason for forcing frame pointers). --- src/bootstrap/compile.rs | 10 ++++++++++ src/librustc_target/spec/riscv32i_unknown_none_elf.rs | 1 - .../spec/riscv32imac_unknown_none_elf.rs | 1 - .../spec/riscv32imc_unknown_none_elf.rs | 1 - src/librustc_target/spec/riscv64gc_unknown_none_elf.rs | 1 - .../spec/riscv64imac_unknown_none_elf.rs | 1 - 6 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index b3999118e3de4..46c43be992ded 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -244,6 +244,16 @@ pub fn std_cargo(builder: &Builder<'_>, target: Interned, stage: u32, ca if stage >= 1 { cargo.rustflag("-Cembed-bitcode=yes"); } + + // By default, rustc does not include unwind tables unless they are required + // for a particular target. They are not required by RISC-V targets, but + // compiling the standard library with them means that users can get + // backtraces without having to recompile the standard library themselves. + // + // This choice was discussed in https://github.com/rust-lang/rust/pull/69890 + if target.contains("riscv") { + cargo.rustflag("-Cforce-unwind-tables=yes"); + } } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] diff --git a/src/librustc_target/spec/riscv32i_unknown_none_elf.rs b/src/librustc_target/spec/riscv32i_unknown_none_elf.rs index aade1e708232e..d7b3e7e15307a 100644 --- a/src/librustc_target/spec/riscv32i_unknown_none_elf.rs +++ b/src/librustc_target/spec/riscv32i_unknown_none_elf.rs @@ -25,7 +25,6 @@ pub fn target() -> TargetResult { relocation_model: RelocModel::Static, emit_debug_gdb_scripts: false, abi_blacklist: super::riscv_base::abi_blacklist(), - eliminate_frame_pointer: false, ..Default::default() }, }) diff --git a/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs b/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs index e2990eeb826f7..b93b6fcf8002a 100644 --- a/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs +++ b/src/librustc_target/spec/riscv32imac_unknown_none_elf.rs @@ -25,7 +25,6 @@ pub fn target() -> TargetResult { relocation_model: RelocModel::Static, emit_debug_gdb_scripts: false, abi_blacklist: super::riscv_base::abi_blacklist(), - eliminate_frame_pointer: false, ..Default::default() }, }) diff --git a/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs b/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs index 55a4d58dfccca..a16e7e31c6619 100644 --- a/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs +++ b/src/librustc_target/spec/riscv32imc_unknown_none_elf.rs @@ -25,7 +25,6 @@ pub fn target() -> TargetResult { relocation_model: RelocModel::Static, emit_debug_gdb_scripts: false, abi_blacklist: super::riscv_base::abi_blacklist(), - eliminate_frame_pointer: false, ..Default::default() }, }) diff --git a/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs b/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs index 7376a14e951f5..e5147a12ed320 100644 --- a/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs +++ b/src/librustc_target/spec/riscv64gc_unknown_none_elf.rs @@ -26,7 +26,6 @@ pub fn target() -> TargetResult { code_model: Some(CodeModel::Medium), emit_debug_gdb_scripts: false, abi_blacklist: super::riscv_base::abi_blacklist(), - eliminate_frame_pointer: false, ..Default::default() }, }) diff --git a/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs b/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs index a3b0eb5334f40..dc056b55b3868 100644 --- a/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs +++ b/src/librustc_target/spec/riscv64imac_unknown_none_elf.rs @@ -26,7 +26,6 @@ pub fn target() -> TargetResult { code_model: Some(CodeModel::Medium), emit_debug_gdb_scripts: false, abi_blacklist: super::riscv_base::abi_blacklist(), - eliminate_frame_pointer: false, ..Default::default() }, }) From f0d2e78d39d0efdb431af0b59c498d532d8146e2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 12 Jun 2020 18:17:30 +0200 Subject: [PATCH 07/12] add raw_ref macros --- src/libcore/ptr/mod.rs | 67 ++++++++++++++++++++++++++++++++++++++++++ src/libstd/lib.rs | 1 + 2 files changed, 68 insertions(+) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 777284ca5c096..199f08c3d5058 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -1399,3 +1399,70 @@ fnptr_impls_args! { A, B, C, D, E, F, G, H, I } fnptr_impls_args! { A, B, C, D, E, F, G, H, I, J } fnptr_impls_args! { A, B, C, D, E, F, G, H, I, J, K } fnptr_impls_args! { A, B, C, D, E, F, G, H, I, J, K, L } + +/// Create a `const` raw pointer to a place, without creating an intermediate reference. +/// +/// Creating a reference with `&`/`&mut` is only allowed if the pointer is properly aligned +/// and points to initialized data. For cases where those requirements do not hold, +/// raw pointers should be used instead. However, `&expr as *const _` creates a reference +/// before casting it to a raw pointer, and that reference is subject to the same rules +/// as all other references. This macro can create a raw pointer *without* creating +/// a reference first. +/// +/// # Example +/// +/// ``` +/// #![feature(raw_ref_macros)] +/// use std::ptr; +/// +/// #[repr(packed)] +/// struct Packed { +/// f1: u8, +/// f2: u16, +/// } +/// +/// let packed = Packed { f1: 1, f2: 2 }; +/// // `&packed.f2` would create an unaligned reference, and thus be Undefined Behavior! +/// let raw_f2 = ptr::raw_const!(packed.f2); +/// assert_eq!(unsafe { raw_f2.read_unaligned() }, 2); +/// ``` +#[unstable(feature = "raw_ref_macros", issue = "none")] +#[rustc_macro_transparency = "semitransparent"] +#[allow_internal_unstable(raw_ref_op)] +pub macro raw_const($e:expr) { + &raw const $e +} + +/// Create a `mut` raw pointer to a place, without creating an intermediate reference. +/// +/// Creating a reference with `&`/`&mut` is only allowed if the pointer is properly aligned +/// and points to initialized data. For cases where those requirements do not hold, +/// raw pointers should be used instead. However, `&mut expr as *mut _` creates a reference +/// before casting it to a raw pointer, and that reference is subject to the same rules +/// as all other references. This macro can create a raw pointer *without* creating +/// a reference first. +/// +/// # Example +/// +/// ``` +/// #![feature(raw_ref_macros)] +/// use std::ptr; +/// +/// #[repr(packed)] +/// struct Packed { +/// f1: u8, +/// f2: u16, +/// } +/// +/// let mut packed = Packed { f1: 1, f2: 2 }; +/// // `&mut packed.f2` would create an unaligned reference, and thus be Undefined Behavior! +/// let raw_f2 = ptr::raw_mut!(packed.f2); +/// unsafe { raw_f2.write_unaligned(42); } +/// assert_eq!({packed.f2}, 42); // `{...}` forces copying the field instead of creating a reference. +/// ``` +#[unstable(feature = "raw_ref_macros", issue = "none")] +#[rustc_macro_transparency = "semitransparent"] +#[allow_internal_unstable(raw_ref_op)] +pub macro raw_mut($e:expr) { + &raw mut $e +} diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index d6493454db591..ef699ede2a140 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -298,6 +298,7 @@ #![feature(prelude_import)] #![feature(ptr_internals)] #![feature(raw)] +#![feature(raw_ref_macros)] #![feature(renamed_spin_loop)] #![feature(rustc_attrs)] #![feature(rustc_private)] From 724dfba460e2c98311cacb5d4f6bb6da36ceec67 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 13 Jun 2020 15:05:37 +0200 Subject: [PATCH 08/12] Clean up some weird command strings --- src/librustdoc/lib.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 82d6cda986a9a..95d113166e001 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -165,9 +165,8 @@ fn opts() -> Vec { o.optmulti( "", "passes", - "list of passes to also run, you might want \ - to pass it multiple times; a value of `list` \ - will print available passes", + "list of passes to also run, you might want to pass it multiple times; a value of \ + `list` will print available passes", "PASSES", ) }), @@ -248,8 +247,8 @@ fn opts() -> Vec { "e", "extend-css", "To add some CSS rules with a given file to generate doc with your \ - own theme. However, your theme might break if the rustdoc's generated HTML \ - changes, so be careful!", + own theme. However, your theme might break if the rustdoc's generated HTML \ + changes, so be careful!", "PATH", ) }), @@ -262,7 +261,7 @@ fn opts() -> Vec { "", "playground-url", "URL to send code snippets to, may be reset by --markdown-playground-url \ - or `#![doc(html_playground_url=...)]`", + or `#![doc(html_playground_url=...)]`", "URL", ) }), @@ -276,8 +275,7 @@ fn opts() -> Vec { o.optflag( "", "sort-modules-by-appearance", - "sort modules by where they appear in the \ - program, rather than alphabetically", + "sort modules by where they appear in the program, rather than alphabetically", ) }), stable("theme", |o| { @@ -358,7 +356,7 @@ fn opts() -> Vec { "", "static-root-path", "Path string to force loading static files from in output pages. \ - If not set, uses combinations of '../' to reach the documentation root.", + If not set, uses combinations of '../' to reach the documentation root.", "PATH", ) }), From 0906066ae7d8a5e217e8cbf7f17de78a00d4ed83 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Mon, 15 Jun 2020 00:50:56 -0400 Subject: [PATCH 09/12] Test that bounds checks are elided when slice len is checked up-front --- src/test/codegen/issue-69101-bounds-check.rs | 26 ++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 src/test/codegen/issue-69101-bounds-check.rs diff --git a/src/test/codegen/issue-69101-bounds-check.rs b/src/test/codegen/issue-69101-bounds-check.rs new file mode 100644 index 0000000000000..cdbe51da03cc2 --- /dev/null +++ b/src/test/codegen/issue-69101-bounds-check.rs @@ -0,0 +1,26 @@ +// no-system-llvm +// compile-flags: -O +#![crate_type = "lib"] + +// CHECK-LABEL: @already_sliced_no_bounds_check +#[no_mangle] +pub fn already_sliced_no_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { + // CHECK: slice_index_len_fail + // CHECK-NOT: panic_bounds_check + let _ = (&a[..2048], &b[..2048], &mut c[..2048]); + for i in 0..1024 { + c[i] = a[i] ^ b[i]; + } +} + +// make sure we're checking for the right thing: there can be a panic if the slice is too small +// CHECK-LABEL: @already_sliced_bounds_check +#[no_mangle] +pub fn already_sliced_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { + // CHECK: slice_index_len_fail + // CHECK: panic_bounds_check + let _ = (&a[..1023], &b[..2048], &mut c[..2048]); + for i in 0..1024 { + c[i] = a[i] ^ b[i]; + } +} From e0975b9b0150118d376ea19908cc9bfdf400bfd5 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Mon, 15 Jun 2020 18:19:54 -0400 Subject: [PATCH 10/12] elaborate, add check for exact bounds --- src/test/codegen/issue-69101-bounds-check.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/test/codegen/issue-69101-bounds-check.rs b/src/test/codegen/issue-69101-bounds-check.rs index cdbe51da03cc2..cf062b6ad7588 100644 --- a/src/test/codegen/issue-69101-bounds-check.rs +++ b/src/test/codegen/issue-69101-bounds-check.rs @@ -2,6 +2,12 @@ // compile-flags: -O #![crate_type = "lib"] +// Make sure no bounds checks are emitted in the loop when upfront slicing +// ensures that the slices are big enough. +// In particular, bounds checks were not always optimized out if the upfront +// check was for a greater len than the loop requires. +// (i.e. `already_sliced_no_bounds_check` was not always optimized even when +// `already_sliced_no_bounds_check_exact` was) // CHECK-LABEL: @already_sliced_no_bounds_check #[no_mangle] pub fn already_sliced_no_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { @@ -13,7 +19,18 @@ pub fn already_sliced_no_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { } } -// make sure we're checking for the right thing: there can be a panic if the slice is too small +// CHECK-LABEL: @already_sliced_no_bounds_check_exact +#[no_mangle] +pub fn already_sliced_no_bounds_check_exact(a: &[u8], b: &[u8], c: &mut [u8]) { + // CHECK: slice_index_len_fail + // CHECK-NOT: panic_bounds_check + let _ = (&a[..1024], &b[..1024], &mut c[..1024]); + for i in 0..1024 { + c[i] = a[i] ^ b[i]; + } +} + +// Make sure we're checking for the right thing: there can be a panic if the slice is too small. // CHECK-LABEL: @already_sliced_bounds_check #[no_mangle] pub fn already_sliced_bounds_check(a: &[u8], b: &[u8], c: &mut [u8]) { From 0265e4e61bcd51b11f0b13b712245feb9c59ab50 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Jun 2020 09:25:29 +0200 Subject: [PATCH 11/12] add tracking issue --- src/libcore/ptr/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/ptr/mod.rs b/src/libcore/ptr/mod.rs index 199f08c3d5058..30c0f9a375714 100644 --- a/src/libcore/ptr/mod.rs +++ b/src/libcore/ptr/mod.rs @@ -1426,7 +1426,7 @@ fnptr_impls_args! { A, B, C, D, E, F, G, H, I, J, K, L } /// let raw_f2 = ptr::raw_const!(packed.f2); /// assert_eq!(unsafe { raw_f2.read_unaligned() }, 2); /// ``` -#[unstable(feature = "raw_ref_macros", issue = "none")] +#[unstable(feature = "raw_ref_macros", issue = "73394")] #[rustc_macro_transparency = "semitransparent"] #[allow_internal_unstable(raw_ref_op)] pub macro raw_const($e:expr) { @@ -1460,7 +1460,7 @@ pub macro raw_const($e:expr) { /// unsafe { raw_f2.write_unaligned(42); } /// assert_eq!({packed.f2}, 42); // `{...}` forces copying the field instead of creating a reference. /// ``` -#[unstable(feature = "raw_ref_macros", issue = "none")] +#[unstable(feature = "raw_ref_macros", issue = "73394")] #[rustc_macro_transparency = "semitransparent"] #[allow_internal_unstable(raw_ref_op)] pub macro raw_mut($e:expr) { From 9f2e8adc3597b21389dda31b687540e1a688fe87 Mon Sep 17 00:00:00 2001 From: pierwill <19642016+pierwill@users.noreply.github.com> Date: Tue, 16 Jun 2020 18:11:47 -0700 Subject: [PATCH 12/12] Fix typo in librustc_ast docs Fixed sentence by removing a word. --- src/librustc_ast/ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_ast/ast.rs b/src/librustc_ast/ast.rs index 62406552e318f..ce186c4834d72 100644 --- a/src/librustc_ast/ast.rs +++ b/src/librustc_ast/ast.rs @@ -5,7 +5,7 @@ //! additional metadata), and [`ItemKind`] (which represents a concrete type and contains //! information specific to the type of the item). //! -//! Other module items that worth mentioning: +//! Other module items worth mentioning: //! - [`Ty`] and [`TyKind`]: A parsed Rust type. //! - [`Expr`] and [`ExprKind`]: A parsed Rust expression. //! - [`Pat`] and [`PatKind`]: A parsed Rust pattern. Patterns are often dual to expressions.