From 73fb18c7712e4a513d537451b2eb195a8c6a63e5 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 17 Jan 2020 21:17:01 +0200 Subject: [PATCH 1/2] rustc_hir: add Expr! pattern macro and try it out in a couple places. --- src/librustc_hir/hir.rs | 12 +++++++++ src/librustc_hir/lib.rs | 1 + src/librustc_lint/builtin.rs | 45 +++++++++++++++++++--------------- src/librustc_lint/lib.rs | 1 + src/librustc_typeck/astconv.rs | 13 ++++------ 5 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index 5c1d600c837c..08f5a5c42d92 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -1330,6 +1330,18 @@ pub struct Expr<'hir> { pub span: Span, } +/// `Expr` pattern alias to facilitate pattern-matching. +/// +/// Usage examples: +/// * `hir::Expr!(Unary(_, hir::Expr!(Lit(_))))` +/// * `hir::Expr! { Loop(..), hir_id: loop_id }` +pub macro Expr($variant:ident $($rest:tt)*) { + $crate::hir::Expr { + kind: $crate::hir::ExprKind::$variant $($rest)*, + .. + } +} + // `Expr` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] rustc_data_structures::static_assert_size!(Expr<'static>, 64); diff --git a/src/librustc_hir/lib.rs b/src/librustc_hir/lib.rs index f54fa291bd6e..65b25301e7f5 100644 --- a/src/librustc_hir/lib.rs +++ b/src/librustc_hir/lib.rs @@ -4,6 +4,7 @@ #![feature(crate_visibility_modifier)] #![feature(const_fn)] // For the unsizing cast on `&[]` +#![feature(decl_macro)] #![feature(in_band_lifetimes)] #![feature(specialization)] #![recursion_limit = "256"] diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 6314c2b99539..7b3c3cb05a24 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1885,10 +1885,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { const TRANSMUTE_PATH: &[Symbol] = &[sym::core, sym::intrinsics, kw::Invalid, sym::transmute]; - if let hir::ExprKind::Call(ref path_expr, ref args) = expr.kind { + match expr { // Find calls to `mem::{uninitialized,zeroed}` methods. - if let hir::ExprKind::Path(ref qpath) = path_expr.kind { - let def_id = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?; + hir::Expr!(Call(hir::Expr! { Path(ref qpath), hir_id: path_hir_id }, ref args)) => { + let def_id = cx.tables.qpath_res(qpath, *path_hir_id).opt_def_id()?; if cx.tcx.is_diagnostic_item(sym::mem_zeroed, def_id) { return Some(InitKind::Zeroed); @@ -1900,25 +1900,30 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { } } } - } else if let hir::ExprKind::MethodCall(_, _, ref args) = expr.kind { - // Find problematic calls to `MaybeUninit::assume_init`. - let def_id = cx.tables.type_dependent_def_id(expr.hir_id)?; - if cx.tcx.is_diagnostic_item(sym::assume_init, def_id) { - // This is a call to *some* method named `assume_init`. - // See if the `self` parameter is one of the dangerous constructors. - if let hir::ExprKind::Call(ref path_expr, _) = args[0].kind { - if let hir::ExprKind::Path(ref qpath) = path_expr.kind { - let def_id = - cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?; - - if cx.tcx.is_diagnostic_item(sym::maybe_uninit_zeroed, def_id) { - return Some(InitKind::Zeroed); - } else if cx.tcx.is_diagnostic_item(sym::maybe_uninit_uninit, def_id) { - return Some(InitKind::Uninit); - } - } + + // Find problematic calls to `MaybeUninit::assume_init`, where + // the `self` parameter is one of the dangerous constructors. + hir::Expr!( + MethodCall( + _, + _, + [hir::Expr!(Call(hir::Expr! { Path(ref qpath), hir_id: path_hir_id }, _)), ..], + ) + ) if cx.tcx.is_diagnostic_item( + sym::assume_init, + cx.tables.type_dependent_def_id(expr.hir_id)?, + ) => + { + let def_id = cx.tables.qpath_res(qpath, *path_hir_id).opt_def_id()?; + + if cx.tcx.is_diagnostic_item(sym::maybe_uninit_zeroed, def_id) { + return Some(InitKind::Zeroed); + } else if cx.tcx.is_diagnostic_item(sym::maybe_uninit_uninit, def_id) { + return Some(InitKind::Uninit); } } + + _ => {} } None diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 78e9d0f14f2d..0a1a0db97022 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -33,6 +33,7 @@ #![feature(crate_visibility_modifier)] #![feature(never_type)] #![feature(nll)] +#![feature(slice_patterns)] #![recursion_limit = "256"] #[macro_use] diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index a3be264ddc12..c38d125d9638 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -2703,14 +2703,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { let expr = &tcx.hir().body(ast_const.body).value; - let lit_input = match expr.kind { - hir::ExprKind::Lit(ref lit) => Some(LitToConstInput { lit: &lit.node, ty, neg: false }), - hir::ExprKind::Unary(hir::UnOp::UnNeg, ref expr) => match expr.kind { - hir::ExprKind::Lit(ref lit) => { - Some(LitToConstInput { lit: &lit.node, ty, neg: true }) - } - _ => None, - }, + let lit_input = match expr { + hir::Expr!(Lit(ref lit)) => Some(LitToConstInput { lit: &lit.node, ty, neg: false }), + hir::Expr!(Unary(hir::UnOp::UnNeg, hir::Expr!(Lit(ref lit)))) => { + Some(LitToConstInput { lit: &lit.node, ty, neg: true }) + } _ => None, }; From 08ded005b298ef941774ec7779eddeecbf420cfe Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 17 Jan 2020 21:17:14 +0200 Subject: [PATCH 2/2] [WIP] use hir::Expr! in more places. --- src/librustc/infer/error_reporting/mod.rs | 20 +++++++------ .../traits/error_reporting/suggestions.rs | 11 +++----- .../borrow_check/diagnostics/region_name.rs | 6 ++-- src/librustc_mir_build/build/mod.rs | 4 +-- src/librustc_typeck/check/callee.rs | 18 ++++-------- src/librustc_typeck/check/demand.rs | 28 ++++++------------- src/librustc_typeck/collect.rs | 10 +++---- 7 files changed, 36 insertions(+), 61 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index febf4f21a675..bce4575e4666 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -98,15 +98,17 @@ pub(super) fn note_and_explain_region( let span = scope.span(tcx, region_scope_tree); let tag = match tcx.hir().find(scope.hir_id(region_scope_tree)) { Some(Node::Block(_)) => "block", - Some(Node::Expr(expr)) => match expr.kind { - hir::ExprKind::Call(..) => "call", - hir::ExprKind::MethodCall(..) => "method call", - hir::ExprKind::Match(.., hir::MatchSource::IfLetDesugar { .. }) => "if let", - hir::ExprKind::Match(.., hir::MatchSource::WhileLetDesugar) => "while let", - hir::ExprKind::Match(.., hir::MatchSource::ForLoopDesugar) => "for", - hir::ExprKind::Match(..) => "match", - _ => "expression", - }, + Some(Node::Expr(hir::Expr!(Call(..)))) => "call", + Some(Node::Expr(hir::Expr!(MethodCall(..)))) => "method call", + Some(Node::Expr(hir::Expr!(Match(.., hir::MatchSource::IfLetDesugar { .. })))) => { + "if let" + } + Some(Node::Expr(hir::Expr!(Match(.., hir::MatchSource::WhileLetDesugar)))) => { + "while let" + } + Some(Node::Expr(hir::Expr!(Match(.., hir::MatchSource::ForLoopDesugar)))) => "for", + Some(Node::Expr(hir::Expr!(Match(..)))) => "match", + Some(Node::Expr(_)) => "expression", Some(Node::Stmt(_)) => "statement", Some(Node::Item(it)) => item_scope_tag(&it), Some(Node::TraitItem(it)) => trait_item_scope_tag(&it), diff --git a/src/librustc/traits/error_reporting/suggestions.rs b/src/librustc/traits/error_reporting/suggestions.rs index bf6891214ace..ade89cf9d477 100644 --- a/src/librustc/traits/error_reporting/suggestions.rs +++ b/src/librustc/traits/error_reporting/suggestions.rs @@ -178,7 +178,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let parent_node = self.tcx.hir().get_parent_node(hir_id); if let Some(Node::Local(ref local)) = self.tcx.hir().find(parent_node) { if let Some(ref expr) = local.init { - if let hir::ExprKind::Index(_, _) = expr.kind { + if let hir::Expr!(Index(_, _)) = expr { if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(expr.span) { err.span_suggestion( expr.span, @@ -753,10 +753,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { /// `report_arg_count_mismatch`. pub fn get_fn_like_arguments(&self, node: Node<'_>) -> (Span, Vec) { match node { - Node::Expr(&hir::Expr { - kind: hir::ExprKind::Closure(_, ref _decl, id, span, _), - .. - }) => ( + Node::Expr(&hir::Expr!(Closure(_, ref _decl, id, span, _))) => ( self.tcx.sess.source_map().def_span(span), self.tcx .hir() @@ -1699,7 +1696,7 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { } fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { - if let hir::ExprKind::Ret(Some(ex)) = ex.kind { + if let hir::Expr!(Ret(Some(ex))) = ex { self.0.push(ex); } hir::intravisit::walk_expr(self, ex); @@ -1707,7 +1704,7 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { fn visit_body(&mut self, body: &'v hir::Body<'v>) { if body.generator_kind().is_none() { - if let hir::ExprKind::Block(block, None) = body.value.kind { + if let hir::Expr!(Block(block, None)) = &body.value { if let Some(expr) = block.expr { self.0.push(expr); } diff --git a/src/librustc_mir/borrow_check/diagnostics/region_name.rs b/src/librustc_mir/borrow_check/diagnostics/region_name.rs index 734e3861c62d..4fbe2fddfc5d 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_name.rs @@ -759,9 +759,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { let mir_hir_id = tcx.hir().as_local_hir_id(mbcx.mir_def_id).expect("non-local mir"); let yield_span = match tcx.hir().get(mir_hir_id) { - hir::Node::Expr(hir::Expr { - kind: hir::ExprKind::Closure(_, _, _, span, _), .. - }) => (tcx.sess.source_map().end_point(*span)), + hir::Node::Expr(hir::Expr!(Closure(_, _, _, span, _))) => { + (tcx.sess.source_map().end_point(*span)) + } _ => mbcx.body.span, }; diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index 44ff493b5b4f..713326d2fb05 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -30,9 +30,7 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { // Figure out what primary body this item has. let (body_id, return_ty_span) = match tcx.hir().get(id) { - Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, decl, body_id, _, _), .. }) => { - (*body_id, decl.output.span()) - } + Node::Expr(hir::Expr!(Closure(_, decl, body_id, _, _))) => (*body_id, decl.output.span()), Node::Item(hir::Item { kind: hir::ItemKind::Fn(hir::FnSig { decl, .. }, _, body_id), .. diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index 58f407b89027..6e181a5bdbca 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -229,18 +229,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, err: &mut DiagnosticBuilder<'a>, hir_id: hir::HirId, - callee_node: &hir::ExprKind<'_>, - callee_span: Span, + callee: &hir::Expr<'_>, ) { let hir_id = self.tcx.hir().get_parent_node(hir_id); let parent_node = self.tcx.hir().get(hir_id); - if let ( - hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, _, _, sp, ..), .. }), - hir::ExprKind::Block(..), - ) = (parent_node, callee_node) + if let (hir::Node::Expr(hir::Expr!(Closure(_, _, _, sp, ..))), hir::Expr!(Block(..))) = + (parent_node, callee) { let start = sp.shrink_to_lo(); - let end = callee_span.shrink_to_hi(); + let end = callee.span.shrink_to_hi(); err.multipart_suggestion( "if you meant to create this closure and immediately call it, surround the \ closure with parenthesis", @@ -285,12 +282,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } ); - self.identify_bad_closure_def_and_call( - &mut err, - call_expr.hir_id, - &callee.kind, - callee.span, - ); + self.identify_bad_closure_def_and_call(&mut err, call_expr.hir_id, callee); if let Some(ref path) = unit_variant { err.span_suggestion( diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index e0f9fcc69325..8dbf65878b21 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -277,11 +277,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let param_parent = self.tcx.hir().get_parent_node(*param_hir_id); let (expr_hir_id, closure_fn_decl) = match self.tcx.hir().find(param_parent) { - Some(Node::Expr(hir::Expr { - hir_id, - kind: hir::ExprKind::Closure(_, decl, ..), - .. - })) => (hir_id, decl), + Some(Node::Expr(hir::Expr! { Closure(_, decl, ..), hir_id })) => (hir_id, decl), _ => return None, }; @@ -289,13 +285,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let hir = self.tcx.hir().find(expr_parent); let closure_params_len = closure_fn_decl.inputs.len(); let (method_path, method_span, method_expr) = match (hir, closure_params_len) { - ( - Some(Node::Expr(hir::Expr { - kind: hir::ExprKind::MethodCall(path, span, expr), - .. - })), - 1, - ) => (path, span, expr), + (Some(Node::Expr(hir::Expr!(MethodCall(path, span, expr)))), 1) => (path, span, expr), _ => return None, }; @@ -323,15 +313,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> bool { let cm = self.sess().source_map(); let parent_id = self.tcx.hir().get_parent_node(hir_id); - if let Some(parent) = self.tcx.hir().find(parent_id) { + if let Some(Node::Expr(hir::Expr!(Struct(_, fields, ..)))) = self.tcx.hir().find(parent_id) + { // Account for fields - if let Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(_, fields, ..), .. }) = parent - { - if let Ok(src) = cm.span_to_snippet(sp) { - for field in *fields { - if field.ident.as_str() == src && field.is_shorthand { - return true; - } + if let Ok(src) = cm.span_to_snippet(sp) { + for field in *fields { + if field.ident.as_str() == src && field.is_shorthand { + return true; } } } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index dca3289747e4..b3b49eef264e 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1031,9 +1031,7 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics { None } } - Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(..), .. }) => { - Some(tcx.closure_base_def_id(def_id)) - } + Node::Expr(hir::Expr!(Closure(..))) => Some(tcx.closure_base_def_id(def_id)), Node::Item(item) => match item.kind { ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn, .. }) => impl_trait_fn, _ => None, @@ -1172,7 +1170,7 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics { // provide junk type parameter defs - the only place that // cares about anything but the length is instantiation, // and we don't do that for closures. - if let Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(.., gen), .. }) = node { + if let Node::Expr(hir::Expr!(Closure(.., gen))) = node { let dummy_args = if gen.is_some() { &["", "", ""][..] } else { @@ -1433,8 +1431,8 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> { } Node::Ty(&hir::Ty { kind: hir::TyKind::Path(_), .. }) - | Node::Expr(&hir::Expr { kind: ExprKind::Struct(..), .. }) - | Node::Expr(&hir::Expr { kind: ExprKind::Path(_), .. }) + | Node::Expr(hir::Expr!(Struct(..))) + | Node::Expr(hir::Expr!(Path(_))) | Node::TraitRef(..) => { let path = match parent_node { Node::Ty(&hir::Ty {