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 ec1aeb8a7d1e9..06cb1b02fcf4c 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 978d20ea94789..1cbfcc1664323 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 a214ec88c6654..1cffe5ace5a6e 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 f633703be56d4..3b9358d7c46a9 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 8badcbfc1b301..837960a0f1d95 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 ae9bb37842990..7cc5d756ff311 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 fc4bd05476f6c..1e287ccc85c78 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 d7acdefcc7d71..d92800c7b9537 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 8ff60e5f56225..4da99f3c0f4b9 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 c1aa25b6b75c2..0a65ce6fe8468 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 a78cf1a471b4b..190d3d1eb5096 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 ce2a348950622..76fbe2119f310 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 205f8941d1ee2..587b28b3edde2 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 ea8b2826732b4..6fd31ffba3997 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 604b31e7167a9..f97f7dcf17f4e 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 f0b559f80a28c..4831232a9f919 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 1b07385d4d1f4..ba97c3d2549af 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 | 29 ++++----- 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 | 80 ++++++++++++------------- src/librustc_passes/hir_stats.rs | 5 -- src/librustc_passes/rvalue_promotion.rs | 26 ++++---- src/librustc_typeck/check/mod.rs | 20 ++----- 16 files changed, 150 insertions(+), 272 deletions(-) diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index 1cbfcc1664323..6122fe6370940 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]) + 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::StmtKind::Item(_) => { + pred } - 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]) + self.expr(&expr, pred) } - } - } - - fn decl(&mut self, decl: &hir::Decl, pred: CFGIndex) -> CFGIndex { - match decl.node { - hir::DeclKind::Local(ref local) => { - let init_exit = self.opt_expr(&local.init, pred); - self.pat(&local.pat, init_exit) - } - - hir::DeclKind::Item(_) => 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 1cffe5ace5a6e..9fa5f4aabe512 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 3b9358d7c46a9..a3eda804aa70c 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 837960a0f1d95..02e66300f24d9 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 1e287ccc85c78..5660b879c9733 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 d92800c7b9537..e950f25c2ac9a 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 4da99f3c0f4b9..9aa8e4c229f47 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 f5a7919ef09c8..837a3645d1c94 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 a373faab3a204..3caa88dbc898e 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 0a65ce6fe8468..08210c3f075ce 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 190d3d1eb5096..13464242f1ced 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 76fbe2119f310..819dd8aa7d53e 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 6fd31ffba3997..c50d9ddcb152e 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)), - }; - - let mut pattern = cx.pattern_from_hir(&local.pat); + 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)), + }; - 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); - 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, - }))); + 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, + }))); } } } diff --git a/src/librustc_passes/hir_stats.rs b/src/librustc_passes/hir_stats.rs index f97f7dcf17f4e..74d6d75a7f528 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 4831232a9f919..49914dc7078fd 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 ba97c3d2549af..daade27768ae3 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 a82a0d3ce5232..d1c54f824fe9d 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 0000000000000..50f1fe873cb5f --- /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 0000000000000..edaa60e5b8d8b --- /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 0000000000000..a27df240d0792 --- /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 0000000000000..54f7b108c3dd4 --- /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 d1c54f824fe9d..28114b527933d 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 c399650d3258b..f8da9282f5fb7 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 45482447ebe2c..592c84bd17d0e 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 54f7b108c3dd4..b94a9f4df4875 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 28114b527933d..f040781e56f7c 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 1b07385d4d1f4..ce876a799644a 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 f8da9282f5fb7..c399650d3258b 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 592c84bd17d0e..45482447ebe2c 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 ce876a799644a..7b9b97ff065a6 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 b94a9f4df4875..be3d2c0db4ca1 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 f040781e56f7c..7f053d6af9611 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 a27df240d0792..95b40368143ef 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 be3d2c0db4ca1..62da0787b02a9 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 7f053d6af9611..dd63b4f20fa55 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 9a33d29766fb9..47bb5e475b473 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 f03aaa67edbb8..b341879ab919a 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 bbcbbdbe8dd5b..a970b80fdbbd9 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 1f9103a6d2ee8..db524907b656f 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 c84dd1953a07b..2862ae641df15 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 eadf3d6d3350e..1170f5c17c18a 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 221806f731f60..cf59f42683d72 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 059b88a4d702a..85bf1e70ebf42 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;