From 9bf577311473aa0accbf0d456cbd26b7530b200f Mon Sep 17 00:00:00 2001 From: William Chargin Date: Sun, 8 Sep 2019 14:03:09 -0700 Subject: [PATCH 01/50] Fix `Stdio::piped` example code and lint Summary: Invoking `rev` does not add a trailing newline when none is present in the input (at least on my Debian). Nearby examples use `echo` rather than `rev`, which probably explains the source of the discrepancy. Also, a `mut` qualifier is unused. Test Plan: Copy the code block into with a `fn main` wrapper, and run it. Note that it compiles and runs cleanly; prior to this commit, it would emit an `unused_mut` warning and then panic. wchargin-branch: stdio-piped-docs --- src/libstd/process.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/process.rs b/src/libstd/process.rs index 000f80f99e7a9..cecd9a5ab56b7 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -935,12 +935,12 @@ impl Stdio { /// .expect("Failed to spawn child process"); /// /// { - /// let mut stdin = child.stdin.as_mut().expect("Failed to open stdin"); + /// let stdin = child.stdin.as_mut().expect("Failed to open stdin"); /// stdin.write_all("Hello, world!".as_bytes()).expect("Failed to write to stdin"); /// } /// /// let output = child.wait_with_output().expect("Failed to read stdout"); - /// assert_eq!(String::from_utf8_lossy(&output.stdout), "!dlrow ,olleH\n"); + /// assert_eq!(String::from_utf8_lossy(&output.stdout), "!dlrow ,olleH"); /// ``` #[stable(feature = "process", since = "1.0.0")] pub fn piped() -> Stdio { Stdio(imp::Stdio::MakePipe) } From 612ef5f518198448c43959a6416b9da2964f9167 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 12 Sep 2019 17:04:32 -0400 Subject: [PATCH 02/50] add new tests for re_rebalance_coherence --- ...]-foreign[foreign[t],local]-for-foreign.rs | 14 +++++++++++++ ...pl[t]-foreign[local]-for-fundamental[t].rs | 21 +++++++++++++++++++ .../coherence/impl[t]-foreign[local]-for-t.rs | 16 ++++++++++++++ .../impl[t]-foreign[local]-for-t.stderr | 11 ++++++++++ .../coherence/re-rebalance-coherence-rpass.rs | 14 ------------- 5 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[local]-for-t.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[local]-for-t.stderr delete mode 100644 src/test/ui/coherence/re-rebalance-coherence-rpass.rs diff --git a/src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs b/src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs new file mode 100644 index 0000000000000..61f2637c0c297 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs @@ -0,0 +1,14 @@ +#![feature(re_rebalance_coherence)] + +// run-pass +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; +impl Remote2, Local> for usize { } + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs new file mode 100644 index 0000000000000..586b8de9e95c9 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs @@ -0,0 +1,21 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs +// run-pass + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for Box { + // FIXME(#64412) -- this is expected to error +} + +impl Remote1 for &T { + // FIXME(#64412) -- this is expected to error +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-t.rs b/src/test/ui/coherence/impl[t]-foreign[local]-for-t.rs new file mode 100644 index 0000000000000..6f35c6c9dbc88 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-t.rs @@ -0,0 +1,16 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for T { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-t.stderr b/src/test/ui/coherence/impl[t]-foreign[local]-for-t.stderr new file mode 100644 index 0000000000000..be7de8cccb467 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-t.stderr @@ -0,0 +1,11 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[local]-for-t.rs:12:1 + | +LL | impl Remote1 for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/re-rebalance-coherence-rpass.rs b/src/test/ui/coherence/re-rebalance-coherence-rpass.rs deleted file mode 100644 index bacd3b89fad29..0000000000000 --- a/src/test/ui/coherence/re-rebalance-coherence-rpass.rs +++ /dev/null @@ -1,14 +0,0 @@ -#![allow(dead_code)] -#![feature(re_rebalance_coherence)] - -// run-pass -// aux-build:re_rebalance_coherence_lib.rs - -extern crate re_rebalance_coherence_lib as lib; -use lib::*; - -struct Oracle; -impl Backend for Oracle {} -impl<'a, T:'a, Tab> QueryFragment for BatchInsert<'a, T, Tab> {} - -fn main() {} From e69d1b67b6603e0635c553eff693a0606d282d75 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 13 Sep 2019 14:57:06 -0400 Subject: [PATCH 03/50] change to check-pass --- .../coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs | 2 +- .../ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs b/src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs index 61f2637c0c297..54d4bf04a583c 100644 --- a/src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs +++ b/src/test/ui/coherence/impl[t]-foreign[foreign[t],local]-for-foreign.rs @@ -1,6 +1,6 @@ #![feature(re_rebalance_coherence)] -// run-pass +// check-pass // compile-flags:--crate-name=test // aux-build:coherence_lib.rs diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs index 586b8de9e95c9..db671cb9bcaba 100644 --- a/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs @@ -2,7 +2,7 @@ // compile-flags:--crate-name=test // aux-build:coherence_lib.rs -// run-pass +// check-pass extern crate coherence_lib as lib; use lib::*; From 146cb8eda667e5d76bebdef3acc61adaeaa025b0 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 7 Sep 2019 15:49:58 +0200 Subject: [PATCH 04/50] or-patterns: remove hack from lowering. --- src/librustc/hir/lowering.rs | 29 ------------------ src/librustc/hir/lowering/expr.rs | 51 ++++++++++++------------------- src/libsyntax/visit.rs | 2 -- 3 files changed, 19 insertions(+), 63 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 58789a10609b7..a5f08cd96061f 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -434,35 +434,6 @@ impl<'a> LoweringContext<'a> { visit::walk_pat(self, p) } - // HACK(or_patterns; Centril | dlrobertson): Avoid creating - // HIR nodes for `PatKind::Or` for the top level of a `ast::Arm`. - // This is a temporary hack that should go away once we push down - // `arm.pats: HirVec>` -> `arm.pat: P` to HIR. // Centril - fn visit_arm(&mut self, arm: &'tcx Arm) { - match &arm.pat.node { - PatKind::Or(pats) => pats.iter().for_each(|p| self.visit_pat(p)), - _ => self.visit_pat(&arm.pat), - } - walk_list!(self, visit_expr, &arm.guard); - self.visit_expr(&arm.body); - walk_list!(self, visit_attribute, &arm.attrs); - } - - // HACK(or_patterns; Centril | dlrobertson): Same as above. // Centril - fn visit_expr(&mut self, e: &'tcx Expr) { - if let ExprKind::Let(pat, scrutinee) = &e.node { - walk_list!(self, visit_attribute, e.attrs.iter()); - match &pat.node { - PatKind::Or(pats) => pats.iter().for_each(|p| self.visit_pat(p)), - _ => self.visit_pat(&pat), - } - self.visit_expr(scrutinee); - self.visit_expr_post(e); - return; - } - visit::walk_expr(self, e) - } - fn visit_item(&mut self, item: &'tcx Item) { let hir_id = self.lctx.allocate_hir_id_counter(item.id); diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index a46cdabbb518f..d71dc53d104af 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -250,14 +250,14 @@ impl LoweringContext<'_> { // 4. The return type of the block is `bool` which seems like what the user wanted. let scrutinee = self.lower_expr(scrutinee); let then_arm = { - let pat = self.lower_pat_top_hack(pat); + let pat = self.lower_pat(pat); let expr = self.expr_bool(span, true); self.arm(pat, P(expr)) }; let else_arm = { let pat = self.pat_wild(span); let expr = self.expr_bool(span, false); - self.arm(hir_vec![pat], P(expr)) + self.arm(pat, P(expr)) }; hir::ExprKind::Match( P(scrutinee), @@ -281,7 +281,7 @@ impl LoweringContext<'_> { None => (self.expr_block_empty(span), false), Some(els) => (self.lower_expr(els), true), }; - let else_arm = self.arm(hir_vec![else_pat], P(else_expr)); + let else_arm = self.arm(else_pat, P(else_expr)); // Handle then + scrutinee: let then_blk = self.lower_block(then, false); @@ -290,7 +290,7 @@ impl LoweringContext<'_> { // ` => `: ExprKind::Let(ref pat, ref scrutinee) => { let scrutinee = self.lower_expr(scrutinee); - let pat = self.lower_pat_top_hack(pat); + let pat = self.lower_pat(pat); (pat, scrutinee, hir::MatchSource::IfLetDesugar { contains_else_clause }) } // `true => `: @@ -307,7 +307,7 @@ impl LoweringContext<'_> { // let temporaries live outside of `cond`. let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new()); let pat = self.pat_bool(span, true); - (hir_vec![pat], cond, hir::MatchSource::IfDesugar { contains_else_clause }) + (pat, cond, hir::MatchSource::IfDesugar { contains_else_clause }) } }; let then_arm = self.arm(then_pat, P(then_expr)); @@ -331,7 +331,7 @@ impl LoweringContext<'_> { let else_arm = { let else_pat = self.pat_wild(span); let else_expr = self.expr_break(span, ThinVec::new()); - self.arm(hir_vec![else_pat], else_expr) + self.arm(else_pat, else_expr) }; // Handle then + scrutinee: @@ -348,7 +348,7 @@ impl LoweringContext<'_> { // } // } let scrutinee = self.with_loop_condition_scope(|t| t.lower_expr(scrutinee)); - let pat = self.lower_pat_top_hack(pat); + let pat = self.lower_pat(pat); (pat, scrutinee, hir::MatchSource::WhileLetDesugar, hir::LoopSource::WhileLet) } _ => { @@ -376,7 +376,7 @@ impl LoweringContext<'_> { let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new()); // `true => `: let pat = self.pat_bool(span, true); - (hir_vec![pat], cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While) + (pat, cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While) } }; let then_arm = self.arm(then_pat, P(then_expr)); @@ -429,7 +429,7 @@ impl LoweringContext<'_> { hir::Arm { hir_id: self.next_id(), attrs: self.lower_attrs(&arm.attrs), - pats: self.lower_pat_top_hack(&arm.pat), + pat: self.lower_pat(&arm.pat), guard: match arm.guard { Some(ref x) => Some(hir::Guard::If(P(self.lower_expr(x)))), _ => None, @@ -439,16 +439,6 @@ impl LoweringContext<'_> { } } - /// HACK(or_patterns; Centril | dlrobertson): For now we don't push down top level or-patterns - /// `p | q` into `hir::PatKind::Or(...)` as post-lowering bits of the compiler are not ready - /// to deal with it. This should by fixed by pushing it down to HIR and then HAIR. - fn lower_pat_top_hack(&mut self, pat: &Pat) -> HirVec> { - match pat.node { - PatKind::Or(ref ps) => ps.iter().map(|x| self.lower_pat(x)).collect(), - _ => hir_vec![self.lower_pat(pat)], - } - } - pub(super) fn make_async_expr( &mut self, capture_clause: CaptureBy, @@ -597,7 +587,7 @@ impl LoweringContext<'_> { ); P(this.expr(await_span, expr_break, ThinVec::new())) }); - self.arm(hir_vec![ready_pat], break_x) + self.arm(ready_pat, break_x) }; // `::std::task::Poll::Pending => {}` @@ -608,7 +598,7 @@ impl LoweringContext<'_> { hir_vec![], ); let empty_block = P(self.expr_block_empty(span)); - self.arm(hir_vec![pending_pat], empty_block) + self.arm(pending_pat, empty_block) }; let inner_match_stmt = { @@ -650,7 +640,7 @@ impl LoweringContext<'_> { }); // mut pinned => loop { ... } - let pinned_arm = self.arm(hir_vec![pinned_pat], loop_expr); + let pinned_arm = self.arm(pinned_pat, loop_expr); // match { // mut pinned => loop { .. } @@ -1084,7 +1074,7 @@ impl LoweringContext<'_> { ThinVec::new(), )); let some_pat = self.pat_some(pat.span, val_pat); - self.arm(hir_vec![some_pat], assign) + self.arm(some_pat, assign) }; // `::std::option::Option::None => break` @@ -1092,7 +1082,7 @@ impl LoweringContext<'_> { let break_expr = self.with_loop_scope(e.id, |this| this.expr_break(e.span, ThinVec::new())); let pat = self.pat_none(e.span); - self.arm(hir_vec![pat], break_expr) + self.arm(pat, break_expr) }; // `mut iter` @@ -1163,7 +1153,7 @@ impl LoweringContext<'_> { }); // `mut iter => { ... }` - let iter_arm = self.arm(hir_vec![iter_pat], loop_expr); + let iter_arm = self.arm(iter_pat, loop_expr); // `match ::std::iter::IntoIterator::into_iter() { ... }` let into_iter_expr = { @@ -1249,7 +1239,7 @@ impl LoweringContext<'_> { ThinVec::from(attrs.clone()), )); let ok_pat = self.pat_ok(span, val_pat); - self.arm(hir_vec![ok_pat], val_expr) + self.arm(ok_pat, val_expr) }; // `Err(err) => #[allow(unreachable_code)] @@ -1284,7 +1274,7 @@ impl LoweringContext<'_> { }; let err_pat = self.pat_err(try_span, err_local); - self.arm(hir_vec![err_pat], ret_expr) + self.arm(err_pat, ret_expr) }; hir::ExprKind::Match( @@ -1479,14 +1469,11 @@ impl LoweringContext<'_> { } } - /// HACK(or_patterns; Centril | dlrobertson): For now we don't push down top level or-patterns - /// `p | q` into `hir::PatKind::Or(...)` as post-lowering bits of the compiler are not ready - /// to deal with it. This should by fixed by pushing it down to HIR and then HAIR. - fn arm(&mut self, pats: HirVec>, expr: P) -> hir::Arm { + fn arm(&mut self, pat: P, expr: P) -> hir::Arm { hir::Arm { hir_id: self.next_id(), attrs: hir_vec![], - pats, + pat, guard: None, span: expr.span, body: expr, diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index d7c537be89668..4fc29d70540fd 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -834,8 +834,6 @@ pub fn walk_param<'a, V: Visitor<'a>>(visitor: &mut V, param: &'a Param) { pub fn walk_arm<'a, V: Visitor<'a>>(visitor: &mut V, arm: &'a Arm) { visitor.visit_pat(&arm.pat); - // NOTE(or_patterns; Centril | dlrobertson): - // If you change this, also change the hack in `lowering.rs`. walk_list!(visitor, visit_expr, &arm.guard); visitor.visit_expr(&arm.body); walk_list!(visitor, visit_attribute, &arm.attrs); From b2f903c066185d32bdfc02ff6a35f4e50c54728d Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 7 Sep 2019 16:00:09 +0200 Subject: [PATCH 05/50] or-patterns: `hir::Arm::pats` -> `::pat` + `Arm::top_pats_hack`. --- src/librustc/hir/mod.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 2c8590aa4e3fa..1b32979eda5f4 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1259,21 +1259,32 @@ pub struct Local { } /// Represents a single arm of a `match` expression, e.g. -/// ` (if ) => `. +/// ` (if ) => `. #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)] pub struct Arm { #[stable_hasher(ignore)] pub hir_id: HirId, pub span: Span, pub attrs: HirVec, - /// Multiple patterns can be combined with `|` - pub pats: HirVec>, + /// If this pattern and the optional guard matches, then `body` is evaluated. + pub pat: P, /// Optional guard clause. pub guard: Option, /// The expression the arm evaluates to if this arm matches. pub body: P, } +impl Arm { + // HACK(or_patterns; Centril | dlrobertson): Remove this and + // correctly handle each case in which this method is used. + pub fn top_pats_hack(&self) -> &[P] { + match &self.pat.node { + PatKind::Or(pats) => pats, + _ => std::slice::from_ref(&self.pat), + } + } +} + #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum Guard { If(P), From 6579d136b13d6b8a3ad14afe688a8c368a538b57 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 7 Sep 2019 16:00:39 +0200 Subject: [PATCH 06/50] or-patterns: normalize HIR pretty priting. --- src/librustc/hir/print.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index cfbfb5eceb550..3cbc9ebec0ad8 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -1790,16 +1790,7 @@ impl<'a> State<'a> { self.ann.pre(self, AnnNode::Arm(arm)); self.ibox(0); self.print_outer_attributes(&arm.attrs); - let mut first = true; - for p in &arm.pats { - if first { - first = false; - } else { - self.s.space(); - self.word_space("|"); - } - self.print_pat(&p); - } + self.print_pat(&arm.pat); self.s.space(); if let Some(ref g) = arm.guard { match g { From 75fb42a11fa91a323e2109a0d42230d96c73e3bb Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 7 Sep 2019 16:01:12 +0200 Subject: [PATCH 07/50] or-patterns: use `top_pats_hack` to make things compile. --- src/librustc/hir/intravisit.rs | 2 +- src/librustc/hir/pat_util.rs | 2 +- src/librustc/middle/dead.rs | 5 +++-- src/librustc/middle/expr_use_visitor.rs | 4 ++-- src/librustc/middle/liveness.rs | 6 +++--- src/librustc_ast_borrowck/cfg/construct.rs | 2 +- src/librustc_mir/hair/cx/expr.rs | 2 +- src/librustc_mir/hair/pattern/check_match.rs | 6 +++--- src/librustc_passes/rvalue_promotion.rs | 2 +- src/librustc_typeck/check/_match.rs | 8 +++----- src/librustc_typeck/check/regionck.rs | 4 ++-- 11 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 1f125de967216..91fc004b893ba 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -1103,7 +1103,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) { pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) { visitor.visit_id(arm.hir_id); - walk_list!(visitor, visit_pat, &arm.pats); + visitor.visit_pat(&arm.pat); if let Some(ref g) = arm.guard { match g { Guard::If(ref e) => visitor.visit_expr(e), diff --git a/src/librustc/hir/pat_util.rs b/src/librustc/hir/pat_util.rs index 0d2c7d393bb89..3d82f37e1a9c3 100644 --- a/src/librustc/hir/pat_util.rs +++ b/src/librustc/hir/pat_util.rs @@ -170,7 +170,7 @@ impl hir::Arm { // for #42640 (default match binding modes). // // See #44848. - self.pats.iter() + self.top_pats_hack().iter() .filter_map(|pat| pat.contains_explicit_ref_binding()) .max_by_key(|m| match *m { hir::MutMutable => 1, diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index d4805a7c78322..666c1cc96f2f7 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -259,8 +259,9 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> { } fn visit_arm(&mut self, arm: &'tcx hir::Arm) { - if arm.pats.len() == 1 { - let variants = arm.pats[0].necessary_variants(); + let pats = arm.top_pats_hack(); + if pats.len() == 1 { + let variants = pats[0].necessary_variants(); // Inside the body, ignore constructions of variants // necessary for the pattern to match. Those construction sites diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index de6dadabcbf56..ef84c9bbad60d 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -779,14 +779,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { fn arm_move_mode(&mut self, discr_cmt: mc::cmt<'tcx>, arm: &hir::Arm) -> TrackMatchMode { let mut mode = Unknown; - for pat in &arm.pats { + for pat in arm.top_pats_hack() { self.determine_pat_move_mode(discr_cmt.clone(), &pat, &mut mode); } mode } fn walk_arm(&mut self, discr_cmt: mc::cmt<'tcx>, arm: &hir::Arm, mode: MatchMode) { - for pat in &arm.pats { + for pat in arm.top_pats_hack() { self.walk_pat(discr_cmt.clone(), &pat, mode); } diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 00013bfc574f4..5e91c81dd4ded 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -456,7 +456,7 @@ fn visit_local<'tcx>(ir: &mut IrMaps<'tcx>, local: &'tcx hir::Local) { } fn visit_arm<'tcx>(ir: &mut IrMaps<'tcx>, arm: &'tcx hir::Arm) { - for pat in &arm.pats { + for pat in arm.top_pats_hack() { add_from_pat(ir, pat); } intravisit::walk_arm(ir, arm); @@ -1080,7 +1080,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { // the same bindings, and we also consider the first pattern to be // the "authoritative" set of ids let arm_succ = - self.define_bindings_in_arm_pats(arm.pats.first().map(|p| &**p), + self.define_bindings_in_arm_pats(arm.top_pats_hack().first().map(|p| &**p), guard_succ); self.merge_from_succ(ln, arm_succ, first_merge); first_merge = false; @@ -1422,7 +1422,7 @@ fn check_arm<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, arm: &'tcx hir::Arm) { // patterns so the suggestions to prefix with underscores will apply to those too. let mut vars: BTreeMap)> = Default::default(); - for pat in &arm.pats { + for pat in arm.top_pats_hack() { this.arm_pats_bindings(Some(&*pat), |this, ln, var, sp, id| { let name = this.ir.variable_name(var); vars.entry(name) diff --git a/src/librustc_ast_borrowck/cfg/construct.rs b/src/librustc_ast_borrowck/cfg/construct.rs index 0dc999083a91a..e2c5de648a276 100644 --- a/src/librustc_ast_borrowck/cfg/construct.rs +++ b/src/librustc_ast_borrowck/cfg/construct.rs @@ -390,7 +390,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { // patterns and the guard (if there is one) in the arm. let bindings_exit = self.add_dummy_node(&[]); - for pat in &arm.pats { + for pat in arm.top_pats_hack() { // Visit the pattern, coming from the discriminant exit let mut pat_exit = self.pat(&pat, discr_exit); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index a33d7207ed4e1..f3d699fa4f008 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -862,7 +862,7 @@ impl ToBorrowKind for hir::Mutability { fn convert_arm<'a, 'tcx>(cx: &mut Cx<'a, 'tcx>, arm: &'tcx hir::Arm) -> Arm<'tcx> { Arm { - patterns: arm.pats.iter().map(|p| cx.pattern_from_hir(p)).collect(), + patterns: arm.top_pats_hack().iter().map(|p| cx.pattern_from_hir(p)).collect(), guard: match arm.guard { Some(hir::Guard::If(ref e)) => Some(Guard::If(e.to_ref())), _ => None, diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 161c58a175579..c3542d4ab6c5f 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -137,7 +137,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { ) { for arm in arms { // First, check legality of move bindings. - self.check_patterns(arm.guard.is_some(), &arm.pats); + self.check_patterns(arm.guard.is_some(), &arm.top_pats_hack()); // Second, if there is a guard on each arm, make sure it isn't // assigning or borrowing anything mutably. @@ -146,7 +146,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { } // Third, perform some lints. - for pat in &arm.pats { + for pat in arm.top_pats_hack() { check_for_bindings_named_same_as_variants(self, pat); } } @@ -156,7 +156,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { let mut have_errors = false; let inlined_arms : Vec<(Vec<_>, _)> = arms.iter().map(|arm| ( - arm.pats.iter().map(|pat| { + arm.top_pats_hack().iter().map(|pat| { let mut patcx = PatternContext::new(self.tcx, self.param_env.and(self.identity_substs), self.tables); diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs index f2461f7016131..7470f5b7a5f0f 100644 --- a/src/librustc_passes/rvalue_promotion.rs +++ b/src/librustc_passes/rvalue_promotion.rs @@ -503,7 +503,7 @@ fn check_expr_kind<'a, 'tcx>( // Compute the most demanding borrow from all the arms' // patterns and set that on the discriminator. let mut mut_borrow = false; - for pat in hirvec_arm.iter().flat_map(|arm| &arm.pats) { + for pat in hirvec_arm.iter().flat_map(|arm| arm.top_pats_hack()) { mut_borrow = v.remove_mut_rvalue_borrow(pat); } if mut_borrow { diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 308a3d8ebc2cf..7b971803db599 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -58,11 +58,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // collection into `Vec`), so we get types for all bindings. let all_arm_pats_diverge: Vec<_> = arms.iter().map(|arm| { let mut all_pats_diverge = Diverges::WarnedAlways; - for p in &arm.pats { - self.diverges.set(Diverges::Maybe); - self.check_pat_top(&p, discrim_ty, Some(discrim.span)); - all_pats_diverge &= self.diverges.get(); - } + self.diverges.set(Diverges::Maybe); + self.check_pat_top(&arm.pat, discrim_ty, Some(discrim.span)); + all_pats_diverge &= self.diverges.get(); // As discussed with @eddyb, this is for disabling unreachable_code // warnings on patterns (they're now subsumed by unreachable_patterns diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index fc01a820e2359..86ec477a6aae8 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -488,7 +488,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RegionCtxt<'a, 'tcx> { fn visit_arm(&mut self, arm: &'tcx hir::Arm) { // see above - for p in &arm.pats { + for p in arm.top_pats_hack() { self.constrain_bindings_in_pat(p); } intravisit::walk_arm(self, arm); @@ -1069,7 +1069,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> { let discr_cmt = Rc::new(ignore_err!(self.with_mc(|mc| mc.cat_expr(discr)))); debug!("discr_cmt={:?}", discr_cmt); for arm in arms { - for root_pat in &arm.pats { + for root_pat in arm.top_pats_hack() { self.link_pattern(discr_cmt.clone(), &root_pat); } } From 89bbef302646c4b33148e2a34b69de7643a55ffa Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 02:48:53 +0200 Subject: [PATCH 08/50] check_match: misc cleanup. --- src/librustc_mir/hair/pattern/check_match.rs | 43 ++++++++------------ src/librustc_mir/lib.rs | 1 + 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index c3542d4ab6c5f..dc884d40ac157 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -563,40 +563,30 @@ fn check_legality_of_move_bindings( } }) } + let span_vec = &mut Vec::new(); - let check_move = | - cx: &mut MatchVisitor<'_, '_>, - p: &Pat, - sub: Option<&Pat>, - span_vec: &mut Vec, - | { - // check legality of moving out of the enum - - // x @ Foo(..) is legal, but x @ Foo(y) isn't. + let mut check_move = |p: &Pat, sub: Option<&Pat>| { + // Check legality of moving out of the enum. + // + // `x @ Foo(..)` is legal, but `x @ Foo(y)` isn't. if sub.map_or(false, |p| p.contains_bindings()) { - struct_span_err!(cx.tcx.sess, p.span, E0007, - "cannot bind by-move with sub-bindings") + struct_span_err!(cx.tcx.sess, p.span, E0007, "cannot bind by-move with sub-bindings") .span_label(p.span, "binds an already bound by-move value by moving it") .emit(); - } else if !has_guard { - if let Some(_by_ref_span) = by_ref_span { - span_vec.push(p.span); - } + } else if !has_guard && by_ref_span.is_some() { + span_vec.push(p.span); } }; for pat in pats { pat.walk(|p| { - if let PatKind::Binding(_, _, _, ref sub) = p.node { + if let PatKind::Binding(.., sub) = &p.node { if let Some(&bm) = cx.tables.pat_binding_modes().get(p.hir_id) { - match bm { - ty::BindByValue(..) => { - let pat_ty = cx.tables.node_type(p.hir_id); - if !pat_ty.is_copy_modulo_regions(cx.tcx, cx.param_env, pat.span) { - check_move(cx, p, sub.as_ref().map(|p| &**p), span_vec); - } + if let ty::BindByValue(..) = bm { + let pat_ty = cx.tables.node_type(p.hir_id); + if !pat_ty.is_copy_modulo_regions(cx.tcx, cx.param_env, pat.span) { + check_move(p, sub.as_deref()); } - _ => {} } } else { cx.tcx.sess.delay_span_bug(pat.span, "missing binding mode"); @@ -605,11 +595,10 @@ fn check_legality_of_move_bindings( true }); } - if !span_vec.is_empty(){ - let span = MultiSpan::from_spans(span_vec.clone()); + if !span_vec.is_empty() { let mut err = struct_span_err!( cx.tcx.sess, - span, + MultiSpan::from_spans(span_vec.clone()), E0009, "cannot bind by-move and by-ref in the same pattern", ); @@ -627,7 +616,7 @@ fn check_legality_of_move_bindings( /// because of the way rvalues are handled in the borrow check. (See issue /// #14587.) fn check_legality_of_bindings_in_at_patterns(cx: &MatchVisitor<'_, '_>, pat: &Pat) { - AtBindingPatternVisitor { cx: cx, bindings_allowed: true }.visit_pat(pat); + AtBindingPatternVisitor { cx, bindings_allowed: true }.visit_pat(pat); } struct AtBindingPatternVisitor<'a, 'b, 'tcx> { diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index f27db351b74db..ac756747aa326 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -6,6 +6,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![feature(nll)] #![feature(in_band_lifetimes)] +#![feature(inner_deref)] #![feature(slice_patterns)] #![feature(box_patterns)] #![feature(box_syntax)] From 4c346939b088cf07ece818b7dc093f993eae2578 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 02:49:37 +0200 Subject: [PATCH 09/50] or-patterns: fix problems in typeck. --- src/librustc_typeck/check/pat.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/librustc_typeck/check/pat.rs b/src/librustc_typeck/check/pat.rs index 8502b89de1469..c8869132fb654 100644 --- a/src/librustc_typeck/check/pat.rs +++ b/src/librustc_typeck/check/pat.rs @@ -97,11 +97,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.check_pat_struct(pat, qpath, fields, *etc, expected, def_bm, discrim_span) } PatKind::Or(pats) => { - let expected_ty = self.structurally_resolved_type(pat.span, expected); for pat in pats { self.check_pat(pat, expected, def_bm, discrim_span); } - expected_ty + expected } PatKind::Tuple(elements, ddpos) => { self.check_pat_tuple(pat.span, elements, *ddpos, expected, def_bm, discrim_span) @@ -208,7 +207,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match pat.node { PatKind::Struct(..) | PatKind::TupleStruct(..) | - PatKind::Or(_) | PatKind::Tuple(..) | PatKind::Box(_) | PatKind::Range(..) | @@ -226,6 +224,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => true, } } + // FIXME(or_patterns; Centril | dlrobertson): To keep things compiling + // for or-patterns at the top level, we need to make `p_0 | ... | p_n` + // a "non reference pattern". For example the following currently compiles: + // ``` + // match &1 { + // e @ &(1...2) | e @ &(3...4) => {} + // _ => {} + // } + // ``` + // + // We should consider whether we should do something special in nested or-patterns. + PatKind::Or(_) | PatKind::Wild | PatKind::Binding(..) | PatKind::Ref(..) => false, @@ -426,12 +436,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If the binding is like `ref x | ref const x | ref mut x` // then `x` is assigned a value of type `&M T` where M is the // mutability and T is the expected type. - let region_ty = self.new_ref_ty(pat.span, mutbl, expected); - + // // `x` is assigned a value of type `&M T`, hence `&M T <: typeof(x)` // is required. However, we use equality, which is stronger. // See (note_1) for an explanation. - region_ty + self.new_ref_ty(pat.span, mutbl, expected) } // Otherwise, the type of x is the expected type `T`. ty::BindByValue(_) => { From 9d1c3c96e7866d6f82b92fffa00fac59f418b601 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 03:26:51 +0200 Subject: [PATCH 10/50] simplify `hir::Pat::walk_`. --- src/librustc/hir/mod.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 1b32979eda5f4..edb55ab3caaf4 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -889,19 +889,14 @@ impl Pat { return false; } - match self.node { - PatKind::Binding(.., Some(ref p)) => p.walk_(it), - PatKind::Struct(_, ref fields, _) => { - fields.iter().all(|field| field.pat.walk_(it)) - } - PatKind::TupleStruct(_, ref s, _) | PatKind::Tuple(ref s, _) => { + match &self.node { + PatKind::Binding(.., Some(p)) => p.walk_(it), + PatKind::Struct(_, fields, _) => fields.iter().all(|field| field.pat.walk_(it)), + PatKind::TupleStruct(_, s, _) | PatKind::Tuple(s, _) | PatKind::Or(s) => { s.iter().all(|p| p.walk_(it)) } - PatKind::Or(ref pats) => pats.iter().all(|p| p.walk_(it)), - PatKind::Box(ref s) | PatKind::Ref(ref s, _) => { - s.walk_(it) - } - PatKind::Slice(ref before, ref slice, ref after) => { + PatKind::Box(s) | PatKind::Ref(s, _) => s.walk_(it), + PatKind::Slice(before, slice, after) => { before.iter() .chain(slice.iter()) .chain(after.iter()) From cc5fe6d520683e847d1d5fd2a4544e2a5dba9ab4 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 03:55:34 +0200 Subject: [PATCH 11/50] or-patterns: liveness/`visit_arm`: remove `top_pats_hack`. --- src/librustc/middle/liveness.rs | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 5e91c81dd4ded..3c264f9ac6e6c 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -404,7 +404,7 @@ fn visit_fn<'tcx>( lsets.warn_about_unused_args(body, entry_ln); } -fn add_from_pat<'tcx>(ir: &mut IrMaps<'tcx>, pat: &P) { +fn add_from_pat(ir: &mut IrMaps<'_>, pat: &P) { // For struct patterns, take note of which fields used shorthand // (`x` rather than `x: x`). let mut shorthand_field_ids = HirIdSet::default(); @@ -412,26 +412,21 @@ fn add_from_pat<'tcx>(ir: &mut IrMaps<'tcx>, pat: &P) { pats.push_back(pat); while let Some(pat) = pats.pop_front() { use crate::hir::PatKind::*; - match pat.node { - Binding(_, _, _, ref inner_pat) => { + match &pat.node { + Binding(.., inner_pat) => { pats.extend(inner_pat.iter()); } - Struct(_, ref fields, _) => { - for field in fields { - if field.is_shorthand { - shorthand_field_ids.insert(field.pat.hir_id); - } - } + Struct(_, fields, _) => { + let ids = fields.iter().filter(|f| f.is_shorthand).map(|f| f.pat.hir_id); + shorthand_field_ids.extend(ids); } - Ref(ref inner_pat, _) | - Box(ref inner_pat) => { + Ref(inner_pat, _) | Box(inner_pat) => { pats.push_back(inner_pat); } - TupleStruct(_, ref inner_pats, _) | - Tuple(ref inner_pats, _) => { + TupleStruct(_, inner_pats, _) | Tuple(inner_pats, _) | Or(inner_pats) => { pats.extend(inner_pats.iter()); } - Slice(ref pre_pats, ref inner_pat, ref post_pats) => { + Slice(pre_pats, inner_pat, post_pats) => { pats.extend(pre_pats.iter()); pats.extend(inner_pat.iter()); pats.extend(post_pats.iter()); @@ -440,7 +435,7 @@ fn add_from_pat<'tcx>(ir: &mut IrMaps<'tcx>, pat: &P) { } } - pat.each_binding(|_bm, hir_id, _sp, ident| { + pat.each_binding(|_, hir_id, _, ident| { ir.add_live_node_for_node(hir_id, VarDefNode(ident.span)); ir.add_variable(Local(LocalInfo { id: hir_id, @@ -456,9 +451,7 @@ fn visit_local<'tcx>(ir: &mut IrMaps<'tcx>, local: &'tcx hir::Local) { } fn visit_arm<'tcx>(ir: &mut IrMaps<'tcx>, arm: &'tcx hir::Arm) { - for pat in arm.top_pats_hack() { - add_from_pat(ir, pat); - } + add_from_pat(ir, &arm.pat); intravisit::walk_arm(ir, arm); } From 9ca42a56000d261be74db41339beff06bc6088c9 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 03:41:21 +0200 Subject: [PATCH 12/50] or-patterns: liveness: generalize + remove `top_pats_hack`. --- src/librustc/hir/pat_util.rs | 28 ++++ src/librustc/middle/liveness.rs | 288 +++++++++++++------------------- 2 files changed, 141 insertions(+), 175 deletions(-) diff --git a/src/librustc/hir/pat_util.rs b/src/librustc/hir/pat_util.rs index 3d82f37e1a9c3..b68886ba62c2d 100644 --- a/src/librustc/hir/pat_util.rs +++ b/src/librustc/hir/pat_util.rs @@ -77,6 +77,34 @@ impl hir::Pat { }); } + /// Call `f` on every "binding" in a pattern, e.g., on `a` in + /// `match foo() { Some(a) => (), None => () }`. + /// + /// When encountering an or-pattern `p_0 | ... | p_n` only `p_0` will be visited. + pub fn each_binding_or_first(&self, c: &mut F) + where F: FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident), + { + match &self.node { + PatKind::Binding(bm, _, ident, sub) => { + c(*bm, self.hir_id, self.span, *ident); + sub.iter().for_each(|p| p.each_binding_or_first(c)); + } + PatKind::Or(ps) => ps[0].each_binding_or_first(c), + PatKind::Struct(_, fs, _) => fs.iter().for_each(|f| f.pat.each_binding_or_first(c)), + PatKind::TupleStruct(_, ps, _) | PatKind::Tuple(ps, _) => { + ps.iter().for_each(|p| p.each_binding_or_first(c)); + } + PatKind::Box(p) | PatKind::Ref(p, _) => p.each_binding_or_first(c), + PatKind::Slice(before, slice, after) => { + before.iter() + .chain(slice.iter()) + .chain(after.iter()) + .for_each(|p| p.each_binding_or_first(c)); + } + PatKind::Wild | PatKind::Lit(_) | PatKind::Range(..) | PatKind::Path(_) => {} + } + } + /// Checks if the pattern contains any patterns that bind something to /// an ident, e.g., `foo`, or `Foo(foo)` or `foo @ Bar(..)`. pub fn contains_bindings(&self) -> bool { diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 3c264f9ac6e6c..9afd147ae34e2 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -96,7 +96,11 @@ use self::LiveNodeKind::*; use self::VarKind::*; +use crate::hir; +use crate::hir::{Expr, HirId}; use crate::hir::def::*; +use crate::hir::def_id::DefId; +use crate::hir::intravisit::{self, Visitor, FnKind, NestedVisitorMap}; use crate::hir::Node; use crate::hir::ptr::P; use crate::ty::{self, TyCtxt}; @@ -105,20 +109,16 @@ use crate::lint; use crate::util::nodemap::{HirIdMap, HirIdSet}; use errors::Applicability; -use std::collections::{BTreeMap, VecDeque}; +use rustc_data_structures::fx::FxIndexMap; +use std::collections::VecDeque; use std::{fmt, u32}; use std::io::prelude::*; use std::io; use std::rc::Rc; use syntax::ast; -use syntax::symbol::{kw, sym}; +use syntax::symbol::sym; use syntax_pos::Span; -use crate::hir; -use crate::hir::{Expr, HirId}; -use crate::hir::def_id::DefId; -use crate::hir::intravisit::{self, Visitor, FnKind, NestedVisitorMap}; - #[derive(Copy, Clone, PartialEq)] struct Variable(u32); @@ -727,35 +727,15 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { self.ir.variable(hir_id, span) } - fn pat_bindings(&mut self, pat: &hir::Pat, mut f: F) where - F: FnMut(&mut Liveness<'a, 'tcx>, LiveNode, Variable, Span, HirId), - { - pat.each_binding(|_bm, hir_id, sp, n| { - let ln = self.live_node(hir_id, sp); - let var = self.variable(hir_id, n.span); - f(self, ln, var, n.span, hir_id); - }) - } - - fn arm_pats_bindings(&mut self, pat: Option<&hir::Pat>, f: F) where - F: FnMut(&mut Liveness<'a, 'tcx>, LiveNode, Variable, Span, HirId), - { - if let Some(pat) = pat { - self.pat_bindings(pat, f); - } - } - - fn define_bindings_in_pat(&mut self, pat: &hir::Pat, succ: LiveNode) - -> LiveNode { - self.define_bindings_in_arm_pats(Some(pat), succ) - } - - fn define_bindings_in_arm_pats(&mut self, pat: Option<&hir::Pat>, succ: LiveNode) - -> LiveNode { - let mut succ = succ; - self.arm_pats_bindings(pat, |this, ln, var, _sp, _id| { - this.init_from_succ(ln, succ); - this.define(ln, var); + fn define_bindings_in_pat(&mut self, pat: &hir::Pat, mut succ: LiveNode) -> LiveNode { + // In an or-pattern, only consider the first pattern; any later patterns + // must have the same bindings, and we also consider the first pattern + // to be the "authoritative" set of ids. + pat.each_binding_or_first(&mut |_, hir_id, pat_sp, ident| { + let ln = self.live_node(hir_id, pat_sp); + let var = self.variable(hir_id, ident.span); + self.init_from_succ(ln, succ); + self.define(ln, var); succ = ln; }); succ @@ -1069,12 +1049,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { arm.guard.as_ref().map(|hir::Guard::If(e)| &**e), body_succ ); - // only consider the first pattern; any later patterns must have - // the same bindings, and we also consider the first pattern to be - // the "authoritative" set of ids - let arm_succ = - self.define_bindings_in_arm_pats(arm.top_pats_hack().first().map(|p| &**p), - guard_succ); + let arm_succ = self.define_bindings_in_pat(&arm.pat, guard_succ); self.merge_from_succ(ln, arm_succ, first_merge); first_merge = false; }; @@ -1381,74 +1356,36 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> { NestedVisitorMap::None } - fn visit_local(&mut self, l: &'tcx hir::Local) { - check_local(self, l); - } - fn visit_expr(&mut self, ex: &'tcx Expr) { - check_expr(self, ex); - } - fn visit_arm(&mut self, a: &'tcx hir::Arm) { - check_arm(self, a); - } -} + fn visit_local(&mut self, local: &'tcx hir::Local) { + self.check_unused_vars_in_pat(&local.pat, None, |spans, hir_id, ln, var| { + if local.init.is_some() { + self.warn_about_dead_assign(spans, hir_id, ln, var); + } + }); -fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local) { - match local.init { - Some(_) => { - this.warn_about_unused_or_dead_vars_in_pat(&local.pat); - }, - None => { - this.pat_bindings(&local.pat, |this, ln, var, sp, id| { - let span = local.pat.simple_ident().map_or(sp, |ident| ident.span); - this.warn_about_unused(vec![span], id, ln, var); - }) - } + intravisit::walk_local(self, local); } - intravisit::walk_local(this, local); -} - -fn check_arm<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, arm: &'tcx hir::Arm) { - // Only consider the variable from the first pattern; any later patterns must have - // the same bindings, and we also consider the first pattern to be the "authoritative" set of - // ids. However, we should take the spans of variables with the same name from the later - // patterns so the suggestions to prefix with underscores will apply to those too. - let mut vars: BTreeMap)> = Default::default(); - - for pat in arm.top_pats_hack() { - this.arm_pats_bindings(Some(&*pat), |this, ln, var, sp, id| { - let name = this.ir.variable_name(var); - vars.entry(name) - .and_modify(|(.., spans)| { - spans.push(sp); - }) - .or_insert_with(|| { - (ln, var, id, vec![sp]) - }); - }); + fn visit_expr(&mut self, ex: &'tcx Expr) { + check_expr(self, ex); } - for (_, (ln, var, id, spans)) in vars { - this.warn_about_unused(spans, id, ln, var); + fn visit_arm(&mut self, arm: &'tcx hir::Arm) { + self.check_unused_vars_in_pat(&arm.pat, None, |_, _, _, _| {}); + intravisit::walk_arm(self, arm); } - - intravisit::walk_arm(this, arm); } -fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) { +fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr) { match expr.node { hir::ExprKind::Assign(ref l, _) => { this.check_place(&l); - - intravisit::walk_expr(this, expr); } hir::ExprKind::AssignOp(_, ref l, _) => { if !this.tables.is_method_call(expr) { this.check_place(&l); } - - intravisit::walk_expr(this, expr); } hir::ExprKind::InlineAsm(ref ia, ref outputs, ref inputs) => { @@ -1463,8 +1400,6 @@ fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) { } this.visit_expr(output); } - - intravisit::walk_expr(this, expr); } // no correctness conditions related to liveness @@ -1477,13 +1412,13 @@ fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) { hir::ExprKind::Lit(_) | hir::ExprKind::Block(..) | hir::ExprKind::AddrOf(..) | hir::ExprKind::Struct(..) | hir::ExprKind::Repeat(..) | hir::ExprKind::Closure(..) | hir::ExprKind::Path(_) | hir::ExprKind::Yield(..) | - hir::ExprKind::Box(..) | hir::ExprKind::Type(..) | hir::ExprKind::Err => { - intravisit::walk_expr(this, expr); - } + hir::ExprKind::Box(..) | hir::ExprKind::Type(..) | hir::ExprKind::Err => {} } + + intravisit::walk_expr(this, expr); } -impl<'a, 'tcx> Liveness<'a, 'tcx> { +impl<'tcx> Liveness<'_, 'tcx> { fn check_place(&mut self, expr: &'tcx Expr) { match expr.node { hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) => { @@ -1496,7 +1431,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { // as being used. let ln = self.live_node(expr.hir_id, expr.span); let var = self.variable(var_hid, expr.span); - self.warn_about_dead_assign(expr.span, expr.hir_id, ln, var); + self.warn_about_dead_assign(vec![expr.span], expr.hir_id, ln, var); } } } @@ -1518,109 +1453,112 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { } fn warn_about_unused_args(&self, body: &hir::Body, entry_ln: LiveNode) { - for param in &body.params { - param.pat.each_binding(|_bm, hir_id, _, ident| { - let sp = ident.span; - let var = self.variable(hir_id, sp); - // Ignore unused self. - if ident.name != kw::SelfLower { - if !self.warn_about_unused(vec![sp], hir_id, entry_ln, var) { - if self.live_on_entry(entry_ln, var).is_none() { - self.report_dead_assign(hir_id, sp, var, true); - } - } + for p in &body.params { + self.check_unused_vars_in_pat(&p.pat, Some(entry_ln), |spans, hir_id, ln, var| { + if self.live_on_entry(ln, var).is_none() { + self.report_dead_assign(hir_id, spans, var, true); } - }) + }); } } - fn warn_about_unused_or_dead_vars_in_pat(&mut self, pat: &hir::Pat) { - self.pat_bindings(pat, |this, ln, var, sp, id| { - if !this.warn_about_unused(vec![sp], id, ln, var) { - this.warn_about_dead_assign(sp, id, ln, var); + fn check_unused_vars_in_pat( + &self, + pat: &hir::Pat, + entry_ln: Option, + on_used_on_entry: impl Fn(Vec, HirId, LiveNode, Variable), + ) { + // In an or-pattern, only consider the variable; any later patterns must have the same + // bindings, and we also consider the first pattern to be the "authoritative" set of ids. + // However, we should take the spans of variables with the same name from the later + // patterns so the suggestions to prefix with underscores will apply to those too. + let mut vars: FxIndexMap)> = <_>::default(); + + pat.each_binding(|_, hir_id, pat_sp, ident| { + let ln = entry_ln.unwrap_or_else(|| self.live_node(hir_id, pat_sp)); + let var = self.variable(hir_id, ident.span); + vars.entry(self.ir.variable_name(var)) + .and_modify(|(.., spans)| spans.push(ident.span)) + .or_insert_with(|| (ln, var, hir_id, vec![ident.span])); + }); + + for (_, (ln, var, id, spans)) in vars { + if self.used_on_entry(ln, var) { + on_used_on_entry(spans, id, ln, var); + } else { + self.report_unused(spans, id, ln, var); } - }) + } } - fn warn_about_unused(&self, - spans: Vec, - hir_id: HirId, - ln: LiveNode, - var: Variable) - -> bool { - if !self.used_on_entry(ln, var) { - let r = self.should_warn(var); - if let Some(name) = r { - // annoying: for parameters in funcs like `fn(x: i32) - // {ret}`, there is only one node, so asking about - // assigned_on_exit() is not meaningful. - let is_assigned = if ln == self.s.exit_ln { - false - } else { - self.assigned_on_exit(ln, var).is_some() - }; + fn report_unused(&self, spans: Vec, hir_id: HirId, ln: LiveNode, var: Variable) { + if let Some(name) = self.should_warn(var).filter(|name| name != "self") { + // annoying: for parameters in funcs like `fn(x: i32) + // {ret}`, there is only one node, so asking about + // assigned_on_exit() is not meaningful. + let is_assigned = if ln == self.s.exit_ln { + false + } else { + self.assigned_on_exit(ln, var).is_some() + }; - if is_assigned { - self.ir.tcx.lint_hir_note( - lint::builtin::UNUSED_VARIABLES, - hir_id, - spans, - &format!("variable `{}` is assigned to, but never used", name), - &format!("consider using `_{}` instead", name), - ); - } else if name != "self" { - let mut err = self.ir.tcx.struct_span_lint_hir( - lint::builtin::UNUSED_VARIABLES, - hir_id, - spans.clone(), - &format!("unused variable: `{}`", name), - ); + if is_assigned { + self.ir.tcx.lint_hir_note( + lint::builtin::UNUSED_VARIABLES, + hir_id, + spans, + &format!("variable `{}` is assigned to, but never used", name), + &format!("consider using `_{}` instead", name), + ); + } else { + let mut err = self.ir.tcx.struct_span_lint_hir( + lint::builtin::UNUSED_VARIABLES, + hir_id, + spans.clone(), + &format!("unused variable: `{}`", name), + ); + + if self.ir.variable_is_shorthand(var) { + if let Node::Binding(pat) = self.ir.tcx.hir().get(hir_id) { + // Handle `ref` and `ref mut`. + let spans = spans.iter() + .map(|_span| (pat.span, format!("{}: _", name))) + .collect(); - if self.ir.variable_is_shorthand(var) { - if let Node::Binding(pat) = self.ir.tcx.hir().get(hir_id) { - // Handle `ref` and `ref mut`. - let spans = spans.iter() - .map(|_span| (pat.span, format!("{}: _", name))) - .collect(); - - err.multipart_suggestion( - "try ignoring the field", - spans, - Applicability::MachineApplicable, - ); - } - } else { err.multipart_suggestion( - "consider prefixing with an underscore", - spans.iter().map(|span| (*span, format!("_{}", name))).collect(), + "try ignoring the field", + spans, Applicability::MachineApplicable, ); } - - err.emit() + } else { + err.multipart_suggestion( + "consider prefixing with an underscore", + spans.iter().map(|span| (*span, format!("_{}", name))).collect(), + Applicability::MachineApplicable, + ); } + + err.emit() } - true - } else { - false } } - fn warn_about_dead_assign(&self, sp: Span, hir_id: HirId, ln: LiveNode, var: Variable) { + fn warn_about_dead_assign(&self, spans: Vec, hir_id: HirId, ln: LiveNode, var: Variable) { if self.live_on_exit(ln, var).is_none() { - self.report_dead_assign(hir_id, sp, var, false); + self.report_dead_assign(hir_id, spans, var, false); } } - fn report_dead_assign(&self, hir_id: HirId, sp: Span, var: Variable, is_argument: bool) { + fn report_dead_assign(&self, hir_id: HirId, spans: Vec, var: Variable, is_argument: bool) { if let Some(name) = self.should_warn(var) { if is_argument { - self.ir.tcx.struct_span_lint_hir(lint::builtin::UNUSED_ASSIGNMENTS, hir_id, sp, + self.ir.tcx.struct_span_lint_hir(lint::builtin::UNUSED_ASSIGNMENTS, hir_id, spans, &format!("value passed to `{}` is never read", name)) .help("maybe it is overwritten before being read?") .emit(); } else { - self.ir.tcx.struct_span_lint_hir(lint::builtin::UNUSED_ASSIGNMENTS, hir_id, sp, + self.ir.tcx.struct_span_lint_hir(lint::builtin::UNUSED_ASSIGNMENTS, hir_id, spans, &format!("value assigned to `{}` is never read", name)) .help("maybe it is overwritten before being read?") .emit(); From 57eeb61cc861f32351a9d45eed09b6b8ca3c9c5b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 04:07:56 +0200 Subject: [PATCH 13/50] or-patterns: remove `Arm::contains_explicit_ref_binding`. --- src/librustc/hir/pat_util.rs | 17 ----------------- src/librustc_typeck/check/_match.rs | 10 +++++----- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/src/librustc/hir/pat_util.rs b/src/librustc/hir/pat_util.rs index b68886ba62c2d..ea35418bc1ba7 100644 --- a/src/librustc/hir/pat_util.rs +++ b/src/librustc/hir/pat_util.rs @@ -189,20 +189,3 @@ impl hir::Pat { result } } - -impl hir::Arm { - /// Checks if the patterns for this arm contain any `ref` or `ref mut` - /// bindings, and if yes whether its containing mutable ones or just immutables ones. - pub fn contains_explicit_ref_binding(&self) -> Option { - // FIXME(tschottdorf): contains_explicit_ref_binding() must be removed - // for #42640 (default match binding modes). - // - // See #44848. - self.top_pats_hack().iter() - .filter_map(|pat| pat.contains_explicit_ref_binding()) - .max_by_key(|m| match *m { - hir::MutMutable => 1, - hir::MutImmutable => 0, - }) - } -} diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 7b971803db599..e0b53525dc90c 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -411,11 +411,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // // See #44848. let contains_ref_bindings = arms.iter() - .filter_map(|a| a.contains_explicit_ref_binding()) - .max_by_key(|m| match *m { - hir::MutMutable => 1, - hir::MutImmutable => 0, - }); + .filter_map(|a| a.pat.contains_explicit_ref_binding()) + .max_by_key(|m| match *m { + hir::MutMutable => 1, + hir::MutImmutable => 0, + }); if let Some(m) = contains_ref_bindings { self.check_expr_with_needs(discrim, Needs::maybe_mut_place(m)) From fb2cfec433421ada440a5dc83686f739dc1ab521 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 04:17:55 +0200 Subject: [PATCH 14/50] or-patterns: euv/`arm_move_mode`: remove `top_pats_hack`. --- src/librustc/middle/expr_use_visitor.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index ef84c9bbad60d..0e97a3b59d082 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -779,9 +779,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { fn arm_move_mode(&mut self, discr_cmt: mc::cmt<'tcx>, arm: &hir::Arm) -> TrackMatchMode { let mut mode = Unknown; - for pat in arm.top_pats_hack() { - self.determine_pat_move_mode(discr_cmt.clone(), &pat, &mut mode); - } + self.determine_pat_move_mode(discr_cmt.clone(), &arm.pat, &mut mode); mode } From d7139f3e6d9dfa11d03f8529907d643bbfa8005a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 04:26:00 +0200 Subject: [PATCH 15/50] or-patterns: euv/`walk_arm`: remove `top_pats_hack`. --- src/librustc/middle/expr_use_visitor.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 0e97a3b59d082..a3110c000fb22 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -784,9 +784,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } fn walk_arm(&mut self, discr_cmt: mc::cmt<'tcx>, arm: &hir::Arm, mode: MatchMode) { - for pat in arm.top_pats_hack() { - self.walk_pat(discr_cmt.clone(), &pat, mode); - } + self.walk_pat(discr_cmt.clone(), &arm.pat, mode); if let Some(hir::Guard::If(ref e)) = arm.guard { self.consume_expr(e) From 07deb93bb291917967d167fa6cdacc62b0186599 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 18:44:32 +0200 Subject: [PATCH 16/50] or-patterns: middle/dead: make a hack less hacky. --- src/librustc/middle/dead.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index 666c1cc96f2f7..1818dede3e229 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -260,8 +260,8 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> { fn visit_arm(&mut self, arm: &'tcx hir::Arm) { let pats = arm.top_pats_hack(); - if pats.len() == 1 { - let variants = pats[0].necessary_variants(); + if let [pat] = pats { + let variants = pat.necessary_variants(); // Inside the body, ignore constructions of variants // necessary for the pattern to match. Those construction sites From 0dfd706257326cf826cca0a1029cf5bc0c2aee97 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 19:17:21 +0200 Subject: [PATCH 17/50] or-patterns: rvalue_promotion: remove `top_pats_hack`. --- src/librustc_passes/rvalue_promotion.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs index 7470f5b7a5f0f..12978cd117b52 100644 --- a/src/librustc_passes/rvalue_promotion.rs +++ b/src/librustc_passes/rvalue_promotion.rs @@ -499,19 +499,15 @@ fn check_expr_kind<'a, 'tcx>( } // Conditional control flow (possible to implement). - hir::ExprKind::Match(ref expr, ref hirvec_arm, ref _match_source) => { + hir::ExprKind::Match(ref expr, ref arms, ref _match_source) => { // Compute the most demanding borrow from all the arms' // patterns and set that on the discriminator. - let mut mut_borrow = false; - for pat in hirvec_arm.iter().flat_map(|arm| arm.top_pats_hack()) { - mut_borrow = v.remove_mut_rvalue_borrow(pat); - } - if mut_borrow { + if arms.iter().fold(false, |_, arm| v.remove_mut_rvalue_borrow(&arm.pat)) { v.mut_rvalue_borrows.insert(expr.hir_id); } let _ = v.check_expr(expr); - for index in hirvec_arm.iter() { + for index in arms.iter() { let _ = v.check_expr(&*index.body); if let Some(hir::Guard::If(ref expr)) = index.guard { let _ = v.check_expr(&expr); From 38a5ae9cc1bc2c4fb25a4be639b0eafaeb51426a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 19:24:40 +0200 Subject: [PATCH 18/50] or-patterns: regionck/visit_arm: remove `top_pats_hack`. --- src/librustc_typeck/check/regionck.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 86ec477a6aae8..698b49515fb66 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -488,9 +488,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RegionCtxt<'a, 'tcx> { fn visit_arm(&mut self, arm: &'tcx hir::Arm) { // see above - for p in arm.top_pats_hack() { - self.constrain_bindings_in_pat(p); - } + self.constrain_bindings_in_pat(&arm.pat); intravisit::walk_arm(self, arm); } From 9b406f1069852ed2fb6c51e4abb8f51b947c0955 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 19:38:03 +0200 Subject: [PATCH 19/50] or-patterns: regionck/`link_match`: remove `top_pats_hack`. --- src/librustc_typeck/check/regionck.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 698b49515fb66..6fa8a016b5458 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -1067,9 +1067,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> { let discr_cmt = Rc::new(ignore_err!(self.with_mc(|mc| mc.cat_expr(discr)))); debug!("discr_cmt={:?}", discr_cmt); for arm in arms { - for root_pat in arm.top_pats_hack() { - self.link_pattern(discr_cmt.clone(), &root_pat); - } + self.link_pattern(discr_cmt.clone(), &arm.pat); } } From 6bd8c6d4f44884a756471dda6ff3e5c66dbbd042 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 19:53:59 +0200 Subject: [PATCH 20/50] or-patterns: check_match: remove `top_pats_hack` for `check_for_bindings_named_same_as_variants`. --- src/librustc_mir/hair/pattern/check_match.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index dc884d40ac157..eb3698957157b 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -146,9 +146,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { } // Third, perform some lints. - for pat in arm.top_pats_hack() { - check_for_bindings_named_same_as_variants(self, pat); - } + check_for_bindings_named_same_as_variants(self, &arm.pat); } let module = self.tcx.hir().get_module_parent(scrut.hir_id); From 549756bef89fa7f9e39cf5e3d04553076ca952ae Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 15 Sep 2019 20:54:38 +0200 Subject: [PATCH 21/50] or-patterns: check_match: nix `top_pats_hack` passed to `check_patterns`. --- src/librustc_mir/hair/pattern/check_match.rs | 70 +++++++++----------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index eb3698957157b..6301d9c390b02 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -14,7 +14,6 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc::hir::def::*; use rustc::hir::def_id::DefId; use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap}; -use rustc::hir::ptr::P; use rustc::hir::{self, Pat, PatKind}; use smallvec::smallvec; @@ -76,7 +75,7 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> { }); // Check legality of move bindings and `@` patterns. - self.check_patterns(false, slice::from_ref(&loc.pat)); + self.check_patterns(false, &loc.pat); } fn visit_body(&mut self, body: &'tcx hir::Body) { @@ -84,7 +83,7 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> { for param in &body.params { self.check_irrefutable(¶m.pat, "function argument"); - self.check_patterns(false, slice::from_ref(¶m.pat)); + self.check_patterns(false, ¶m.pat); } } } @@ -122,11 +121,9 @@ impl PatternContext<'_, '_> { } impl<'tcx> MatchVisitor<'_, 'tcx> { - fn check_patterns(&mut self, has_guard: bool, pats: &[P]) { - check_legality_of_move_bindings(self, has_guard, pats); - for pat in pats { - check_legality_of_bindings_in_at_patterns(self, pat); - } + fn check_patterns(&mut self, has_guard: bool, pat: &Pat) { + check_legality_of_move_bindings(self, has_guard, pat); + check_legality_of_bindings_in_at_patterns(self, pat); } fn check_match( @@ -137,7 +134,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { ) { for arm in arms { // First, check legality of move bindings. - self.check_patterns(arm.guard.is_some(), &arm.top_pats_hack()); + self.check_patterns(arm.guard.is_some(), &arm.pat); // Second, if there is a guard on each arm, make sure it isn't // assigning or borrowing anything mutably. @@ -543,24 +540,18 @@ fn maybe_point_at_variant(ty: Ty<'_>, patterns: &[Pattern<'_>]) -> Vec { covered } -// Legality of move bindings checking -fn check_legality_of_move_bindings( - cx: &mut MatchVisitor<'_, '_>, - has_guard: bool, - pats: &[P], -) { +// Check the legality of legality of by-move bindings. +fn check_legality_of_move_bindings(cx: &mut MatchVisitor<'_, '_>, has_guard: bool, pat: &Pat) { let mut by_ref_span = None; - for pat in pats { - pat.each_binding(|_, hir_id, span, _path| { - if let Some(&bm) = cx.tables.pat_binding_modes().get(hir_id) { - if let ty::BindByReference(..) = bm { - by_ref_span = Some(span); - } - } else { - cx.tcx.sess.delay_span_bug(pat.span, "missing binding mode"); + pat.each_binding(|_, hir_id, span, _| { + if let Some(&bm) = cx.tables.pat_binding_modes().get(hir_id) { + if let ty::BindByReference(..) = bm { + by_ref_span = Some(span); } - }) - } + } else { + cx.tcx.sess.delay_span_bug(pat.span, "missing binding mode"); + } + }); let span_vec = &mut Vec::new(); let mut check_move = |p: &Pat, sub: Option<&Pat>| { @@ -576,23 +567,22 @@ fn check_legality_of_move_bindings( } }; - for pat in pats { - pat.walk(|p| { - if let PatKind::Binding(.., sub) = &p.node { - if let Some(&bm) = cx.tables.pat_binding_modes().get(p.hir_id) { - if let ty::BindByValue(..) = bm { - let pat_ty = cx.tables.node_type(p.hir_id); - if !pat_ty.is_copy_modulo_regions(cx.tcx, cx.param_env, pat.span) { - check_move(p, sub.as_deref()); - } + pat.walk(|p| { + if let PatKind::Binding(.., sub) = &p.node { + if let Some(&bm) = cx.tables.pat_binding_modes().get(p.hir_id) { + if let ty::BindByValue(..) = bm { + let pat_ty = cx.tables.node_type(p.hir_id); + if !pat_ty.is_copy_modulo_regions(cx.tcx, cx.param_env, pat.span) { + check_move(p, sub.as_deref()); } - } else { - cx.tcx.sess.delay_span_bug(pat.span, "missing binding mode"); } + } else { + cx.tcx.sess.delay_span_bug(pat.span, "missing binding mode"); } - true - }); - } + } + true + }); + if !span_vec.is_empty() { let mut err = struct_span_err!( cx.tcx.sess, @@ -603,7 +593,7 @@ fn check_legality_of_move_bindings( if let Some(by_ref_span) = by_ref_span { err.span_label(by_ref_span, "both by-ref and by-move used"); } - for span in span_vec.iter(){ + for span in span_vec.iter() { err.span_label(*span, "by-move pattern here"); } err.emit(); From 56b055a6abf5f3cf0d4ae13026154eb82298b21b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 16 Sep 2019 03:32:44 +0200 Subject: [PATCH 22/50] or-patterns: HAIR: `Arm.patterns: Vec>` -> `.pattern: Pattern<'_>`. --- src/librustc_mir/build/matches/mod.rs | 6 +++--- src/librustc_mir/hair/cx/expr.rs | 4 ++-- src/librustc_mir/hair/mod.rs | 13 ++++++++++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index aa261f8eb6fb2..f23abf86bcfc8 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -142,7 +142,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Step 2. Create the otherwise and prebinding blocks. // create binding start block for link them by false edges - let candidate_count = arms.iter().map(|c| c.patterns.len()).sum::(); + let candidate_count = arms.iter().map(|c| c.top_pats_hack().len()).sum::(); let pre_binding_blocks: Vec<_> = (0..candidate_count) .map(|_| self.cfg.start_new_block()) .collect(); @@ -159,7 +159,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .map(|arm| { let arm_has_guard = arm.guard.is_some(); match_has_guard |= arm_has_guard; - let arm_candidates: Vec<_> = arm.patterns + let arm_candidates: Vec<_> = arm.top_pats_hack() .iter() .zip(candidate_pre_binding_blocks.by_ref()) .map( @@ -238,7 +238,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let scope = this.declare_bindings( None, arm.span, - &arm.patterns[0], + &arm.top_pats_hack()[0], ArmHasGuard(arm.guard.is_some()), Some((Some(&scrutinee_place), scrutinee_span)), ); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index f3d699fa4f008..bdfcacd0f4629 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -860,9 +860,9 @@ impl ToBorrowKind for hir::Mutability { } } -fn convert_arm<'a, 'tcx>(cx: &mut Cx<'a, 'tcx>, arm: &'tcx hir::Arm) -> Arm<'tcx> { +fn convert_arm<'tcx>(cx: &mut Cx<'_, 'tcx>, arm: &'tcx hir::Arm) -> Arm<'tcx> { Arm { - patterns: arm.top_pats_hack().iter().map(|p| cx.pattern_from_hir(p)).collect(), + pattern: cx.pattern_from_hir(&arm.pat), guard: match arm.guard { Some(hir::Guard::If(ref e)) => Some(Guard::If(e.to_ref())), _ => None, diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 0638cb462f73b..63a9a83154b4f 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -293,7 +293,7 @@ pub struct FruInfo<'tcx> { #[derive(Clone, Debug)] pub struct Arm<'tcx> { - pub patterns: Vec>, + pub pattern: Pattern<'tcx>, pub guard: Option>, pub body: ExprRef<'tcx>, pub lint_level: LintLevel, @@ -301,6 +301,17 @@ pub struct Arm<'tcx> { pub span: Span, } +impl Arm<'tcx> { + // HACK(or_patterns; Centril | dlrobertson): Remove this and + // correctly handle each case in which this method is used. + pub fn top_pats_hack(&self) -> &[Pattern<'tcx>] { + match &*self.pattern.kind { + PatternKind::Or { pats } => pats, + _ => std::slice::from_ref(&self.pattern), + } + } +} + #[derive(Clone, Debug)] pub enum Guard<'tcx> { If(ExprRef<'tcx>), From 05cc3c0a83fc7faab20a10f2e0eca61dc34a6394 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 16 Sep 2019 16:35:36 +0200 Subject: [PATCH 23/50] or-patterns: liveness: `is_argument` -> `is_param`. Pacify `tidy`. It's also more correct in this context. --- src/librustc/middle/liveness.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 9afd147ae34e2..9f6611712a8aa 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -1550,9 +1550,9 @@ impl<'tcx> Liveness<'_, 'tcx> { } } - fn report_dead_assign(&self, hir_id: HirId, spans: Vec, var: Variable, is_argument: bool) { + fn report_dead_assign(&self, hir_id: HirId, spans: Vec, var: Variable, is_param: bool) { if let Some(name) = self.should_warn(var) { - if is_argument { + if is_param { self.ir.tcx.struct_span_lint_hir(lint::builtin::UNUSED_ASSIGNMENTS, hir_id, spans, &format!("value passed to `{}` is never read", name)) .help("maybe it is overwritten before being read?") From 370fbcc0225a226096773fc8641c59eb5e635f7a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 16 Sep 2019 20:49:48 +0200 Subject: [PATCH 24/50] or-patterns: #47390: we rely on names to exercise `IndexMap`. --- ...47390-unused-variable-in-struct-pattern.rs | 3 +++ ...0-unused-variable-in-struct-pattern.stderr | 20 +++++++++---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs index 4cb35e907c8f9..7870d394c8bfb 100644 --- a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs +++ b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.rs @@ -33,6 +33,9 @@ fn main() { let mut mut_unused_var = 1; let (mut var, unused_var) = (1, 2); + // NOTE: `var` comes after `unused_var` lexicographically yet the warning + // for `var` will be emitted before the one for `unused_var`. We use an + // `IndexMap` to ensure this is the case instead of a `BTreeMap`. if let SoulHistory { corridors_of_light, mut hours_are_suns, diff --git a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr index a0b34d220c8d9..74bbef8adad05 100644 --- a/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr +++ b/src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr @@ -30,13 +30,13 @@ LL | let (mut var, unused_var) = (1, 2); | ^^^^^^^^^^ help: consider prefixing with an underscore: `_unused_var` warning: unused variable: `corridors_of_light` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:26 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:26 | LL | if let SoulHistory { corridors_of_light, | ^^^^^^^^^^^^^^^^^^ help: try ignoring the field: `corridors_of_light: _` warning: variable `hours_are_suns` is assigned to, but never used - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:38:30 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:41:30 | LL | mut hours_are_suns, | ^^^^^^^^^^^^^^ @@ -44,7 +44,7 @@ LL | mut hours_are_suns, = note: consider using `_hours_are_suns` instead warning: value assigned to `hours_are_suns` is never read - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:9 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:43:9 | LL | hours_are_suns = false; | ^^^^^^^^^^^^^^ @@ -58,43 +58,43 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896) = help: maybe it is overwritten before being read? warning: unused variable: `fire` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:44:32 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:47:32 | LL | let LovelyAmbition { lips, fire } = the_spirit; | ^^^^ help: try ignoring the field: `fire: _` warning: unused variable: `case` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:53:23 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:56:23 | LL | Large::Suit { case } => {} | ^^^^ help: try ignoring the field: `case: _` warning: unused variable: `case` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:58:24 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:61:24 | LL | &Large::Suit { case } => {} | ^^^^ help: try ignoring the field: `case: _` warning: unused variable: `case` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:63:27 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:66:27 | LL | box Large::Suit { case } => {} | ^^^^ help: try ignoring the field: `case: _` warning: unused variable: `case` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:68:24 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:71:24 | LL | (Large::Suit { case },) => {} | ^^^^ help: try ignoring the field: `case: _` warning: unused variable: `case` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:73:24 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:76:24 | LL | [Large::Suit { case }] => {} | ^^^^ help: try ignoring the field: `case: _` warning: unused variable: `case` - --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:78:29 + --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:81:29 | LL | Tuple(Large::Suit { case }, ()) => {} | ^^^^ help: try ignoring the field: `case: _` From 3f004a1bc44859857f05a9f692a578124b3f3e01 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 17 Sep 2019 14:40:36 +0200 Subject: [PATCH 25/50] Fix re-rebalance coherence implementation for fundamental types Fixes #64412 --- src/librustc/traits/coherence.rs | 10 +++++++++- ...pl[t]-foreign[local]-for-fundamental[t].rs | 5 ++--- ...]-foreign[local]-for-fundamental[t].stderr | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].stderr diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index b6f0addd77107..bc6bcb1f76f96 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -378,7 +378,15 @@ fn orphan_check_trait_ref<'tcx>( // Let Ti be the first such type. // - No uncovered type parameters P1..=Pn may appear in T0..Ti (excluding Ti) // - for input_ty in trait_ref.input_types() { + fn uncover_fundamental_ty(ty: Ty<'_>) -> Vec> { + if fundamental_ty(ty) { + ty.walk_shallow().flat_map(|ty| uncover_fundamental_ty(ty)).collect() + } else { + vec![ty] + } + } + + for input_ty in trait_ref.input_types().flat_map(uncover_fundamental_ty) { debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty); if ty_is_local(tcx, input_ty, in_crate) { debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty); diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs index db671cb9bcaba..54425b6d708aa 100644 --- a/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].rs @@ -2,7 +2,6 @@ // compile-flags:--crate-name=test // aux-build:coherence_lib.rs -// check-pass extern crate coherence_lib as lib; use lib::*; @@ -11,11 +10,11 @@ use std::rc::Rc; struct Local; impl Remote1 for Box { - // FIXME(#64412) -- this is expected to error + //~^ ERROR type parameter `T` must be used as the type parameter for some local type } impl Remote1 for &T { - // FIXME(#64412) -- this is expected to error + //~^ ERROR type parameter `T` must be used as the type parameter for some local type } fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].stderr b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].stderr new file mode 100644 index 0000000000000..7859665a7bb58 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-fundamental[t].stderr @@ -0,0 +1,19 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[local]-for-fundamental[t].rs:12:1 + | +LL | impl Remote1 for Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[local]-for-fundamental[t].rs:16:1 + | +LL | impl Remote1 for &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. From a9c38d9d01bd50b5a264cd62fd32cec37f006ab9 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 18 Sep 2019 22:14:33 +0200 Subject: [PATCH 26/50] Add more tests --- .../impl-foreign[foreign]-for-foreign.rs | 16 +++++++++++++++ .../impl-foreign[foreign]-for-foreign.stderr | 12 +++++++++++ .../impl-foreign[foreign]-for-local.rs | 16 +++++++++++++++ ...[t]-foreign[foreign]-for-fundamental[t].rs | 20 +++++++++++++++++++ ...foreign[foreign]-for-fundamental[t].stderr | 19 ++++++++++++++++++ .../impl[t]-foreign[foreign]-for-t.rs | 16 +++++++++++++++ .../impl[t]-foreign[foreign]-for-t.stderr | 11 ++++++++++ ...reign[fundamental[t],local]-for-foreign.rs | 20 +++++++++++++++++++ ...n[fundamental[t],local]-for-foreign.stderr | 19 ++++++++++++++++++ ...[t]-foreign[fundamental[t]]-for-foreign.rs | 20 +++++++++++++++++++ ...foreign[fundamental[t]]-for-foreign.stderr | 19 ++++++++++++++++++ ...eign[fundamental[t]]-for-fundamental[t].rs | 19 ++++++++++++++++++ ...[fundamental[t]]-for-fundamental[t].stderr | 19 ++++++++++++++++++ ...pl[t]-foreign[fundamental[t]]-for-local.rs | 17 ++++++++++++++++ .../impl[t]-foreign[fundamental[t]]-for-t.rs | 19 ++++++++++++++++++ ...pl[t]-foreign[fundamental[t]]-for-t.stderr | 19 ++++++++++++++++++ ...eign[local, fundamental[t]]-for-foreign.rs | 19 ++++++++++++++++++ .../impl[t]-foreign[local]-for-foreign.rs | 16 +++++++++++++++ .../impl[t]-foreign[local]-for-local.rs | 15 ++++++++++++++ .../impl[t]-foreign[t]-for-foreign.rs | 16 +++++++++++++++ .../impl[t]-foreign[t]-for-foreign.stderr | 11 ++++++++++ .../impl[t]-foreign[t]-for-fundamental.rs | 20 +++++++++++++++++++ .../impl[t]-foreign[t]-for-fundamental.stderr | 19 ++++++++++++++++++ .../coherence/impl[t]-foreign[t]-for-local.rs | 15 ++++++++++++++ .../ui/coherence/impl[t]-foreign[t]-for-t.rs | 16 +++++++++++++++ .../coherence/impl[t]-foreign[t]-for-t.stderr | 11 ++++++++++ 26 files changed, 439 insertions(+) create mode 100644 src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs create mode 100644 src/test/ui/coherence/impl-foreign[foreign]-for-foreign.stderr create mode 100644 src/test/ui/coherence/impl-foreign[foreign]-for-local.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-local.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[local, fundamental[t]]-for-foreign.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[local]-for-foreign.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[local]-for-local.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.stderr create mode 100644 src/test/ui/coherence/impl[t]-foreign[t]-for-local.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[t]-for-t.rs create mode 100644 src/test/ui/coherence/impl[t]-foreign[t]-for-t.stderr diff --git a/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs b/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs new file mode 100644 index 0000000000000..57738c64e3779 --- /dev/null +++ b/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs @@ -0,0 +1,16 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for f64 { + //~^ ERROR only traits defined in the current crate can be implemented for arbitrary types [E0117] +} + +fn main() {} diff --git a/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.stderr b/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.stderr new file mode 100644 index 0000000000000..04e96f29230fb --- /dev/null +++ b/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.stderr @@ -0,0 +1,12 @@ +error[E0117]: only traits defined in the current crate can be implemented for arbitrary types + --> $DIR/impl-foreign[foreign]-for-foreign.rs:12:1 + | +LL | impl Remote1 for f64 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate + | + = note: the impl does not reference only types defined in this crate + = note: define and implement a trait or new type instead + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0117`. diff --git a/src/test/ui/coherence/impl-foreign[foreign]-for-local.rs b/src/test/ui/coherence/impl-foreign[foreign]-for-local.rs new file mode 100644 index 0000000000000..33e85c164763e --- /dev/null +++ b/src/test/ui/coherence/impl-foreign[foreign]-for-local.rs @@ -0,0 +1,16 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs +// check-pass + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for Local { +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].rs b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].rs new file mode 100644 index 0000000000000..66a4d9d273469 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].rs @@ -0,0 +1,20 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for Box { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +impl<'a, T> Remote1 for &'a T { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].stderr b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].stderr new file mode 100644 index 0000000000000..2467097b1a8b3 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-fundamental[t].stderr @@ -0,0 +1,19 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[foreign]-for-fundamental[t].rs:12:1 + | +LL | impl Remote1 for Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[foreign]-for-fundamental[t].rs:16:1 + | +LL | impl<'a, T> Remote1 for &'a T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.rs b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.rs new file mode 100644 index 0000000000000..0a67ebcbba44c --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.rs @@ -0,0 +1,16 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for T { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.stderr b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.stderr new file mode 100644 index 0000000000000..5c28406f113fc --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[foreign]-for-t.stderr @@ -0,0 +1,11 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[foreign]-for-t.rs:12:1 + | +LL | impl Remote1 for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.rs b/src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.rs new file mode 100644 index 0000000000000..24e0f309c4555 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.rs @@ -0,0 +1,20 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote2, Local> for u32 { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +impl<'a, T> Remote2<&'a T, Local> for u32 { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.stderr b/src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.stderr new file mode 100644 index 0000000000000..da670bcfc3fc9 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t],local]-for-foreign.stderr @@ -0,0 +1,19 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t],local]-for-foreign.rs:12:1 + | +LL | impl Remote2, Local> for u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t],local]-for-foreign.rs:16:1 + | +LL | impl<'a, T> Remote2<&'a T, Local> for u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.rs b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.rs new file mode 100644 index 0000000000000..71598dae96ab3 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.rs @@ -0,0 +1,20 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1> for u32 { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +impl<'a, T> Remote1<&'a T> for u32 { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.stderr b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.stderr new file mode 100644 index 0000000000000..dd9702650795e --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-foreign.stderr @@ -0,0 +1,19 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t]]-for-foreign.rs:12:1 + | +LL | impl Remote1> for u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t]]-for-foreign.rs:16:1 + | +LL | impl<'a, T> Remote1<&'a T> for u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].rs b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].rs new file mode 100644 index 0000000000000..7bf0306f29ba4 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].rs @@ -0,0 +1,19 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl<'a, T> Remote1> for &'a T { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} +impl<'a, T> Remote1<&'a T> for Box { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].stderr b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].stderr new file mode 100644 index 0000000000000..eec57fccea762 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-fundamental[t].stderr @@ -0,0 +1,19 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t]]-for-fundamental[t].rs:12:1 + | +LL | impl<'a, T> Remote1> for &'a T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t]]-for-fundamental[t].rs:15:1 + | +LL | impl<'a, T> Remote1<&'a T> for Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-local.rs b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-local.rs new file mode 100644 index 0000000000000..54d577c749248 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-local.rs @@ -0,0 +1,17 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs +// check-pass + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1> for Local {} + +impl<'a, T> Remote1<&'a T> for Local {} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.rs b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.rs new file mode 100644 index 0000000000000..7af929006ef7f --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.rs @@ -0,0 +1,19 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1> for T { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} +impl<'a, T> Remote1<&'a T> for T { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.stderr b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.stderr new file mode 100644 index 0000000000000..e017c3ffe6c05 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[fundamental[t]]-for-t.stderr @@ -0,0 +1,19 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t]]-for-t.rs:12:1 + | +LL | impl Remote1> for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[fundamental[t]]-for-t.rs:15:1 + | +LL | impl<'a, T> Remote1<&'a T> for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[local, fundamental[t]]-for-foreign.rs b/src/test/ui/coherence/impl[t]-foreign[local, fundamental[t]]-for-foreign.rs new file mode 100644 index 0000000000000..be0875d0110fd --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[local, fundamental[t]]-for-foreign.rs @@ -0,0 +1,19 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs +// check-pass + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; +struct Local2(Rc); + +impl Remote2> for u32 {} +impl<'a, T> Remote2 for u32 {} +impl Remote2, Box> for u32 {} +impl<'a, T> Remote2, &'a T> for u32 {} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-foreign.rs b/src/test/ui/coherence/impl[t]-foreign[local]-for-foreign.rs new file mode 100644 index 0000000000000..81cf3c3f6eca9 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-foreign.rs @@ -0,0 +1,16 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs +// check-pass + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for Rc {} +impl Remote1 for Vec> {} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[local]-for-local.rs b/src/test/ui/coherence/impl[t]-foreign[local]-for-local.rs new file mode 100644 index 0000000000000..6b1d93cd94442 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[local]-for-local.rs @@ -0,0 +1,15 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs +// check-pass + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for Local {} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.rs b/src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.rs new file mode 100644 index 0000000000000..5e89c2077330a --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.rs @@ -0,0 +1,16 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for u32 { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.stderr b/src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.stderr new file mode 100644 index 0000000000000..5544729b5d640 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[t]-for-foreign.stderr @@ -0,0 +1,11 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[t]-for-foreign.rs:12:1 + | +LL | impl Remote1 for u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.rs b/src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.rs new file mode 100644 index 0000000000000..300a2c4d48a9c --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.rs @@ -0,0 +1,20 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for Box { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +impl<'a, A, B> Remote1 for &'a B { + //~^ ERROR type parameter `B` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.stderr b/src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.stderr new file mode 100644 index 0000000000000..be8cc29a6e5b0 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[t]-for-fundamental.stderr @@ -0,0 +1,19 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[t]-for-fundamental.rs:12:1 + | +LL | impl Remote1 for Box { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error[E0210]: type parameter `B` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[t]-for-fundamental.rs:16:1 + | +LL | impl<'a, A, B> Remote1 for &'a B { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `B` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0210`. diff --git a/src/test/ui/coherence/impl[t]-foreign[t]-for-local.rs b/src/test/ui/coherence/impl[t]-foreign[t]-for-local.rs new file mode 100644 index 0000000000000..769147ea7eabd --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[t]-for-local.rs @@ -0,0 +1,15 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs +// check-pass + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for Local {} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[t]-for-t.rs b/src/test/ui/coherence/impl[t]-foreign[t]-for-t.rs new file mode 100644 index 0000000000000..c8513380ff73e --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[t]-for-t.rs @@ -0,0 +1,16 @@ +#![feature(re_rebalance_coherence)] + +// compile-flags:--crate-name=test +// aux-build:coherence_lib.rs + +extern crate coherence_lib as lib; +use lib::*; +use std::rc::Rc; + +struct Local; + +impl Remote1 for T { + //~^ ERROR type parameter `T` must be used as the type parameter for some local type +} + +fn main() {} diff --git a/src/test/ui/coherence/impl[t]-foreign[t]-for-t.stderr b/src/test/ui/coherence/impl[t]-foreign[t]-for-t.stderr new file mode 100644 index 0000000000000..de857afd20b15 --- /dev/null +++ b/src/test/ui/coherence/impl[t]-foreign[t]-for-t.stderr @@ -0,0 +1,11 @@ +error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) + --> $DIR/impl[t]-foreign[t]-for-t.rs:12:1 + | +LL | impl Remote1 for T { + | ^^^^^^^^^^^^^^^^^^^^^^^^ type parameter `T` must be used as the type parameter for some local type + | + = note: only traits defined in the current crate can be implemented for a type parameter + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0210`. From 31b301219f534090690674e43e05c3cbfd2d5005 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 18 Sep 2019 22:36:04 +0200 Subject: [PATCH 27/50] Split line to fix tidy --- src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs b/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs index 57738c64e3779..b08fedc5e11c2 100644 --- a/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs +++ b/src/test/ui/coherence/impl-foreign[foreign]-for-foreign.rs @@ -10,7 +10,8 @@ use std::rc::Rc; struct Local; impl Remote1 for f64 { - //~^ ERROR only traits defined in the current crate can be implemented for arbitrary types [E0117] + //~^ ERROR only traits defined in the current crate + // | can be implemented for arbitrary types [E0117] } fn main() {} From e625f62c517b9546df0d3e8f48bbd07c2a31724a Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 19 Sep 2019 18:33:22 -0700 Subject: [PATCH 28/50] Support run-fail ui tests --- src/tools/compiletest/src/common.rs | 2 ++ src/tools/compiletest/src/header.rs | 5 +++++ src/tools/compiletest/src/runtest.rs | 22 ++++++++++++++++++---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index edb9eb7d860e2..2358a065d62d1 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -100,6 +100,7 @@ pub enum PassMode { Check, Build, Run, + RunFail, } impl FromStr for PassMode { @@ -120,6 +121,7 @@ impl fmt::Display for PassMode { PassMode::Check => "check", PassMode::Build => "build", PassMode::Run => "run", + PassMode::RunFail => "run-fail", }; fmt::Display::fmt(s, f) } diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 48dd68d0f61ee..df56448dd225d 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -610,6 +610,11 @@ impl TestProps { panic!("`run-pass` header is only supported in UI tests") } Some(PassMode::Run) + } else if config.parse_name_directive(ln, "run-fail") { + if config.mode != Mode::Ui { + panic!("`run-fail` header is only supported in UI tests") + } + Some(PassMode::RunFail) } else { None }; diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index baed27dd15152..ea31f37c7a52b 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -326,6 +326,14 @@ impl<'test> TestCx<'test> { self.props.pass_mode(self.config) } + fn should_run(&self) -> bool { + let pass_mode = self.pass_mode(); + match self.config.mode { + Ui => pass_mode == Some(PassMode::Run) || pass_mode == Some(PassMode::RunFail), + mode => panic!("unimplemented for mode {:?}", mode), + } + } + fn should_run_successfully(&self) -> bool { let pass_mode = self.pass_mode(); match self.config.mode { @@ -1534,7 +1542,7 @@ impl<'test> TestCx<'test> { fn compile_test(&self) -> ProcRes { // Only use `make_exe_name` when the test ends up being executed. let will_execute = match self.config.mode { - Ui => self.should_run_successfully(), + Ui => self.should_run(), Incremental => self.revision.unwrap().starts_with("r"), RunFail | RunPassValgrind | MirOpt | DebugInfoCdb | DebugInfoGdbLldb | DebugInfoGdb | DebugInfoLldb => true, @@ -3107,7 +3115,7 @@ impl<'test> TestCx<'test> { let expected_errors = errors::load_errors(&self.testpaths.file, self.revision); - if self.should_run_successfully() { + if self.should_run() { let proc_res = self.exec_compiled_test(); let run_output_errors = if self.props.check_run_results { self.load_compare_outputs(&proc_res, TestOutput::Run, explicit) @@ -3120,8 +3128,14 @@ impl<'test> TestCx<'test> { &proc_res, ); } - if !proc_res.status.success() { - self.fatal_proc_rec("test run failed!", &proc_res); + if self.should_run_successfully() { + if !proc_res.status.success() { + self.fatal_proc_rec("test run failed!", &proc_res); + } + } else { + if proc_res.status.success() { + self.fatal_proc_rec("test run succeeded!", &proc_res); + } } } From 78ff1105f9a6159a89e10b0797224df1ca6ca868 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 11 Sep 2019 17:13:34 -0700 Subject: [PATCH 29/50] Spawn one subprocess per unit test --- src/libtest/lib.rs | 271 +++++++++++++++++++++++++++++++++++-------- src/libtest/tests.rs | 14 +-- 2 files changed, 228 insertions(+), 57 deletions(-) diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index 09d5fcc89520e..fab1452cb7be3 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -21,7 +21,8 @@ #![unstable(feature = "test", issue = "50297")] #![doc(html_root_url = "https://doc.rust-lang.org/nightly/", test(attr(deny(warnings))))] #![feature(asm)] -#![cfg_attr(any(unix, target_os = "cloudabi"), feature(libc, rustc_private))] +#![cfg_attr(any(unix, target_os = "cloudabi"), feature(libc))] +#![feature(rustc_private)] #![feature(nll)] #![feature(set_stdio)] #![feature(panic_unwind)] @@ -56,15 +57,16 @@ use std::any::Any; use std::borrow::Cow; use std::cmp; use std::collections::BTreeMap; +use std::convert::{TryFrom, TryInto}; use std::env; use std::fmt; use std::fs::File; use std::io; use std::io::prelude::*; -use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::panic::{self, catch_unwind, AssertUnwindSafe, PanicInfo}; use std::path::PathBuf; use std::process; -use std::process::Termination; +use std::process::{Command, Termination}; use std::sync::mpsc::{channel, Sender}; use std::sync::{Arc, Mutex}; use std::thread; @@ -76,13 +78,15 @@ mod tests; const TEST_WARN_TIMEOUT_S: u64 = 60; const QUIET_MODE_MAX_COLUMN: usize = 100; // insert a '\n' after 100 tests in quiet mode +const SECONDARY_TEST_INVOKER_VAR: &'static str = "__RUST_TEST_INVOKE"; + // to be used by rustc to compile tests in libtest pub mod test { pub use crate::{ assert_test_result, filter_tests, parse_opts, run_test, test_main, test_main_static, - Bencher, DynTestFn, DynTestName, Metric, MetricMap, Options, RunIgnored, ShouldPanic, - StaticBenchFn, StaticTestFn, StaticTestName, TestDesc, TestDescAndFn, TestName, TestOpts, - TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk, + Bencher, DynTestFn, DynTestName, Metric, MetricMap, Options, RunIgnored, RunStrategy, + ShouldPanic, StaticBenchFn, StaticTestFn, StaticTestName, TestDesc, TestDescAndFn, TestName, + TestOpts, TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk, }; } @@ -275,6 +279,19 @@ impl Options { // The default console test runner. It accepts the command line // arguments and a vector of test_descs. pub fn test_main(args: &[String], tests: Vec, options: Option) { + // If we're being run in SpawnedSecondary mode, run the test here. run_test + // will then exit the process. + if let Ok(name) = env::var(SECONDARY_TEST_INVOKER_VAR) { + let test = tests + .into_iter() + .filter(|test| test.desc.name.as_slice() == name) + .next() + .expect("couldn't find a test with the provided name"); + let opts = parse_opts(&[]).unwrap().unwrap(); + run_test(&opts, false, test, RunStrategy::SpawnedSecondary, Concurrent::No); + unreachable!(); + } + let mut opts = match parse_opts(args) { Some(Ok(o)) => o, Some(Err(msg)) => { @@ -677,6 +694,41 @@ pub enum TestResult { unsafe impl Send for TestResult {} +// Return codes for TestResult. +// Start somewhere other than 0 so we know the return code means what we think +// it means. +const TR_OK: i32 = 50; +const TR_FAILED: i32 = 51; +const TR_IGNORED: i32 = 52; +const TR_ALLOWED_FAIL: i32 = 53; + +impl TryFrom for i32 { + type Error = &'static str; + fn try_from(val: TestResult) -> Result { + Ok(match val { + TrOk => TR_OK, + TrFailed => TR_FAILED, + TrIgnored => TR_IGNORED, + TrAllowedFail => TR_ALLOWED_FAIL, + TrFailedMsg(..) | + TrBench(..) => return Err("can't convert variant to i32"), + }) + } +} + +impl TryFrom for TestResult { + type Error = &'static str; + fn try_from(val: i32) -> Result { + Ok(match val { + TR_OK => TrOk, + TR_FAILED => TrFailed, + TR_IGNORED => TrIgnored, + TR_ALLOWED_FAIL => TrAllowedFail, + _ => return Err("unrecognized return code"), + }) + } +} + enum OutputLocation { Pretty(Box), Raw(T), @@ -1021,6 +1073,33 @@ impl Write for Sink { } } +#[derive(Clone)] +pub enum RunStrategy { + /// Runs the test in the current process, and sends the result back over the + /// supplied channel. + InProcess(Sender), + + /// 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 + /// that's currently running. + SpawnPrimary(Sender), + + /// Runs the test in the current process, then exits the process. + /// Prints to stdout the custom result format that the parent process + /// expects in SpawnPrimary mode. + SpawnedSecondary, +} + +impl RunStrategy { + fn monitor_ch(self) -> Option> { + match self { + RunStrategy::InProcess(ch) => Some(ch), + RunStrategy::SpawnPrimary(ch) => Some(ch), + RunStrategy::SpawnedSecondary => None, + } + } +} + pub fn run_tests(opts: &TestOpts, tests: Vec, mut callback: F) -> io::Result<()> where F: FnMut(TestEvent) -> io::Result<()>, @@ -1068,6 +1147,12 @@ where let mut pending = 0; let (tx, rx) = channel::(); + // TODO + let run_strategy = if true { + RunStrategy::SpawnPrimary(tx) + } else { + RunStrategy::InProcess(tx) + }; let mut running_tests: TestMap = HashMap::default(); @@ -1104,7 +1189,7 @@ where while !remaining.is_empty() { let test = remaining.pop().unwrap(); callback(TeWait(test.desc.clone()))?; - run_test(opts, !opts.run_tests, test, tx.clone(), Concurrent::No); + run_test(opts, !opts.run_tests, test, run_strategy.clone(), Concurrent::No); let (test, result, stdout) = rx.recv().unwrap(); callback(TeResult(test, result, stdout))?; } @@ -1115,7 +1200,7 @@ where let timeout = Instant::now() + Duration::from_secs(TEST_WARN_TIMEOUT_S); running_tests.insert(test.desc.clone(), timeout); callback(TeWait(test.desc.clone()))?; //here no pad - run_test(opts, !opts.run_tests, test, tx.clone(), Concurrent::Yes); + run_test(opts, !opts.run_tests, test, run_strategy.clone(), Concurrent::Yes); pending += 1; } @@ -1147,7 +1232,7 @@ where // All benchmarks run at the end, in serial. for b in filtered_benchs { callback(TeWait(b.desc.clone()))?; - run_test(opts, false, b, tx.clone(), Concurrent::No); + run_test(opts, false, b, run_strategy.clone(), Concurrent::No); let (test, result, stdout) = rx.recv().unwrap(); callback(TeResult(test, result, stdout))?; } @@ -1374,7 +1459,7 @@ pub fn run_test( opts: &TestOpts, force_ignore: bool, test: TestDescAndFn, - monitor_ch: Sender, + strategy: RunStrategy, concurrency: Concurrent, ) { let TestDescAndFn { desc, testfn } = test; @@ -1384,44 +1469,125 @@ pub fn run_test( && desc.should_panic != ShouldPanic::No; if force_ignore || desc.ignore || ignore_because_panic_abort { - monitor_ch.send((desc, TrIgnored, Vec::new())).unwrap(); + match strategy { + RunStrategy::InProcess(tx) | RunStrategy::SpawnPrimary(tx) => { + tx.send((desc, TrIgnored, Vec::new())).unwrap(); + } + RunStrategy::SpawnedSecondary => panic!(), + } return; } fn run_test_inner( desc: TestDesc, - monitor_ch: Sender, nocapture: bool, + strategy: RunStrategy, testfn: Box, concurrency: Concurrent, ) { // Buffer for capturing standard I/O let data = Arc::new(Mutex::new(Vec::new())); - let data2 = data.clone(); let name = desc.name.clone(); let runtest = move || { - let oldio = if !nocapture { - Some(( - io::set_print(Some(Box::new(Sink(data2.clone())))), - io::set_panic(Some(Box::new(Sink(data2)))), - )) - } else { - None - }; + // TODO split into functions + match strategy { + RunStrategy::InProcess(monitor_ch) => { + let oldio = if !nocapture { + Some(( + io::set_print(Some(Box::new(Sink(data.clone())))), + io::set_panic(Some(Box::new(Sink(data.clone())))), + )) + } else { + None + }; + + let result = catch_unwind(AssertUnwindSafe(testfn)); + + if let Some((printio, panicio)) = oldio { + io::set_print(printio); + io::set_panic(panicio); + } - let result = catch_unwind(AssertUnwindSafe(testfn)); + let test_result = match result { + Ok(()) => calc_result(&desc, Ok(())), + Err(e) => calc_result(&desc, Err(e.as_ref())), + }; + let stdout = data.lock().unwrap().to_vec(); + monitor_ch.send((desc.clone(), test_result, stdout)).unwrap(); + } - if let Some((printio, panicio)) = oldio { - io::set_print(printio); - io::set_panic(panicio); - }; + RunStrategy::SpawnPrimary(monitor_ch) => { + let (result, test_output) = (|| { + let args = env::args().collect::>(); + let current_exe = &args[0]; + let output = match Command::new(current_exe) + .env(SECONDARY_TEST_INVOKER_VAR, desc.name.as_slice()) + .output() { + Ok(out) => out, + Err(e) => { + let err = format!("Failed to spawn {} as child for test: {:?}", + args[0], e); + return (TrFailed, err.into_bytes()); + } + }; + + let std::process::Output { stdout, stderr, status } = output; + let mut test_output = stdout; + test_output.extend_from_slice(&stderr); + + let result = match (move || { + let exit_code = status.code().ok_or("child process was terminated")?; + TestResult::try_from(exit_code).map_err(|_| { + format!("child process returned unexpected exit code {}", exit_code) + }) + })() { + Ok(r) => r, + Err(e) => { + write!(&mut test_output, "Unexpected error: {}", e).unwrap(); + TrFailed + } + }; + + (result, test_output) + })(); + + monitor_ch.send((desc.clone(), result, test_output)).unwrap(); + } - let test_result = calc_result(&desc, result); - let stdout = data.lock().unwrap().to_vec(); - monitor_ch - .send((desc.clone(), test_result, stdout)) - .unwrap(); + RunStrategy::SpawnedSecondary => { + let record_lock = Mutex::new(()); + let builtin_panic_hook = panic::take_hook(); + let record_result = Arc::new(move |panic_info: Option<&'_ PanicInfo<'_>>| { + let _lock = record_lock.lock().unwrap(); + + let test_result = match panic_info { + Some(info) => calc_result(&desc, Err(info.payload())), + None => calc_result(&desc, Ok(())), + }; + + // We don't support serializing TrFailedMsg, so just + // print the message out to stderr. + let test_result = match test_result { + TrFailedMsg(msg) => { + eprintln!("{}", msg); + TrFailed + } + _ => test_result, + }; + + if let Some(info) = panic_info { + builtin_panic_hook(info); + } + process::exit(test_result.try_into().unwrap()); + }); + let record_result2 = record_result.clone(); + panic::set_hook(Box::new(move |info| record_result2(Some(&info)))); + testfn(); + record_result(None); + unreachable!("panic=abort callback should have exited the process"); + } + } }; // If the platform is single-threaded we're just going to run @@ -1438,26 +1604,29 @@ pub fn run_test( match testfn { DynBenchFn(bencher) => { - crate::bench::benchmark(desc, monitor_ch, opts.nocapture, |harness| { + // Benchmarks aren't expected to panic, so we run them all in-process. + crate::bench::benchmark(opts, desc, strategy.monitor_ch().unwrap(), |harness| { bencher.run(harness) }); } StaticBenchFn(benchfn) => { - crate::bench::benchmark(desc, monitor_ch, opts.nocapture, |harness| { + // Benchmarks aren't expected to panic, so we run them all in-process. + crate::bench::benchmark(opts, desc, strategy.monitor_ch().unwrap(), |harness| { (benchfn.clone())(harness) }); } DynTestFn(f) => { + match strategy { + RunStrategy::InProcess(_) => (), + _ => panic!("Cannot run dynamic test fn out-of-process"), + }; + let cb = move || __rust_begin_short_backtrace(f); + run_test_inner(desc, opts.nocapture, strategy, Box::new(cb), concurrency); + } + StaticTestFn(f) => { let cb = move || __rust_begin_short_backtrace(f); - run_test_inner(desc, monitor_ch, opts.nocapture, Box::new(cb), concurrency) + run_test_inner(desc, opts.nocapture, strategy, Box::new(cb), concurrency); } - StaticTestFn(f) => run_test_inner( - desc, - monitor_ch, - opts.nocapture, - Box::new(move || __rust_begin_short_backtrace(f)), - concurrency, - ), } } @@ -1467,7 +1636,9 @@ fn __rust_begin_short_backtrace(f: F) { f() } -fn calc_result(desc: &TestDesc, task_result: Result<(), Box>) -> TestResult { +fn calc_result<'a>(desc: &TestDesc, + task_result: Result<(), &'a (dyn Any + 'static + Send)>) +-> TestResult { match (&desc.should_panic, task_result) { (&ShouldPanic::No, Ok(())) | (&ShouldPanic::Yes, Err(_)) => TrOk, (&ShouldPanic::YesWithMessage(msg), Err(ref err)) => { @@ -1640,14 +1811,16 @@ where } pub mod bench { - use super::{BenchMode, BenchSamples, Bencher, MonitorMsg, Sender, Sink, TestDesc, TestResult}; + use super::{ + BenchMode, BenchSamples, Bencher, MonitorMsg, Sender, Sink, TestDesc, TestOpts, TestResult + }; use crate::stats; use std::cmp; use std::io; use std::panic::{catch_unwind, AssertUnwindSafe}; use std::sync::{Arc, Mutex}; - pub fn benchmark(desc: TestDesc, monitor_ch: Sender, nocapture: bool, f: F) + pub fn benchmark(opts: &TestOpts, desc: TestDesc, monitor_ch: Sender, f: F) where F: FnMut(&mut Bencher), { @@ -1658,12 +1831,10 @@ pub mod bench { }; let data = Arc::new(Mutex::new(Vec::new())); - let data2 = data.clone(); - - let oldio = if !nocapture { + let oldio = if !opts.nocapture { Some(( - io::set_print(Some(Box::new(Sink(data2.clone())))), - io::set_panic(Some(Box::new(Sink(data2)))), + io::set_print(Some(Box::new(Sink(data.clone())))), + io::set_panic(Some(Box::new(Sink(data.clone())))), )) } else { None @@ -1674,7 +1845,7 @@ pub mod bench { if let Some((printio, panicio)) = oldio { io::set_print(printio); io::set_panic(panicio); - }; + } let test_result = match result { //bs.bench(f) { diff --git a/src/libtest/tests.rs b/src/libtest/tests.rs index afc4217ec1ba2..2c4320df12565 100644 --- a/src/libtest/tests.rs +++ b/src/libtest/tests.rs @@ -1,7 +1,7 @@ use super::*; use crate::test::{ - filter_tests, parse_opts, run_test, DynTestFn, DynTestName, MetricMap, RunIgnored, + filter_tests, parse_opts, run_test, DynTestFn, DynTestName, MetricMap, RunIgnored, RunStrategy, ShouldPanic, StaticTestName, TestDesc, TestDescAndFn, TestOpts, TrFailed, TrFailedMsg, TrIgnored, TrOk, }; @@ -66,7 +66,7 @@ pub fn do_not_run_ignored_tests() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, desc, tx, Concurrent::No); + run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No); let (_, res, _) = rx.recv().unwrap(); assert!(res != TrOk); } @@ -84,7 +84,7 @@ pub fn ignored_tests_result_in_ignored() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, desc, tx, Concurrent::No); + run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No); let (_, res, _) = rx.recv().unwrap(); assert!(res == TrIgnored); } @@ -104,7 +104,7 @@ fn test_should_panic() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, desc, tx, Concurrent::No); + run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No); let (_, res, _) = rx.recv().unwrap(); assert!(res == TrOk); } @@ -124,7 +124,7 @@ fn test_should_panic_good_message() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, desc, tx, Concurrent::No); + run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No); let (_, res, _) = rx.recv().unwrap(); assert!(res == TrOk); } @@ -146,7 +146,7 @@ fn test_should_panic_bad_message() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, desc, tx, Concurrent::No); + run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No); let (_, res, _) = rx.recv().unwrap(); assert!(res == TrFailedMsg(format!("{} '{}'", failed_msg, expected))); } @@ -164,7 +164,7 @@ fn test_should_panic_but_succeeds() { testfn: DynTestFn(Box::new(f)), }; let (tx, rx) = channel(); - run_test(&TestOpts::new(), false, desc, tx, Concurrent::No); + run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No); let (_, res, _) = rx.recv().unwrap(); assert!(res == TrFailed); } From 52e49198dc367a9428d731f90c4d18b8e84a3bb2 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 11 Sep 2019 21:22:06 -0700 Subject: [PATCH 30/50] Only spawn subprocesses when panic=abort --- src/librustc_interface/passes.rs | 1 + src/libsyntax_ext/test_harness.rs | 14 ++++- src/libtest/lib.rs | 96 +++++++++++++++++++------------ 3 files changed, 73 insertions(+), 38 deletions(-) diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index e8e8da6733471..84462f65a4e87 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -446,6 +446,7 @@ fn configure_and_expand_inner<'a>( &mut krate, sess.diagnostic(), &sess.features_untracked(), + sess.panic_strategy(), ) }); diff --git a/src/libsyntax_ext/test_harness.rs b/src/libsyntax_ext/test_harness.rs index 56de0c97f81c0..73841a39f5884 100644 --- a/src/libsyntax_ext/test_harness.rs +++ b/src/libsyntax_ext/test_harness.rs @@ -2,6 +2,7 @@ use log::debug; use smallvec::{smallvec, SmallVec}; +use rustc_target::spec::PanicStrategy; use syntax::ast::{self, Ident}; use syntax::attr; use syntax::entry::{self, EntryPointType}; @@ -25,6 +26,7 @@ struct Test { struct TestCtxt<'a> { ext_cx: ExtCtxt<'a>, + panic_strategy: PanicStrategy, def_site: Span, test_cases: Vec, reexport_test_harness_main: Option, @@ -40,6 +42,7 @@ pub fn inject( krate: &mut ast::Crate, span_diagnostic: &errors::Handler, features: &Features, + panic_strategy: PanicStrategy, ) { // Check for #![reexport_test_harness_main = "some_name"] which gives the // main test function the name `some_name` without hygiene. This needs to be @@ -54,7 +57,7 @@ pub fn inject( if should_test { generate_test_harness(sess, resolver, reexport_test_harness_main, - krate, features, test_runner) + krate, features, panic_strategy, test_runner) } } @@ -183,6 +186,7 @@ fn generate_test_harness(sess: &ParseSess, reexport_test_harness_main: Option, krate: &mut ast::Crate, features: &Features, + panic_strategy: PanicStrategy, test_runner: Option) { let mut econfig = ExpansionConfig::default("test".to_string()); econfig.features = Some(features); @@ -203,6 +207,7 @@ fn generate_test_harness(sess: &ParseSess, let cx = TestCtxt { ext_cx, + panic_strategy, def_site, test_cases: Vec::new(), reexport_test_harness_main, @@ -248,9 +253,14 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P { let ecx = &cx.ext_cx; let test_id = Ident::new(sym::test, sp); + let runner_name = match cx.panic_strategy { + PanicStrategy::Unwind => "test_main_static", + PanicStrategy::Abort => "test_main_static_abort", + }; + // test::test_main_static(...) let mut test_runner = cx.test_runner.clone().unwrap_or( - ecx.path(sp, vec![test_id, ecx.ident_of("test_main_static", sp)])); + ecx.path(sp, vec![test_id, ecx.ident_of(runner_name, sp)])); test_runner.span = sp; diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index fab1452cb7be3..3a5a339121584 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -261,12 +261,14 @@ impl Metric { #[derive(Copy, Clone, Debug)] pub struct Options { display_output: bool, + panic_abort: bool, } impl Options { pub fn new() -> Options { Options { display_output: false, + panic_abort: false, } } @@ -274,24 +276,16 @@ impl Options { self.display_output = display_output; self } + + pub fn panic_abort(mut self, panic_abort: bool) -> Options { + self.panic_abort = panic_abort; + self + } } // The default console test runner. It accepts the command line // arguments and a vector of test_descs. pub fn test_main(args: &[String], tests: Vec, options: Option) { - // If we're being run in SpawnedSecondary mode, run the test here. run_test - // will then exit the process. - if let Ok(name) = env::var(SECONDARY_TEST_INVOKER_VAR) { - let test = tests - .into_iter() - .filter(|test| test.desc.name.as_slice() == name) - .next() - .expect("couldn't find a test with the provided name"); - let opts = parse_opts(&[]).unwrap().unwrap(); - run_test(&opts, false, test, RunStrategy::SpawnedSecondary, Concurrent::No); - unreachable!(); - } - let mut opts = match parse_opts(args) { Some(Ok(o)) => o, Some(Err(msg)) => { @@ -320,32 +314,63 @@ pub fn test_main(args: &[String], tests: Vec, options: Option is used in order to effect ownership-transfer -// semantics into parallel test runners, which in turn requires a Vec<> -// rather than a &[]. +/// A variant optimized for invocation with a static test vector. +/// This will panic (intentionally) when fed any dynamic tests. +/// +/// This is the entry point for the main function generated by `rustc --test` +/// when panic=unwind. pub fn test_main_static(tests: &[&TestDescAndFn]) { let args = env::args().collect::>(); - let owned_tests = tests - .iter() - .map(|t| match t.testfn { - StaticTestFn(f) => TestDescAndFn { - testfn: StaticTestFn(f), - desc: t.desc.clone(), - }, - StaticBenchFn(f) => TestDescAndFn { - testfn: StaticBenchFn(f), - desc: t.desc.clone(), - }, - _ => panic!("non-static tests passed to test::test_main_static"), - }) - .collect(); + let owned_tests: Vec<_> = tests.iter().map(make_owned_test).collect(); test_main(&args, owned_tests, None) } +/// A variant optimized for invocation with a static test vector. +/// This will panic (intentionally) when fed any dynamic tests. +/// +/// Runs tests in panic=abort mode, which involves spawning subprocesses for +/// tests. +/// +/// This is the entry point for the main function generated by `rustc --test` +/// when panic=abort. +pub fn test_main_static_abort(tests: &[&TestDescAndFn]) { + // If we're being run in SpawnedSecondary mode, run the test here. run_test + // will then exit the process. + if let Ok(name) = env::var(SECONDARY_TEST_INVOKER_VAR) { + let test = tests + .iter() + .filter(|test| test.desc.name.as_slice() == name) + .map(make_owned_test) + .next() + .expect("couldn't find a test with the provided name"); + let opts = parse_opts(&[]).unwrap().unwrap(); + run_test(&opts, false, test, RunStrategy::SpawnedSecondary, Concurrent::No); + unreachable!(); + } + + let args = env::args().collect::>(); + let owned_tests: Vec<_> = tests.iter().map(make_owned_test).collect(); + test_main(&args, owned_tests, Some(Options::new().panic_abort(true))) +} + +/// Clones static values for putting into a dynamic vector, which test_main() +/// needs to hand out ownership of tests to parallel test runners. +/// +/// This will panic when fed any dynamic tests, because they cannot be cloned. +fn make_owned_test(test: &&TestDescAndFn) -> TestDescAndFn { + match test.testfn { + StaticTestFn(f) => TestDescAndFn { + testfn: StaticTestFn(f), + desc: test.desc.clone(), + }, + StaticBenchFn(f) => TestDescAndFn { + testfn: StaticBenchFn(f), + desc: test.desc.clone(), + }, + _ => panic!("non-static tests passed to test::test_main_static"), + } +} + /// Invoked when unit tests terminate. Should panic if the unit /// Tests is considered a failure. By default, invokes `report()` /// and checks for a `0` result. @@ -1147,8 +1172,7 @@ where let mut pending = 0; let (tx, rx) = channel::(); - // TODO - let run_strategy = if true { + let run_strategy = if opts.options.panic_abort { RunStrategy::SpawnPrimary(tx) } else { RunStrategy::InProcess(tx) From c6b4fb7340b19fb686b2852ddc71ddb60f90023f Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 12 Sep 2019 16:19:53 -0700 Subject: [PATCH 31/50] Remove panic_unwind requirement from libtest --- src/libtest/lib.rs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index 3a5a339121584..605c9477361d5 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -35,16 +35,6 @@ use getopts; extern crate libc; use term; -// FIXME(#54291): rustc and/or LLVM don't yet support building with panic-unwind -// on aarch64-pc-windows-msvc, or thumbv7a-pc-windows-msvc -// so we don't link libtest against libunwind (for the time being) -// even though it means that libtest won't be fully functional on -// these platforms. -// -// See also: https://github.com/rust-lang/rust/issues/54190#issuecomment-422904437 -#[cfg(not(all(windows, any(target_arch = "aarch64", target_arch = "arm"))))] -extern crate panic_unwind; - pub use self::ColorConfig::*; use self::NamePadding::*; use self::OutputLocation::*; @@ -1488,11 +1478,7 @@ pub fn run_test( ) { let TestDescAndFn { desc, testfn } = test; - let ignore_because_panic_abort = cfg!(target_arch = "wasm32") - && !cfg!(target_os = "emscripten") - && desc.should_panic != ShouldPanic::No; - - if force_ignore || desc.ignore || ignore_because_panic_abort { + if force_ignore || desc.ignore { match strategy { RunStrategy::InProcess(tx) | RunStrategy::SpawnPrimary(tx) => { tx.send((desc, TrIgnored, Vec::new())).unwrap(); From df24055bcae3269908ed0da22b7119b8d282c5cd Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 12 Sep 2019 19:04:37 -0700 Subject: [PATCH 32/50] Factor inner test runners out, simplify RunStrategy --- src/libtest/lib.rs | 271 ++++++++++++++++++++++----------------------- 1 file changed, 131 insertions(+), 140 deletions(-) diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index 605c9477361d5..f270d0341b226 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -333,9 +333,12 @@ pub fn test_main_static_abort(tests: &[&TestDescAndFn]) { .map(make_owned_test) .next() .expect("couldn't find a test with the provided name"); - let opts = parse_opts(&[]).unwrap().unwrap(); - run_test(&opts, false, test, RunStrategy::SpawnedSecondary, Concurrent::No); - unreachable!(); + let TestDescAndFn { desc, testfn } = test; + let testfn = match testfn { + StaticTestFn(f) => f, + _ => panic!("only static tests are supported"), + }; + run_test_in_spawned_subprocess(desc, Box::new(testfn)); } let args = env::args().collect::>(); @@ -1088,31 +1091,16 @@ impl Write for Sink { } } -#[derive(Clone)] +#[derive(Clone, Copy)] pub enum RunStrategy { /// Runs the test in the current process, and sends the result back over the /// supplied channel. - InProcess(Sender), + 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 /// that's currently running. - SpawnPrimary(Sender), - - /// Runs the test in the current process, then exits the process. - /// Prints to stdout the custom result format that the parent process - /// expects in SpawnPrimary mode. - SpawnedSecondary, -} - -impl RunStrategy { - fn monitor_ch(self) -> Option> { - match self { - RunStrategy::InProcess(ch) => Some(ch), - RunStrategy::SpawnPrimary(ch) => Some(ch), - RunStrategy::SpawnedSecondary => None, - } - } + SpawnPrimary, } pub fn run_tests(opts: &TestOpts, tests: Vec, mut callback: F) -> io::Result<()> @@ -1163,9 +1151,9 @@ where let (tx, rx) = channel::(); let run_strategy = if opts.options.panic_abort { - RunStrategy::SpawnPrimary(tx) + RunStrategy::SpawnPrimary } else { - RunStrategy::InProcess(tx) + RunStrategy::InProcess }; let mut running_tests: TestMap = HashMap::default(); @@ -1203,7 +1191,7 @@ where while !remaining.is_empty() { let test = remaining.pop().unwrap(); callback(TeWait(test.desc.clone()))?; - run_test(opts, !opts.run_tests, test, run_strategy.clone(), Concurrent::No); + run_test(opts, !opts.run_tests, test, run_strategy, tx.clone(), Concurrent::No); let (test, result, stdout) = rx.recv().unwrap(); callback(TeResult(test, result, stdout))?; } @@ -1214,7 +1202,7 @@ where let timeout = Instant::now() + Duration::from_secs(TEST_WARN_TIMEOUT_S); running_tests.insert(test.desc.clone(), timeout); callback(TeWait(test.desc.clone()))?; //here no pad - run_test(opts, !opts.run_tests, test, run_strategy.clone(), Concurrent::Yes); + run_test(opts, !opts.run_tests, test, run_strategy, tx.clone(), Concurrent::Yes); pending += 1; } @@ -1246,7 +1234,7 @@ where // All benchmarks run at the end, in serial. for b in filtered_benchs { callback(TeWait(b.desc.clone()))?; - run_test(opts, false, b, run_strategy.clone(), Concurrent::No); + run_test(opts, false, b, run_strategy, tx.clone(), Concurrent::No); let (test, result, stdout) = rx.recv().unwrap(); callback(TeResult(test, result, stdout))?; } @@ -1474,17 +1462,17 @@ pub fn run_test( force_ignore: bool, test: TestDescAndFn, strategy: RunStrategy, + monitor_ch: Sender, concurrency: Concurrent, ) { let TestDescAndFn { desc, testfn } = test; - if force_ignore || desc.ignore { - match strategy { - RunStrategy::InProcess(tx) | RunStrategy::SpawnPrimary(tx) => { - tx.send((desc, TrIgnored, Vec::new())).unwrap(); - } - RunStrategy::SpawnedSecondary => panic!(), - } + let ignore_because_no_process_support = cfg!(target_arch = "wasm32") + && !cfg!(target_os = "emscripten") + && desc.should_panic != ShouldPanic::No; + + if force_ignore || desc.ignore || ignore_because_no_process_support { + monitor_ch.send((desc, TrIgnored, Vec::new())).unwrap(); return; } @@ -1492,111 +1480,16 @@ pub fn run_test( desc: TestDesc, nocapture: bool, strategy: RunStrategy, + monitor_ch: Sender, testfn: Box, concurrency: Concurrent, ) { - // Buffer for capturing standard I/O - let data = Arc::new(Mutex::new(Vec::new())); - let name = desc.name.clone(); + let runtest = move || { - // TODO split into functions match strategy { - RunStrategy::InProcess(monitor_ch) => { - let oldio = if !nocapture { - Some(( - io::set_print(Some(Box::new(Sink(data.clone())))), - io::set_panic(Some(Box::new(Sink(data.clone())))), - )) - } else { - None - }; - - let result = catch_unwind(AssertUnwindSafe(testfn)); - - if let Some((printio, panicio)) = oldio { - io::set_print(printio); - io::set_panic(panicio); - } - - let test_result = match result { - Ok(()) => calc_result(&desc, Ok(())), - Err(e) => calc_result(&desc, Err(e.as_ref())), - }; - let stdout = data.lock().unwrap().to_vec(); - monitor_ch.send((desc.clone(), test_result, stdout)).unwrap(); - } - - RunStrategy::SpawnPrimary(monitor_ch) => { - let (result, test_output) = (|| { - let args = env::args().collect::>(); - let current_exe = &args[0]; - let output = match Command::new(current_exe) - .env(SECONDARY_TEST_INVOKER_VAR, desc.name.as_slice()) - .output() { - Ok(out) => out, - Err(e) => { - let err = format!("Failed to spawn {} as child for test: {:?}", - args[0], e); - return (TrFailed, err.into_bytes()); - } - }; - - let std::process::Output { stdout, stderr, status } = output; - let mut test_output = stdout; - test_output.extend_from_slice(&stderr); - - let result = match (move || { - let exit_code = status.code().ok_or("child process was terminated")?; - TestResult::try_from(exit_code).map_err(|_| { - format!("child process returned unexpected exit code {}", exit_code) - }) - })() { - Ok(r) => r, - Err(e) => { - write!(&mut test_output, "Unexpected error: {}", e).unwrap(); - TrFailed - } - }; - - (result, test_output) - })(); - - monitor_ch.send((desc.clone(), result, test_output)).unwrap(); - } - - RunStrategy::SpawnedSecondary => { - let record_lock = Mutex::new(()); - let builtin_panic_hook = panic::take_hook(); - let record_result = Arc::new(move |panic_info: Option<&'_ PanicInfo<'_>>| { - let _lock = record_lock.lock().unwrap(); - - let test_result = match panic_info { - Some(info) => calc_result(&desc, Err(info.payload())), - None => calc_result(&desc, Ok(())), - }; - - // We don't support serializing TrFailedMsg, so just - // print the message out to stderr. - let test_result = match test_result { - TrFailedMsg(msg) => { - eprintln!("{}", msg); - TrFailed - } - _ => test_result, - }; - - if let Some(info) = panic_info { - builtin_panic_hook(info); - } - process::exit(test_result.try_into().unwrap()); - }); - let record_result2 = record_result.clone(); - panic::set_hook(Box::new(move |info| record_result2(Some(&info)))); - testfn(); - record_result(None); - unreachable!("panic=abort callback should have exited the process"); - } + RunStrategy::InProcess => run_test_in_process(desc, nocapture, testfn, monitor_ch), + RunStrategy::SpawnPrimary => spawn_test_subprocess(desc, monitor_ch), } }; @@ -1615,27 +1508,27 @@ pub fn run_test( match testfn { DynBenchFn(bencher) => { // Benchmarks aren't expected to panic, so we run them all in-process. - crate::bench::benchmark(opts, desc, strategy.monitor_ch().unwrap(), |harness| { + crate::bench::benchmark(desc, monitor_ch, opts.nocapture, |harness| { bencher.run(harness) }); } StaticBenchFn(benchfn) => { // Benchmarks aren't expected to panic, so we run them all in-process. - crate::bench::benchmark(opts, desc, strategy.monitor_ch().unwrap(), |harness| { + crate::bench::benchmark(desc, monitor_ch, opts.nocapture, |harness| { (benchfn.clone())(harness) }); } DynTestFn(f) => { match strategy { - RunStrategy::InProcess(_) => (), + RunStrategy::InProcess => (), _ => panic!("Cannot run dynamic test fn out-of-process"), }; let cb = move || __rust_begin_short_backtrace(f); - run_test_inner(desc, opts.nocapture, strategy, Box::new(cb), concurrency); + run_test_inner(desc, opts.nocapture, strategy, monitor_ch, Box::new(cb), concurrency); } StaticTestFn(f) => { let cb = move || __rust_begin_short_backtrace(f); - run_test_inner(desc, opts.nocapture, strategy, Box::new(cb), concurrency); + run_test_inner(desc, opts.nocapture, strategy, monitor_ch, Box::new(cb), concurrency); } } } @@ -1673,6 +1566,104 @@ fn calc_result<'a>(desc: &TestDesc, } } +fn run_test_in_process(desc: TestDesc, + nocapture: bool, + testfn: Box, + monitor_ch: Sender) { + // Buffer for capturing standard I/O + let data = Arc::new(Mutex::new(Vec::new())); + + let oldio = if !nocapture { + Some(( + io::set_print(Some(Box::new(Sink(data.clone())))), + io::set_panic(Some(Box::new(Sink(data.clone())))), + )) + } else { + None + }; + + let result = catch_unwind(AssertUnwindSafe(testfn)); + + if let Some((printio, panicio)) = oldio { + io::set_print(printio); + io::set_panic(panicio); + } + + let test_result = match result { + Ok(()) => calc_result(&desc, Ok(())), + Err(e) => calc_result(&desc, Err(e.as_ref())), + }; + let stdout = data.lock().unwrap().to_vec(); + monitor_ch.send((desc.clone(), test_result, stdout)).unwrap(); +} + +fn spawn_test_subprocess(desc: TestDesc, monitor_ch: Sender) { + let (result, test_output) = (|| { + let args = env::args().collect::>(); + let current_exe = &args[0]; + let output = match Command::new(current_exe) + .env(SECONDARY_TEST_INVOKER_VAR, desc.name.as_slice()) + .output() { + Ok(out) => out, + Err(e) => { + let err = format!("Failed to spawn {} as child for test: {:?}", args[0], e); + return (TrFailed, err.into_bytes()); + } + }; + + let std::process::Output { stdout, stderr, status } = output; + let mut test_output = stdout; + test_output.extend_from_slice(&stderr); + + let result = match (move || { + let exit_code = status.code().ok_or("child process was terminated")?; + TestResult::try_from(exit_code).map_err(|_| { + format!("child process returned unexpected exit code {}", exit_code) + }) + })() { + Ok(r) => r, + Err(e) => { + write!(&mut test_output, "Unexpected error: {}", e).unwrap(); + TrFailed + } + }; + + (result, test_output) + })(); + + monitor_ch.send((desc.clone(), result, test_output)).unwrap(); +} + +fn run_test_in_spawned_subprocess(desc: TestDesc, testfn: Box) -> ! { + let builtin_panic_hook = panic::take_hook(); + let record_result = Arc::new(move |panic_info: Option<&'_ PanicInfo<'_>>| { + let test_result = match panic_info { + Some(info) => calc_result(&desc, Err(info.payload())), + None => calc_result(&desc, Ok(())), + }; + + // We don't support serializing TrFailedMsg, so just + // print the message out to stderr. + let test_result = match test_result { + TrFailedMsg(msg) => { + eprintln!("{}", msg); + TrFailed + } + _ => test_result, + }; + + if let Some(info) = panic_info { + builtin_panic_hook(info); + } + process::exit(test_result.try_into().unwrap()); + }); + let record_result2 = record_result.clone(); + panic::set_hook(Box::new(move |info| record_result2(Some(&info)))); + testfn(); + record_result(None); + unreachable!("panic=abort callback should have exited the process") +} + #[derive(Clone, PartialEq)] pub struct MetricMap(BTreeMap); @@ -1822,7 +1813,7 @@ where pub mod bench { use super::{ - BenchMode, BenchSamples, Bencher, MonitorMsg, Sender, Sink, TestDesc, TestOpts, TestResult + BenchMode, BenchSamples, Bencher, MonitorMsg, Sender, Sink, TestDesc, TestResult }; use crate::stats; use std::cmp; @@ -1830,7 +1821,7 @@ pub mod bench { use std::panic::{catch_unwind, AssertUnwindSafe}; use std::sync::{Arc, Mutex}; - pub fn benchmark(opts: &TestOpts, desc: TestDesc, monitor_ch: Sender, f: F) + pub fn benchmark(desc: TestDesc, monitor_ch: Sender, nocapture: bool, f: F) where F: FnMut(&mut Bencher), { @@ -1841,7 +1832,7 @@ pub mod bench { }; let data = Arc::new(Mutex::new(Vec::new())); - let oldio = if !opts.nocapture { + let oldio = if !nocapture { Some(( io::set_print(Some(Box::new(Sink(data.clone())))), io::set_panic(Some(Box::new(Sink(data.clone())))), From e6015e41705d3b55a5e108cc4b9854ec37a78436 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 13 Sep 2019 17:44:09 -0700 Subject: [PATCH 33/50] Include signal from terminated child process on unix --- src/libtest/formatters/mod.rs | 9 +++++++++ src/libtest/lib.rs | 22 ++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/libtest/formatters/mod.rs b/src/libtest/formatters/mod.rs index cc30b06e5ec38..f431c9de7f118 100644 --- a/src/libtest/formatters/mod.rs +++ b/src/libtest/formatters/mod.rs @@ -21,3 +21,12 @@ pub(crate) trait OutputFormatter { ) -> io::Result<()>; fn write_run_finish(&mut self, state: &ConsoleTestState) -> io::Result; } + +pub(crate) fn write_stderr_delimiter(test_output: &mut Vec, test_name: &TestName) { + match test_output.last() { + Some(b'\n') => (), + Some(_) => test_output.push(b'\n'), + None => (), + } + write!(test_output, "---- {} stderr ----\n", test_name).unwrap(); +} diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index f270d0341b226..813463ff7dfe1 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -56,7 +56,7 @@ use std::io::prelude::*; use std::panic::{self, catch_unwind, AssertUnwindSafe, PanicInfo}; use std::path::PathBuf; use std::process; -use std::process::{Command, Termination}; +use std::process::{ExitStatus, Command, Termination}; use std::sync::mpsc::{channel, Sender}; use std::sync::{Arc, Mutex}; use std::thread; @@ -1613,10 +1613,11 @@ fn spawn_test_subprocess(desc: TestDesc, monitor_ch: Sender) { let std::process::Output { stdout, stderr, status } = output; let mut test_output = stdout; + formatters::write_stderr_delimiter(&mut test_output, &desc.name); test_output.extend_from_slice(&stderr); let result = match (move || { - let exit_code = status.code().ok_or("child process was terminated")?; + let exit_code = get_exit_code(status)?; TestResult::try_from(exit_code).map_err(|_| { format!("child process returned unexpected exit code {}", exit_code) }) @@ -1664,6 +1665,23 @@ fn run_test_in_spawned_subprocess(desc: TestDesc, testfn: Box Result { + status.code().ok_or("received no exit code from child process".into()) +} + +#[cfg(unix)] +fn get_exit_code(status: ExitStatus) -> Result { + use std::os::unix::process::ExitStatusExt; + match status.code() { + Some(code) => Ok(code), + None => match status.signal() { + Some(signal) => Err(format!("child process exited with signal {}", signal)), + None => Err("child process exited with unknown signal".into()), + } + } +} + #[derive(Clone, PartialEq)] pub struct MetricMap(BTreeMap); From c792c251f38a0de3cc2a2826b34dab18b1949ad0 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 18 Sep 2019 18:19:13 -0700 Subject: [PATCH 34/50] Use just two return codes to communicate result --- src/libtest/lib.rs | 74 ++++++++++++++++------------------------------ 1 file changed, 26 insertions(+), 48 deletions(-) diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index 813463ff7dfe1..2a2f32f97162d 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -47,7 +47,6 @@ use std::any::Any; use std::borrow::Cow; use std::cmp; use std::collections::BTreeMap; -use std::convert::{TryFrom, TryInto}; use std::env; use std::fmt; use std::fs::File; @@ -70,6 +69,12 @@ const QUIET_MODE_MAX_COLUMN: usize = 100; // insert a '\n' after 100 tests in qu const SECONDARY_TEST_INVOKER_VAR: &'static str = "__RUST_TEST_INVOKE"; +// Return codes for secondary process. +// Start somewhere other than 0 so we know the return code means what we think +// it means. +const TR_OK: i32 = 50; +const TR_FAILED: i32 = 51; + // to be used by rustc to compile tests in libtest pub mod test { pub use crate::{ @@ -712,41 +717,6 @@ pub enum TestResult { unsafe impl Send for TestResult {} -// Return codes for TestResult. -// Start somewhere other than 0 so we know the return code means what we think -// it means. -const TR_OK: i32 = 50; -const TR_FAILED: i32 = 51; -const TR_IGNORED: i32 = 52; -const TR_ALLOWED_FAIL: i32 = 53; - -impl TryFrom for i32 { - type Error = &'static str; - fn try_from(val: TestResult) -> Result { - Ok(match val { - TrOk => TR_OK, - TrFailed => TR_FAILED, - TrIgnored => TR_IGNORED, - TrAllowedFail => TR_ALLOWED_FAIL, - TrFailedMsg(..) | - TrBench(..) => return Err("can't convert variant to i32"), - }) - } -} - -impl TryFrom for TestResult { - type Error = &'static str; - fn try_from(val: i32) -> Result { - Ok(match val { - TR_OK => TrOk, - TR_FAILED => TrFailed, - TR_IGNORED => TrIgnored, - TR_ALLOWED_FAIL => TrAllowedFail, - _ => return Err("unrecognized return code"), - }) - } -} - enum OutputLocation { Pretty(Box), Raw(T), @@ -1566,6 +1536,15 @@ fn calc_result<'a>(desc: &TestDesc, } } +fn get_result_from_exit_code(desc: &TestDesc, code: i32) -> TestResult { + match (desc.allow_fail, code) { + (_, TR_OK) => TrOk, + (true, TR_FAILED) => TrAllowedFail, + (false, TR_FAILED) => TrFailed, + (_, _) => TrFailedMsg(format!("got unexpected return code {}", code)), + } +} + fn run_test_in_process(desc: TestDesc, nocapture: bool, testfn: Box, @@ -1616,11 +1595,9 @@ fn spawn_test_subprocess(desc: TestDesc, monitor_ch: Sender) { formatters::write_stderr_delimiter(&mut test_output, &desc.name); test_output.extend_from_slice(&stderr); - let result = match (move || { + let result = match (|| -> Result { let exit_code = get_exit_code(status)?; - TestResult::try_from(exit_code).map_err(|_| { - format!("child process returned unexpected exit code {}", exit_code) - }) + Ok(get_result_from_exit_code(&desc, exit_code)) })() { Ok(r) => r, Err(e) => { @@ -1645,18 +1622,19 @@ fn run_test_in_spawned_subprocess(desc: TestDesc, testfn: Box { - eprintln!("{}", msg); - TrFailed - } - _ => test_result, - }; + if let TrFailedMsg(msg) = &test_result { + eprintln!("{}", msg); + } if let Some(info) = panic_info { builtin_panic_hook(info); } - process::exit(test_result.try_into().unwrap()); + + if let TrOk = test_result { + process::exit(TR_OK); + } else { + process::exit(TR_FAILED); + } }); let record_result2 = record_result.clone(); panic::set_hook(Box::new(move |info| record_result2(Some(&info)))); From c396c126c1cc69a813a986d66749cbf1d004dfd5 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 19 Sep 2019 18:34:23 -0700 Subject: [PATCH 35/50] Add test for libtest panic=abort mode --- src/test/ui/test-panic-abort.rs | 39 +++++++++++++++++++++++++ src/test/ui/test-panic-abort.run.stdout | 30 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 src/test/ui/test-panic-abort.rs create mode 100644 src/test/ui/test-panic-abort.run.stdout diff --git a/src/test/ui/test-panic-abort.rs b/src/test/ui/test-panic-abort.rs new file mode 100644 index 0000000000000..9947355863f23 --- /dev/null +++ b/src/test/ui/test-panic-abort.rs @@ -0,0 +1,39 @@ +// no-prefer-dynamic +// compile-flags: --test -Cpanic=abort +// run-flags: --test-threads=1 +// run-fail +// check-run-results + +#![cfg(test)] + +use std::io::Write; + +#[test] +fn it_works() { + assert_eq!(1 + 1, 2); +} + +#[test] +#[should_panic] +fn it_panics() { + assert_eq!(1 + 1, 4); +} + +#[test] +fn it_fails() { + println!("hello, world"); + writeln!(std::io::stdout(), "testing123").unwrap(); + writeln!(std::io::stderr(), "testing321").unwrap(); + assert_eq!(1 + 1, 5); +} + +#[test] +fn it_exits() { + std::process::exit(123); +} + +#[test] +fn it_segfaults() { + let x = unsafe { *(0 as *const u64) }; + println!("output: {}", x); +} diff --git a/src/test/ui/test-panic-abort.run.stdout b/src/test/ui/test-panic-abort.run.stdout new file mode 100644 index 0000000000000..0e21b44c6669e --- /dev/null +++ b/src/test/ui/test-panic-abort.run.stdout @@ -0,0 +1,30 @@ + +running 5 tests +test it_exits ... FAILED +test it_fails ... FAILED +test it_panics ... ok +test it_segfaults ... ok +test it_works ... ok + +failures: + +---- it_exits stdout ---- +---- it_exits stderr ---- +note: got unexpected return code 123 +---- it_fails stdout ---- +hello, world +testing123 +---- it_fails stderr ---- +testing321 +thread 'main' panicked at 'assertion failed: `(left == right)` + left: `2`, + right: `5`', $DIR/test-panic-abort.rs:27:5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. + + +failures: + it_exits + it_fails + +test result: FAILED. 3 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out + From 6f36f8710acfcba8de9688c61136e129e8b11ccb Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 19 Sep 2019 19:33:38 -0700 Subject: [PATCH 36/50] Put panic=abort test support behind -Z panic_abort_tests --- src/librustc/session/config.rs | 2 ++ src/librustc_interface/passes.rs | 1 + src/libsyntax_ext/test_harness.rs | 5 +++++ src/test/ui/test-panic-abort-disabled.rs | 18 ++++++++++++++++++ .../ui/test-panic-abort-disabled.run.stdout | 3 +++ src/test/ui/test-panic-abort.rs | 2 +- 6 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/test-panic-abort-disabled.rs create mode 100644 src/test/ui/test-panic-abort-disabled.run.stdout diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 723855c7c29cf..e566a2ae42e58 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1295,6 +1295,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "show extended diagnostic help"), terminal_width: Option = (None, parse_opt_uint, [UNTRACKED], "set the current terminal width"), + panic_abort_tests: bool = (false, parse_bool, [TRACKED], + "support compiling tests with panic=abort"), continue_parse_after_error: bool = (false, parse_bool, [TRACKED], "attempt to recover from parse errors (experimental)"), dep_tasks: bool = (false, parse_bool, [UNTRACKED], diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 84462f65a4e87..032bd2c02561d 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -447,6 +447,7 @@ fn configure_and_expand_inner<'a>( sess.diagnostic(), &sess.features_untracked(), sess.panic_strategy(), + sess.opts.debugging_opts.panic_abort_tests, ) }); diff --git a/src/libsyntax_ext/test_harness.rs b/src/libsyntax_ext/test_harness.rs index 73841a39f5884..158a7c2df8a0f 100644 --- a/src/libsyntax_ext/test_harness.rs +++ b/src/libsyntax_ext/test_harness.rs @@ -43,6 +43,7 @@ pub fn inject( span_diagnostic: &errors::Handler, features: &Features, panic_strategy: PanicStrategy, + enable_panic_abort_tests: bool, ) { // Check for #![reexport_test_harness_main = "some_name"] which gives the // main test function the name `some_name` without hygiene. This needs to be @@ -55,6 +56,10 @@ pub fn inject( // even in non-test builds let test_runner = get_test_runner(span_diagnostic, &krate); + let panic_strategy = match (panic_strategy, enable_panic_abort_tests) { + (PanicStrategy::Abort, true) => PanicStrategy::Abort, + _ => PanicStrategy::Unwind, + }; if should_test { generate_test_harness(sess, resolver, reexport_test_harness_main, krate, features, panic_strategy, test_runner) diff --git a/src/test/ui/test-panic-abort-disabled.rs b/src/test/ui/test-panic-abort-disabled.rs new file mode 100644 index 0000000000000..a0e7cb1a0db05 --- /dev/null +++ b/src/test/ui/test-panic-abort-disabled.rs @@ -0,0 +1,18 @@ +// no-prefer-dynamic +// compile-flags: --test -Cpanic=abort +// run-flags: --test-threads=1 +// run-fail +// check-run-results + +#![cfg(test)] + +#[test] +fn it_works() { + assert_eq!(1 + 1, 2); +} + +#[test] +#[should_panic] +fn it_panics() { + assert_eq!(1 + 1, 4); +} diff --git a/src/test/ui/test-panic-abort-disabled.run.stdout b/src/test/ui/test-panic-abort-disabled.run.stdout new file mode 100644 index 0000000000000..171fd2edcf63e --- /dev/null +++ b/src/test/ui/test-panic-abort-disabled.run.stdout @@ -0,0 +1,3 @@ + +running 2 tests +test it_panics ... \ No newline at end of file diff --git a/src/test/ui/test-panic-abort.rs b/src/test/ui/test-panic-abort.rs index 9947355863f23..0daccb3ec8aae 100644 --- a/src/test/ui/test-panic-abort.rs +++ b/src/test/ui/test-panic-abort.rs @@ -1,5 +1,5 @@ // no-prefer-dynamic -// compile-flags: --test -Cpanic=abort +// compile-flags: --test -Cpanic=abort -Zpanic_abort_tests // run-flags: --test-threads=1 // run-fail // check-run-results From 9c2d227fbafd6d2e64890d0b3e041a87054a6d2c Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 20 Sep 2019 15:02:42 -0700 Subject: [PATCH 37/50] Ignore panic=abort tests on wasm, emscripten --- src/test/ui/test-panic-abort-disabled.rs | 3 +++ src/test/ui/test-panic-abort.rs | 3 +++ src/test/ui/test-panic-abort.run.stdout | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/ui/test-panic-abort-disabled.rs b/src/test/ui/test-panic-abort-disabled.rs index a0e7cb1a0db05..ea979258a556e 100644 --- a/src/test/ui/test-panic-abort-disabled.rs +++ b/src/test/ui/test-panic-abort-disabled.rs @@ -4,6 +4,9 @@ // run-fail // check-run-results +// ignore-wasm no panic or subprocess support +// ignore-emscripten no panic or subprocess support + #![cfg(test)] #[test] diff --git a/src/test/ui/test-panic-abort.rs b/src/test/ui/test-panic-abort.rs index 0daccb3ec8aae..9fe54ab737357 100644 --- a/src/test/ui/test-panic-abort.rs +++ b/src/test/ui/test-panic-abort.rs @@ -4,6 +4,9 @@ // run-fail // check-run-results +// ignore-wasm no panic or subprocess support +// ignore-emscripten no panic or subprocess support + #![cfg(test)] use std::io::Write; diff --git a/src/test/ui/test-panic-abort.run.stdout b/src/test/ui/test-panic-abort.run.stdout index 0e21b44c6669e..3934e2d5e4884 100644 --- a/src/test/ui/test-panic-abort.run.stdout +++ b/src/test/ui/test-panic-abort.run.stdout @@ -18,7 +18,7 @@ testing123 testing321 thread 'main' panicked at 'assertion failed: `(left == right)` left: `2`, - right: `5`', $DIR/test-panic-abort.rs:27:5 + right: `5`', $DIR/test-panic-abort.rs:30:5 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. From 8233b28169b5770e15abcbc651b7d6328956b61d Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 20 Sep 2019 16:29:36 -0700 Subject: [PATCH 38/50] Remove segfault test since behavior varies by platform --- src/test/ui/test-panic-abort.rs | 6 ------ src/test/ui/test-panic-abort.run.stdout | 5 ++--- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/test/ui/test-panic-abort.rs b/src/test/ui/test-panic-abort.rs index 9fe54ab737357..415ecbf7b38cd 100644 --- a/src/test/ui/test-panic-abort.rs +++ b/src/test/ui/test-panic-abort.rs @@ -34,9 +34,3 @@ fn it_fails() { fn it_exits() { std::process::exit(123); } - -#[test] -fn it_segfaults() { - let x = unsafe { *(0 as *const u64) }; - println!("output: {}", x); -} diff --git a/src/test/ui/test-panic-abort.run.stdout b/src/test/ui/test-panic-abort.run.stdout index 3934e2d5e4884..32c96b4f849c8 100644 --- a/src/test/ui/test-panic-abort.run.stdout +++ b/src/test/ui/test-panic-abort.run.stdout @@ -1,9 +1,8 @@ -running 5 tests +running 4 tests test it_exits ... FAILED test it_fails ... FAILED test it_panics ... ok -test it_segfaults ... ok test it_works ... ok failures: @@ -26,5 +25,5 @@ failures: it_exits it_fails -test result: FAILED. 3 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out +test result: FAILED. 2 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out From 0918dc4e5990592c28484d1d64bec41cb9a4a301 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 21 Sep 2019 15:49:15 +0200 Subject: [PATCH 39/50] or-patterns: middle/dead: remove `top_pats_hack`. Also tweak walkers on `Pat`. --- src/librustc/hir/mod.rs | 62 ++++++++++++------- src/librustc/hir/pat_util.rs | 112 ++++++++++++++++------------------- src/librustc/middle/dead.rs | 21 +++---- 3 files changed, 100 insertions(+), 95 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index edb55ab3caaf4..87fc385acd64d 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -882,39 +882,61 @@ impl fmt::Debug for Pat { impl Pat { // FIXME(#19596) this is a workaround, but there should be a better way - fn walk_(&self, it: &mut G) -> bool - where G: FnMut(&Pat) -> bool - { + fn walk_short_(&self, it: &mut impl FnMut(&Pat) -> bool) -> bool { if !it(self) { return false; } + use PatKind::*; match &self.node { - PatKind::Binding(.., Some(p)) => p.walk_(it), - PatKind::Struct(_, fields, _) => fields.iter().all(|field| field.pat.walk_(it)), - PatKind::TupleStruct(_, s, _) | PatKind::Tuple(s, _) | PatKind::Or(s) => { - s.iter().all(|p| p.walk_(it)) - } - PatKind::Box(s) | PatKind::Ref(s, _) => s.walk_(it), - PatKind::Slice(before, slice, after) => { + Wild | Lit(_) | Range(..) | Binding(.., None) | Path(_) => true, + Box(s) | Ref(s, _) | Binding(.., Some(s)) => s.walk_short_(it), + Struct(_, fields, _) => fields.iter().all(|field| field.pat.walk_short_(it)), + TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().all(|p| p.walk_short_(it)), + Slice(before, slice, after) => { before.iter() .chain(slice.iter()) .chain(after.iter()) - .all(|p| p.walk_(it)) + .all(|p| p.walk_short_(it)) } - PatKind::Wild | - PatKind::Lit(_) | - PatKind::Range(..) | - PatKind::Binding(..) | - PatKind::Path(_) => { - true + } + } + + /// Walk the pattern in left-to-right order, + /// short circuiting (with `.all(..)`) if `false` is returned. + /// + /// Note that when visiting e.g. `Tuple(ps)`, + /// if visiting `ps[0]` returns `false`, + /// then `ps[1]` will not be visited. + pub fn walk_short(&self, mut it: impl FnMut(&Pat) -> bool) -> bool { + self.walk_short_(&mut it) + } + + // FIXME(#19596) this is a workaround, but there should be a better way + fn walk_(&self, it: &mut impl FnMut(&Pat) -> bool) { + if !it(self) { + return; + } + + use PatKind::*; + match &self.node { + Wild | Lit(_) | Range(..) | Binding(.., None) | Path(_) => {}, + Box(s) | Ref(s, _) | Binding(.., Some(s)) => s.walk_(it), + Struct(_, fields, _) => fields.iter().for_each(|field| field.pat.walk_(it)), + TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().for_each(|p| p.walk_(it)), + Slice(before, slice, after) => { + before.iter() + .chain(slice.iter()) + .chain(after.iter()) + .for_each(|p| p.walk_(it)) } } } - pub fn walk(&self, mut it: F) -> bool - where F: FnMut(&Pat) -> bool - { + /// Walk the pattern in left-to-right order. + /// + /// If `it(pat)` returns `false`, the children are not visited. + pub fn walk(&self, mut it: impl FnMut(&Pat) -> bool) { self.walk_(&mut it) } } diff --git a/src/librustc/hir/pat_util.rs b/src/librustc/hir/pat_util.rs index ea35418bc1ba7..118e168f87767 100644 --- a/src/librustc/hir/pat_util.rs +++ b/src/librustc/hir/pat_util.rs @@ -66,9 +66,7 @@ impl hir::Pat { /// Call `f` on every "binding" in a pattern, e.g., on `a` in /// `match foo() { Some(a) => (), None => () }` - pub fn each_binding(&self, mut f: F) - where F: FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident), - { + pub fn each_binding(&self, mut f: impl FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident)) { self.walk(|p| { if let PatKind::Binding(binding_mode, _, ident, _) = p.node { f(binding_mode, p.hir_id, p.span, ident); @@ -81,59 +79,53 @@ impl hir::Pat { /// `match foo() { Some(a) => (), None => () }`. /// /// When encountering an or-pattern `p_0 | ... | p_n` only `p_0` will be visited. - pub fn each_binding_or_first(&self, c: &mut F) - where F: FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident), - { - match &self.node { - PatKind::Binding(bm, _, ident, sub) => { - c(*bm, self.hir_id, self.span, *ident); - sub.iter().for_each(|p| p.each_binding_or_first(c)); - } - PatKind::Or(ps) => ps[0].each_binding_or_first(c), - PatKind::Struct(_, fs, _) => fs.iter().for_each(|f| f.pat.each_binding_or_first(c)), - PatKind::TupleStruct(_, ps, _) | PatKind::Tuple(ps, _) => { - ps.iter().for_each(|p| p.each_binding_or_first(c)); - } - PatKind::Box(p) | PatKind::Ref(p, _) => p.each_binding_or_first(c), - PatKind::Slice(before, slice, after) => { - before.iter() - .chain(slice.iter()) - .chain(after.iter()) - .for_each(|p| p.each_binding_or_first(c)); + pub fn each_binding_or_first( + &self, + f: &mut impl FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident), + ) { + self.walk(|p| match &p.node { + PatKind::Or(ps) => { + ps[0].each_binding_or_first(f); + false + }, + PatKind::Binding(bm, _, ident, _) => { + f(*bm, p.hir_id, p.span, *ident); + true } - PatKind::Wild | PatKind::Lit(_) | PatKind::Range(..) | PatKind::Path(_) => {} - } + _ => true, + }) } /// Checks if the pattern contains any patterns that bind something to /// an ident, e.g., `foo`, or `Foo(foo)` or `foo @ Bar(..)`. pub fn contains_bindings(&self) -> bool { - let mut contains_bindings = false; - self.walk(|p| { - if let PatKind::Binding(..) = p.node { - contains_bindings = true; - false // there's at least one binding, can short circuit now. - } else { - true - } - }); - contains_bindings + self.satisfies(|p| match p.node { + PatKind::Binding(..) => true, + _ => false, + }) } /// Checks if the pattern contains any patterns that bind something to /// an ident or wildcard, e.g., `foo`, or `Foo(_)`, `foo @ Bar(..)`, pub fn contains_bindings_or_wild(&self) -> bool { - let mut contains_bindings = false; - self.walk(|p| { - match p.node { - PatKind::Binding(..) | PatKind::Wild => { - contains_bindings = true; - false // there's at least one binding/wildcard, can short circuit now. - } - _ => true + self.satisfies(|p| match p.node { + PatKind::Binding(..) | PatKind::Wild => true, + _ => false, + }) + } + + /// Checks if the pattern satisfies the given predicate on some sub-pattern. + fn satisfies(&self, pred: impl Fn(&Self) -> bool) -> bool { + let mut satisfies = false; + self.walk_short(|p| { + if pred(p) { + satisfies = true; + false // Found one, can short circuit now. + } else { + true } }); - contains_bindings + satisfies } pub fn simple_ident(&self) -> Option { @@ -147,20 +139,20 @@ impl hir::Pat { /// Returns variants that are necessary to exist for the pattern to match. pub fn necessary_variants(&self) -> Vec { let mut variants = vec![]; - self.walk(|p| { - match p.node { - PatKind::Path(hir::QPath::Resolved(_, ref path)) | - PatKind::TupleStruct(hir::QPath::Resolved(_, ref path), ..) | - PatKind::Struct(hir::QPath::Resolved(_, ref path), ..) => { - match path.res { - Res::Def(DefKind::Variant, id) => variants.push(id), - Res::Def(DefKind::Ctor(CtorOf::Variant, ..), id) => variants.push(id), - _ => () - } + self.walk(|p| match &p.node { + PatKind::Or(_) => false, + PatKind::Path(hir::QPath::Resolved(_, path)) | + PatKind::TupleStruct(hir::QPath::Resolved(_, path), ..) | + PatKind::Struct(hir::QPath::Resolved(_, path), ..) => { + if let Res::Def(DefKind::Variant, id) + | Res::Def(DefKind::Ctor(CtorOf::Variant, ..), id) + = path.res + { + variants.push(id); } - _ => () + true } - true + _ => true, }); variants.sort(); variants.dedup(); @@ -176,14 +168,12 @@ impl hir::Pat { let mut result = None; self.each_binding(|annotation, _, _, _| { match annotation { - hir::BindingAnnotation::Ref => { - match result { - None | Some(hir::MutImmutable) => result = Some(hir::MutImmutable), - _ => (), - } + hir::BindingAnnotation::Ref => match result { + None | Some(hir::MutImmutable) => result = Some(hir::MutImmutable), + _ => {} } hir::BindingAnnotation::RefMut => result = Some(hir::MutMutable), - _ => (), + _ => {} } }); result diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index 1818dede3e229..2746794e2a2c1 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -259,20 +259,13 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> { } fn visit_arm(&mut self, arm: &'tcx hir::Arm) { - let pats = arm.top_pats_hack(); - if let [pat] = pats { - let variants = pat.necessary_variants(); - - // Inside the body, ignore constructions of variants - // necessary for the pattern to match. Those construction sites - // can't be reached unless the variant is constructed elsewhere. - let len = self.ignore_variant_stack.len(); - self.ignore_variant_stack.extend_from_slice(&variants); - intravisit::walk_arm(self, arm); - self.ignore_variant_stack.truncate(len); - } else { - intravisit::walk_arm(self, arm); - } + // Inside the body, ignore constructions of variants + // necessary for the pattern to match. Those construction sites + // can't be reached unless the variant is constructed elsewhere. + let len = self.ignore_variant_stack.len(); + self.ignore_variant_stack.extend(arm.pat.necessary_variants()); + intravisit::walk_arm(self, arm); + self.ignore_variant_stack.truncate(len); } fn visit_pat(&mut self, pat: &'tcx hir::Pat) { From 67d88f607ed831cf692387703c34441019b8db96 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Sep 2019 15:01:10 -0400 Subject: [PATCH 40/50] Remove constraints argument from path_all It was never used --- src/libsyntax/ext/build.rs | 11 +++++------ src/libsyntax_ext/deriving/clone.rs | 2 +- src/libsyntax_ext/deriving/cmp/eq.rs | 2 +- src/libsyntax_ext/deriving/generic/mod.rs | 2 +- src/libsyntax_ext/deriving/generic/ty.rs | 8 ++++---- src/libsyntax_ext/env.rs | 2 +- 6 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index f1d0e0b68f735..84a27fcb7dd94 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -14,20 +14,19 @@ pub trait AstBuilder {} impl<'a> ExtCtxt<'a> { pub fn path(&self, span: Span, strs: Vec ) -> ast::Path { - self.path_all(span, false, strs, vec![], vec![]) + self.path_all(span, false, strs, vec![]) } pub fn path_ident(&self, span: Span, id: ast::Ident) -> ast::Path { self.path(span, vec![id]) } pub fn path_global(&self, span: Span, strs: Vec ) -> ast::Path { - self.path_all(span, true, strs, vec![], vec![]) + self.path_all(span, true, strs, vec![]) } pub fn path_all(&self, span: Span, global: bool, mut idents: Vec , - args: Vec, - constraints: Vec ) + args: Vec) -> ast::Path { assert!(!idents.is_empty()); let add_root = global && !idents[0].is_path_segment_keyword(); @@ -39,8 +38,8 @@ impl<'a> ExtCtxt<'a> { segments.extend(idents.into_iter().map(|ident| { ast::PathSegment::from_ident(ident.with_span_pos(span)) })); - let args = if !args.is_empty() || !constraints.is_empty() { - ast::AngleBracketedArgs { args, constraints, span }.into() + let args = if !args.is_empty() { + ast::AngleBracketedArgs { args, constraints: Vec::new(), span }.into() } else { None }; diff --git a/src/libsyntax_ext/deriving/clone.rs b/src/libsyntax_ext/deriving/clone.rs index 4dd0ecfebefd4..9a4c540dc6f1f 100644 --- a/src/libsyntax_ext/deriving/clone.rs +++ b/src/libsyntax_ext/deriving/clone.rs @@ -115,7 +115,7 @@ fn cs_clone_shallow(name: &str, let span = cx.with_def_site_ctxt(span); let assert_path = cx.path_all(span, true, cx.std_path(&[sym::clone, Symbol::intern(helper_name)]), - vec![GenericArg::Type(ty)], vec![]); + vec![GenericArg::Type(ty)]); stmts.push(cx.stmt_let_type_only(span, cx.ty_path(assert_path))); } fn process_variant(cx: &mut ExtCtxt<'_>, stmts: &mut Vec, variant: &VariantData) { diff --git a/src/libsyntax_ext/deriving/cmp/eq.rs b/src/libsyntax_ext/deriving/cmp/eq.rs index 32ab47969ada4..471c92dd99949 100644 --- a/src/libsyntax_ext/deriving/cmp/eq.rs +++ b/src/libsyntax_ext/deriving/cmp/eq.rs @@ -56,7 +56,7 @@ fn cs_total_eq_assert(cx: &mut ExtCtxt<'_>, let span = cx.with_def_site_ctxt(span); let assert_path = cx.path_all(span, true, cx.std_path(&[sym::cmp, Symbol::intern(helper_name)]), - vec![GenericArg::Type(ty)], vec![]); + vec![GenericArg::Type(ty)]); stmts.push(cx.stmt_let_type_only(span, cx.ty_path(assert_path))); } fn process_variant(cx: &mut ExtCtxt<'_>, diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index aceee62e89b0c..5c332eccb62cc 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -664,7 +664,7 @@ impl<'a> TraitDef<'a> { }).collect(); // Create the type of `self`. - let path = cx.path_all(self.span, false, vec![type_ident], self_params, vec![]); + let path = cx.path_all(self.span, false, vec![type_ident], self_params); let self_type = cx.ty_path(path); let attr = cx.attribute(cx.meta_word(self.span, sym::automatically_derived)); diff --git a/src/libsyntax_ext/deriving/generic/ty.rs b/src/libsyntax_ext/deriving/generic/ty.rs index b341a07675206..6ae02a5cab199 100644 --- a/src/libsyntax_ext/deriving/generic/ty.rs +++ b/src/libsyntax_ext/deriving/generic/ty.rs @@ -82,12 +82,12 @@ impl<'a> Path<'a> { .collect(); match self.kind { - PathKind::Global => cx.path_all(span, true, idents, params, Vec::new()), - PathKind::Local => cx.path_all(span, false, idents, params, Vec::new()), + PathKind::Global => cx.path_all(span, true, idents, params), + PathKind::Local => cx.path_all(span, false, idents, params), PathKind::Std => { let def_site = cx.with_def_site_ctxt(DUMMY_SP); idents.insert(0, Ident::new(kw::DollarCrate, def_site)); - cx.path_all(span, false, idents, params, Vec::new()) + cx.path_all(span, false, idents, params) } } @@ -183,7 +183,7 @@ impl<'a> Ty<'a> { } }).collect(); - cx.path_all(span, false, vec![self_ty], params, vec![]) + cx.path_all(span, false, vec![self_ty], params) } Literal(ref p) => p.to_path(cx, span, self_ty, generics), Ptr(..) => cx.span_bug(span, "pointer in a path in generic `derive`"), diff --git a/src/libsyntax_ext/env.rs b/src/libsyntax_ext/env.rs index 70e1fbe6af78a..02757bf6b1689 100644 --- a/src/libsyntax_ext/env.rs +++ b/src/libsyntax_ext/env.rs @@ -32,7 +32,7 @@ pub fn expand_option_env<'cx>(cx: &'cx mut ExtCtxt<'_>, Ident::new(sym::str, sp)), Some(lt), ast::Mutability::Immutable))], - vec![])) + )) } Ok(s) => { cx.expr_call_global(sp, From 2aa9d29c6a054d0aca249eb2f9a002acea4ba175 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Sep 2019 15:07:28 -0400 Subject: [PATCH 41/50] Remove unused code --- src/libsyntax/ext/build.rs | 294 +------------------------------------ 1 file changed, 3 insertions(+), 291 deletions(-) diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 84a27fcb7dd94..70f915bf8c4db 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -1,12 +1,11 @@ -use crate::ast::{self, Ident, Generics, Expr, BlockCheckMode, UnOp, PatKind}; +use crate::ast::{self, Ident, Expr, BlockCheckMode, UnOp, PatKind}; use crate::attr; -use crate::source_map::{dummy_spanned, respan, Spanned}; +use crate::source_map::{respan, Spanned}; use crate::ext::base::ExtCtxt; use crate::ptr::P; use crate::symbol::{kw, sym, Symbol}; use crate::ThinVec; -use rustc_target::spec::abi::Abi; use syntax_pos::{Pos, Span}; // Left so that Cargo tests don't break, this can be removed once those no longer use it @@ -51,42 +50,6 @@ impl<'a> ExtCtxt<'a> { ast::Path { span, segments } } - /// Constructs a qualified path. - /// - /// Constructs a path like `::ident`. - pub fn qpath(&self, - self_type: P, - trait_path: ast::Path, - ident: ast::Ident) - -> (ast::QSelf, ast::Path) { - self.qpath_all(self_type, trait_path, ident, vec![], vec![]) - } - - /// Constructs a qualified path. - /// - /// Constructs a path like `::ident<'a, T, A = Bar>`. - pub fn qpath_all(&self, - self_type: P, - trait_path: ast::Path, - ident: ast::Ident, - args: Vec, - constraints: Vec) - -> (ast::QSelf, ast::Path) { - let mut path = trait_path; - let args = if !args.is_empty() || !constraints.is_empty() { - ast::AngleBracketedArgs { args, constraints, span: ident.span }.into() - } else { - None - }; - path.segments.push(ast::PathSegment { ident, id: ast::DUMMY_NODE_ID, args }); - - (ast::QSelf { - ty: self_type, - path_span: path.span, - position: path.segments.len() - 1 - }, path) - } - pub fn ty_mt(&self, ty: P, mutbl: ast::Mutability) -> ast::MutTy { ast::MutTy { ty, @@ -219,14 +182,6 @@ impl<'a> ExtCtxt<'a> { } } - pub fn stmt_semi(&self, expr: P) -> ast::Stmt { - ast::Stmt { - id: ast::DUMMY_NODE_ID, - span: expr.span, - node: ast::StmtKind::Semi(expr), - } - } - pub fn stmt_let(&self, sp: Span, mutbl: bool, ident: ast::Ident, ex: P) -> ast::Stmt { let pat = if mutbl { @@ -250,34 +205,6 @@ impl<'a> ExtCtxt<'a> { } } - pub fn stmt_let_typed(&self, - sp: Span, - mutbl: bool, - ident: ast::Ident, - typ: P, - ex: P) - -> ast::Stmt { - let pat = if mutbl { - let binding_mode = ast::BindingMode::ByValue(ast::Mutability::Mutable); - self.pat_ident_binding_mode(sp, ident, binding_mode) - } else { - self.pat_ident(sp, ident) - }; - let local = P(ast::Local { - pat, - ty: Some(typ), - init: Some(ex), - id: ast::DUMMY_NODE_ID, - span: sp, - attrs: ThinVec::new(), - }); - ast::Stmt { - id: ast::DUMMY_NODE_ID, - node: ast::StmtKind::Local(local), - span: sp, - } - } - // Generates `let _: Type;`, which is usually used for type assertions. pub fn stmt_let_type_only(&self, span: Span, ty: P) -> ast::Stmt { let local = P(ast::Local { @@ -332,11 +259,6 @@ impl<'a> ExtCtxt<'a> { self.expr(path.span, ast::ExprKind::Path(None, path)) } - /// Constructs a `QPath` expression. - pub fn expr_qpath(&self, span: Span, qself: ast::QSelf, path: ast::Path) -> P { - self.expr(span, ast::ExprKind::Path(Some(qself), path)) - } - pub fn expr_ident(&self, span: Span, id: ast::Ident) -> P { self.expr_path(self.path_ident(span, id)) } @@ -350,27 +272,12 @@ impl<'a> ExtCtxt<'a> { } pub fn expr_deref(&self, sp: Span, e: P) -> P { - self.expr_unary(sp, UnOp::Deref, e) - } - pub fn expr_unary(&self, sp: Span, op: ast::UnOp, e: P) -> P { - self.expr(sp, ast::ExprKind::Unary(op, e)) + self.expr(sp, ast::ExprKind::Unary(UnOp::Deref, e)) } - pub fn expr_field_access( - &self, sp: Span, expr: P, ident: ast::Ident, - ) -> P { - self.expr(sp, ast::ExprKind::Field(expr, ident.with_span_pos(sp))) - } - pub fn expr_tup_field_access(&self, sp: Span, expr: P, idx: usize) -> P { - let ident = Ident::new(sym::integer(idx), sp); - self.expr(sp, ast::ExprKind::Field(expr, ident)) - } pub fn expr_addr_of(&self, sp: Span, e: P) -> P { self.expr(sp, ast::ExprKind::AddrOf(ast::Mutability::Immutable, e)) } - pub fn expr_mut_addr_of(&self, sp: Span, e: P) -> P { - self.expr(sp, ast::ExprKind::AddrOf(ast::Mutability::Mutable, e)) - } pub fn expr_call( &self, span: Span, expr: P, args: Vec>, @@ -426,28 +333,10 @@ impl<'a> ExtCtxt<'a> { self.expr_lit(span, ast::LitKind::Int(i as u128, ast::LitIntType::Unsigned(ast::UintTy::Usize))) } - pub fn expr_isize(&self, sp: Span, i: isize) -> P { - if i < 0 { - let i = (-i) as u128; - let lit_ty = ast::LitIntType::Signed(ast::IntTy::Isize); - let lit = self.expr_lit(sp, ast::LitKind::Int(i, lit_ty)); - self.expr_unary(sp, ast::UnOp::Neg, lit) - } else { - self.expr_lit(sp, ast::LitKind::Int(i as u128, - ast::LitIntType::Signed(ast::IntTy::Isize))) - } - } pub fn expr_u32(&self, sp: Span, u: u32) -> P { self.expr_lit(sp, ast::LitKind::Int(u as u128, ast::LitIntType::Unsigned(ast::UintTy::U32))) } - pub fn expr_u16(&self, sp: Span, u: u16) -> P { - self.expr_lit(sp, ast::LitKind::Int(u as u128, - ast::LitIntType::Unsigned(ast::UintTy::U16))) - } - pub fn expr_u8(&self, sp: Span, u: u8) -> P { - self.expr_lit(sp, ast::LitKind::Int(u as u128, ast::LitIntType::Unsigned(ast::UintTy::U8))) - } pub fn expr_bool(&self, sp: Span, value: bool) -> P { self.expr_lit(sp, ast::LitKind::Bool(value)) } @@ -455,10 +344,6 @@ impl<'a> ExtCtxt<'a> { pub fn expr_vec(&self, sp: Span, exprs: Vec>) -> P { self.expr(sp, ast::ExprKind::Array(exprs)) } - pub fn expr_vec_ng(&self, sp: Span) -> P { - self.expr_call_global(sp, self.std_path(&[sym::vec, sym::Vec, sym::new]), - Vec::new()) - } pub fn expr_vec_slice(&self, sp: Span, exprs: Vec>) -> P { self.expr_addr_of(sp, self.expr_vec(sp, exprs)) } @@ -475,16 +360,6 @@ impl<'a> ExtCtxt<'a> { self.expr_call_global(sp, some, vec![expr]) } - pub fn expr_none(&self, sp: Span) -> P { - let none = self.std_path(&[sym::option, sym::Option, sym::None]); - let none = self.path_global(sp, none); - self.expr_path(none) - } - - pub fn expr_break(&self, sp: Span) -> P { - self.expr(sp, ast::ExprKind::Break(None, None)) - } - pub fn expr_tuple(&self, sp: Span, exprs: Vec>) -> P { self.expr(sp, ast::ExprKind::Tup(exprs)) } @@ -513,11 +388,6 @@ impl<'a> ExtCtxt<'a> { self.expr_call_global(sp, ok, vec![expr]) } - pub fn expr_err(&self, sp: Span, expr: P) -> P { - let err = self.std_path(&[sym::result, sym::Result, sym::Err]); - self.expr_call_global(sp, err, vec![expr]) - } - pub fn expr_try(&self, sp: Span, head: P) -> P { let ok = self.std_path(&[sym::result, sym::Result, sym::Ok]); let ok_path = self.path_global(sp, ok); @@ -634,10 +504,6 @@ impl<'a> ExtCtxt<'a> { self.expr(span, ast::ExprKind::If(cond, self.block_expr(then), els)) } - pub fn expr_loop(&self, span: Span, block: P) -> P { - self.expr(span, ast::ExprKind::Loop(block, None)) - } - pub fn lambda_fn_decl(&self, span: Span, fn_decl: P, @@ -681,16 +547,6 @@ impl<'a> ExtCtxt<'a> { self.lambda(span, vec![ident], body) } - pub fn lambda_stmts(&self, - span: Span, - ids: Vec, - stmts: Vec) - -> P { - self.lambda(span, ids, self.expr_block(self.block(span, stmts))) - } - pub fn lambda_stmts_0(&self, span: Span, stmts: Vec) -> P { - self.lambda0(span, self.expr_block(self.block(span, stmts))) - } pub fn lambda_stmts_1(&self, span: Span, stmts: Vec, ident: ast::Ident) -> P { self.lambda1(span, self.expr_block(self.block(span, stmts)), ident) @@ -732,43 +588,6 @@ impl<'a> ExtCtxt<'a> { }) } - pub fn item_fn_poly(&self, - span: Span, - name: Ident, - inputs: Vec , - output: P, - generics: Generics, - body: P) -> P { - self.item(span, - name, - Vec::new(), - ast::ItemKind::Fn(self.fn_decl(inputs, ast::FunctionRetTy::Ty(output)), - ast::FnHeader { - unsafety: ast::Unsafety::Normal, - asyncness: dummy_spanned(ast::IsAsync::NotAsync), - constness: dummy_spanned(ast::Constness::NotConst), - abi: Abi::Rust, - }, - generics, - body)) - } - - pub fn item_fn(&self, - span: Span, - name: Ident, - inputs: Vec , - output: P, - body: P - ) -> P { - self.item_fn_poly( - span, - name, - inputs, - output, - Generics::default(), - body) - } - pub fn variant(&self, span: Span, ident: Ident, tys: Vec> ) -> ast::Variant { let fields: Vec<_> = tys.into_iter().map(|ty| { ast::StructField { @@ -799,52 +618,6 @@ impl<'a> ExtCtxt<'a> { } } - pub fn item_enum_poly(&self, span: Span, name: Ident, - enum_definition: ast::EnumDef, - generics: Generics) -> P { - self.item(span, name, Vec::new(), ast::ItemKind::Enum(enum_definition, generics)) - } - - pub fn item_enum(&self, span: Span, name: Ident, - enum_definition: ast::EnumDef) -> P { - self.item_enum_poly(span, name, enum_definition, - Generics::default()) - } - - pub fn item_struct(&self, span: Span, name: Ident, - struct_def: ast::VariantData) -> P { - self.item_struct_poly( - span, - name, - struct_def, - Generics::default() - ) - } - - pub fn item_struct_poly(&self, span: Span, name: Ident, - struct_def: ast::VariantData, generics: Generics) -> P { - self.item(span, name, Vec::new(), ast::ItemKind::Struct(struct_def, generics)) - } - - pub fn item_mod(&self, span: Span, inner_span: Span, name: Ident, - attrs: Vec, - items: Vec>) -> P { - self.item( - span, - name, - attrs, - ast::ItemKind::Mod(ast::Mod { - inner: inner_span, - items, - inline: true - }) - ) - } - - pub fn item_extern_crate(&self, span: Span, name: Ident) -> P { - self.item(span, name, Vec::new(), ast::ItemKind::ExternCrate(None)) - } - pub fn item_static(&self, span: Span, name: Ident, @@ -864,15 +637,6 @@ impl<'a> ExtCtxt<'a> { self.item(span, name, Vec::new(), ast::ItemKind::Const(ty, expr)) } - pub fn item_ty_poly(&self, span: Span, name: Ident, ty: P, - generics: Generics) -> P { - self.item(span, name, Vec::new(), ast::ItemKind::TyAlias(ty, generics)) - } - - pub fn item_ty(&self, span: Span, name: Ident, ty: P) -> P { - self.item_ty_poly(span, name, ty, Generics::default()) - } - pub fn attribute(&self, mi: ast::MetaItem) -> ast::Attribute { attr::mk_attr_outer(mi) } @@ -894,56 +658,4 @@ impl<'a> ExtCtxt<'a> { -> ast::MetaItem { attr::mk_name_value_item(Ident::new(name, span), lit_kind, span) } - - pub fn item_use(&self, sp: Span, - vis: ast::Visibility, vp: P) -> P { - P(ast::Item { - id: ast::DUMMY_NODE_ID, - ident: Ident::invalid(), - attrs: vec![], - node: ast::ItemKind::Use(vp), - vis, - span: sp, - tokens: None, - }) - } - - pub fn item_use_simple(&self, sp: Span, vis: ast::Visibility, path: ast::Path) -> P { - self.item_use_simple_(sp, vis, None, path) - } - - pub fn item_use_simple_(&self, sp: Span, vis: ast::Visibility, - rename: Option, path: ast::Path) -> P { - self.item_use(sp, vis, P(ast::UseTree { - span: sp, - prefix: path, - kind: ast::UseTreeKind::Simple(rename, ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID), - })) - } - - pub fn item_use_list(&self, sp: Span, vis: ast::Visibility, - path: Vec, imports: &[ast::Ident]) -> P { - let imports = imports.iter().map(|id| { - (ast::UseTree { - span: sp, - prefix: self.path(sp, vec![*id]), - kind: ast::UseTreeKind::Simple(None, ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID), - }, ast::DUMMY_NODE_ID) - }).collect(); - - self.item_use(sp, vis, P(ast::UseTree { - span: sp, - prefix: self.path(sp, path), - kind: ast::UseTreeKind::Nested(imports), - })) - } - - pub fn item_use_glob(&self, sp: Span, - vis: ast::Visibility, path: Vec) -> P { - self.item_use(sp, vis, P(ast::UseTree { - span: sp, - prefix: self.path(sp, path), - kind: ast::UseTreeKind::Glob, - })) - } } From e41aa8c0d04d9a663638e66a82868f0fbeb83bd8 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Sep 2019 15:05:31 -0400 Subject: [PATCH 42/50] Inline ty_infer --- src/libsyntax/ext/build.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 70f915bf8c4db..cadd7d563dab2 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -111,10 +111,6 @@ impl<'a> ExtCtxt<'a> { ast::TyKind::Ptr(self.ty_mt(ty, mutbl))) } - pub fn ty_infer(&self, span: Span) -> P { - self.ty(span, ast::TyKind::Infer) - } - pub fn typaram(&self, span: Span, ident: ast::Ident, @@ -524,7 +520,7 @@ impl<'a> ExtCtxt<'a> { body: P) -> P { let fn_decl = self.fn_decl( - ids.iter().map(|id| self.param(span, *id, self.ty_infer(span))).collect(), + ids.iter().map(|id| self.param(span, *id, self.ty(span, ast::TyKind::Infer))).collect(), ast::FunctionRetTy::Default(span)); // FIXME -- We are using `span` as the span of the `|...|` From 8417ac67c3fe979098facad586438ea5244617b3 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 21 Sep 2019 15:26:15 -0400 Subject: [PATCH 43/50] Inline attribute constructors --- src/libsyntax/ext/build.rs | 14 -------------- src/libsyntax_ext/deriving/cmp/eq.rs | 6 +++--- src/libsyntax_ext/deriving/generic/mod.rs | 7 +++++-- src/libsyntax_ext/test.rs | 4 ++-- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index cadd7d563dab2..ccd73bb66c677 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -640,18 +640,4 @@ impl<'a> ExtCtxt<'a> { pub fn meta_word(&self, sp: Span, w: ast::Name) -> ast::MetaItem { attr::mk_word_item(Ident::new(w, sp)) } - - pub fn meta_list_item_word(&self, sp: Span, w: ast::Name) -> ast::NestedMetaItem { - attr::mk_nested_word_item(Ident::new(w, sp)) - } - - pub fn meta_list(&self, sp: Span, name: ast::Name, mis: Vec) - -> ast::MetaItem { - attr::mk_list_item(Ident::new(name, sp), mis) - } - - pub fn meta_name_value(&self, span: Span, name: ast::Name, lit_kind: ast::LitKind) - -> ast::MetaItem { - attr::mk_name_value_item(Ident::new(name, span), lit_kind, span) - } } diff --git a/src/libsyntax_ext/deriving/cmp/eq.rs b/src/libsyntax_ext/deriving/cmp/eq.rs index 471c92dd99949..c92339dd2fbd5 100644 --- a/src/libsyntax_ext/deriving/cmp/eq.rs +++ b/src/libsyntax_ext/deriving/cmp/eq.rs @@ -2,7 +2,7 @@ use crate::deriving::path_std; use crate::deriving::generic::*; use crate::deriving::generic::ty::*; -use syntax::ast::{self, Expr, MetaItem, GenericArg}; +use syntax::ast::{self, Ident, Expr, MetaItem, GenericArg}; use syntax::ext::base::{Annotatable, ExtCtxt, SpecialDerives}; use syntax::ptr::P; use syntax::symbol::{sym, Symbol}; @@ -16,8 +16,8 @@ pub fn expand_deriving_eq(cx: &mut ExtCtxt<'_>, cx.resolver.add_derives(cx.current_expansion.id.expn_data().parent, SpecialDerives::EQ); let inline = cx.meta_word(span, sym::inline); - let hidden = cx.meta_list_item_word(span, sym::hidden); - let doc = cx.meta_list(span, sym::doc, vec![hidden]); + let hidden = syntax::attr::mk_nested_word_item(Ident::new(sym::hidden, span)); + let doc = syntax::attr::mk_list_item(Ident::new(sym::doc, span), vec![hidden]); let attrs = vec![cx.attribute(inline), cx.attribute(doc)]; let trait_def = TraitDef { span, diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 5c332eccb62cc..fec035d331dc5 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -672,8 +672,11 @@ impl<'a> TraitDef<'a> { attr::mark_used(&attr); let opt_trait_ref = Some(trait_ref); let unused_qual = { - let word = cx.meta_list_item_word(self.span, Symbol::intern("unused_qualifications")); - cx.attribute(cx.meta_list(self.span, sym::allow, vec![word])) + let word = syntax::attr::mk_nested_word_item( + Ident::new(Symbol::intern("unused_qualifications"), self.span)); + let list = syntax::attr::mk_list_item( + Ident::new(sym::allow, self.span), vec![word]); + cx.attribute(list) }; let mut a = vec![attr, unused_qual]; diff --git a/src/libsyntax_ext/test.rs b/src/libsyntax_ext/test.rs index 0910c00a8a2a9..6c7e3e3eb9875 100644 --- a/src/libsyntax_ext/test.rs +++ b/src/libsyntax_ext/test.rs @@ -145,8 +145,8 @@ pub fn expand_test_or_bench( let mut test_const = cx.item(sp, ast::Ident::new(item.ident.name, sp), vec![ // #[cfg(test)] - cx.attribute(cx.meta_list(attr_sp, sym::cfg, vec![ - cx.meta_list_item_word(attr_sp, sym::test) + cx.attribute(attr::mk_list_item(ast::Ident::new(sym::cfg, attr_sp), vec![ + attr::mk_nested_word_item(ast::Ident::new(sym::test, attr_sp)) ])), // #[rustc_test_marker] cx.attribute(cx.meta_word(attr_sp, sym::rustc_test_marker)), From 3e6b84474d7cd813bda0956568966024915d0ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 21 Sep 2019 16:31:27 -0700 Subject: [PATCH 44/50] Propagate `types.err` in locals further to avoid spurious knock-down errors --- src/librustc_typeck/check/coercion.rs | 8 ++++++-- src/librustc_typeck/check/mod.rs | 23 ++++++++++++++++++++++- src/test/ui/issues/issue-33575.rs | 4 ++++ src/test/ui/issues/issue-33575.stderr | 9 +++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/issues/issue-33575.rs create mode 100644 src/test/ui/issues/issue-33575.stderr diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index ee4f0a868c10a..ac1bf8e91a149 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -163,7 +163,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { // Just ignore error types. if a.references_error() || b.references_error() { - return success(vec![], b, vec![]); + return success(vec![], self.fcx.tcx.types.err, vec![]); } if a.is_never() { @@ -821,7 +821,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let (adjustments, _) = self.register_infer_ok_obligations(ok); self.apply_adjustments(expr, adjustments); - Ok(target) + if expr_ty.references_error() { + Ok(self.tcx.types.err) + } else { + Ok(target) + } } /// Same as `try_coerce()`, but without side-effects. diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 0eeeee01c82f1..75942731d8790 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -153,7 +153,7 @@ use self::method::{MethodCallee, SelfSource}; use self::TupleArgumentsFlag::*; /// The type of a local binding, including the revealed type for anon types. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] pub struct LocalTy<'tcx> { decl_ty: Ty<'tcx>, revealed_ty: Ty<'tcx> @@ -3752,14 +3752,35 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let Some(ref init) = local.init { let init_ty = self.check_decl_initializer(local, &init); if init_ty.references_error() { + // Override the types everywhere with `types.err` to avoid knock down errors. self.write_ty(local.hir_id, init_ty); + self.write_ty(local.pat.hir_id, init_ty); + self.locals.borrow_mut().insert(local.hir_id, LocalTy { + decl_ty: t, + revealed_ty: init_ty, + }); + self.locals.borrow_mut().insert(local.pat.hir_id, LocalTy { + decl_ty: t, + revealed_ty: init_ty, + }); } } self.check_pat_top(&local.pat, t, None); let pat_ty = self.node_ty(local.pat.hir_id); + debug!("check_decl_local pat_ty {:?}", pat_ty); if pat_ty.references_error() { + // Override the types everywhere with `types.err` to avoid knock down errors. self.write_ty(local.hir_id, pat_ty); + self.write_ty(local.pat.hir_id, pat_ty); + self.locals.borrow_mut().insert(local.hir_id, LocalTy { + decl_ty: t, + revealed_ty: pat_ty, + }); + self.locals.borrow_mut().insert(local.pat.hir_id, LocalTy { + decl_ty: t, + revealed_ty: pat_ty, + }); } } diff --git a/src/test/ui/issues/issue-33575.rs b/src/test/ui/issues/issue-33575.rs new file mode 100644 index 0000000000000..d97afc3d31d5c --- /dev/null +++ b/src/test/ui/issues/issue-33575.rs @@ -0,0 +1,4 @@ +fn main() { + let baz = ().foo(); //~ ERROR no method named `foo` found for type `()` in the current scope + ::from_str(&baz); // No complains about `str` being unsized +} diff --git a/src/test/ui/issues/issue-33575.stderr b/src/test/ui/issues/issue-33575.stderr new file mode 100644 index 0000000000000..e6b74d262c340 --- /dev/null +++ b/src/test/ui/issues/issue-33575.stderr @@ -0,0 +1,9 @@ +error[E0599]: no method named `foo` found for type `()` in the current scope + --> $DIR/issue-33575.rs:2:18 + | +LL | let baz = ().foo(); + | ^^^ method not found in `()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. From 60560bc2a2c149e179cd7e58a8b48e06c2c4e3f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 21 Sep 2019 17:11:09 -0700 Subject: [PATCH 45/50] Parse assoc type bounds in generic params and provide custom diagnostic --- src/libsyntax/parse/parser/generics.rs | 99 ++++++++++++------- src/test/ui/parser/assoc-type-in-type-arg.rs | 11 +++ .../ui/parser/assoc-type-in-type-arg.stderr | 8 ++ 3 files changed, 80 insertions(+), 38 deletions(-) create mode 100644 src/test/ui/parser/assoc-type-in-type-arg.rs create mode 100644 src/test/ui/parser/assoc-type-in-type-arg.stderr diff --git a/src/libsyntax/parse/parser/generics.rs b/src/libsyntax/parse/parser/generics.rs index 3e6118ad86f47..1ffdda19c80b5 100644 --- a/src/libsyntax/parse/parser/generics.rs +++ b/src/libsyntax/parse/parser/generics.rs @@ -100,13 +100,31 @@ impl<'a> Parser<'a> { } else if self.check_ident() { // Parse type parameter. params.push(self.parse_ty_param(attrs)?); + } else if self.token.can_begin_type() { + // Trying to write an associated type bound? (#26271) + let snapshot = self.clone(); + match self.parse_ty_where_predicate() { + Ok(where_predicate) => { + self.struct_span_err( + where_predicate.span(), + "associated type bounds do not belong here", + ) + .span_label(where_predicate.span(), "belongs in `where` clause") + .emit(); + } + Err(mut err) => { + err.cancel(); + std::mem::replace(self, snapshot); + break + } + } } else { // Check for trailing attributes and stop parsing. if !attrs.is_empty() { if !params.is_empty() { self.struct_span_err( attrs[0].span, - &format!("trailing attribute after generic parameter"), + "trailing attribute after generic parameter", ) .span_label(attrs[0].span, "attributes must go before parameters") .emit(); @@ -202,43 +220,7 @@ impl<'a> Parser<'a> { } )); } else if self.check_type() { - // Parse optional `for<'a, 'b>`. - // This `for` is parsed greedily and applies to the whole predicate, - // the bounded type can have its own `for` applying only to it. - // Examples: - // * `for<'a> Trait1<'a>: Trait2<'a /* ok */>` - // * `(for<'a> Trait1<'a>): Trait2<'a /* not ok */>` - // * `for<'a> for<'b> Trait1<'a, 'b>: Trait2<'a /* ok */, 'b /* not ok */>` - let lifetime_defs = self.parse_late_bound_lifetime_defs()?; - - // Parse type with mandatory colon and (possibly empty) bounds, - // or with mandatory equality sign and the second type. - let ty = self.parse_ty()?; - if self.eat(&token::Colon) { - let bounds = self.parse_generic_bounds(Some(self.prev_span))?; - where_clause.predicates.push(ast::WherePredicate::BoundPredicate( - ast::WhereBoundPredicate { - span: lo.to(self.prev_span), - bound_generic_params: lifetime_defs, - bounded_ty: ty, - bounds, - } - )); - // FIXME: Decide what should be used here, `=` or `==`. - // FIXME: We are just dropping the binders in lifetime_defs on the floor here. - } else if self.eat(&token::Eq) || self.eat(&token::EqEq) { - let rhs_ty = self.parse_ty()?; - where_clause.predicates.push(ast::WherePredicate::EqPredicate( - ast::WhereEqPredicate { - span: lo.to(self.prev_span), - lhs_ty: ty, - rhs_ty, - id: ast::DUMMY_NODE_ID, - } - )); - } else { - return self.unexpected(); - } + where_clause.predicates.push(self.parse_ty_where_predicate()?); } else { break } @@ -252,6 +234,47 @@ impl<'a> Parser<'a> { Ok(where_clause) } + fn parse_ty_where_predicate(&mut self) -> PResult<'a, ast::WherePredicate> { + let lo = self.token.span; + // Parse optional `for<'a, 'b>`. + // This `for` is parsed greedily and applies to the whole predicate, + // the bounded type can have its own `for` applying only to it. + // Examples: + // * `for<'a> Trait1<'a>: Trait2<'a /* ok */>` + // * `(for<'a> Trait1<'a>): Trait2<'a /* not ok */>` + // * `for<'a> for<'b> Trait1<'a, 'b>: Trait2<'a /* ok */, 'b /* not ok */>` + let lifetime_defs = self.parse_late_bound_lifetime_defs()?; + + // Parse type with mandatory colon and (possibly empty) bounds, + // or with mandatory equality sign and the second type. + let ty = self.parse_ty()?; + if self.eat(&token::Colon) { + let bounds = self.parse_generic_bounds(Some(self.prev_span))?; + Ok(ast::WherePredicate::BoundPredicate( + ast::WhereBoundPredicate { + span: lo.to(self.prev_span), + bound_generic_params: lifetime_defs, + bounded_ty: ty, + bounds, + } + )) + // FIXME: Decide what should be used here, `=` or `==`. + // FIXME: We are just dropping the binders in lifetime_defs on the floor here. + } else if self.eat(&token::Eq) || self.eat(&token::EqEq) { + let rhs_ty = self.parse_ty()?; + Ok(ast::WherePredicate::EqPredicate( + ast::WhereEqPredicate { + span: lo.to(self.prev_span), + lhs_ty: ty, + rhs_ty, + id: ast::DUMMY_NODE_ID, + } + )) + } else { + self.unexpected() + } + } + pub(super) fn choose_generics_over_qpath(&self) -> bool { // There's an ambiguity between generic parameters and qualified paths in impls. // If we see `<` it may start both, so we have to inspect some following tokens. diff --git a/src/test/ui/parser/assoc-type-in-type-arg.rs b/src/test/ui/parser/assoc-type-in-type-arg.rs new file mode 100644 index 0000000000000..09765f01371aa --- /dev/null +++ b/src/test/ui/parser/assoc-type-in-type-arg.rs @@ -0,0 +1,11 @@ +trait Tr { + type TrSubtype; +} + +struct Bar<'a, Item: Tr, ::TrSubtype: 'a> { + //~^ ERROR associated type bounds do not belong here + item: Item, + item_sub: &'a ::TrSubtype, +} + +fn main() {} diff --git a/src/test/ui/parser/assoc-type-in-type-arg.stderr b/src/test/ui/parser/assoc-type-in-type-arg.stderr new file mode 100644 index 0000000000000..06addb0241725 --- /dev/null +++ b/src/test/ui/parser/assoc-type-in-type-arg.stderr @@ -0,0 +1,8 @@ +error: associated type bounds do not belong here + --> $DIR/assoc-type-in-type-arg.rs:5:26 + | +LL | struct Bar<'a, Item: Tr, ::TrSubtype: 'a> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ belongs in `where` clause + +error: aborting due to previous error + From c3d791740f7bc5f85d24e30c835f23cc1423797c Mon Sep 17 00:00:00 2001 From: Tshepang Lekhonkhobe Date: Sun, 22 Sep 2019 02:23:41 +0200 Subject: [PATCH 46/50] remove outdated comment --- src/librustc_mir/borrow_check/flows.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/flows.rs b/src/librustc_mir/borrow_check/flows.rs index 4400e0c8395a2..1f17ab69f6660 100644 --- a/src/librustc_mir/borrow_check/flows.rs +++ b/src/librustc_mir/borrow_check/flows.rs @@ -23,7 +23,6 @@ use std::rc::Rc; crate type PoloniusOutput = Output; -// (forced to be `pub` due to its use as an associated type below.) crate struct Flows<'b, 'tcx> { borrows: FlowAtLocation<'tcx, Borrows<'b, 'tcx>>, pub uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'tcx>>, From daed67481511b65475069214cd8325ca9d018509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 21 Sep 2019 17:28:07 -0700 Subject: [PATCH 47/50] review comments --- src/librustc_typeck/check/coercion.rs | 8 +++--- src/librustc_typeck/check/mod.rs | 39 ++++++++++----------------- src/test/ui/issues/issue-33575.rs | 2 +- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index ac1bf8e91a149..d98e1f3e1283f 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -821,11 +821,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let (adjustments, _) = self.register_infer_ok_obligations(ok); self.apply_adjustments(expr, adjustments); - if expr_ty.references_error() { - Ok(self.tcx.types.err) + Ok(if expr_ty.references_error() { + self.tcx.types.err } else { - Ok(target) - } + target + }) } /// Same as `try_coerce()`, but without side-effects. diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 75942731d8790..07eb034054c95 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3751,36 +3751,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let Some(ref init) = local.init { let init_ty = self.check_decl_initializer(local, &init); - if init_ty.references_error() { - // Override the types everywhere with `types.err` to avoid knock down errors. - self.write_ty(local.hir_id, init_ty); - self.write_ty(local.pat.hir_id, init_ty); - self.locals.borrow_mut().insert(local.hir_id, LocalTy { - decl_ty: t, - revealed_ty: init_ty, - }); - self.locals.borrow_mut().insert(local.pat.hir_id, LocalTy { - decl_ty: t, - revealed_ty: init_ty, - }); - } + self.overwrite_local_ty_if_err(local, t, init_ty); } self.check_pat_top(&local.pat, t, None); let pat_ty = self.node_ty(local.pat.hir_id); - debug!("check_decl_local pat_ty {:?}", pat_ty); - if pat_ty.references_error() { + self.overwrite_local_ty_if_err(local, t, pat_ty); + } + + fn overwrite_local_ty_if_err(&self, local: &'tcx hir::Local, decl_ty: Ty<'tcx>, ty: Ty<'tcx>) { + if ty.references_error() { // Override the types everywhere with `types.err` to avoid knock down errors. - self.write_ty(local.hir_id, pat_ty); - self.write_ty(local.pat.hir_id, pat_ty); - self.locals.borrow_mut().insert(local.hir_id, LocalTy { - decl_ty: t, - revealed_ty: pat_ty, - }); - self.locals.borrow_mut().insert(local.pat.hir_id, LocalTy { - decl_ty: t, - revealed_ty: pat_ty, - }); + self.write_ty(local.hir_id, ty); + self.write_ty(local.pat.hir_id, ty); + let local_ty = LocalTy { + decl_ty, + revealed_ty: ty, + }; + self.locals.borrow_mut().insert(local.hir_id, local_ty); + self.locals.borrow_mut().insert(local.pat.hir_id, local_ty); } } diff --git a/src/test/ui/issues/issue-33575.rs b/src/test/ui/issues/issue-33575.rs index d97afc3d31d5c..09c499452adb6 100644 --- a/src/test/ui/issues/issue-33575.rs +++ b/src/test/ui/issues/issue-33575.rs @@ -1,4 +1,4 @@ fn main() { let baz = ().foo(); //~ ERROR no method named `foo` found for type `()` in the current scope - ::from_str(&baz); // No complains about `str` being unsized + ::from_str(&baz); // No complaints about `str` being unsized } From 0f2f16db5364663b4a2f092bba61dc3b056902e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 21 Sep 2019 18:57:37 -0700 Subject: [PATCH 48/50] review comments: wording --- src/libsyntax/parse/parser/generics.rs | 2 +- src/test/ui/parser/assoc-type-in-type-arg.rs | 2 +- src/test/ui/parser/assoc-type-in-type-arg.stderr | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/parse/parser/generics.rs b/src/libsyntax/parse/parser/generics.rs index 1ffdda19c80b5..2ecd9cca3c64b 100644 --- a/src/libsyntax/parse/parser/generics.rs +++ b/src/libsyntax/parse/parser/generics.rs @@ -107,7 +107,7 @@ impl<'a> Parser<'a> { Ok(where_predicate) => { self.struct_span_err( where_predicate.span(), - "associated type bounds do not belong here", + "bounds on associated types do not belong here", ) .span_label(where_predicate.span(), "belongs in `where` clause") .emit(); diff --git a/src/test/ui/parser/assoc-type-in-type-arg.rs b/src/test/ui/parser/assoc-type-in-type-arg.rs index 09765f01371aa..000956ea24fad 100644 --- a/src/test/ui/parser/assoc-type-in-type-arg.rs +++ b/src/test/ui/parser/assoc-type-in-type-arg.rs @@ -3,7 +3,7 @@ trait Tr { } struct Bar<'a, Item: Tr, ::TrSubtype: 'a> { - //~^ ERROR associated type bounds do not belong here + //~^ ERROR bounds on associated types do not belong here item: Item, item_sub: &'a ::TrSubtype, } diff --git a/src/test/ui/parser/assoc-type-in-type-arg.stderr b/src/test/ui/parser/assoc-type-in-type-arg.stderr index 06addb0241725..b637702f21e90 100644 --- a/src/test/ui/parser/assoc-type-in-type-arg.stderr +++ b/src/test/ui/parser/assoc-type-in-type-arg.stderr @@ -1,4 +1,4 @@ -error: associated type bounds do not belong here +error: bounds on associated types do not belong here --> $DIR/assoc-type-in-type-arg.rs:5:26 | LL | struct Bar<'a, Item: Tr, ::TrSubtype: 'a> { From 3f2855e4a6003f7e4d5736843d9ca5f327bef9d7 Mon Sep 17 00:00:00 2001 From: ben Date: Sun, 22 Sep 2019 17:24:09 +1200 Subject: [PATCH 49/50] Infer consts consistently. Moved some logic into super_combined_consts, also removed some duplicated logic from TypeRelation methods. --- src/librustc/infer/combine.rs | 7 ++++ src/librustc/infer/equate.rs | 40 ++---------------- src/librustc/infer/glb.rs | 5 --- src/librustc/infer/lub.rs | 5 --- src/librustc/infer/sub.rs | 42 ++----------------- src/test/ui/const-generics/issue-64519.rs | 21 ++++++++++ src/test/ui/const-generics/issue-64519.stderr | 8 ++++ 7 files changed, 42 insertions(+), 86 deletions(-) create mode 100644 src/test/ui/const-generics/issue-64519.rs create mode 100644 src/test/ui/const-generics/issue-64519.stderr diff --git a/src/librustc/infer/combine.rs b/src/librustc/infer/combine.rs index 4a9b68f24371d..966c5810171af 100644 --- a/src/librustc/infer/combine.rs +++ b/src/librustc/infer/combine.rs @@ -30,6 +30,7 @@ use super::sub::Sub; use super::type_variable::TypeVariableValue; use super::unify_key::{ConstVarValue, ConstVariableValue}; use super::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; +use super::unify_key::replace_if_possible; use crate::hir::def_id::DefId; use crate::mir::interpret::ConstValue; @@ -127,6 +128,12 @@ impl<'infcx, 'tcx> InferCtxt<'infcx, 'tcx> { where R: TypeRelation<'tcx>, { + debug!("{}.consts({:?}, {:?})", relation.tag(), a, b); + if a == b { return Ok(a); } + + let a = replace_if_possible(self.const_unification_table.borrow_mut(), a); + let b = replace_if_possible(self.const_unification_table.borrow_mut(), b); + let a_is_expected = relation.a_is_expected(); match (a.val, b.val) { diff --git a/src/librustc/infer/equate.rs b/src/librustc/infer/equate.rs index 96d40bc81add2..6065387647fa7 100644 --- a/src/librustc/infer/equate.rs +++ b/src/librustc/infer/equate.rs @@ -1,14 +1,12 @@ -use super::combine::{CombineFields, RelationDir, const_unification_error}; +use super::combine::{CombineFields, RelationDir}; use super::Subtype; use crate::hir::def_id::DefId; -use crate::ty::{self, Ty, TyCtxt, InferConst}; +use crate::ty::{self, Ty, TyCtxt}; use crate::ty::TyVar; use crate::ty::subst::SubstsRef; use crate::ty::relate::{self, Relate, RelateResult, TypeRelation}; -use crate::mir::interpret::ConstValue; -use crate::infer::unify_key::replace_if_possible; /// Ensures `a` is made equal to `b`. Returns `a` on success. pub struct Equate<'combine, 'infcx, 'tcx> { @@ -108,39 +106,7 @@ impl TypeRelation<'tcx> for Equate<'combine, 'infcx, 'tcx> { a: &'tcx ty::Const<'tcx>, b: &'tcx ty::Const<'tcx>, ) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> { - debug!("{}.consts({:?}, {:?})", self.tag(), a, b); - if a == b { return Ok(a); } - - let infcx = self.fields.infcx; - let a = replace_if_possible(infcx.const_unification_table.borrow_mut(), a); - let b = replace_if_possible(infcx.const_unification_table.borrow_mut(), b); - let a_is_expected = self.a_is_expected(); - - match (a.val, b.val) { - (ConstValue::Infer(InferConst::Var(a_vid)), - ConstValue::Infer(InferConst::Var(b_vid))) => { - infcx.const_unification_table - .borrow_mut() - .unify_var_var(a_vid, b_vid) - .map_err(|e| const_unification_error(a_is_expected, e))?; - return Ok(a); - } - - (ConstValue::Infer(InferConst::Var(a_id)), _) => { - self.fields.infcx.unify_const_variable(a_is_expected, a_id, b)?; - return Ok(a); - } - - (_, ConstValue::Infer(InferConst::Var(b_id))) => { - self.fields.infcx.unify_const_variable(!a_is_expected, b_id, a)?; - return Ok(a); - } - - _ => {} - } - - self.fields.infcx.super_combine_consts(self, a, b)?; - Ok(a) + self.fields.infcx.super_combine_consts(self, a, b) } fn binders(&mut self, a: &ty::Binder, b: &ty::Binder) diff --git a/src/librustc/infer/glb.rs b/src/librustc/infer/glb.rs index 10e45321a6d6a..37de54a7e8558 100644 --- a/src/librustc/infer/glb.rs +++ b/src/librustc/infer/glb.rs @@ -66,11 +66,6 @@ impl TypeRelation<'tcx> for Glb<'combine, 'infcx, 'tcx> { a: &'tcx ty::Const<'tcx>, b: &'tcx ty::Const<'tcx>, ) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> { - debug!("{}.consts({:?}, {:?})", self.tag(), a, b); - if a == b { - return Ok(a); - } - self.fields.infcx.super_combine_consts(self, a, b) } diff --git a/src/librustc/infer/lub.rs b/src/librustc/infer/lub.rs index 8b64cda7bd26d..a1a94865e74e3 100644 --- a/src/librustc/infer/lub.rs +++ b/src/librustc/infer/lub.rs @@ -66,11 +66,6 @@ impl TypeRelation<'tcx> for Lub<'combine, 'infcx, 'tcx> { a: &'tcx ty::Const<'tcx>, b: &'tcx ty::Const<'tcx>, ) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> { - debug!("{}.consts({:?}, {:?})", self.tag(), a, b); - if a == b { - return Ok(a); - } - self.fields.infcx.super_combine_consts(self, a, b) } diff --git a/src/librustc/infer/sub.rs b/src/librustc/infer/sub.rs index 76db55ecfa8ef..67c97ef5d8b29 100644 --- a/src/librustc/infer/sub.rs +++ b/src/librustc/infer/sub.rs @@ -1,13 +1,11 @@ use super::SubregionOrigin; -use super::combine::{CombineFields, RelationDir, const_unification_error}; +use super::combine::{CombineFields, RelationDir}; use crate::traits::Obligation; -use crate::ty::{self, Ty, TyCtxt, InferConst}; +use crate::ty::{self, Ty, TyCtxt}; use crate::ty::TyVar; use crate::ty::fold::TypeFoldable; use crate::ty::relate::{Cause, Relate, RelateResult, TypeRelation}; -use crate::infer::unify_key::replace_if_possible; -use crate::mir::interpret::ConstValue; use std::mem; /// Ensures `a` is made a subtype of `b`. Returns `a` on success. @@ -142,41 +140,7 @@ impl TypeRelation<'tcx> for Sub<'combine, 'infcx, 'tcx> { a: &'tcx ty::Const<'tcx>, b: &'tcx ty::Const<'tcx>, ) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> { - debug!("{}.consts({:?}, {:?})", self.tag(), a, b); - if a == b { return Ok(a); } - - let infcx = self.fields.infcx; - let a = replace_if_possible(infcx.const_unification_table.borrow_mut(), a); - let b = replace_if_possible(infcx.const_unification_table.borrow_mut(), b); - - // Consts can only be equal or unequal to each other: there's no subtyping - // relation, so we're just going to perform equating here instead. - let a_is_expected = self.a_is_expected(); - match (a.val, b.val) { - (ConstValue::Infer(InferConst::Var(a_vid)), - ConstValue::Infer(InferConst::Var(b_vid))) => { - infcx.const_unification_table - .borrow_mut() - .unify_var_var(a_vid, b_vid) - .map_err(|e| const_unification_error(a_is_expected, e))?; - return Ok(a); - } - - (ConstValue::Infer(InferConst::Var(a_id)), _) => { - self.fields.infcx.unify_const_variable(a_is_expected, a_id, b)?; - return Ok(a); - } - - (_, ConstValue::Infer(InferConst::Var(b_id))) => { - self.fields.infcx.unify_const_variable(!a_is_expected, b_id, a)?; - return Ok(a); - } - - _ => {} - } - - self.fields.infcx.super_combine_consts(self, a, b)?; - Ok(a) + self.fields.infcx.super_combine_consts(self, a, b) } fn binders(&mut self, a: &ty::Binder, b: &ty::Binder) diff --git a/src/test/ui/const-generics/issue-64519.rs b/src/test/ui/const-generics/issue-64519.rs new file mode 100644 index 0000000000000..72cce9b4843d7 --- /dev/null +++ b/src/test/ui/const-generics/issue-64519.rs @@ -0,0 +1,21 @@ +// check-pass + +#![feature(const_generics)] +//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash + +struct Foo { + state: Option<[u8; D]>, +} + +impl Iterator for Foo<{D}> { + type Item = [u8; D]; + fn next(&mut self) -> Option { + if true { + return Some(self.state.unwrap().clone()); + } else { + return Some(self.state.unwrap().clone()); + } + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/issue-64519.stderr b/src/test/ui/const-generics/issue-64519.stderr new file mode 100644 index 0000000000000..d368f39d903a0 --- /dev/null +++ b/src/test/ui/const-generics/issue-64519.stderr @@ -0,0 +1,8 @@ +warning: the feature `const_generics` is incomplete and may cause the compiler to crash + --> $DIR/issue-64519.rs:3:12 + | +LL | #![feature(const_generics)] + | ^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + From 3ee292021d00857586af0d2ef1fa6c31f74181ce Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Sun, 22 Sep 2019 12:59:10 +0200 Subject: [PATCH 50/50] Fix some unused variable warnings --- src/test/ui/coherence/auxiliary/coherence_lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/coherence/auxiliary/coherence_lib.rs b/src/test/ui/coherence/auxiliary/coherence_lib.rs index 9a5ec82430639..c22819831ab24 100644 --- a/src/test/ui/coherence/auxiliary/coherence_lib.rs +++ b/src/test/ui/coherence/auxiliary/coherence_lib.rs @@ -5,11 +5,11 @@ pub trait Remote { } pub trait Remote1 { - fn foo(&self, t: T) { } + fn foo(&self, _t: T) { } } pub trait Remote2 { - fn foo(&self, t: T, u: U) { } + fn foo(&self, _t: T, _u: U) { } } pub struct Pair(T,U);