From 5fc6fc3d90b32c504b3ffe1802e48987cb7a7e64 Mon Sep 17 00:00:00 2001 From: Steve Klabnik Date: Tue, 8 Jan 2019 15:44:57 -0500 Subject: [PATCH 01/11] Improve docs for Formatter --- src/libcore/fmt/mod.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index ec1aeb8a7d1e..06cb1b02fcf4 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -232,9 +232,18 @@ impl Write for &mut W { } } -/// A struct to represent both where to emit formatting strings to and how they -/// should be formatted. A mutable version of this is passed to all formatting -/// traits. +/// Configuration for formatting. +/// +/// A `Formatter` represents various options related to formatting. Users do not +/// construct `Formatter`s directly; a mutable reference to one is passed to +/// the `fmt` method of all formatting traits, like [`Debug`] and [`Display`]. +/// +/// To interact with a `Formatter`, you'll call various methods to change the +/// various options related to formatting. For examples, please see the +/// documentation of the methods defined on `Formatter` below. +/// +/// [`Debug`]: trait.Debug.html +/// [`Display`]: trait.Display.html #[allow(missing_debug_implementations)] #[stable(feature = "rust1", since = "1.0.0")] pub struct Formatter<'a> { From b2ce5a90995be655b1a1e8fd764e2b1c1f0dab14 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 17 Jan 2019 09:45:02 +1100 Subject: [PATCH 02/11] Make `hir::Stmt` a separate struct. Benefits: - It lets us move the `NodeId` field out of every `hir::StmtKind` variant `NodeId` to a more sensible spot. - It eliminates sadness in `Stmt::fmt`. - It makes `hir::Stmt` match `ast::Stmt`. --- src/librustc/cfg/construct.rs | 8 ++-- src/librustc/hir/check_attr.rs | 2 +- src/librustc/hir/intravisit.rs | 9 ++--- src/librustc/hir/lowering.rs | 50 ++++++++++++++----------- src/librustc/hir/map/collector.rs | 2 +- src/librustc/hir/mod.rs | 39 ++++++++----------- src/librustc/hir/print.rs | 10 ++--- src/librustc/ich/impls_hir.rs | 13 +++++-- src/librustc/middle/expr_use_visitor.rs | 6 +-- src/librustc/middle/liveness.rs | 4 +- src/librustc/middle/region.rs | 2 +- src/librustc_lint/unused.rs | 4 +- src/librustc_mir/hair/cx/block.rs | 10 ++--- src/librustc_passes/hir_stats.rs | 2 +- src/librustc_passes/rvalue_promotion.rs | 6 +-- src/librustc_typeck/check/mod.rs | 12 +++--- 16 files changed, 92 insertions(+), 87 deletions(-) diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index 978d20ea9478..1cbfcc166432 100644 --- a/src/librustc/cfg/construct.rs +++ b/src/librustc/cfg/construct.rs @@ -99,15 +99,15 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { } fn stmt(&mut self, stmt: &hir::Stmt, pred: CFGIndex) -> CFGIndex { - let hir_id = self.tcx.hir().node_to_hir_id(stmt.node.id()); + let hir_id = self.tcx.hir().node_to_hir_id(stmt.id); match stmt.node { - hir::StmtKind::Decl(ref decl, _) => { + hir::StmtKind::Decl(ref decl) => { let exit = self.decl(&decl, pred); self.add_ast_node(hir_id.local_id, &[exit]) } - hir::StmtKind::Expr(ref expr, _) | - hir::StmtKind::Semi(ref expr, _) => { + hir::StmtKind::Expr(ref expr) | + hir::StmtKind::Semi(ref expr) => { let exit = self.expr(&expr, pred); self.add_ast_node(hir_id.local_id, &[exit]) } diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index a214ec88c665..1cffe5ace5a6 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -298,7 +298,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { fn check_stmt_attributes(&self, stmt: &hir::Stmt) { // When checking statements ignore expressions, they will be checked later - if let hir::StmtKind::Decl(_, _) = stmt.node { + if let hir::StmtKind::Decl(..) = stmt.node { for attr in stmt.node.attrs() { if attr.check_name("inline") { self.check_inline(attr, &stmt.span, Target::Statement); diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index f633703be56d..3b9358d7c46a 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -953,14 +953,13 @@ pub fn walk_block<'v, V: Visitor<'v>>(visitor: &mut V, block: &'v Block) { } pub fn walk_stmt<'v, V: Visitor<'v>>(visitor: &mut V, statement: &'v Stmt) { + visitor.visit_id(statement.id); match statement.node { - StmtKind::Decl(ref declaration, id) => { - visitor.visit_id(id); + StmtKind::Decl(ref declaration) => { visitor.visit_decl(declaration) } - StmtKind::Expr(ref expression, id) | - StmtKind::Semi(ref expression, id) => { - visitor.visit_id(id); + StmtKind::Expr(ref expression) | + StmtKind::Semi(ref expression) => { visitor.visit_expr(expression) } } diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 8badcbfc1b30..837960a0f1d9 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -4331,10 +4331,11 @@ impl<'a> LoweringContext<'a> { ThinVec::new(), )) }; - let match_stmt = respan( - head_sp, - hir::StmtKind::Expr(match_expr, self.next_id().node_id) - ); + let match_stmt = hir::Stmt { + id: self.next_id().node_id, + node: hir::StmtKind::Expr(match_expr), + span: head_sp, + }; let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat.id)); @@ -4357,10 +4358,11 @@ impl<'a> LoweringContext<'a> { let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false)); let body_expr = P(self.expr_block(body_block, ThinVec::new())); - let body_stmt = respan( - body.span, - hir::StmtKind::Expr(body_expr, self.next_id().node_id) - ); + let body_stmt = hir::Stmt { + id: self.next_id().node_id, + node: hir::StmtKind::Expr(body_expr), + span: body.span, + }; let loop_block = P(self.block_all( e.span, @@ -4533,24 +4535,24 @@ impl<'a> LoweringContext<'a> { let (l, item_ids) = self.lower_local(l); let mut ids: SmallVec<[hir::Stmt; 1]> = item_ids .into_iter() - .map(|item_id| Spanned { + .map(|item_id| hir::Stmt { + id: self.next_id().node_id, node: hir::StmtKind::Decl( P(Spanned { node: hir::DeclKind::Item(item_id), span: s.span, }), - self.next_id().node_id, ), span: s.span, }) .collect(); - ids.push(Spanned { + ids.push(hir::Stmt { + id: self.lower_node_id(s.id).node_id, node: hir::StmtKind::Decl( P(Spanned { node: hir::DeclKind::Local(l), span: s.span, }), - self.lower_node_id(s.id).node_id, ), span: s.span, }); @@ -4561,26 +4563,28 @@ impl<'a> LoweringContext<'a> { let mut id = Some(s.id); return self.lower_item_id(it) .into_iter() - .map(|item_id| Spanned { + .map(|item_id| hir::Stmt { + id: id.take() + .map(|id| self.lower_node_id(id).node_id) + .unwrap_or_else(|| self.next_id().node_id), node: hir::StmtKind::Decl( P(Spanned { node: hir::DeclKind::Item(item_id), span: s.span, }), - id.take() - .map(|id| self.lower_node_id(id).node_id) - .unwrap_or_else(|| self.next_id().node_id), ), span: s.span, }) .collect(); } - StmtKind::Expr(ref e) => Spanned { - node: hir::StmtKind::Expr(P(self.lower_expr(e)), self.lower_node_id(s.id).node_id), + StmtKind::Expr(ref e) => hir::Stmt { + id: self.lower_node_id(s.id).node_id, + node: hir::StmtKind::Expr(P(self.lower_expr(e))), span: s.span, }, - StmtKind::Semi(ref e) => Spanned { - node: hir::StmtKind::Semi(P(self.lower_expr(e)), self.lower_node_id(s.id).node_id), + StmtKind::Semi(ref e) => hir::Stmt { + id: self.lower_node_id(s.id).node_id, + node: hir::StmtKind::Semi(P(self.lower_expr(e))), span: s.span, }, StmtKind::Mac(..) => panic!("Shouldn't exist here"), @@ -4806,7 +4810,11 @@ impl<'a> LoweringContext<'a> { source, }); let decl = respan(sp, hir::DeclKind::Local(local)); - respan(sp, hir::StmtKind::Decl(P(decl), self.next_id().node_id)) + hir::Stmt { + id: self.next_id().node_id, + node: hir::StmtKind::Decl(P(decl)), + span: sp + } } fn stmt_let( diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index ae9bb3784299..7cc5d756ff31 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -426,7 +426,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_stmt(&mut self, stmt: &'hir Stmt) { - let id = stmt.node.id(); + let id = stmt.id; self.insert(stmt.span, id, Node::Stmt(stmt)); self.with_parent(id, |this| { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index fc4bd05476f6..1e287ccc85c7 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -16,7 +16,7 @@ use util::nodemap::{NodeMap, FxHashSet}; use mir::mono::Linkage; use syntax_pos::{Span, DUMMY_SP, symbol::InternedString}; -use syntax::source_map::{self, Spanned}; +use syntax::source_map::Spanned; use rustc_target::spec::abi::Abi; use syntax::ast::{self, CrateSugar, Ident, Name, NodeId, DUMMY_NODE_ID, AsmDialect}; use syntax::ast::{Attribute, Lit, StrStyle, FloatTy, IntTy, UintTy}; @@ -1144,45 +1144,38 @@ impl UnOp { } /// A statement -pub type Stmt = Spanned; +#[derive(Clone, RustcEncodable, RustcDecodable)] +pub struct Stmt { + pub id: NodeId, + pub node: StmtKind, + pub span: Span, +} -impl fmt::Debug for StmtKind { +impl fmt::Debug for Stmt { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // Sadness. - let spanned = source_map::dummy_spanned(self.clone()); - write!(f, - "stmt({}: {})", - spanned.node.id(), - print::to_string(print::NO_ANN, |s| s.print_stmt(&spanned))) + write!(f, "stmt({}: {})", self.id, + print::to_string(print::NO_ANN, |s| s.print_stmt(self))) } } #[derive(Clone, RustcEncodable, RustcDecodable)] pub enum StmtKind { /// Could be an item or a local (let) binding: - Decl(P, NodeId), + Decl(P), /// Expr without trailing semi-colon (must have unit type): - Expr(P, NodeId), + Expr(P), /// Expr with trailing semi-colon (may have any type): - Semi(P, NodeId), + Semi(P), } impl StmtKind { pub fn attrs(&self) -> &[Attribute] { match *self { - StmtKind::Decl(ref d, _) => d.node.attrs(), - StmtKind::Expr(ref e, _) | - StmtKind::Semi(ref e, _) => &e.attrs, - } - } - - pub fn id(&self) -> NodeId { - match *self { - StmtKind::Decl(_, id) | - StmtKind::Expr(_, id) | - StmtKind::Semi(_, id) => id, + StmtKind::Decl(ref d) => d.node.attrs(), + StmtKind::Expr(ref e) | + StmtKind::Semi(ref e) => &e.attrs, } } } diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index d7acdefcc7d7..d92800c7b953 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -992,14 +992,14 @@ impl<'a> State<'a> { pub fn print_stmt(&mut self, st: &hir::Stmt) -> io::Result<()> { self.maybe_print_comment(st.span.lo())?; match st.node { - hir::StmtKind::Decl(ref decl, _) => { + hir::StmtKind::Decl(ref decl) => { self.print_decl(&decl)?; } - hir::StmtKind::Expr(ref expr, _) => { + hir::StmtKind::Expr(ref expr) => { self.space_if_not_bol()?; self.print_expr(&expr)?; } - hir::StmtKind::Semi(ref expr, _) => { + hir::StmtKind::Semi(ref expr) => { self.space_if_not_bol()?; self.print_expr(&expr)?; self.s.word(";")?; @@ -2401,13 +2401,13 @@ fn expr_requires_semi_to_be_stmt(e: &hir::Expr) -> bool { /// seen the semicolon, and thus don't need another. fn stmt_ends_with_semi(stmt: &hir::StmtKind) -> bool { match *stmt { - hir::StmtKind::Decl(ref d, _) => { + hir::StmtKind::Decl(ref d) => { match d.node { hir::DeclKind::Local(_) => true, hir::DeclKind::Item(_) => false, } } - hir::StmtKind::Expr(ref e, _) => { + hir::StmtKind::Expr(ref e) => { expr_requires_semi_to_be_stmt(&e) } hir::StmtKind::Semi(..) => { diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 8ff60e5f5622..4da99f3c0f4b 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -483,7 +483,12 @@ impl_stable_hash_for!(enum hir::UnOp { UnNeg }); -impl_stable_hash_for_spanned!(hir::StmtKind); +impl_stable_hash_for!(struct hir::Stmt { + id, + node, + span, +}); + impl_stable_hash_for!(struct hir::Local { pat, @@ -941,9 +946,9 @@ impl_stable_hash_for!(enum hir::ForeignItemKind { }); impl_stable_hash_for!(enum hir::StmtKind { - Decl(decl, id), - Expr(expr, id), - Semi(expr, id) + Decl(decl), + Expr(expr), + Semi(expr) }); impl_stable_hash_for!(struct hir::Arg { diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index c1aa25b6b75c..0a65ce6fe846 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -589,7 +589,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { fn walk_stmt(&mut self, stmt: &hir::Stmt) { match stmt.node { - hir::StmtKind::Decl(ref decl, _) => { + hir::StmtKind::Decl(ref decl) => { match decl.node { hir::DeclKind::Local(ref local) => { self.walk_local(&local); @@ -602,8 +602,8 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { } } - hir::StmtKind::Expr(ref expr, _) | - hir::StmtKind::Semi(ref expr, _) => { + hir::StmtKind::Expr(ref expr) | + hir::StmtKind::Semi(ref expr) => { self.consume_expr(&expr); } } diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index a78cf1a471b4..190d3d1eb509 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -956,11 +956,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { fn propagate_through_stmt(&mut self, stmt: &hir::Stmt, succ: LiveNode) -> LiveNode { match stmt.node { - hir::StmtKind::Decl(ref decl, _) => { + hir::StmtKind::Decl(ref decl) => { self.propagate_through_decl(&decl, succ) } - hir::StmtKind::Expr(ref expr, _) | hir::StmtKind::Semi(ref expr, _) => { + hir::StmtKind::Expr(ref expr) | hir::StmtKind::Semi(ref expr) => { self.propagate_through_expr(&expr, succ) } } diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index ce2a34895062..76fbe2119f31 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -835,7 +835,7 @@ fn resolve_pat<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, pat: & } fn resolve_stmt<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, stmt: &'tcx hir::Stmt) { - let stmt_id = visitor.tcx.hir().node_to_hir_id(stmt.node.id()).local_id; + let stmt_id = visitor.tcx.hir().node_to_hir_id(stmt.id).local_id; debug!("resolve_stmt(stmt.id={:?})", stmt_id); // Every statement will clean up the temporaries created during diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 205f8941d1ee..587b28b3edde 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -41,7 +41,7 @@ impl LintPass for UnusedResults { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { fn check_stmt(&mut self, cx: &LateContext, s: &hir::Stmt) { let expr = match s.node { - hir::StmtKind::Semi(ref expr, _) => &**expr, + hir::StmtKind::Semi(ref expr) => &**expr, _ => return, }; @@ -205,7 +205,7 @@ impl LintPass for PathStatements { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PathStatements { fn check_stmt(&mut self, cx: &LateContext, s: &hir::Stmt) { - if let hir::StmtKind::Semi(ref expr, _) = s.node { + if let hir::StmtKind::Semi(ref expr) = s.node { if let hir::ExprKind::Path(_) = expr.node { cx.span_lint(PATH_STATEMENTS, s.span, "path statement with no effect"); } diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index ea8b2826732b..6fd31ffba399 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -46,12 +46,12 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, -> Vec> { let mut result = vec![]; for (index, stmt) in stmts.iter().enumerate() { - let hir_id = cx.tcx.hir().node_to_hir_id(stmt.node.id()); + let hir_id = cx.tcx.hir().node_to_hir_id(stmt.id); let opt_dxn_ext = cx.region_scope_tree.opt_destruction_scope(hir_id.local_id); - let stmt_span = StatementSpan(cx.tcx.hir().span(stmt.node.id())); + let stmt_span = StatementSpan(cx.tcx.hir().span(stmt.id)); match stmt.node { - hir::StmtKind::Expr(ref expr, _) | - hir::StmtKind::Semi(ref expr, _) => { + hir::StmtKind::Expr(ref expr) | + hir::StmtKind::Semi(ref expr) => { result.push(StmtRef::Mirror(Box::new(Stmt { kind: StmtKind::Expr { scope: region::Scope { @@ -64,7 +64,7 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, span: stmt_span, }))) } - hir::StmtKind::Decl(ref decl, _) => { + hir::StmtKind::Decl(ref decl) => { match decl.node { hir::DeclKind::Item(..) => { // ignore for purposes of the MIR diff --git a/src/librustc_passes/hir_stats.rs b/src/librustc_passes/hir_stats.rs index 604b31e7167a..f97f7dcf17f4 100644 --- a/src/librustc_passes/hir_stats.rs +++ b/src/librustc_passes/hir_stats.rs @@ -144,7 +144,7 @@ impl<'v> hir_visit::Visitor<'v> for StatCollector<'v> { } fn visit_stmt(&mut self, s: &'v hir::Stmt) { - self.record("Stmt", Id::Node(s.node.id()), s); + self.record("Stmt", Id::Node(s.id), s); hir_visit::walk_stmt(self, s) } diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs index f0b559f80a28..4831232a9f91 100644 --- a/src/librustc_passes/rvalue_promotion.rs +++ b/src/librustc_passes/rvalue_promotion.rs @@ -220,7 +220,7 @@ impl<'a, 'tcx> CheckCrateVisitor<'a, 'tcx> { fn check_stmt(&mut self, stmt: &'tcx hir::Stmt) -> Promotability { match stmt.node { - hir::StmtKind::Decl(ref decl, _node_id) => { + hir::StmtKind::Decl(ref decl) => { match &decl.node { hir::DeclKind::Local(local) => { if self.remove_mut_rvalue_borrow(&local.pat) { @@ -238,8 +238,8 @@ impl<'a, 'tcx> CheckCrateVisitor<'a, 'tcx> { hir::DeclKind::Item(_) => Promotable } } - hir::StmtKind::Expr(ref box_expr, _node_id) | - hir::StmtKind::Semi(ref box_expr, _node_id) => { + hir::StmtKind::Expr(ref box_expr) | + hir::StmtKind::Semi(ref box_expr) => { let _ = self.check_expr(box_expr); NotPromotable } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 1b07385d4d1f..ba97c3d2549a 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4840,7 +4840,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { pub fn check_stmt(&self, stmt: &'gcx hir::Stmt) { // Don't do all the complex logic below for `DeclItem`. match stmt.node { - hir::StmtKind::Decl(ref decl, _) => { + hir::StmtKind::Decl(ref decl) => { if let hir::DeclKind::Item(_) = decl.node { return } @@ -4848,7 +4848,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { hir::StmtKind::Expr(..) | hir::StmtKind::Semi(..) => {} } - self.warn_if_unreachable(stmt.node.id(), stmt.span, "statement"); + self.warn_if_unreachable(stmt.id, stmt.span, "statement"); // Hide the outer diverging and `has_errors` flags. let old_diverges = self.diverges.get(); @@ -4857,7 +4857,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.has_errors.set(false); match stmt.node { - hir::StmtKind::Decl(ref decl, _) => { + hir::StmtKind::Decl(ref decl) => { match decl.node { hir::DeclKind::Local(ref l) => { self.check_decl_local(&l); @@ -4866,11 +4866,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { hir::DeclKind::Item(_) => () } } - hir::StmtKind::Expr(ref expr, _) => { + hir::StmtKind::Expr(ref expr) => { // Check with expected type of `()`. self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit()); } - hir::StmtKind::Semi(ref expr, _) => { + hir::StmtKind::Semi(ref expr) => { self.check_expr(&expr); } } @@ -5273,7 +5273,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { None => return None, }; let last_expr = match last_stmt.node { - hir::StmtKind::Semi(ref e, _) => e, + hir::StmtKind::Semi(ref e) => e, _ => return None, }; let last_expr_ty = self.node_ty(last_expr.hir_id); From afbd004d696063adf1301ae461c41565f2f1921a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 17 Jan 2019 10:39:24 +1100 Subject: [PATCH 03/11] Remove `hir::StmtKind::Decl`. It's a level of indirection that hurts far more than it helps. The code is simpler without it. (This commit cuts more than 120 lines of code.) In particular, this commit removes some unnecessary `Span`s within `DeclKind` that were always identical to those in the enclosing `Stmt`, and some unnecessary allocations via `P`. --- src/librustc/cfg/construct.rs | 31 ++++------ src/librustc/hir/check_attr.rs | 4 +- src/librustc/hir/intravisit.rs | 15 +---- src/librustc/hir/lowering.rs | 34 +++------- src/librustc/hir/mod.rs | 35 ++--------- src/librustc/hir/print.rs | 59 +++++++----------- src/librustc/ich/impls_hir.rs | 9 +-- src/librustc/lint/context.rs | 5 -- src/librustc/lint/mod.rs | 1 - src/librustc/middle/expr_use_visitor.rs | 16 ++--- src/librustc/middle/liveness.rs | 51 ++++++--------- src/librustc/middle/region.rs | 33 +++++----- src/librustc_mir/hair/cx/block.rs | 82 ++++++++++++------------- src/librustc_passes/hir_stats.rs | 5 -- src/librustc_passes/rvalue_promotion.rs | 26 ++++---- src/librustc_typeck/check/mod.rs | 20 ++---- 16 files changed, 152 insertions(+), 274 deletions(-) diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index 1cbfcc166432..6122fe637094 100644 --- a/src/librustc/cfg/construct.rs +++ b/src/librustc/cfg/construct.rs @@ -100,29 +100,20 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { fn stmt(&mut self, stmt: &hir::Stmt, pred: CFGIndex) -> CFGIndex { let hir_id = self.tcx.hir().node_to_hir_id(stmt.id); - match stmt.node { - hir::StmtKind::Decl(ref decl) => { - let exit = self.decl(&decl, pred); - self.add_ast_node(hir_id.local_id, &[exit]) - } - - hir::StmtKind::Expr(ref expr) | - hir::StmtKind::Semi(ref expr) => { - let exit = self.expr(&expr, pred); - self.add_ast_node(hir_id.local_id, &[exit]) - } - } - } - - fn decl(&mut self, decl: &hir::Decl, pred: CFGIndex) -> CFGIndex { - match decl.node { - hir::DeclKind::Local(ref local) => { + let exit = match stmt.node { + hir::StmtKind::Local(ref local) => { let init_exit = self.opt_expr(&local.init, pred); self.pat(&local.pat, init_exit) } - - hir::DeclKind::Item(_) => pred, - } + hir::StmtKind::Item(_) => { + pred + } + hir::StmtKind::Expr(ref expr) | + hir::StmtKind::Semi(ref expr) => { + self.expr(&expr, pred) + } + }; + self.add_ast_node(hir_id.local_id, &[exit]) } fn pat(&mut self, pat: &hir::Pat, pred: CFGIndex) -> CFGIndex { diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 1cffe5ace5a6..9fa5f4aabe51 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -298,8 +298,8 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { fn check_stmt_attributes(&self, stmt: &hir::Stmt) { // When checking statements ignore expressions, they will be checked later - if let hir::StmtKind::Decl(..) = stmt.node { - for attr in stmt.node.attrs() { + if let hir::StmtKind::Local(ref l) = stmt.node { + for attr in l.attrs.iter() { if attr.check_name("inline") { self.check_inline(attr, &stmt.span, Target::Statement); } diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 3b9358d7c46a..a3eda804aa70 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -260,9 +260,6 @@ pub trait Visitor<'v> : Sized { fn visit_pat(&mut self, p: &'v Pat) { walk_pat(self, p) } - fn visit_decl(&mut self, d: &'v Decl) { - walk_decl(self, d) - } fn visit_anon_const(&mut self, c: &'v AnonConst) { walk_anon_const(self, c) } @@ -955,9 +952,8 @@ pub fn walk_block<'v, V: Visitor<'v>>(visitor: &mut V, block: &'v Block) { pub fn walk_stmt<'v, V: Visitor<'v>>(visitor: &mut V, statement: &'v Stmt) { visitor.visit_id(statement.id); match statement.node { - StmtKind::Decl(ref declaration) => { - visitor.visit_decl(declaration) - } + StmtKind::Local(ref local) => visitor.visit_local(local), + StmtKind::Item(ref item) => visitor.visit_nested_item(**item), StmtKind::Expr(ref expression) | StmtKind::Semi(ref expression) => { visitor.visit_expr(expression) @@ -965,13 +961,6 @@ pub fn walk_stmt<'v, V: Visitor<'v>>(visitor: &mut V, statement: &'v Stmt) { } } -pub fn walk_decl<'v, V: Visitor<'v>>(visitor: &mut V, declaration: &'v Decl) { - match declaration.node { - DeclKind::Local(ref local) => visitor.visit_local(local), - DeclKind::Item(item) => visitor.visit_nested_item(item), - } -} - pub fn walk_anon_const<'v, V: Visitor<'v>>(visitor: &mut V, constant: &'v AnonConst) { visitor.visit_id(constant.id); visitor.visit_nested_body(constant.body); diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 837960a0f1d9..02e66300f24d 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1957,7 +1957,7 @@ impl<'a> LoweringContext<'a> { ) } - fn lower_local(&mut self, l: &Local) -> (P, SmallVec<[hir::ItemId; 1]>) { + fn lower_local(&mut self, l: &Local) -> (hir::Local, SmallVec<[hir::ItemId; 1]>) { let LoweredNodeId { node_id, hir_id } = self.lower_node_id(l.id); let mut ids = SmallVec::<[hir::ItemId; 1]>::new(); if self.sess.features_untracked().impl_trait_in_bindings { @@ -1967,7 +1967,7 @@ impl<'a> LoweringContext<'a> { } } let parent_def_id = DefId::local(self.current_hir_id_owner.last().unwrap().0); - (P(hir::Local { + (hir::Local { id: node_id, hir_id, ty: l.ty @@ -1984,7 +1984,7 @@ impl<'a> LoweringContext<'a> { span: l.span, attrs: l.attrs.clone(), source: hir::LocalSource::Normal, - }), ids) + }, ids) } fn lower_mutability(&mut self, m: Mutability) -> hir::Mutability { @@ -4537,23 +4537,13 @@ impl<'a> LoweringContext<'a> { .into_iter() .map(|item_id| hir::Stmt { id: self.next_id().node_id, - node: hir::StmtKind::Decl( - P(Spanned { - node: hir::DeclKind::Item(item_id), - span: s.span, - }), - ), + node: hir::StmtKind::Item(P(item_id)), span: s.span, }) .collect(); ids.push(hir::Stmt { id: self.lower_node_id(s.id).node_id, - node: hir::StmtKind::Decl( - P(Spanned { - node: hir::DeclKind::Local(l), - span: s.span, - }), - ), + node: hir::StmtKind::Local(P(l)), span: s.span, }); return ids; @@ -4567,12 +4557,7 @@ impl<'a> LoweringContext<'a> { id: id.take() .map(|id| self.lower_node_id(id).node_id) .unwrap_or_else(|| self.next_id().node_id), - node: hir::StmtKind::Decl( - P(Spanned { - node: hir::DeclKind::Item(item_id), - span: s.span, - }), - ), + node: hir::StmtKind::Item(P(item_id)), span: s.span, }) .collect(); @@ -4799,7 +4784,7 @@ impl<'a> LoweringContext<'a> { ) -> hir::Stmt { let LoweredNodeId { node_id, hir_id } = self.next_id(); - let local = P(hir::Local { + let local = hir::Local { pat, ty: None, init: ex, @@ -4808,11 +4793,10 @@ impl<'a> LoweringContext<'a> { span: sp, attrs: ThinVec::new(), source, - }); - let decl = respan(sp, hir::DeclKind::Local(local)); + }; hir::Stmt { id: self.next_id().node_id, - node: hir::StmtKind::Decl(P(decl)), + node: hir::StmtKind::Local(P(local)), span: sp } } diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 1e287ccc85c7..5660b879c973 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1160,8 +1160,10 @@ impl fmt::Debug for Stmt { #[derive(Clone, RustcEncodable, RustcDecodable)] pub enum StmtKind { - /// Could be an item or a local (let) binding: - Decl(P), + /// A local (let) binding: + Local(P), + /// An item binding: + Item(P), /// Expr without trailing semi-colon (must have unit type): Expr(P), @@ -1173,7 +1175,8 @@ pub enum StmtKind { impl StmtKind { pub fn attrs(&self) -> &[Attribute] { match *self { - StmtKind::Decl(ref d) => d.node.attrs(), + StmtKind::Local(ref l) => &l.attrs, + StmtKind::Item(_) => &[], StmtKind::Expr(ref e) | StmtKind::Semi(ref e) => &e.attrs, } @@ -1194,32 +1197,6 @@ pub struct Local { pub source: LocalSource, } -pub type Decl = Spanned; - -#[derive(Clone, RustcEncodable, RustcDecodable, Debug)] -pub enum DeclKind { - /// A local (let) binding: - Local(P), - /// An item binding: - Item(ItemId), -} - -impl DeclKind { - pub fn attrs(&self) -> &[Attribute] { - match *self { - DeclKind::Local(ref l) => &l.attrs, - DeclKind::Item(_) => &[] - } - } - - pub fn is_local(&self) -> bool { - match *self { - DeclKind::Local(_) => true, - _ => false, - } - } -} - /// represents one arm of a 'match' #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct Arm { diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index d92800c7b953..e950f25c2ac9 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -992,8 +992,23 @@ impl<'a> State<'a> { pub fn print_stmt(&mut self, st: &hir::Stmt) -> io::Result<()> { self.maybe_print_comment(st.span.lo())?; match st.node { - hir::StmtKind::Decl(ref decl) => { - self.print_decl(&decl)?; + hir::StmtKind::Local(ref loc) => { + self.space_if_not_bol()?; + self.ibox(indent_unit)?; + self.word_nbsp("let")?; + + self.ibox(indent_unit)?; + self.print_local_decl(&loc)?; + self.end()?; + if let Some(ref init) = loc.init { + self.nbsp()?; + self.word_space("=")?; + self.print_expr(&init)?; + } + self.end()? + } + hir::StmtKind::Item(ref item) => { + self.ann.nested(self, Nested::Item(**item))? } hir::StmtKind::Expr(ref expr) => { self.space_if_not_bol()?; @@ -1562,30 +1577,6 @@ impl<'a> State<'a> { Ok(()) } - pub fn print_decl(&mut self, decl: &hir::Decl) -> io::Result<()> { - self.maybe_print_comment(decl.span.lo())?; - match decl.node { - hir::DeclKind::Local(ref loc) => { - self.space_if_not_bol()?; - self.ibox(indent_unit)?; - self.word_nbsp("let")?; - - self.ibox(indent_unit)?; - self.print_local_decl(&loc)?; - self.end()?; - if let Some(ref init) = loc.init { - self.nbsp()?; - self.word_space("=")?; - self.print_expr(&init)?; - } - self.end() - } - hir::DeclKind::Item(item) => { - self.ann.nested(self, Nested::Item(item)) - } - } - } - pub fn print_usize(&mut self, i: usize) -> io::Result<()> { self.s.word(i.to_string()) } @@ -2401,18 +2392,10 @@ fn expr_requires_semi_to_be_stmt(e: &hir::Expr) -> bool { /// seen the semicolon, and thus don't need another. fn stmt_ends_with_semi(stmt: &hir::StmtKind) -> bool { match *stmt { - hir::StmtKind::Decl(ref d) => { - match d.node { - hir::DeclKind::Local(_) => true, - hir::DeclKind::Item(_) => false, - } - } - hir::StmtKind::Expr(ref e) => { - expr_requires_semi_to_be_stmt(&e) - } - hir::StmtKind::Semi(..) => { - false - } + hir::StmtKind::Local(_) => true, + hir::StmtKind::Item(_) => false, + hir::StmtKind::Expr(ref e) => expr_requires_semi_to_be_stmt(&e), + hir::StmtKind::Semi(..) => false, } } diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 4da99f3c0f4b..9aa8e4c229f4 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -501,12 +501,6 @@ impl_stable_hash_for!(struct hir::Local { source }); -impl_stable_hash_for_spanned!(hir::DeclKind); -impl_stable_hash_for!(enum hir::DeclKind { - Local(local), - Item(item_id) -}); - impl_stable_hash_for!(struct hir::Arm { attrs, pats, @@ -946,7 +940,8 @@ impl_stable_hash_for!(enum hir::ForeignItemKind { }); impl_stable_hash_for!(enum hir::StmtKind { - Decl(decl), + Local(local), + Item(item_id), Expr(expr), Semi(expr) }); diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index f5a7919ef09c..837a3645d1c9 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -941,11 +941,6 @@ impl<'a, 'tcx> hir_visit::Visitor<'tcx> for LateContext<'a, 'tcx> { hir_visit::walk_arm(self, a); } - fn visit_decl(&mut self, d: &'tcx hir::Decl) { - run_lints!(self, check_decl, d); - hir_visit::walk_decl(self, d); - } - fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam) { run_lints!(self, check_generic_param, p); hir_visit::walk_generic_param(self, p); diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index a373faab3a20..3caa88dbc898 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -191,7 +191,6 @@ macro_rules! late_lint_methods { fn check_stmt(a: &$hir hir::Stmt); fn check_arm(a: &$hir hir::Arm); fn check_pat(a: &$hir hir::Pat); - fn check_decl(a: &$hir hir::Decl); fn check_expr(a: &$hir hir::Expr); fn check_expr_post(a: &$hir hir::Expr); fn check_ty(a: &$hir hir::Ty); diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 0a65ce6fe846..08210c3f075c 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -589,17 +589,13 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { fn walk_stmt(&mut self, stmt: &hir::Stmt) { match stmt.node { - hir::StmtKind::Decl(ref decl) => { - match decl.node { - hir::DeclKind::Local(ref local) => { - self.walk_local(&local); - } + hir::StmtKind::Local(ref local) => { + self.walk_local(&local); + } - hir::DeclKind::Item(_) => { - // we don't visit nested items in this visitor, - // only the fn body we were given. - } - } + hir::StmtKind::Item(_) => { + // we don't visit nested items in this visitor, + // only the fn body we were given. } hir::StmtKind::Expr(ref expr) | diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 190d3d1eb509..13464242f1ce 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -956,46 +956,31 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { fn propagate_through_stmt(&mut self, stmt: &hir::Stmt, succ: LiveNode) -> LiveNode { match stmt.node { - hir::StmtKind::Decl(ref decl) => { - self.propagate_through_decl(&decl, succ) - } + hir::StmtKind::Local(ref local) => { + // Note: we mark the variable as defined regardless of whether + // there is an initializer. Initially I had thought to only mark + // the live variable as defined if it was initialized, and then we + // could check for uninit variables just by scanning what is live + // at the start of the function. But that doesn't work so well for + // immutable variables defined in a loop: + // loop { let x; x = 5; } + // because the "assignment" loops back around and generates an error. + // + // So now we just check that variables defined w/o an + // initializer are not live at the point of their + // initialization, which is mildly more complex than checking + // once at the func header but otherwise equivalent. + let succ = self.propagate_through_opt_expr(local.init.as_ref().map(|e| &**e), succ); + self.define_bindings_in_pat(&local.pat, succ) + } + hir::StmtKind::Item(..) => succ, hir::StmtKind::Expr(ref expr) | hir::StmtKind::Semi(ref expr) => { self.propagate_through_expr(&expr, succ) } } } - fn propagate_through_decl(&mut self, decl: &hir::Decl, succ: LiveNode) - -> LiveNode { - match decl.node { - hir::DeclKind::Local(ref local) => { - self.propagate_through_local(&local, succ) - } - hir::DeclKind::Item(_) => succ, - } - } - - fn propagate_through_local(&mut self, local: &hir::Local, succ: LiveNode) - -> LiveNode { - // Note: we mark the variable as defined regardless of whether - // there is an initializer. Initially I had thought to only mark - // the live variable as defined if it was initialized, and then we - // could check for uninit variables just by scanning what is live - // at the start of the function. But that doesn't work so well for - // immutable variables defined in a loop: - // loop { let x; x = 5; } - // because the "assignment" loops back around and generates an error. - // - // So now we just check that variables defined w/o an - // initializer are not live at the point of their - // initialization, which is mildly more complex than checking - // once at the func header but otherwise equivalent. - - let succ = self.propagate_through_opt_expr(local.init.as_ref().map(|e| &**e), succ); - self.define_bindings_in_pat(&local.pat, succ) - } - fn propagate_through_exprs(&mut self, exprs: &[Expr], succ: LiveNode) -> LiveNode { exprs.iter().rev().fold(succ, |succ, expr| { diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 76fbe2119f31..819dd8aa7d53 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -784,20 +784,25 @@ fn resolve_block<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, blk: // index information.) for (i, statement) in blk.stmts.iter().enumerate() { - if let hir::StmtKind::Decl(..) = statement.node { - // Each StmtKind::Decl introduces a subscope for bindings - // introduced by the declaration; this subscope covers - // a suffix of the block . Each subscope in a block - // has the previous subscope in the block as a parent, - // except for the first such subscope, which has the - // block itself as a parent. - visitor.enter_scope( - Scope { - id: blk.hir_id.local_id, - data: ScopeData::Remainder(FirstStatementIndex::new(i)) - } - ); - visitor.cx.var_parent = visitor.cx.parent; + match statement.node { + hir::StmtKind::Local(..) | + hir::StmtKind::Item(..) => { + // Each declaration introduces a subscope for bindings + // introduced by the declaration; this subscope covers a + // suffix of the block. Each subscope in a block has the + // previous subscope in the block as a parent, except for + // the first such subscope, which has the block itself as a + // parent. + visitor.enter_scope( + Scope { + id: blk.hir_id.local_id, + data: ScopeData::Remainder(FirstStatementIndex::new(i)) + } + ); + visitor.cx.var_parent = visitor.cx.parent; + } + hir::StmtKind::Expr(..) | + hir::StmtKind::Semi(..) => {} } visitor.visit_stmt(statement) } diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index 6fd31ffba399..c50d9ddcb152 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -64,52 +64,48 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, span: stmt_span, }))) } - hir::StmtKind::Decl(ref decl) => { - match decl.node { - hir::DeclKind::Item(..) => { - // ignore for purposes of the MIR - } - hir::DeclKind::Local(ref local) => { - let remainder_scope = region::Scope { - id: block_id, - data: region::ScopeData::Remainder( - region::FirstStatementIndex::new(index)), + hir::StmtKind::Item(..) => { + // ignore for purposes of the MIR + } + hir::StmtKind::Local(ref local) => { + let remainder_scope = region::Scope { + id: block_id, + data: region::ScopeData::Remainder( + region::FirstStatementIndex::new(index)), + }; + + let mut pattern = cx.pattern_from_hir(&local.pat); + + if let Some(ty) = &local.ty { + if let Some(&user_ty) = cx.tables.user_provided_types().get(ty.hir_id) { + debug!("mirror_stmts: user_ty={:?}", user_ty); + pattern = Pattern { + ty: pattern.ty, + span: pattern.span, + kind: Box::new(PatternKind::AscribeUserType { + user_ty: PatternTypeProjection::from_user_type(user_ty), + user_ty_span: ty.span, + subpattern: pattern, + variance: ty::Variance::Covariant, + }) }; - - let mut pattern = cx.pattern_from_hir(&local.pat); - - if let Some(ty) = &local.ty { - if let Some(&user_ty) = cx.tables.user_provided_types().get(ty.hir_id) { - debug!("mirror_stmts: user_ty={:?}", user_ty); - pattern = Pattern { - ty: pattern.ty, - span: pattern.span, - kind: Box::new(PatternKind::AscribeUserType { - user_ty: PatternTypeProjection::from_user_type(user_ty), - user_ty_span: ty.span, - subpattern: pattern, - variance: ty::Variance::Covariant, - }) - }; - } - } - - result.push(StmtRef::Mirror(Box::new(Stmt { - kind: StmtKind::Let { - remainder_scope: remainder_scope, - init_scope: region::Scope { - id: hir_id.local_id, - data: region::ScopeData::Node - }, - pattern, - initializer: local.init.to_ref(), - lint_level: cx.lint_level_of(local.id), - }, - opt_destruction_scope: opt_dxn_ext, - span: stmt_span, - }))); } } + + result.push(StmtRef::Mirror(Box::new(Stmt { + kind: StmtKind::Let { + remainder_scope: remainder_scope, + init_scope: region::Scope { + id: hir_id.local_id, + data: region::ScopeData::Node + }, + pattern, + initializer: local.init.to_ref(), + lint_level: cx.lint_level_of(local.id), + }, + opt_destruction_scope: opt_dxn_ext, + span: stmt_span, + }))); } } } diff --git a/src/librustc_passes/hir_stats.rs b/src/librustc_passes/hir_stats.rs index f97f7dcf17f4..74d6d75a7f52 100644 --- a/src/librustc_passes/hir_stats.rs +++ b/src/librustc_passes/hir_stats.rs @@ -158,11 +158,6 @@ impl<'v> hir_visit::Visitor<'v> for StatCollector<'v> { hir_visit::walk_pat(self, p) } - fn visit_decl(&mut self, d: &'v hir::Decl) { - self.record("Decl", Id::None, d); - hir_visit::walk_decl(self, d) - } - fn visit_expr(&mut self, ex: &'v hir::Expr) { self.record("Expr", Id::Node(ex.id), ex); hir_visit::walk_expr(self, ex) diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs index 4831232a9f91..49914dc7078f 100644 --- a/src/librustc_passes/rvalue_promotion.rs +++ b/src/librustc_passes/rvalue_promotion.rs @@ -220,24 +220,20 @@ impl<'a, 'tcx> CheckCrateVisitor<'a, 'tcx> { fn check_stmt(&mut self, stmt: &'tcx hir::Stmt) -> Promotability { match stmt.node { - hir::StmtKind::Decl(ref decl) => { - match &decl.node { - hir::DeclKind::Local(local) => { - if self.remove_mut_rvalue_borrow(&local.pat) { - if let Some(init) = &local.init { - self.mut_rvalue_borrows.insert(init.id); - } - } - - if let Some(ref expr) = local.init { - let _ = self.check_expr(&expr); - } - NotPromotable + hir::StmtKind::Local(ref local) => { + if self.remove_mut_rvalue_borrow(&local.pat) { + if let Some(init) = &local.init { + self.mut_rvalue_borrows.insert(init.id); } - // Item statements are allowed - hir::DeclKind::Item(_) => Promotable } + + if let Some(ref expr) = local.init { + let _ = self.check_expr(&expr); + } + NotPromotable } + // Item statements are allowed + hir::StmtKind::Item(..) => Promotable, hir::StmtKind::Expr(ref box_expr) | hir::StmtKind::Semi(ref box_expr) => { let _ = self.check_expr(box_expr); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index ba97c3d2549a..daade27768ae 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4840,12 +4840,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { pub fn check_stmt(&self, stmt: &'gcx hir::Stmt) { // Don't do all the complex logic below for `DeclItem`. match stmt.node { - hir::StmtKind::Decl(ref decl) => { - if let hir::DeclKind::Item(_) = decl.node { - return - } - } - hir::StmtKind::Expr(..) | hir::StmtKind::Semi(..) => {} + hir::StmtKind::Item(..) => return, + hir::StmtKind::Local(..) | hir::StmtKind::Expr(..) | hir::StmtKind::Semi(..) => {} } self.warn_if_unreachable(stmt.id, stmt.span, "statement"); @@ -4857,15 +4853,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.has_errors.set(false); match stmt.node { - hir::StmtKind::Decl(ref decl) => { - match decl.node { - hir::DeclKind::Local(ref l) => { - self.check_decl_local(&l); - } - // Ignore for now. - hir::DeclKind::Item(_) => () - } + hir::StmtKind::Local(ref l) => { + self.check_decl_local(&l); } + // Ignore for now. + hir::StmtKind::Item(_) => {} hir::StmtKind::Expr(ref expr) => { // Check with expected type of `()`. self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit()); From 90507295db1665b7567f66fc12ceb0b15e32d44d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 17 Jan 2019 20:26:00 -0800 Subject: [PATCH 04/11] Do not give incorrect label for return type mismatch --- src/librustc_typeck/check/coercion.rs | 28 ++++++++-- ...o-type-err-cause-on-impl-trait-return-2.rs | 17 ++++++ ...pe-err-cause-on-impl-trait-return-2.stderr | 12 +++++ ...-to-type-err-cause-on-impl-trait-return.rs | 36 +++++++++++++ ...type-err-cause-on-impl-trait-return.stderr | 52 +++++++++++++++++++ 5 files changed, 142 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.rs create mode 100644 src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.stderr create mode 100644 src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs create mode 100644 src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index a82a0d3ce523..d1c54f824fe9 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -1224,9 +1224,31 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E> cause.span, blk_id, ); - if let Some(sp) = fcx.ret_coercion_span.borrow().as_ref() { - if !sp.overlaps(cause.span) { - db.span_label(*sp, reason_label); + // TODO: replace with navigating up the chain until hitting an fn or + // bailing if no "pass-through" Node is found, in order to provide a + // suggestion when encountering something like: + // ``` + // fn foo(a: bool) -> impl Debug { + // if a { + // bar()?; + // } + // { + // let x = unsafe { bar() }; + // x + // } + // } + // ``` + // + // Verify that this is a tail expression of a function, otherwise the + // label pointing out the cause for the type coercion will be wrong + // as prior return coercions would not be relevant (#57664). + let parent_id = fcx.tcx.hir().get_parent_node(blk_id); + let parent = fcx.tcx.hir().get(fcx.tcx.hir().get_parent_node(parent_id)); + if fcx.get_node_fn_decl(parent).is_some() { + if let Some(sp) = fcx.ret_coercion_span.borrow().as_ref() { + if !sp.overlaps(cause.span) { + db.span_label(*sp, reason_label); + } } } } diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.rs b/src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.rs new file mode 100644 index 000000000000..50f1fe873cb5 --- /dev/null +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.rs @@ -0,0 +1,17 @@ +fn unrelated() -> Result<(), std::string::ParseError> { // #57664 + let x = 0; + + match x { + 1 => { + let property_value_as_string = "a".parse()?; + } + 2 => { + let value: &bool = unsafe { &42 }; + //~^ ERROR mismatched types + } + }; + + Ok(()) +} + +fn main() {} diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.stderr b/src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.stderr new file mode 100644 index 000000000000..edaa60e5b8d8 --- /dev/null +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return-2.stderr @@ -0,0 +1,12 @@ +error[E0308]: mismatched types + --> $DIR/point-to-type-err-cause-on-impl-trait-return-2.rs:9:41 + | +LL | let value: &bool = unsafe { &42 }; + | ^^^ expected bool, found integer + | + = note: expected type `&bool` + found type `&{integer}` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs new file mode 100644 index 000000000000..a27df240d079 --- /dev/null +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs @@ -0,0 +1,36 @@ +fn foo() -> impl std::fmt::Display { + if false { + return 0i32; + } + 1u32 + //~^ ERROR mismatched types +} + +fn bar() -> impl std::fmt::Display { + if false { + return 0i32; + } else { + return 1u32; + //~^ ERROR mismatched types + } +} + +fn baz() -> impl std::fmt::Display { + if false { + //~^ ERROR mismatched types + return 0i32; + } else { + 1u32 + } +} + +fn qux() -> impl std::fmt::Display { + if false { + //~^ ERROR if and else have incompatible types + 0i32 + } else { + 1u32 + } +} + +fn main() {} diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr new file mode 100644 index 000000000000..54f7b108c3dd --- /dev/null +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr @@ -0,0 +1,52 @@ +error[E0308]: mismatched types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:5:5 + | +LL | return 0i32; + | ---- expected because of this statement +LL | } +LL | 1u32 + | ^^^^ expected i32, found u32 + | + = note: expected type `i32` + found type `u32` + +error[E0308]: mismatched types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:13:16 + | +LL | return 1u32; + | ^^^^ expected i32, found u32 + | + = note: expected type `i32` + found type `u32` + +error[E0308]: mismatched types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:19:5 + | +LL | / if false { +LL | | //~^ ERROR mismatched types +LL | | return 0i32; +LL | | } else { +LL | | 1u32 +LL | | } + | |_____^ expected i32, found u32 + | + = note: expected type `i32` + found type `u32` + +error[E0308]: if and else have incompatible types + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:28:5 + | +LL | / if false { +LL | | //~^ ERROR if and else have incompatible types +LL | | 0i32 +LL | | } else { +LL | | 1u32 +LL | | } + | |_____^ expected i32, found u32 + | + = note: expected type `i32` + found type `u32` + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`. From 19255dc2e670085a7bf6864feccb323077309325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 17 Jan 2019 20:45:50 -0800 Subject: [PATCH 05/11] Point more places where expectation comes from --- src/librustc_typeck/check/coercion.rs | 4 +--- src/test/ui/diverging-tuple-parts-39485.stderr | 5 ++++- src/test/ui/issues/issue-10176.stderr | 5 ++++- .../ui/point-to-type-err-cause-on-impl-trait-return.stderr | 1 + 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index d1c54f824fe9..28114b527933 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -1246,9 +1246,7 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E> let parent = fcx.tcx.hir().get(fcx.tcx.hir().get_parent_node(parent_id)); if fcx.get_node_fn_decl(parent).is_some() { if let Some(sp) = fcx.ret_coercion_span.borrow().as_ref() { - if !sp.overlaps(cause.span) { - db.span_label(*sp, reason_label); - } + db.span_label(*sp, reason_label); } } } diff --git a/src/test/ui/diverging-tuple-parts-39485.stderr b/src/test/ui/diverging-tuple-parts-39485.stderr index c399650d3258..f8da9282f5fb 100644 --- a/src/test/ui/diverging-tuple-parts-39485.stderr +++ b/src/test/ui/diverging-tuple-parts-39485.stderr @@ -15,7 +15,10 @@ error[E0308]: mismatched types LL | fn f() -> isize { | ----- expected `isize` because of return type LL | (return 1, return 2) //~ ERROR mismatched types - | ^^^^^^^^^^^^^^^^^^^^ expected isize, found tuple + | ^^^^^^^^^^^^^^^^^^-^ + | | | + | | expected because of this statement + | expected isize, found tuple | = note: expected type `isize` found type `(!, !)` diff --git a/src/test/ui/issues/issue-10176.stderr b/src/test/ui/issues/issue-10176.stderr index 45482447ebe2..592c84bd17d0 100644 --- a/src/test/ui/issues/issue-10176.stderr +++ b/src/test/ui/issues/issue-10176.stderr @@ -4,7 +4,10 @@ error[E0308]: mismatched types LL | fn f() -> isize { | ----- expected `isize` because of return type LL | (return 1, return 2) - | ^^^^^^^^^^^^^^^^^^^^ expected isize, found tuple + | ^^^^^^^^^^^^^^^^^^-^ + | | | + | | expected because of this statement + | expected isize, found tuple | = note: expected type `isize` found type `(!, !)` diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr index 54f7b108c3dd..b94a9f4df487 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr @@ -25,6 +25,7 @@ error[E0308]: mismatched types LL | / if false { LL | | //~^ ERROR mismatched types LL | | return 0i32; + | | ---- expected because of this statement LL | | } else { LL | | 1u32 LL | | } From c4318502bca669f399b7428d3e9a180b1de041bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 17 Jan 2019 21:06:09 -0800 Subject: [PATCH 06/11] Avoid pointing at multiple places on return type error --- src/librustc_typeck/check/coercion.rs | 4 +-- src/librustc_typeck/check/mod.rs | 28 +++++++++++++------ .../ui/diverging-tuple-parts-39485.stderr | 5 +--- src/test/ui/issues/issue-10176.stderr | 5 +--- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 28114b527933..f040781e56f7 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -1216,7 +1216,7 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E> "supposed to be part of a block tail expression, but the \ expression is empty"); }); - fcx.suggest_mismatched_types_on_tail( + let pointing_at_return_type = fcx.suggest_mismatched_types_on_tail( &mut db, expr, expected, @@ -1244,7 +1244,7 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E> // as prior return coercions would not be relevant (#57664). let parent_id = fcx.tcx.hir().get_parent_node(blk_id); let parent = fcx.tcx.hir().get(fcx.tcx.hir().get_parent_node(parent_id)); - if fcx.get_node_fn_decl(parent).is_some() { + if fcx.get_node_fn_decl(parent).is_some() && !pointing_at_return_type { if let Some(sp) = fcx.ret_coercion_span.borrow().as_ref() { db.span_label(*sp, reason_label); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 1b07385d4d1f..ce876a799644 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5089,12 +5089,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { found: Ty<'tcx>, cause_span: Span, blk_id: ast::NodeId, - ) { + ) -> bool { self.suggest_missing_semicolon(err, expression, expected, cause_span); + let mut pointing_at_return_type = false; if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) { - self.suggest_missing_return_type(err, &fn_decl, expected, found, can_suggest); + pointing_at_return_type = self.suggest_missing_return_type( + err, &fn_decl, expected, found, can_suggest); } self.suggest_ref_or_into(err, expression, expected, found); + pointing_at_return_type } pub fn suggest_ref_or_into( @@ -5193,12 +5196,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { /// This routine checks if the return type is left as default, the method is not part of an /// `impl` block and that it isn't the `main` method. If so, it suggests setting the return /// type. - fn suggest_missing_return_type(&self, - err: &mut DiagnosticBuilder<'tcx>, - fn_decl: &hir::FnDecl, - expected: Ty<'tcx>, - found: Ty<'tcx>, - can_suggest: bool) { + fn suggest_missing_return_type( + &self, + err: &mut DiagnosticBuilder<'tcx>, + fn_decl: &hir::FnDecl, + expected: Ty<'tcx>, + found: Ty<'tcx>, + can_suggest: bool, + ) -> bool { // Only suggest changing the return type for methods that // haven't set a return type at all (and aren't `fn main()` or an impl). match (&fn_decl.output, found.is_suggestable(), can_suggest, expected.is_unit()) { @@ -5208,16 +5213,19 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { "try adding a return type", format!("-> {} ", self.resolve_type_vars_with_obligations(found)), Applicability::MachineApplicable); + true } (&hir::FunctionRetTy::DefaultReturn(span), false, true, true) => { err.span_label(span, "possibly return type missing here?"); + true } (&hir::FunctionRetTy::DefaultReturn(span), _, false, true) => { // `fn main()` must return `()`, do not suggest changing return type err.span_label(span, "expected `()` because of default return type"); + true } // expectation was caused by something else, not the default return - (&hir::FunctionRetTy::DefaultReturn(_), _, _, false) => {} + (&hir::FunctionRetTy::DefaultReturn(_), _, _, false) => false, (&hir::FunctionRetTy::Return(ref ty), _, _, _) => { // Only point to return type if the expected type is the return type, as if they // are not, the expectation must have been caused by something else. @@ -5229,7 +5237,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if ty.sty == expected.sty { err.span_label(sp, format!("expected `{}` because of return type", expected)); + return true; } + false } } } diff --git a/src/test/ui/diverging-tuple-parts-39485.stderr b/src/test/ui/diverging-tuple-parts-39485.stderr index f8da9282f5fb..c399650d3258 100644 --- a/src/test/ui/diverging-tuple-parts-39485.stderr +++ b/src/test/ui/diverging-tuple-parts-39485.stderr @@ -15,10 +15,7 @@ error[E0308]: mismatched types LL | fn f() -> isize { | ----- expected `isize` because of return type LL | (return 1, return 2) //~ ERROR mismatched types - | ^^^^^^^^^^^^^^^^^^-^ - | | | - | | expected because of this statement - | expected isize, found tuple + | ^^^^^^^^^^^^^^^^^^^^ expected isize, found tuple | = note: expected type `isize` found type `(!, !)` diff --git a/src/test/ui/issues/issue-10176.stderr b/src/test/ui/issues/issue-10176.stderr index 592c84bd17d0..45482447ebe2 100644 --- a/src/test/ui/issues/issue-10176.stderr +++ b/src/test/ui/issues/issue-10176.stderr @@ -4,10 +4,7 @@ error[E0308]: mismatched types LL | fn f() -> isize { | ----- expected `isize` because of return type LL | (return 1, return 2) - | ^^^^^^^^^^^^^^^^^^-^ - | | | - | | expected because of this statement - | expected isize, found tuple + | ^^^^^^^^^^^^^^^^^^^^ expected isize, found tuple | = note: expected type `isize` found type `(!, !)` From 9b8243ac2450c49f95cfbee517109ce13d00f95e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 17 Jan 2019 21:18:51 -0800 Subject: [PATCH 07/11] Point at more cases involving return types --- src/librustc_typeck/check/mod.rs | 8 ++++++-- .../point-to-type-err-cause-on-impl-trait-return.stderr | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index ce876a799644..7b9b97ff065a 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4347,11 +4347,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { struct_span_err!(self.tcx.sess, expr.span, E0572, "return statement outside of function body").emit(); } else if let Some(ref e) = *expr_opt { - *self.ret_coercion_span.borrow_mut() = Some(e.span); + if self.ret_coercion_span.borrow().is_none() { + *self.ret_coercion_span.borrow_mut() = Some(e.span); + } self.check_return_expr(e); } else { let mut coercion = self.ret_coercion.as_ref().unwrap().borrow_mut(); - *self.ret_coercion_span.borrow_mut() = Some(expr.span); + if self.ret_coercion_span.borrow().is_none() { + *self.ret_coercion_span.borrow_mut() = Some(expr.span); + } let cause = self.cause(expr.span, ObligationCauseCode::ReturnNoExpression); if let Some((fn_decl, _)) = self.get_fn_decl(expr.id) { coercion.coerce_forced_unit( diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr index b94a9f4df487..be3d2c0db4ca 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr @@ -13,6 +13,9 @@ LL | 1u32 error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:13:16 | +LL | return 0i32; + | ---- expected because of this statement +LL | } else { LL | return 1u32; | ^^^^ expected i32, found u32 | From fbb072837dbbaf30a1e81674b457e5ea816dbca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 17 Jan 2019 22:32:59 -0800 Subject: [PATCH 08/11] Fix tidy error --- src/librustc_typeck/check/coercion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index f040781e56f7..7f053d6af961 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -1224,7 +1224,7 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E> cause.span, blk_id, ); - // TODO: replace with navigating up the chain until hitting an fn or + // FIXME: replace with navigating up the chain until hitting an fn or // bailing if no "pass-through" Node is found, in order to provide a // suggestion when encountering something like: // ``` From 954769e2ed16a90bfda2b69395fc099b93581352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 17 Jan 2019 22:51:01 -0800 Subject: [PATCH 09/11] Fix test after rebase --- .../ui/point-to-type-err-cause-on-impl-trait-return.rs | 2 +- .../point-to-type-err-cause-on-impl-trait-return.stderr | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs index a27df240d079..95b40368143e 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs @@ -26,10 +26,10 @@ fn baz() -> impl std::fmt::Display { fn qux() -> impl std::fmt::Display { if false { - //~^ ERROR if and else have incompatible types 0i32 } else { 1u32 + //~^ ERROR if and else have incompatible types } } diff --git a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr index be3d2c0db4ca..62da0787b02a 100644 --- a/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr +++ b/src/test/ui/point-to-type-err-cause-on-impl-trait-return.stderr @@ -38,15 +38,17 @@ LL | | } found type `u32` error[E0308]: if and else have incompatible types - --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:28:5 + --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:31:9 | LL | / if false { -LL | | //~^ ERROR if and else have incompatible types LL | | 0i32 + | | ---- expected because of this LL | | } else { LL | | 1u32 + | | ^^^^ expected i32, found u32 +LL | | //~^ ERROR if and else have incompatible types LL | | } - | |_____^ expected i32, found u32 + | |_____- if and else have incompatible types | = note: expected type `i32` found type `u32` From 2e06d9c91b9f0bccde4a0e445ce2014c3fe85506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 18 Jan 2019 00:12:09 -0800 Subject: [PATCH 10/11] Point at return type when appropriate --- src/librustc_typeck/check/coercion.rs | 16 ++++++++++++++-- .../fully-qualified-type-name2.stderr | 2 ++ .../fully-qualified-type-name4.stderr | 2 ++ src/test/ui/liveness/liveness-forgot-ret.stderr | 2 +- src/test/ui/proc-macro/span-preservation.stderr | 3 +++ src/test/ui/return/return-from-diverging.stderr | 2 ++ src/test/ui/tail-typeck.stderr | 4 +++- src/test/ui/wrong-ret-type.stderr | 4 +++- 8 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 7f053d6af961..dd63b4f20fa5 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -1250,14 +1250,26 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E> } } } - _ => { + ObligationCauseCode::ReturnType(_id) => { db = fcx.report_mismatched_types(cause, expected, found, err); - if let Some(sp) = fcx.ret_coercion_span.borrow().as_ref() { + let _id = fcx.tcx.hir().get_parent_node(_id); + let mut pointing_at_return_type = false; + if let Some((fn_decl, can_suggest)) = fcx.get_fn_decl(_id) { + pointing_at_return_type = fcx.suggest_missing_return_type( + &mut db, &fn_decl, expected, found, can_suggest); + } + if let (Some(sp), false) = ( + fcx.ret_coercion_span.borrow().as_ref(), + pointing_at_return_type, + ) { if !sp.overlaps(cause.span) { db.span_label(*sp, reason_label); } } } + _ => { + db = fcx.report_mismatched_types(cause, expected, found, err); + } } if let Some(augment_error) = augment_error { diff --git a/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr b/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr index 9a33d29766fb..47bb5e475b47 100644 --- a/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr +++ b/src/test/ui/fully-qualified-type/fully-qualified-type-name2.stderr @@ -1,6 +1,8 @@ error[E0308]: mismatched types --> $DIR/fully-qualified-type-name2.rs:12:12 | +LL | fn bar(x: x::Foo) -> y::Foo { + | ------ expected `y::Foo` because of return type LL | return x; | ^ expected enum `y::Foo`, found enum `x::Foo` | diff --git a/src/test/ui/fully-qualified-type/fully-qualified-type-name4.stderr b/src/test/ui/fully-qualified-type/fully-qualified-type-name4.stderr index f03aaa67edbb..b341879ab919 100644 --- a/src/test/ui/fully-qualified-type/fully-qualified-type-name4.stderr +++ b/src/test/ui/fully-qualified-type/fully-qualified-type-name4.stderr @@ -1,6 +1,8 @@ error[E0308]: mismatched types --> $DIR/fully-qualified-type-name4.rs:6:12 | +LL | fn bar(x: usize) -> Option { + | ------------- expected `std::option::Option` because of return type LL | return x; | ^ expected enum `std::option::Option`, found usize | diff --git a/src/test/ui/liveness/liveness-forgot-ret.stderr b/src/test/ui/liveness/liveness-forgot-ret.stderr index bbcbbdbe8dd5..a970b80fdbbd 100644 --- a/src/test/ui/liveness/liveness-forgot-ret.stderr +++ b/src/test/ui/liveness/liveness-forgot-ret.stderr @@ -2,7 +2,7 @@ error[E0308]: mismatched types --> $DIR/liveness-forgot-ret.rs:3:19 | LL | fn f(a: isize) -> isize { if god_exists(a) { return 5; }; } - | - ^^^^^ expected isize, found () - expected because of this statement + | - ^^^^^ expected isize, found () | | | this function's body doesn't return | diff --git a/src/test/ui/proc-macro/span-preservation.stderr b/src/test/ui/proc-macro/span-preservation.stderr index 1f9103a6d2ee..db524907b656 100644 --- a/src/test/ui/proc-macro/span-preservation.stderr +++ b/src/test/ui/proc-macro/span-preservation.stderr @@ -15,6 +15,9 @@ LL | let x: usize = "hello";;;;; //~ ERROR mismatched types error[E0308]: mismatched types --> $DIR/span-preservation.rs:19:29 | +LL | fn b(x: Option) -> usize { + | ----- expected `usize` because of return type +LL | match x { LL | Some(x) => { return x }, //~ ERROR mismatched types | ^ expected usize, found isize diff --git a/src/test/ui/return/return-from-diverging.stderr b/src/test/ui/return/return-from-diverging.stderr index c84dd1953a07..2862ae641df1 100644 --- a/src/test/ui/return/return-from-diverging.stderr +++ b/src/test/ui/return/return-from-diverging.stderr @@ -1,6 +1,8 @@ error[E0308]: mismatched types --> $DIR/return-from-diverging.rs:4:12 | +LL | fn fail() -> ! { + | - expected `!` because of return type LL | return "wow"; //~ ERROR mismatched types | ^^^^^ expected !, found reference | diff --git a/src/test/ui/tail-typeck.stderr b/src/test/ui/tail-typeck.stderr index eadf3d6d3350..1170f5c17c18 100644 --- a/src/test/ui/tail-typeck.stderr +++ b/src/test/ui/tail-typeck.stderr @@ -2,7 +2,9 @@ error[E0308]: mismatched types --> $DIR/tail-typeck.rs:3:26 | LL | fn f() -> isize { return g(); } - | ^^^ expected isize, found usize + | ----- ^^^ expected isize, found usize + | | + | expected `isize` because of return type error: aborting due to previous error diff --git a/src/test/ui/wrong-ret-type.stderr b/src/test/ui/wrong-ret-type.stderr index 221806f731f6..cf59f42683d7 100644 --- a/src/test/ui/wrong-ret-type.stderr +++ b/src/test/ui/wrong-ret-type.stderr @@ -2,7 +2,9 @@ error[E0308]: mismatched types --> $DIR/wrong-ret-type.rs:2:49 | LL | fn mk_int() -> usize { let i: isize = 3; return i; } - | ^ expected usize, found isize + | ----- ^ expected usize, found isize + | | + | expected `usize` because of return type error: aborting due to previous error From 79ef9718bb8e7b1fe45770a1ca43fb79bebbbbf0 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 18 Jan 2019 16:46:51 +0100 Subject: [PATCH 11/11] Remove delay_span_bug from qualify_min_const_fn This is causing issues with a new Clippy lint that will be able to detect possible const functions. As per https://github.com/rust-lang/rust-clippy/pull/3648#discussion_r247927450 --- src/librustc_mir/transform/qualify_min_const_fn.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index 059b88a4d702..85bf1e70ebf4 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -21,6 +21,7 @@ pub fn is_min_const_fn( | Predicate::RegionOutlives(_) | Predicate::TypeOutlives(_) | Predicate::WellFormed(_) + | Predicate::Projection(_) | Predicate::ConstEvaluatable(..) => continue, | Predicate::ObjectSafe(_) => { bug!("object safe predicate on function: {:#?}", predicate) @@ -29,13 +30,6 @@ pub fn is_min_const_fn( bug!("closure kind predicate on function: {:#?}", predicate) } Predicate::Subtype(_) => bug!("subtype predicate on function: {:#?}", predicate), - Predicate::Projection(_) => { - let span = tcx.def_span(current); - // we'll hit a `Predicate::Trait` later which will report an error - tcx.sess - .delay_span_bug(span, "projection without trait bound"); - continue; - } Predicate::Trait(pred) => { if Some(pred.def_id()) == tcx.lang_items().sized_trait() { continue;