From 4aa526f8093dcb46c3b4576ae7b05b5494add48e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 1 Oct 2019 10:32:45 +0200 Subject: [PATCH 01/25] Improve sidebar styling to make its integration easier --- src/librustdoc/html/static/rustdoc.css | 4 ++-- src/librustdoc/html/static/themes/dark.css | 2 +- src/librustdoc/html/static/themes/light.css | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index 244b24af43f35..64c858238dbce 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -183,7 +183,7 @@ nav.sub { position: fixed; left: 0; top: 0; - height: 100vh; + bottom: 0; overflow: auto; } @@ -573,7 +573,7 @@ h4 > code, h3 > code, .invisible > code { margin-top: 0; } -nav { +nav:not(.sidebar) { border-bottom: 1px solid; padding-bottom: 10px; margin-bottom: 10px; diff --git a/src/librustdoc/html/static/themes/dark.css b/src/librustdoc/html/static/themes/dark.css index e44ae2ad10cee..c3116dbe7a242 100644 --- a/src/librustdoc/html/static/themes/dark.css +++ b/src/librustdoc/html/static/themes/dark.css @@ -129,7 +129,7 @@ pre { pre.rust .comment { color: #8d8d8b; } pre.rust .doccomment { color: #8ca375; } -nav { +nav:not(.sidebar) { border-bottom-color: #4e4e4e; } nav.main .current { diff --git a/src/librustdoc/html/static/themes/light.css b/src/librustdoc/html/static/themes/light.css index 4c37000dde2c5..e2bf9f9d2f23a 100644 --- a/src/librustdoc/html/static/themes/light.css +++ b/src/librustdoc/html/static/themes/light.css @@ -129,7 +129,7 @@ pre { pre.rust .comment { color: #8E908C; } pre.rust .doccomment { color: #4D4D4C; } -nav { +nav:not(.sidebar) { border-bottom-color: #e0e0e0; } nav.main .current { From 9fe138aa4350bd981ceea6cba0ba8c2b464da94b Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 30 Sep 2019 11:05:04 +0200 Subject: [PATCH 02/25] regression test for 64453 borrow check error. --- src/test/ui/borrowck/issue-64453.rs | 24 +++++++++++++++++ src/test/ui/borrowck/issue-64453.stderr | 34 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 src/test/ui/borrowck/issue-64453.rs create mode 100644 src/test/ui/borrowck/issue-64453.stderr diff --git a/src/test/ui/borrowck/issue-64453.rs b/src/test/ui/borrowck/issue-64453.rs new file mode 100644 index 0000000000000..d8ab6b6e25f6f --- /dev/null +++ b/src/test/ui/borrowck/issue-64453.rs @@ -0,0 +1,24 @@ +struct Project; +struct Value; + +static settings_dir: String = format!(""); +//~^ ERROR [E0019] +//~| ERROR [E0015] +//~| ERROR [E0015] + +fn from_string(_: String) -> Value { + Value +} +fn set_editor(_: Value) {} + +fn main() { + let settings_data = from_string(settings_dir); + //~^ ERROR cannot move out of static item `settings_dir` [E0507] + let args: i32 = 0; + + match args { + ref x if x == &0 => set_editor(settings_data), + ref x if x == &1 => set_editor(settings_data), + _ => unimplemented!(), + } +} diff --git a/src/test/ui/borrowck/issue-64453.stderr b/src/test/ui/borrowck/issue-64453.stderr new file mode 100644 index 0000000000000..6987417fe192e --- /dev/null +++ b/src/test/ui/borrowck/issue-64453.stderr @@ -0,0 +1,34 @@ +error[E0507]: cannot move out of static item `settings_dir` + --> $DIR/issue-64453.rs:15:37 + | +LL | let settings_data = from_string(settings_dir); + | ^^^^^^^^^^^^ move occurs because `settings_dir` has type `std::string::String`, which does not implement the `Copy` trait + +error[E0019]: static contains unimplemented expression type + --> $DIR/issue-64453.rs:4:31 + | +LL | static settings_dir: String = format!(""); + | ^^^^^^^^^^^ + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error[E0015]: calls in statics are limited to constant functions, tuple structs and tuple variants + --> $DIR/issue-64453.rs:4:31 + | +LL | static settings_dir: String = format!(""); + | ^^^^^^^^^^^ + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error[E0015]: calls in statics are limited to constant functions, tuple structs and tuple variants + --> $DIR/issue-64453.rs:4:31 + | +LL | static settings_dir: String = format!(""); + | ^^^^^^^^^^^ + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0015, E0019, E0507. +For more information about an error, try `rustc --explain E0015`. From 48081065f4badf0544ec02d6ecde673716fd6695 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Mon, 16 Sep 2019 17:16:17 +0300 Subject: [PATCH 03/25] fix typo --- src/librustc_typeck/check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index be5723959fb22..064908de48f30 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -472,7 +472,7 @@ pub enum Diverges { WarnedAlways } -// Convenience impls for combinig `Diverges`. +// Convenience impls for combining `Diverges`. impl ops::BitAnd for Diverges { type Output = Self; From 057569e2c2864ce907d780fc022831064c61e984 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Wed, 18 Sep 2019 18:08:56 +0300 Subject: [PATCH 04/25] fix spurious unreachable_code lints for try{} block ok-wrapping --- src/librustc/hir/lowering/expr.rs | 43 ++++++++++++++++++++++--------- src/librustc/hir/mod.rs | 2 +- src/librustc_typeck/check/expr.rs | 33 ++++++++++++++++++------ 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index 9dcecedd97cae..0d2c7c13a85b9 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -394,17 +394,35 @@ impl LoweringContext<'_> { fn lower_expr_try_block(&mut self, body: &Block) -> hir::ExprKind { self.with_catch_scope(body.id, |this| { - let unstable_span = this.mark_span_with_reason( + let mut block = this.lower_block(body, true).into_inner(); + + let tail_expr = block.expr.take().map_or_else( + || { + let unit_span = this.mark_span_with_reason( + DesugaringKind::TryBlock, + this.sess.source_map().end_point(body.span), + None + ); + this.expr_unit(unit_span) + }, + |x: P| x.into_inner(), + ); + + let from_ok_span = this.mark_span_with_reason( DesugaringKind::TryBlock, - body.span, + tail_expr.span, this.allow_try_trait.clone(), ); - let mut block = this.lower_block(body, true).into_inner(); - let tail = block.expr.take().map_or_else( - || this.expr_unit(this.sess.source_map().end_point(unstable_span)), - |x: P| x.into_inner(), + + let ok_wrapped_span = this.mark_span_with_reason( + DesugaringKind::TryBlock, + tail_expr.span, + None ); - block.expr = Some(this.wrap_in_try_constructor(sym::from_ok, tail, unstable_span)); + + block.expr = Some(this.wrap_in_try_constructor( + sym::from_ok, from_ok_span, tail_expr, ok_wrapped_span)); + hir::ExprKind::Block(P(block), None) }) } @@ -412,12 +430,13 @@ impl LoweringContext<'_> { fn wrap_in_try_constructor( &mut self, method: Symbol, - e: hir::Expr, - unstable_span: Span, + method_span: Span, + expr: hir::Expr, + overall_span: Span, ) -> P { let path = &[sym::ops, sym::Try, method]; - let from_err = P(self.expr_std_path(unstable_span, path, None, ThinVec::new())); - P(self.expr_call(e.span, from_err, hir_vec![e])) + let constructor = P(self.expr_std_path(method_span, path, None, ThinVec::new())); + P(self.expr_call(overall_span, constructor, hir_vec![expr])) } fn lower_arm(&mut self, arm: &Arm) -> hir::Arm { @@ -1244,7 +1263,7 @@ impl LoweringContext<'_> { self.expr_call_std_path(try_span, from_path, hir_vec![err_expr]) }; let from_err_expr = - self.wrap_in_try_constructor(sym::from_error, from_expr, unstable_span); + self.wrap_in_try_constructor(sym::from_error, unstable_span, from_expr, try_span); let thin_attrs = ThinVec::from(attrs); let catch_scope = self.catch_scopes.last().map(|x| *x); let ret_expr = if let Some(catch_node) = catch_scope { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 9ae661f0952a4..9b4d88a5a0967 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -861,7 +861,7 @@ pub struct Block { pub span: Span, /// If true, then there may exist `break 'a` values that aim to /// break out of this block early. - /// Used by `'label: {}` blocks and by `catch` statements. + /// Used by `'label: {}` blocks and by `try {}` blocks. pub targeted_by_break: bool, } diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 04c8536de8dfe..a93ea0cc00467 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -18,6 +18,7 @@ use crate::util::nodemap::FxHashMap; use crate::astconv::AstConv as _; use errors::{Applicability, DiagnosticBuilder, pluralise}; +use syntax_pos::hygiene::DesugaringKind; use syntax::ast; use syntax::symbol::{Symbol, kw, sym}; use syntax::source_map::Span; @@ -150,8 +151,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { debug!(">> type-checking: expr={:?} expected={:?}", expr, expected); + // If when desugaring the try block we ok-wrapped an expression that diverges + // (e.g. `try { return }`) then technically the ok-wrapping expression is unreachable. + // But since it is autogenerated code the resulting warning is confusing for the user + // so we want avoid generating it. + // Ditto for the autogenerated `Try::from_ok(())` at the end of e.g. `try { return; }`. + let (is_try_block_ok_wrapped_expr, is_try_block_generated_expr) = match expr.node { + ExprKind::Call(_, ref args) if expr.span.is_desugaring(DesugaringKind::TryBlock) => { + (true, args.len() == 1 && args[0].span.is_desugaring(DesugaringKind::TryBlock)) + } + _ => (false, false), + }; + // Warn for expressions after diverging siblings. - self.warn_if_unreachable(expr.hir_id, expr.span, "expression"); + if !is_try_block_generated_expr { + self.warn_if_unreachable(expr.hir_id, expr.span, "expression"); + } // Hide the outer diverging and has_errors flags. let old_diverges = self.diverges.get(); @@ -162,13 +177,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let ty = self.check_expr_kind(expr, expected, needs); // Warn for non-block expressions with diverging children. - match expr.kind { - ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {}, - ExprKind::Call(ref callee, _) => - self.warn_if_unreachable(expr.hir_id, callee.span, "call"), - ExprKind::MethodCall(_, ref span, _) => - self.warn_if_unreachable(expr.hir_id, *span, "call"), - _ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"), + if !is_try_block_ok_wrapped_expr { + match expr.kind { + ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {}, + ExprKind::Call(ref callee, _) => + self.warn_if_unreachable(expr.hir_id, callee.span, "call"), + ExprKind::MethodCall(_, ref span, _) => + self.warn_if_unreachable(expr.hir_id, *span, "call"), + _ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"), + } } // Any expression that produces a value of type `!` must have diverged From c474c6e8253fe30684aa908ebef6e2923fb7146a Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Wed, 18 Sep 2019 18:10:19 +0300 Subject: [PATCH 05/25] add tests --- .../ui/try-block/try-block-bad-type.stderr | 8 +- .../try-block-unreachable-code-lint.rs | 76 +++++++++++++++++++ .../try-block-unreachable-code-lint.stderr | 28 +++++++ 3 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/try-block/try-block-unreachable-code-lint.rs create mode 100644 src/test/ui/try-block/try-block-unreachable-code-lint.stderr diff --git a/src/test/ui/try-block/try-block-bad-type.stderr b/src/test/ui/try-block/try-block-bad-type.stderr index e1c2c6b675e9b..0e993cb4e5885 100644 --- a/src/test/ui/try-block/try-block-bad-type.stderr +++ b/src/test/ui/try-block/try-block-bad-type.stderr @@ -32,18 +32,18 @@ LL | let res: Result = try { }; found type `()` error[E0277]: the trait bound `(): std::ops::Try` is not satisfied - --> $DIR/try-block-bad-type.rs:17:23 + --> $DIR/try-block-bad-type.rs:17:25 | LL | let res: () = try { }; - | ^^^ the trait `std::ops::Try` is not implemented for `()` + | ^ the trait `std::ops::Try` is not implemented for `()` | = note: required by `std::ops::Try::from_ok` error[E0277]: the trait bound `i32: std::ops::Try` is not satisfied - --> $DIR/try-block-bad-type.rs:19:24 + --> $DIR/try-block-bad-type.rs:19:26 | LL | let res: i32 = try { 5 }; - | ^^^^^ the trait `std::ops::Try` is not implemented for `i32` + | ^ the trait `std::ops::Try` is not implemented for `i32` | = note: required by `std::ops::Try::from_ok` diff --git a/src/test/ui/try-block/try-block-unreachable-code-lint.rs b/src/test/ui/try-block/try-block-unreachable-code-lint.rs new file mode 100644 index 0000000000000..5a9f662d229b2 --- /dev/null +++ b/src/test/ui/try-block/try-block-unreachable-code-lint.rs @@ -0,0 +1,76 @@ +// Test unreachable_code lint for `try {}` block ok-wrapping. See issues #54165, #63324. + +// compile-flags: --edition 2018 +// check-pass +#![feature(try_blocks)] +#![warn(unreachable_code)] + +fn err() -> Result { + Err(()) +} + +// In the following cases unreachable code is autogenerated and should not be reported. + +fn test_ok_wrapped_divergent_expr_1() { + let res: Result = try { + loop { + err()?; + } + }; + println!("res: {:?}", res); +} + +fn test_ok_wrapped_divergent_expr_2() { + let _: Result = try { + return + }; +} + +fn test_autogenerated_unit_after_divergent_expr() { + let _: Result<(), ()> = try { + return; + }; +} + +// In the following cases unreachable code should be reported. + +fn test_try_block_after_divergent_stmt() { + let _: Result = { + return; + + try { + loop { + err()?; + } + } + // ~^^^^^ WARNING unreachable expression + }; +} + +fn test_wrapped_divergent_expr() { + let _: Result = { + Err(return) + // ~^ WARNING unreachable call + }; +} + +fn test_expr_after_divergent_stmt_in_try_block() { + let res: Result = try { + loop { + err()?; + } + + 42 + // ~^ WARNING unreachable expression + }; + println!("res: {:?}", res); +} + +fn main() { + test_ok_wrapped_divergent_expr_1(); + test_ok_wrapped_divergent_expr_2(); + test_autogenerated_unit_after_divergent_expr(); + test_try_block_after_divergent_stmt(); + test_wrapped_divergent_expr(); + test_expr_after_divergent_stmt_in_try_block(); +} diff --git a/src/test/ui/try-block/try-block-unreachable-code-lint.stderr b/src/test/ui/try-block/try-block-unreachable-code-lint.stderr new file mode 100644 index 0000000000000..621d882aa9fb9 --- /dev/null +++ b/src/test/ui/try-block/try-block-unreachable-code-lint.stderr @@ -0,0 +1,28 @@ +warning: unreachable expression + --> $DIR/try-block-unreachable-code-lint.rs:41:9 + | +LL | / try { +LL | | loop { +LL | | err()?; +LL | | } +LL | | } + | |_________^ + | +note: lint level defined here + --> $DIR/try-block-unreachable-code-lint.rs:6:9 + | +LL | #![warn(unreachable_code)] + | ^^^^^^^^^^^^^^^^ + +warning: unreachable call + --> $DIR/try-block-unreachable-code-lint.rs:52:9 + | +LL | Err(return) + | ^^^ + +warning: unreachable expression + --> $DIR/try-block-unreachable-code-lint.rs:63:9 + | +LL | 42 + | ^^ + From ffa526937e54a22219a265df867e42f848a78a81 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Thu, 19 Sep 2019 01:20:18 +0300 Subject: [PATCH 06/25] address review comments --- src/librustc/hir/lowering/expr.rs | 28 ++++++------- src/librustc_typeck/check/expr.rs | 40 +++++++++---------- .../ui/try-block/try-block-bad-type.stderr | 8 ++-- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index 0d2c7c13a85b9..db5b197c5d673 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -392,36 +392,34 @@ impl LoweringContext<'_> { ) } + /// Desugar `try { ; }` into `{ ; ::std::ops::Try::from_ok() }`, + /// `try { ; }` into `{ ; ::std::ops::Try::from_ok(()) }` + /// and save the block id to use it as a break target for desugaring of the `?` operator. fn lower_expr_try_block(&mut self, body: &Block) -> hir::ExprKind { self.with_catch_scope(body.id, |this| { let mut block = this.lower_block(body, true).into_inner(); - let tail_expr = block.expr.take().map_or_else( - || { - let unit_span = this.mark_span_with_reason( - DesugaringKind::TryBlock, - this.sess.source_map().end_point(body.span), - None - ); - this.expr_unit(unit_span) - }, - |x: P| x.into_inner(), - ); - - let from_ok_span = this.mark_span_with_reason( + let try_span = this.mark_span_with_reason( DesugaringKind::TryBlock, - tail_expr.span, + body.span, this.allow_try_trait.clone(), ); + // Final expression of the block (if present) or `()` with span at the end of block + let tail_expr = block.expr.take().map_or_else( + || this.expr_unit(this.sess.source_map().end_point(try_span)), + |x: P| x.into_inner(), + ); + let ok_wrapped_span = this.mark_span_with_reason( DesugaringKind::TryBlock, tail_expr.span, None ); + // `::std::ops::Try::from_ok($tail_expr)` block.expr = Some(this.wrap_in_try_constructor( - sym::from_ok, from_ok_span, tail_expr, ok_wrapped_span)); + sym::from_ok, try_span, tail_expr, ok_wrapped_span)); hir::ExprKind::Block(P(block), None) }) diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index a93ea0cc00467..9152eee1a2c5a 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -151,20 +151,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { debug!(">> type-checking: expr={:?} expected={:?}", expr, expected); - // If when desugaring the try block we ok-wrapped an expression that diverges - // (e.g. `try { return }`) then technically the ok-wrapping expression is unreachable. - // But since it is autogenerated code the resulting warning is confusing for the user - // so we want avoid generating it. - // Ditto for the autogenerated `Try::from_ok(())` at the end of e.g. `try { return; }`. - let (is_try_block_ok_wrapped_expr, is_try_block_generated_expr) = match expr.node { - ExprKind::Call(_, ref args) if expr.span.is_desugaring(DesugaringKind::TryBlock) => { - (true, args.len() == 1 && args[0].span.is_desugaring(DesugaringKind::TryBlock)) - } - _ => (false, false), + // True if `expr` is a `Try::from_ok(())` that is a result of desugaring a try block + // without the final expr (e.g. `try { return; }`). We don't want to generate an + // unreachable_code lint for it since warnings for autogenerated code are confusing. + let is_try_block_generated_unit_expr = match expr.node { + ExprKind::Call(_, ref args) if expr.span.is_desugaring(DesugaringKind::TryBlock) => + args.len() == 1 && args[0].span.is_desugaring(DesugaringKind::TryBlock), + + _ => false, }; // Warn for expressions after diverging siblings. - if !is_try_block_generated_expr { + if !is_try_block_generated_unit_expr { self.warn_if_unreachable(expr.hir_id, expr.span, "expression"); } @@ -177,15 +175,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let ty = self.check_expr_kind(expr, expected, needs); // Warn for non-block expressions with diverging children. - if !is_try_block_ok_wrapped_expr { - match expr.kind { - ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {}, - ExprKind::Call(ref callee, _) => - self.warn_if_unreachable(expr.hir_id, callee.span, "call"), - ExprKind::MethodCall(_, ref span, _) => - self.warn_if_unreachable(expr.hir_id, *span, "call"), - _ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"), - } + match expr.kind { + ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {}, + // If `expr` is a result of desugaring the try block and is an ok-wrapped + // diverging expression (e.g. it arose from desugaring of `try { return }`), + // we skip issuing a warning because it is autogenerated code. + ExprKind::Call(..) if expr.span.is_desugaring(DesugaringKind::TryBlock) => {}, + ExprKind::Call(ref callee, _) => + self.warn_if_unreachable(expr.hir_id, callee.span, "call"), + ExprKind::MethodCall(_, ref span, _) => + self.warn_if_unreachable(expr.hir_id, *span, "call"), + _ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"), } // Any expression that produces a value of type `!` must have diverged diff --git a/src/test/ui/try-block/try-block-bad-type.stderr b/src/test/ui/try-block/try-block-bad-type.stderr index 0e993cb4e5885..e1c2c6b675e9b 100644 --- a/src/test/ui/try-block/try-block-bad-type.stderr +++ b/src/test/ui/try-block/try-block-bad-type.stderr @@ -32,18 +32,18 @@ LL | let res: Result = try { }; found type `()` error[E0277]: the trait bound `(): std::ops::Try` is not satisfied - --> $DIR/try-block-bad-type.rs:17:25 + --> $DIR/try-block-bad-type.rs:17:23 | LL | let res: () = try { }; - | ^ the trait `std::ops::Try` is not implemented for `()` + | ^^^ the trait `std::ops::Try` is not implemented for `()` | = note: required by `std::ops::Try::from_ok` error[E0277]: the trait bound `i32: std::ops::Try` is not satisfied - --> $DIR/try-block-bad-type.rs:19:26 + --> $DIR/try-block-bad-type.rs:19:24 | LL | let res: i32 = try { 5 }; - | ^ the trait `std::ops::Try` is not implemented for `i32` + | ^^^^^ the trait `std::ops::Try` is not implemented for `i32` | = note: required by `std::ops::Try::from_ok` From cb4ed52fd0d732c9a464937434651810fb666246 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Sun, 22 Sep 2019 17:09:10 +0300 Subject: [PATCH 07/25] fix test after rebase --- .../try-block-unreachable-code-lint.stderr | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/test/ui/try-block/try-block-unreachable-code-lint.stderr b/src/test/ui/try-block/try-block-unreachable-code-lint.stderr index 621d882aa9fb9..54fed04d400e9 100644 --- a/src/test/ui/try-block/try-block-unreachable-code-lint.stderr +++ b/src/test/ui/try-block/try-block-unreachable-code-lint.stderr @@ -1,12 +1,15 @@ warning: unreachable expression --> $DIR/try-block-unreachable-code-lint.rs:41:9 | +LL | return; + | ------ any code following this expression is unreachable +LL | LL | / try { LL | | loop { LL | | err()?; LL | | } LL | | } - | |_________^ + | |_________^ unreachable expression | note: lint level defined here --> $DIR/try-block-unreachable-code-lint.rs:6:9 @@ -18,11 +21,18 @@ warning: unreachable call --> $DIR/try-block-unreachable-code-lint.rs:52:9 | LL | Err(return) - | ^^^ + | ^^^ ------ any code following this expression is unreachable + | | + | unreachable call warning: unreachable expression --> $DIR/try-block-unreachable-code-lint.rs:63:9 | -LL | 42 - | ^^ +LL | / loop { +LL | | err()?; +LL | | } + | |_________- any code following this expression is unreachable +LL | +LL | 42 + | ^^ unreachable expression From 6acdea433647b93b91ab9075c1be8ee869d20bbc Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 1 Oct 2019 12:51:15 -0300 Subject: [PATCH 08/25] Make comment about dummy type a bit more clear --- src/librustc/ty/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 4b9117f71be5c..c0ec2b1cd5a80 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -600,7 +600,8 @@ impl<'tcx> rustc_serialize::UseSpecializedDecodable for Ty<'tcx> {} pub type CanonicalTy<'tcx> = Canonical<'tcx, Ty<'tcx>>; extern { - /// A dummy type used to force `List` to by unsized without requiring fat pointers. + /// A dummy type used to force List to be unsized while not requiring references to it be wide + /// pointers. type OpaqueListContents; } From 738baa734b41b171514280dd0772398a70628083 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 1 Oct 2019 13:03:33 -0300 Subject: [PATCH 09/25] Update src/librustc/ty/mod.rs Co-Authored-By: Oliver Scherer --- src/librustc/ty/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index c0ec2b1cd5a80..d2dc07374ed00 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -600,7 +600,7 @@ impl<'tcx> rustc_serialize::UseSpecializedDecodable for Ty<'tcx> {} pub type CanonicalTy<'tcx> = Canonical<'tcx, Ty<'tcx>>; extern { - /// A dummy type used to force List to be unsized while not requiring references to it be wide + /// A dummy type used to force `List` to be unsized while not requiring references to it be wide /// pointers. type OpaqueListContents; } From 75fdb959321dc04acb70bed73fe8cc84e85ae135 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Tue, 1 Oct 2019 20:04:41 +0300 Subject: [PATCH 10/25] change .node -> .kind after rebase --- src/librustc_typeck/check/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 9152eee1a2c5a..7a6fe9560fbff 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -154,7 +154,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // True if `expr` is a `Try::from_ok(())` that is a result of desugaring a try block // without the final expr (e.g. `try { return; }`). We don't want to generate an // unreachable_code lint for it since warnings for autogenerated code are confusing. - let is_try_block_generated_unit_expr = match expr.node { + let is_try_block_generated_unit_expr = match expr.kind { ExprKind::Call(_, ref args) if expr.span.is_desugaring(DesugaringKind::TryBlock) => args.len() == 1 && args[0].span.is_desugaring(DesugaringKind::TryBlock), From a1ad38f99cfc4ffa0f943f22834cc56afcef06e1 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 1 Oct 2019 11:35:50 -0700 Subject: [PATCH 11/25] Pass attrs to `do_dataflow` in new const checker This is needed to dump graphviz results for the `IndirectlyMutableLocals` analysis. --- src/librustc_mir/transform/check_consts/validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 3045239d7a770..2d7b215b13c45 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -137,7 +137,7 @@ pub fn compute_indirectly_mutable_locals<'mir, 'tcx>( item.tcx, item.body, item.def_id, - &[], + &item.tcx.get_attrs(item.def_id), &dead_unwinds, old_dataflow::IndirectlyMutableLocals::new(item.tcx, item.body, item.param_env), |_, local| old_dataflow::DebugFormatted::new(&local), From 8d84646c7b797a2c3e4ab8ed9da3935ab9e0bb31 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 1 Oct 2019 12:01:14 -0700 Subject: [PATCH 12/25] Don't mark zero-sized arrays as indirectly mutable when borrowed --- .../dataflow/impls/indirect_mutation.rs | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/indirect_mutation.rs b/src/librustc_mir/dataflow/impls/indirect_mutation.rs index 535b803b85f35..990425c3252e0 100644 --- a/src/librustc_mir/dataflow/impls/indirect_mutation.rs +++ b/src/librustc_mir/dataflow/impls/indirect_mutation.rs @@ -97,6 +97,36 @@ struct TransferFunction<'a, 'mir, 'tcx> { param_env: ty::ParamEnv<'tcx>, } +impl<'tcx> TransferFunction<'_, '_, 'tcx> { + /// Returns `true` if this borrow would allow mutation of the `borrowed_place`. + fn borrow_allows_mutation( + &self, + kind: mir::BorrowKind, + borrowed_place: &mir::Place<'tcx>, + ) -> bool { + let borrowed_ty = borrowed_place.ty(self.body, self.tcx).ty; + + // Zero-sized types cannot be mutated, since there is nothing inside to mutate. + // + // FIXME: For now, we only exempt arrays of length zero. We need to carefully + // consider the effects before extending this to all ZSTs. + if let ty::Array(_, len) = borrowed_ty.kind { + if len.try_eval_usize(self.tcx, self.param_env) == Some(0) { + return false; + } + } + + match kind { + mir::BorrowKind::Mut { .. } => true, + + | mir::BorrowKind::Shared + | mir::BorrowKind::Shallow + | mir::BorrowKind::Unique + => !borrowed_ty.is_freeze(self.tcx, self.param_env, DUMMY_SP), + } + } +} + impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> { fn visit_rvalue( &mut self, @@ -104,21 +134,7 @@ impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> { location: Location, ) { if let mir::Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue { - let is_mut = match kind { - mir::BorrowKind::Mut { .. } => true, - - | mir::BorrowKind::Shared - | mir::BorrowKind::Shallow - | mir::BorrowKind::Unique - => { - !borrowed_place - .ty(self.body, self.tcx) - .ty - .is_freeze(self.tcx, self.param_env, DUMMY_SP) - } - }; - - if is_mut { + if self.borrow_allows_mutation(kind, borrowed_place) { match borrowed_place.base { mir::PlaceBase::Local(borrowed_local) if !borrowed_place.is_indirect() => self.trans.gen(borrowed_local), From 4eeedd0953a86258c3b3379e0905ab9569eb3af6 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 1 Oct 2019 13:27:36 -0700 Subject: [PATCH 13/25] Add test cases for #64945 This also tests that `&&[]` no longer causes an ICE in this PR (although the test fails the borrow checker). This could be more complete. --- .../ui/consts/const-eval/generic-slice.rs | 31 +++++++++++++++++++ .../ui/consts/const-eval/generic-slice.stderr | 30 ++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 src/test/ui/consts/const-eval/generic-slice.rs create mode 100644 src/test/ui/consts/const-eval/generic-slice.stderr diff --git a/src/test/ui/consts/const-eval/generic-slice.rs b/src/test/ui/consts/const-eval/generic-slice.rs new file mode 100644 index 0000000000000..21360a1c471f6 --- /dev/null +++ b/src/test/ui/consts/const-eval/generic-slice.rs @@ -0,0 +1,31 @@ +// Several variants of #64945. + +// This struct is not important, we just use it to put `T` and `'a` in scope for our associated +// consts. +struct Generic<'a, T>(std::marker::PhantomData<&'a T>); + +impl<'a, T: 'static> Generic<'a, T> { + const EMPTY_SLICE: &'a [T] = { + let x: &'a [T] = &[]; + x + }; + + const EMPTY_SLICE_REF: &'a &'static [T] = { + let x: &'static [T] = &[]; + &x + //~^ ERROR `x` does not live long enough + }; +} + +static mut INTERIOR_MUT_AND_DROP: &'static [std::cell::RefCell>] = { + let x: &[_] = &[]; + x +}; + +static mut INTERIOR_MUT_AND_DROP_REF: &'static &'static [std::cell::RefCell>] = { + let x: &[_] = &[]; + &x + //~^ ERROR `x` does not live long enough +}; + +fn main() {} diff --git a/src/test/ui/consts/const-eval/generic-slice.stderr b/src/test/ui/consts/const-eval/generic-slice.stderr new file mode 100644 index 0000000000000..c38088df4d8e6 --- /dev/null +++ b/src/test/ui/consts/const-eval/generic-slice.stderr @@ -0,0 +1,30 @@ +error[E0597]: `x` does not live long enough + --> $DIR/generic-slice.rs:15:9 + | +LL | impl<'a, T: 'static> Generic<'a, T> { + | -- lifetime `'a` defined here +... +LL | &x + | ^^ + | | + | borrowed value does not live long enough + | using this value as a constant requires that `x` is borrowed for `'a` +LL | +LL | }; + | - `x` dropped here while still borrowed + +error[E0597]: `x` does not live long enough + --> $DIR/generic-slice.rs:27:5 + | +LL | &x + | ^^ + | | + | borrowed value does not live long enough + | using this value as a static requires that `x` is borrowed for `'static` +LL | +LL | }; + | - `x` dropped here while still borrowed + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0597`. From 242702965b759ad92f49c024d6cf2411af409bd7 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 1 Oct 2019 15:09:53 -0700 Subject: [PATCH 14/25] Fix typo passing flags to rustc --- src/test/ui/issues/issue-23477.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/issues/issue-23477.rs b/src/test/ui/issues/issue-23477.rs index b10b2e49616fb..787c4df96dcfb 100644 --- a/src/test/ui/issues/issue-23477.rs +++ b/src/test/ui/issues/issue-23477.rs @@ -1,5 +1,5 @@ // build-pass (FIXME(62277): could be check-pass?) -// compiler-flags: -g +// compile-flags: -g pub struct Dst { pub a: (), From cf984d22d68cd4b44228936af39fdfc47d6c2553 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 1 Oct 2019 15:10:29 -0700 Subject: [PATCH 15/25] This needs to be build-pass since it involves debuginfo --- src/test/ui/issues/issue-23477.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/issues/issue-23477.rs b/src/test/ui/issues/issue-23477.rs index 787c4df96dcfb..1ce05ba390d76 100644 --- a/src/test/ui/issues/issue-23477.rs +++ b/src/test/ui/issues/issue-23477.rs @@ -1,4 +1,4 @@ -// build-pass (FIXME(62277): could be check-pass?) +// build-pass // compile-flags: -g pub struct Dst { From f18535fa46752ac33ceaebe71386cf77a6df8f9a Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 5 Jul 2019 18:18:38 -0700 Subject: [PATCH 16/25] Refactor `rustc_peek` We now use `DataflowResultsCursor` to get the dataflow state before calls to `rustc_peek`. The original version used a custom implementation that only looked at assignment statements. This also extends `rustc_peek` to take arguments by-value as well as by-reference, and allows *all* dataflow analyses, not just those dependent on `MoveData`, to be inspected. --- src/librustc_mir/transform/rustc_peek.rs | 284 ++++++++++++----------- 1 file changed, 153 insertions(+), 131 deletions(-) diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index f509d2d7104eb..d0ffcbbae8855 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -3,9 +3,9 @@ use syntax::ast; use syntax::symbol::sym; use syntax_pos::Span; -use rustc::ty::{self, TyCtxt}; +use rustc::ty::{self, TyCtxt, Ty}; use rustc::hir::def_id::DefId; -use rustc::mir::{self, Body, Location}; +use rustc::mir::{self, Body, Location, Local}; use rustc_index::bit_set::BitSet; use crate::transform::{MirPass, MirSource}; @@ -13,6 +13,7 @@ use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::MoveDataParamEnv; use crate::dataflow::BitDenotation; use crate::dataflow::DataflowResults; +use crate::dataflow::DataflowResultsCursor; use crate::dataflow::{ DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces }; @@ -88,151 +89,172 @@ pub fn sanity_check_via_rustc_peek<'tcx, O>( def_id: DefId, _attributes: &[ast::Attribute], results: &DataflowResults<'tcx, O>, -) where - O: BitDenotation<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, -{ +) where O: RustcPeekAt<'tcx> { debug!("sanity_check_via_rustc_peek def_id: {:?}", def_id); - // FIXME: this is not DRY. Figure out way to abstract this and - // `dataflow::build_sets`. (But note it is doing non-standard - // stuff, so such generalization may not be realistic.) - for bb in body.basic_blocks().indices() { - each_block(tcx, body, results, bb); - } -} + let mut cursor = DataflowResultsCursor::new(results, body); -fn each_block<'tcx, O>( - tcx: TyCtxt<'tcx>, - body: &Body<'tcx>, - results: &DataflowResults<'tcx, O>, - bb: mir::BasicBlock, -) where - O: BitDenotation<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, -{ - let move_data = results.0.operator.move_data(); - let mir::BasicBlockData { ref statements, ref terminator, is_cleanup: _ } = body[bb]; - - let (args, span) = match is_rustc_peek(tcx, terminator) { - Some(args_and_span) => args_and_span, - None => return, - }; - assert!(args.len() == 1); - let peek_arg_place = match args[0] { - mir::Operand::Copy(ref place @ mir::Place { - base: mir::PlaceBase::Local(_), - projection: box [], - }) | - mir::Operand::Move(ref place @ mir::Place { - base: mir::PlaceBase::Local(_), - projection: box [], - }) => Some(place), - _ => None, - }; - - let peek_arg_place = match peek_arg_place { - Some(arg) => arg, - None => { - tcx.sess.diagnostic().span_err( - span, "dataflow::sanity_check cannot feed a non-temp to rustc_peek."); - return; - } - }; - - let mut on_entry = results.0.sets.entry_set_for(bb.index()).to_owned(); - let mut trans = results.0.sets.trans_for(bb.index()).clone(); - - // Emulate effect of all statements in the block up to (but not - // including) the borrow within `peek_arg_place`. Do *not* include - // call to `peek_arg_place` itself (since we are peeking the state - // of the argument at time immediate preceding Call to - // `rustc_peek`). - - for (j, stmt) in statements.iter().enumerate() { - debug!("rustc_peek: ({:?},{}) {:?}", bb, j, stmt); - let (place, rvalue) = match stmt.kind { - mir::StatementKind::Assign(box(ref place, ref rvalue)) => { - (place, rvalue) + let peek_calls = body + .basic_blocks() + .iter_enumerated() + .filter_map(|(bb, block_data)| { + PeekCall::from_terminator(tcx, block_data.terminator()) + .map(|call| (bb, block_data, call)) + }); + + for (bb, block_data, call) in peek_calls { + // Look for a sequence like the following to indicate that we should be peeking at `_1`: + // _2 = &_1; + // rustc_peek(_2); + // + // /* or */ + // + // _2 = _1; + // rustc_peek(_2); + let (statement_index, peek_rval) = block_data + .statements + .iter() + .enumerate() + .filter_map(|(i, stmt)| value_assigned_to_local(stmt, call.arg).map(|rval| (i, rval))) + .next() + .expect("call to rustc_peek should be preceded by \ + assignment to temporary holding its argument"); + + match (call.kind, peek_rval) { + | (PeekCallKind::ByRef, mir::Rvalue::Ref(_, _, place)) + | (PeekCallKind::ByVal, mir::Rvalue::Use(mir::Operand::Move(place))) + | (PeekCallKind::ByVal, mir::Rvalue::Use(mir::Operand::Copy(place))) + => { + let loc = Location { block: bb, statement_index }; + cursor.seek(loc); + let state = cursor.get(); + results.operator().peek_at(tcx, place, state, call); } - mir::StatementKind::FakeRead(..) | - mir::StatementKind::StorageLive(_) | - mir::StatementKind::StorageDead(_) | - mir::StatementKind::InlineAsm { .. } | - mir::StatementKind::Retag { .. } | - mir::StatementKind::AscribeUserType(..) | - mir::StatementKind::Nop => continue, - mir::StatementKind::SetDiscriminant{ .. } => - span_bug!(stmt.source_info.span, - "sanity_check should run before Deaggregator inserts SetDiscriminant"), - }; - - if place == peek_arg_place { - if let mir::Rvalue::Ref(_, mir::BorrowKind::Shared, ref peeking_at_place) = *rvalue { - // Okay, our search is over. - match move_data.rev_lookup.find(peeking_at_place.as_ref()) { - LookupResult::Exact(peek_mpi) => { - let bit_state = on_entry.contains(peek_mpi); - debug!("rustc_peek({:?} = &{:?}) bit_state: {}", - place, peeking_at_place, bit_state); - if !bit_state { - tcx.sess.span_err(span, "rustc_peek: bit not set"); - } - } - LookupResult::Parent(..) => { - tcx.sess.span_err(span, "rustc_peek: argument untracked"); - } - } - return; - } else { - // Our search should have been over, but the input - // does not match expectations of `rustc_peek` for - // this sanity_check. + + _ => { let msg = "rustc_peek: argument expression \ - must be immediate borrow of form `&expr`"; - tcx.sess.span_err(span, msg); + must be either `place` or `&place`"; + tcx.sess.span_err(call.span, msg); } } + } +} - let lhs_mpi = move_data.rev_lookup.find(place.as_ref()); - - debug!("rustc_peek: computing effect on place: {:?} ({:?}) in stmt: {:?}", - place, lhs_mpi, stmt); - // reset GEN and KILL sets before emulating their effect. - trans.clear(); - results.0.operator.before_statement_effect( - &mut trans, - Location { block: bb, statement_index: j }); - results.0.operator.statement_effect( - &mut trans, - Location { block: bb, statement_index: j }); - trans.apply(&mut on_entry); +/// If `stmt` is an assignment where the LHS is the given local (with no projections), returns the +/// RHS of the assignment. +fn value_assigned_to_local<'a, 'tcx>( + stmt: &'a mir::Statement<'tcx>, + local: Local, +) -> Option<&'a mir::Rvalue<'tcx>> { + if let mir::StatementKind::Assign(box (place, rvalue)) = &stmt.kind { + if let mir::Place { base: mir::PlaceBase::Local(l), projection: box [] } = place { + if local == *l { + return Some(&*rvalue); + } + } } - results.0.operator.before_terminator_effect( - &mut trans, - Location { block: bb, statement_index: statements.len() }); + None +} - tcx.sess.span_err(span, &format!("rustc_peek: MIR did not match \ - anticipated pattern; note that \ - rustc_peek expects input of \ - form `&expr`")); +#[derive(Clone, Copy, Debug)] +enum PeekCallKind { + ByVal, + ByRef, } -fn is_rustc_peek<'a, 'tcx>( - tcx: TyCtxt<'tcx>, - terminator: &'a Option>, -) -> Option<(&'a [mir::Operand<'tcx>], Span)> { - if let Some(mir::Terminator { ref kind, source_info, .. }) = *terminator { - if let mir::TerminatorKind::Call { func: ref oper, ref args, .. } = *kind { - if let mir::Operand::Constant(ref func) = *oper { - if let ty::FnDef(def_id, _) = func.literal.ty.kind { - let abi = tcx.fn_sig(def_id).abi(); - let name = tcx.item_name(def_id); - if abi == Abi::RustIntrinsic && name == sym::rustc_peek { - return Some((args, source_info.span)); +impl PeekCallKind { + fn from_arg_ty(arg: Ty<'_>) -> Self { + match arg.kind { + ty::Ref(_, _, _) => PeekCallKind::ByRef, + _ => PeekCallKind::ByVal, + } + } +} + +#[derive(Clone, Copy, Debug)] +pub struct PeekCall { + arg: Local, + kind: PeekCallKind, + span: Span, +} + +impl PeekCall { + fn from_terminator<'tcx>( + tcx: TyCtxt<'tcx>, + terminator: &mir::Terminator<'tcx>, + ) -> Option { + use mir::{Operand, Place, PlaceBase}; + + let span = terminator.source_info.span; + if let mir::TerminatorKind::Call { func: Operand::Constant(func), args, .. } = + &terminator.kind + { + if let ty::FnDef(def_id, substs) = func.literal.ty.kind { + let sig = tcx.fn_sig(def_id); + let name = tcx.item_name(def_id); + if sig.abi() != Abi::RustIntrinsic || name != sym::rustc_peek { + return None; + } + + assert_eq!(args.len(), 1); + let kind = PeekCallKind::from_arg_ty(substs.type_at(0)); + let arg = match args[0] { + | Operand::Copy(Place { base: PlaceBase::Local(local), projection: box [] }) + | Operand::Move(Place { base: PlaceBase::Local(local), projection: box [] }) + => local, + + _ => { + tcx.sess.diagnostic().span_err( + span, "dataflow::sanity_check cannot feed a non-temp to rustc_peek."); + return None; } + }; + + return Some(PeekCall { + arg, + kind, + span, + }); + } + } + + None + } +} + +pub trait RustcPeekAt<'tcx>: BitDenotation<'tcx> { + fn peek_at( + &self, + tcx: TyCtxt<'tcx>, + place: &mir::Place<'tcx>, + flow_state: &BitSet, + call: PeekCall, + ); +} + +impl<'tcx, O> RustcPeekAt<'tcx> for O + where O: BitDenotation<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, +{ + fn peek_at( + &self, + tcx: TyCtxt<'tcx>, + place: &mir::Place<'tcx>, + flow_state: &BitSet, + call: PeekCall, + ) { + match self.move_data().rev_lookup.find(place.as_ref()) { + LookupResult::Exact(peek_mpi) => { + let bit_state = flow_state.contains(peek_mpi); + debug!("rustc_peek({:?} = &{:?}) bit_state: {}", + call.arg, place, bit_state); + if !bit_state { + tcx.sess.span_err(call.span, "rustc_peek: bit not set"); } } + LookupResult::Parent(..) => { + tcx.sess.span_err(call.span, "rustc_peek: argument untracked"); + } } } - return None; } From 767550ef0fc0a01abd8dc50fead0967d215eca41 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 1 Oct 2019 20:14:01 -0700 Subject: [PATCH 17/25] Add `rustc_peek` support for `IndirectlyMutableLocals` --- src/librustc_mir/transform/rustc_peek.rs | 32 ++++++++++++++++++++++++ src/libsyntax_pos/symbol.rs | 1 + 2 files changed, 33 insertions(+) diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index d0ffcbbae8855..6edd28a4259a5 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -17,6 +17,7 @@ use crate::dataflow::DataflowResultsCursor; use crate::dataflow::{ DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces }; +use crate::dataflow::IndirectlyMutableLocals; use crate::dataflow::move_paths::{MovePathIndex, LookupResult}; use crate::dataflow::move_paths::{HasMoveData, MoveData}; @@ -51,6 +52,10 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { do_dataflow(tcx, body, def_id, &attributes, &dead_unwinds, DefinitelyInitializedPlaces::new(tcx, body, &mdpe), |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); + let flow_indirectly_mut = + do_dataflow(tcx, body, def_id, &attributes, &dead_unwinds, + IndirectlyMutableLocals::new(tcx, body, param_env), + |_, i| DebugFormatted::new(&i)); if has_rustc_mir_with(&attributes, sym::rustc_peek_maybe_init).is_some() { sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_inits); @@ -61,6 +66,9 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(&attributes, sym::rustc_peek_definite_init).is_some() { sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_def_inits); } + if has_rustc_mir_with(&attributes, sym::rustc_peek_indirectly_mutable).is_some() { + sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_indirectly_mut); + } if has_rustc_mir_with(&attributes, sym::stop_after_dataflow).is_some() { tcx.sess.fatal("stop_after_dataflow ended compilation"); } @@ -252,9 +260,33 @@ impl<'tcx, O> RustcPeekAt<'tcx> for O tcx.sess.span_err(call.span, "rustc_peek: bit not set"); } } + LookupResult::Parent(..) => { tcx.sess.span_err(call.span, "rustc_peek: argument untracked"); } } } } + +impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> { + fn peek_at( + &self, + tcx: TyCtxt<'tcx>, + place: &mir::Place<'tcx>, + flow_state: &BitSet, + call: PeekCall, + ) { + warn!("peek_at: place={:?}", place); + let local = match place { + mir::Place { base: mir::PlaceBase::Local(l), projection: box [] } => *l, + _ => { + tcx.sess.span_err(call.span, "rustc_peek: argument was not a local"); + return; + } + }; + + if !flow_state.contains(local) { + tcx.sess.span_err(call.span, "rustc_peek: bit not set"); + } + } +} diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 1769135e7f21a..82c47e6dbb758 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -597,6 +597,7 @@ symbols! { rustc_peek_definite_init, rustc_peek_maybe_init, rustc_peek_maybe_uninit, + rustc_peek_indirectly_mutable, rustc_private, rustc_proc_macro_decls, rustc_promotable, From 33aa5e855ed1deec5d086c7919b2a9572128b7ee Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 1 Oct 2019 20:14:27 -0700 Subject: [PATCH 18/25] Add test exposing unsoundness in `IndirectlyMutableLocals` --- .../mir-dataflow/indirect-mutation-offset.rs | 42 +++++++++++++++++++ .../indirect-mutation-offset.stderr | 10 +++++ 2 files changed, 52 insertions(+) create mode 100644 src/test/ui/mir-dataflow/indirect-mutation-offset.rs create mode 100644 src/test/ui/mir-dataflow/indirect-mutation-offset.stderr diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs new file mode 100644 index 0000000000000..804b70d26527a --- /dev/null +++ b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs @@ -0,0 +1,42 @@ +// compile-flags: -Zunleash-the-miri-inside-of-you + +#![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] + +use std::cell::UnsafeCell; +use std::intrinsics::rustc_peek; + +#[repr(C)] +struct PartialInteriorMut { + zst: [i32; 0], + cell: UnsafeCell, +} + +#[rustc_mir(rustc_peek_indirectly_mutable,stop_after_dataflow)] +#[rustc_mir(borrowck_graphviz_postflow="indirect.dot")] +const BOO: i32 = { + let x = PartialInteriorMut { + zst: [], + cell: UnsafeCell::new(0), + }; + + let p_zst: *const _ = &x.zst ; // Doesn't cause `x` to get marked as indirectly mutable. + + let rmut_cell = unsafe { + // Take advantage of the fact that `zst` and `cell` are at the same location in memory. + // This trick would work with any size type if miri implemented `ptr::offset`. + let p_cell = p_zst as *const UnsafeCell; + + let pmut_cell = (*p_cell).get(); + &mut *pmut_cell + }; + + *rmut_cell = 42; // Mutates `x` indirectly even though `x` is not marked indirectly mutable!!! + let val = *rmut_cell; + unsafe { rustc_peek(x) }; //~ ERROR rustc_peek: bit not set + + val +}; + +fn main() { + println!("{}", BOO); +} diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr b/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr new file mode 100644 index 0000000000000..16bd17813134a --- /dev/null +++ b/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr @@ -0,0 +1,10 @@ +error: rustc_peek: bit not set + --> $DIR/indirect-mutation-offset.rs:35:14 + | +LL | unsafe { rustc_peek(x) }; + | ^^^^^^^^^^^^^ + +error: stop_after_dataflow ended compilation + +error: aborting due to 2 previous errors + From 8e67180c526720262c03adbd8fd4d7ca71212f41 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Wed, 2 Oct 2019 15:34:31 +0900 Subject: [PATCH 19/25] Fix async/await ICE #64964 --- src/librustc/ty/context.rs | 6 +++++ .../check/generator_interior.rs | 22 +++++++++++-------- src/test/ui/async-await/issues/issue-64964.rs | 22 +++++++++++++++++++ 3 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/async-await/issues/issue-64964.rs diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 87b9917f340da..c58811cea190c 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -316,6 +316,12 @@ pub struct GeneratorInteriorTypeCause<'tcx> { pub scope_span: Option, } +BraceStructTypeFoldableImpl! { + impl<'tcx> TypeFoldable<'tcx> for GeneratorInteriorTypeCause<'tcx> { + ty, span, scope_span + } +} + #[derive(RustcEncodable, RustcDecodable, Debug)] pub struct TypeckTables<'tcx> { /// The HirId::owner all ItemLocalIds in this table are relative to. diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index 4608eb51df74e..c947d552a01a7 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -123,13 +123,6 @@ pub fn resolve_interior<'a, 'tcx>( // Sort types by insertion order types.sort_by_key(|t| t.1); - // Store the generator types and spans into the tables for this generator. - let interior_types = types.iter().cloned().map(|t| t.0).collect::>(); - visitor.fcx.inh.tables.borrow_mut().generator_interior_types = interior_types; - - // Extract type components - let type_list = fcx.tcx.mk_type_list(types.into_iter().map(|t| (t.0).ty)); - // The types in the generator interior contain lifetimes local to the generator itself, // which should not be exposed outside of the generator. Therefore, we replace these // lifetimes with existentially-bound lifetimes, which reflect the exact value of the @@ -139,18 +132,29 @@ pub fn resolve_interior<'a, 'tcx>( // if a Sync generator contains an &'α T, we need to check whether &'α T: Sync), // so knowledge of the exact relationships between them isn't particularly important. - debug!("types in generator {:?}, span = {:?}", type_list, body.value.span); + debug!( + "types in generator {:?}, span = {:?}", + types.iter().map(|t| (t.0).ty).collect::>(), + body.value.span, + ); // Replace all regions inside the generator interior with late bound regions // Note that each region slot in the types gets a new fresh late bound region, // which means that none of the regions inside relate to any other, even if // typeck had previously found constraints that would cause them to be related. let mut counter = 0; - let type_list = fcx.tcx.fold_regions(&type_list, &mut false, |_, current_depth| { + let types = fcx.tcx.fold_regions(&types, &mut false, |_, current_depth| { counter += 1; fcx.tcx.mk_region(ty::ReLateBound(current_depth, ty::BrAnon(counter))) }); + // Store the generator types and spans into the tables for this generator. + let interior_types = types.iter().map(|t| t.0.clone()).collect::>(); + visitor.fcx.inh.tables.borrow_mut().generator_interior_types = interior_types; + + // Extract type components + let type_list = fcx.tcx.mk_type_list(types.into_iter().map(|t| (t.0).ty)); + let witness = fcx.tcx.mk_generator_witness(ty::Binder::bind(type_list)); debug!("types in generator after region replacement {:?}, span = {:?}", diff --git a/src/test/ui/async-await/issues/issue-64964.rs b/src/test/ui/async-await/issues/issue-64964.rs new file mode 100644 index 0000000000000..af20bc5bf9a2b --- /dev/null +++ b/src/test/ui/async-await/issues/issue-64964.rs @@ -0,0 +1,22 @@ +// check-pass +// compile-flags: -Z query-dep-graph +// edition:2018 + +// Regression test for ICE related to `await`ing in a method. (#64964) + +struct Body; +impl Body { + async fn next(&mut self) { + async {}.await + } +} + +// Another reproduction: `await`ing with a variable from for-loop. + +async fn bar() { + for x in 0..10 { + async { Some(x) }.await.unwrap(); + } +} + +fn main() {} From f0fddb1a89eda1c5588725e23a50b3073f4e7e97 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Wed, 2 Oct 2019 18:59:05 +0900 Subject: [PATCH 20/25] Do not collect to vec for debug output --- src/librustc_typeck/check/generator_interior.rs | 6 +----- src/test/ui/async-await/issues/issue-64964.rs | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index c947d552a01a7..940537a5f48af 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -132,11 +132,7 @@ pub fn resolve_interior<'a, 'tcx>( // if a Sync generator contains an &'α T, we need to check whether &'α T: Sync), // so knowledge of the exact relationships between them isn't particularly important. - debug!( - "types in generator {:?}, span = {:?}", - types.iter().map(|t| (t.0).ty).collect::>(), - body.value.span, - ); + debug!("types in generator {:?}, span = {:?}", types, body.value.span); // Replace all regions inside the generator interior with late bound regions // Note that each region slot in the types gets a new fresh late bound region, diff --git a/src/test/ui/async-await/issues/issue-64964.rs b/src/test/ui/async-await/issues/issue-64964.rs index af20bc5bf9a2b..11f6cb6af9cc6 100644 --- a/src/test/ui/async-await/issues/issue-64964.rs +++ b/src/test/ui/async-await/issues/issue-64964.rs @@ -2,7 +2,7 @@ // compile-flags: -Z query-dep-graph // edition:2018 -// Regression test for ICE related to `await`ing in a method. (#64964) +// Regression test for ICE related to `await`ing in a method + incr. comp. (#64964) struct Body; impl Body { From 3a8932d9b0d12a67c9b92e0370de708a089d50b4 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 2 Oct 2019 05:58:26 -0400 Subject: [PATCH 21/25] [const-prop] Correctly handle locals that can't be propagated `const_prop()` now handles writing the Rvalue into the Place in the stack frame for us. So if we're not supported to propagate that value, we need to clear it. --- src/librustc_mir/transform/const_prop.rs | 34 +++---------------- src/test/ui/consts/const-eval/issue-64970.rs | 15 ++++++++ .../ui/consts/const-eval/issue-64970.stderr | 8 +++++ 3 files changed, 28 insertions(+), 29 deletions(-) create mode 100644 src/test/ui/consts/const-eval/issue-64970.rs create mode 100644 src/test/ui/consts/const-eval/issue-64970.stderr diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index f34bfbeab3b47..49ac1de8fef64 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -335,34 +335,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } fn get_const(&self, local: Local) -> Option> { - let l = &self.ecx.frame().locals[local]; - - // If the local is `Unitialized` or `Dead` then we haven't propagated a value into it. - // - // `InterpCx::access_local()` mostly takes care of this for us however, for ZSTs, - // it will synthesize a value for us. In doing so, that will cause the - // `get_const(l).is_empty()` assert right before we call `set_const()` in `visit_statement` - // to fail. - if let LocalValue::Uninitialized | LocalValue::Dead = l.value { - return None; - } - self.ecx.access_local(self.ecx.frame(), local, None).ok() } - fn set_const(&mut self, local: Local, c: Const<'tcx>) { - let frame = self.ecx.frame_mut(); - - if let Some(layout) = frame.locals[local].layout.get() { - debug_assert_eq!(c.layout, layout); - } - - frame.locals[local] = LocalState { - value: LocalValue::Live(*c), - layout: Cell::new(Some(c.layout)), - }; - } - fn remove_const(&mut self, local: Local) { self.ecx.frame_mut().locals[local] = LocalState { value: LocalValue::Uninitialized, @@ -735,10 +710,8 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { place) { trace!("checking whether {:?} can be stored to {:?}", value, local); if self.can_const_prop[local] { - trace!("storing {:?} to {:?}", value, local); - assert!(self.get_const(local).is_none() || - self.get_const(local) == Some(value)); - self.set_const(local, value); + trace!("stored {:?} to {:?}", value, local); + assert_eq!(self.get_const(local), Some(value)); if self.should_const_prop() { self.replace_with_const( @@ -747,6 +720,9 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { statement.source_info, ); } + } else { + trace!("can't propagate {:?} to {:?}", value, local); + self.remove_const(local); } } } diff --git a/src/test/ui/consts/const-eval/issue-64970.rs b/src/test/ui/consts/const-eval/issue-64970.rs new file mode 100644 index 0000000000000..ede5081c8a5c2 --- /dev/null +++ b/src/test/ui/consts/const-eval/issue-64970.rs @@ -0,0 +1,15 @@ +// run-pass + +fn main() { + foo(10); +} + +fn foo(mut n: i32) { + if false { + n = 0i32; + } + + if n > 0i32 { + 1i32 / n; + } +} diff --git a/src/test/ui/consts/const-eval/issue-64970.stderr b/src/test/ui/consts/const-eval/issue-64970.stderr new file mode 100644 index 0000000000000..2c44b68cbd1d1 --- /dev/null +++ b/src/test/ui/consts/const-eval/issue-64970.stderr @@ -0,0 +1,8 @@ +warning: unused arithmetic operation that must be used + --> $DIR/issue-64970.rs:13:9 + | +LL | 1i32 / n; + | ^^^^^^^^ + | + = note: `#[warn(unused_must_use)]` on by default + From 675ed489d5a9254e694498a4fc07813778fb3880 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Mon, 30 Sep 2019 13:46:41 -0400 Subject: [PATCH 22/25] Remove inline annotations from dep_node --- src/librustc/dep_graph/dep_node.rs | 40 ++++++++++++++---------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 3d5e7dd0af121..0686bec0621f4 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -114,7 +114,6 @@ macro_rules! define_dep_nodes { impl DepKind { #[allow(unreachable_code)] - #[inline] pub fn can_reconstruct_query_key<$tcx>(&self) -> bool { match *self { $( @@ -150,7 +149,6 @@ macro_rules! define_dep_nodes { } } - #[inline(always)] pub fn is_eval_always(&self) -> bool { match *self { $( @@ -199,7 +197,6 @@ macro_rules! define_dep_nodes { impl DepNode { #[allow(unreachable_code, non_snake_case)] - #[inline(always)] pub fn new<'tcx>(tcx: TyCtxt<'tcx>, dep: DepConstructor<'tcx>) -> DepNode @@ -219,14 +216,16 @@ macro_rules! define_dep_nodes { hash }; - if cfg!(debug_assertions) && - !dep_node.kind.can_reconstruct_query_key() && - (tcx.sess.opts.debugging_opts.incremental_info || - tcx.sess.opts.debugging_opts.query_dep_graph) + #[cfg(debug_assertions)] { - tcx.dep_graph.register_dep_node_debug_str(dep_node, || { - arg.to_debug_str(tcx) - }); + if !dep_node.kind.can_reconstruct_query_key() && + (tcx.sess.opts.debugging_opts.incremental_info || + tcx.sess.opts.debugging_opts.query_dep_graph) + { + tcx.dep_graph.register_dep_node_debug_str(dep_node, || { + arg.to_debug_str(tcx) + }); + } } return dep_node; @@ -242,14 +241,16 @@ macro_rules! define_dep_nodes { hash }; - if cfg!(debug_assertions) && - !dep_node.kind.can_reconstruct_query_key() && - (tcx.sess.opts.debugging_opts.incremental_info || - tcx.sess.opts.debugging_opts.query_dep_graph) + #[cfg(debug_assertions)] { - tcx.dep_graph.register_dep_node_debug_str(dep_node, || { - tupled_args.to_debug_str(tcx) - }); + if !dep_node.kind.can_reconstruct_query_key() && + (tcx.sess.opts.debugging_opts.incremental_info || + tcx.sess.opts.debugging_opts.query_dep_graph) + { + tcx.dep_graph.register_dep_node_debug_str(dep_node, || { + tupled_args.to_debug_str(tcx) + }); + } } return dep_node; @@ -267,7 +268,6 @@ macro_rules! define_dep_nodes { /// Construct a DepNode from the given DepKind and DefPathHash. This /// method will assert that the given DepKind actually requires a /// single DefId/DefPathHash parameter. - #[inline(always)] pub fn from_def_path_hash(kind: DepKind, def_path_hash: DefPathHash) -> DepNode { @@ -281,7 +281,6 @@ macro_rules! define_dep_nodes { /// Creates a new, parameterless DepNode. This method will assert /// that the DepNode corresponding to the given DepKind actually /// does not require any parameters. - #[inline(always)] pub fn new_no_params(kind: DepKind) -> DepNode { debug_assert!(!kind.has_params()); DepNode { @@ -300,7 +299,6 @@ macro_rules! define_dep_nodes { /// DepNode. Condition (2) might not be fulfilled if a DepNode /// refers to something from the previous compilation session that /// has been removed. - #[inline] pub fn extract_def_id(&self, tcx: TyCtxt<'_>) -> Option { if self.kind.can_reconstruct_query_key() { let def_path_hash = DefPathHash(self.hash); @@ -386,14 +384,12 @@ impl fmt::Debug for DepNode { impl DefPathHash { - #[inline(always)] pub fn to_dep_node(self, kind: DepKind) -> DepNode { DepNode::from_def_path_hash(kind, self) } } impl DefId { - #[inline(always)] pub fn to_dep_node(self, tcx: TyCtxt<'_>, kind: DepKind) -> DepNode { DepNode::from_def_path_hash(kind, tcx.def_path_hash(self)) } From 70dcb998890c87e06c5b9df083520779617a0055 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 2 Oct 2019 14:24:59 +0200 Subject: [PATCH 23/25] Remove rustdoc warning --- src/libtest/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index f04d289c4ef33..8b76080fc68c6 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -1109,7 +1109,7 @@ pub enum RunStrategy { InProcess, /// Spawns a subprocess to run the test, and sends the result back over the - /// supplied channel. Requires argv[0] to exist and point to the binary + /// supplied channel. Requires `argv[0]` to exist and point to the binary /// that's currently running. SpawnPrimary, } From 8e9f635a703e0746a8de6796a3343b2ec89d1219 Mon Sep 17 00:00:00 2001 From: Robin Kruppe Date: Wed, 2 Oct 2019 14:41:49 +0200 Subject: [PATCH 24/25] rustc book: nitpick SLP vectorization SLP vectorization (in general and as implemented in LLVM) is not limited to loops. --- src/doc/rustc/src/codegen-options/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 5c41acc6581c5..e73fd43f19a51 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -105,7 +105,7 @@ flag will turn that behavior off. ## no-vectorize-slp -By default, `rustc` will attempt to vectorize loops using [superword-level +By default, `rustc` will attempt to vectorize code using [superword-level parallelism](https://llvm.org/docs/Vectorizers.html#the-slp-vectorizer). This flag will turn that behavior off. From 79dc862d4a8c0690fbc50d7ebd129fab2e199a49 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 30 Sep 2019 13:09:10 -0300 Subject: [PATCH 25/25] Use PlaceBuilder to avoid a lot of slice -> vec -> slice convertions --- src/librustc_mir/build/expr/as_place.rs | 144 +++++++++++++++++++----- 1 file changed, 114 insertions(+), 30 deletions(-) diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index 09b33c6654a9d..ecbf469901582 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -6,13 +6,79 @@ use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::hair::*; use rustc::mir::interpret::{PanicInfo::BoundsCheck}; use rustc::mir::*; -use rustc::ty::{CanonicalUserTypeAnnotation, Variance}; +use rustc::ty::{CanonicalUserTypeAnnotation, Ty, Variance}; use rustc_data_structures::indexed_vec::Idx; +/// `PlaceBuilder` is used to create places during MIR construction. It allows you to "build up" a +/// place by pushing more and more projections onto the end, and then convert the final set into a +/// place using the `into_place` method. +/// +/// This is used internally when building a place for an expression like `a.b.c`. The fields `b` +/// and `c` can be progressively pushed onto the place builder that is created when converting `a`. +#[derive(Clone)] +struct PlaceBuilder<'tcx> { + base: PlaceBase<'tcx>, + projection: Vec>, +} + +impl PlaceBuilder<'tcx> { + fn into_place(self) -> Place<'tcx> { + Place { + base: self.base, + projection: self.projection.into_boxed_slice(), + } + } + + fn field(self, f: Field, ty: Ty<'tcx>) -> Self { + self.project(PlaceElem::Field(f, ty)) + } + + fn deref(self) -> Self { + self.project(PlaceElem::Deref) + } + + fn index(self, index: Local) -> Self { + self.project(PlaceElem::Index(index)) + } + + fn project(mut self, elem: PlaceElem<'tcx>) -> Self { + self.projection.push(elem); + self + } +} + +impl From for PlaceBuilder<'tcx> { + fn from(local: Local) -> Self { + Self { + base: local.into(), + projection: Vec::new(), + } + } +} + +impl From> for PlaceBuilder<'tcx> { + fn from(base: PlaceBase<'tcx>) -> Self { + Self { + base, + projection: Vec::new(), + } + } +} + impl<'a, 'tcx> Builder<'a, 'tcx> { /// Compile `expr`, yielding a place that we can move from etc. - pub fn as_place(&mut self, block: BasicBlock, expr: M) -> BlockAnd> + pub fn as_place(&mut self, mut block: BasicBlock, expr: M) -> BlockAnd> + where + M: Mirror<'tcx, Output = Expr<'tcx>>, + { + let place_builder = unpack!(block = self.as_place_builder(block, expr)); + block.and(place_builder.into_place()) + } + + /// This is used when constructing a compound `Place`, so that we can avoid creating + /// intermediate `Place` values until we know the full set of projections. + fn as_place_builder(&mut self, block: BasicBlock, expr: M) -> BlockAnd> where M: Mirror<'tcx, Output = Expr<'tcx>>, { @@ -25,7 +91,25 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// place. The place itself may or may not be mutable: /// * If this expr is a place expr like a.b, then we will return that place. /// * Otherwise, a temporary is created: in that event, it will be an immutable temporary. - pub fn as_read_only_place(&mut self, block: BasicBlock, expr: M) -> BlockAnd> + pub fn as_read_only_place(&mut self, mut block: BasicBlock, expr: M) -> BlockAnd> + where + M: Mirror<'tcx, Output = Expr<'tcx>>, + { + let place_builder = unpack!(block = self.as_read_only_place_builder(block, expr)); + block.and(place_builder.into_place()) + } + + /// This is used when constructing a compound `Place`, so that we can avoid creating + /// intermediate `Place` values until we know the full set of projections. + /// Mutability note: The caller of this method promises only to read from the resulting + /// place. The place itself may or may not be mutable: + /// * If this expr is a place expr like a.b, then we will return that place. + /// * Otherwise, a temporary is created: in that event, it will be an immutable temporary. + fn as_read_only_place_builder( + &mut self, + block: BasicBlock, + expr: M, + ) -> BlockAnd> where M: Mirror<'tcx, Output = Expr<'tcx>>, { @@ -38,7 +122,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { mut block: BasicBlock, expr: Expr<'tcx>, mutability: Mutability, - ) -> BlockAnd> { + ) -> BlockAnd> { debug!( "expr_as_place(block={:?}, expr={:?}, mutability={:?})", block, expr, mutability @@ -54,25 +138,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { value, } => this.in_scope((region_scope, source_info), lint_level, |this| { if mutability == Mutability::Not { - this.as_read_only_place(block, value) + this.as_read_only_place_builder(block, value) } else { - this.as_place(block, value) + this.as_place_builder(block, value) } }), ExprKind::Field { lhs, name } => { - let place = unpack!(block = this.as_place(block, lhs)); - let place = place.field(name, expr.ty); - block.and(place) + let place_builder = unpack!(block = this.as_place_builder(block, lhs)); + block.and(place_builder.field(name, expr.ty)) } ExprKind::Deref { arg } => { - let place = unpack!(block = this.as_place(block, arg)); - let place = place.deref(); - block.and(place) + let place_builder = unpack!(block = this.as_place_builder(block, arg)); + block.and(place_builder.deref()) } ExprKind::Index { lhs, index } => { let (usize_ty, bool_ty) = (this.hir.usize_ty(), this.hir.bool_ty()); - let slice = unpack!(block = this.as_place(block, lhs)); + let place_builder = unpack!(block = this.as_place_builder(block, lhs)); // Making this a *fresh* temporary also means we do not have to worry about // the index changing later: Nothing will ever change this temporary. // The "retagging" transformation (for Stacked Borrows) relies on this. @@ -83,6 +165,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Mutability::Not, )); + let slice = place_builder.clone().into_place(); // bounds check: let (len, lt) = ( this.temp(usize_ty.clone(), expr_span), @@ -92,7 +175,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block, source_info, // len = len(slice) &len, - Rvalue::Len(slice.clone()), + Rvalue::Len(slice), ); this.cfg.push_assign( block, @@ -110,30 +193,29 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { index: Operand::Copy(Place::from(idx)), }; let success = this.assert(block, Operand::Move(lt), true, msg, expr_span); - success.and(slice.index(idx)) + success.and(place_builder.index(idx)) } - ExprKind::SelfRef => block.and(Place::from(Local::new(1))), + ExprKind::SelfRef => block.and(PlaceBuilder::from(Local::new(1))), ExprKind::VarRef { id } => { - let place = if this.is_bound_var_in_guard(id) { + let place_builder = if this.is_bound_var_in_guard(id) { let index = this.var_local_id(id, RefWithinGuard); - Place::from(index).deref() + PlaceBuilder::from(index).deref() } else { let index = this.var_local_id(id, OutsideGuard); - Place::from(index) + PlaceBuilder::from(index) }; - block.and(place) + block.and(place_builder) } - ExprKind::StaticRef { id } => block.and(Place { - base: PlaceBase::Static(Box::new(Static { + ExprKind::StaticRef { id } => block.and(PlaceBuilder::from( + PlaceBase::Static(Box::new(Static { ty: expr.ty, kind: StaticKind::Static, def_id: id, - })), - projection: box [], - }), + })) + )), ExprKind::PlaceTypeAscription { source, user_ty } => { - let place = unpack!(block = this.as_place(block, source)); + let place_builder = unpack!(block = this.as_place_builder(block, source)); if let Some(user_ty) = user_ty { let annotation_index = this.canonical_user_type_annotations.push( CanonicalUserTypeAnnotation { @@ -142,13 +224,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { inferred_ty: expr.ty, } ); + + let place = place_builder.clone().into_place(); this.cfg.push( block, Statement { source_info, kind: StatementKind::AscribeUserType( box( - place.clone(), + place, UserTypeProjection { base: annotation_index, projs: vec![], } ), Variance::Invariant, @@ -156,7 +240,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ); } - block.and(place) + block.and(place_builder) } ExprKind::ValueTypeAscription { source, user_ty } => { let source = this.hir.mirror(source); @@ -185,7 +269,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ); } - block.and(Place::from(temp)) + block.and(PlaceBuilder::from(temp)) } ExprKind::Array { .. } @@ -221,7 +305,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }); let temp = unpack!(block = this.as_temp(block, expr.temp_lifetime, expr, mutability)); - block.and(Place::from(temp)) + block.and(PlaceBuilder::from(temp)) } } }