Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc_hir: add Expr! pattern macro and try it out in a couple places. #68320

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
11 changes: 4 additions & 7 deletions src/librustc/traits/error_reporting/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<ArgKind>) {
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()
Expand Down Expand Up @@ -1699,15 +1696,15 @@ 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);
}

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);
}
Expand Down
12 changes: 12 additions & 0 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)*) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use pat fragment?

Copy link
Member Author

@eddyb eddyb Jan 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because then people would have to write hir::ExprKind::Foo instead of Foo, inside the macro arguments.
And also this allows some trickery like matching on the hir_id (although maybe that should be done with @ instead of this EDIT: nevermind, @ needs a binding on one side, it's not a conjunction of patterns).

$crate::hir::Expr {
kind: $crate::hir::ExprKind::$variant $($rest)*,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$crate -> crate here and one line above, macros are fully hygienic and don't need $crate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to be conservative, presumably I can even write Expr without the full module path.

..
}
}

// `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);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_hir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
45 changes: 25 additions & 20 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#![feature(crate_visibility_modifier)]
#![feature(never_type)]
#![feature(nll)]
#![feature(slice_patterns)]
#![recursion_limit = "256"]

#[macro_use]
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/borrow_check/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
4 changes: 1 addition & 3 deletions src/librustc_mir_build/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
..
Expand Down
13 changes: 5 additions & 8 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
18 changes: 5 additions & 13 deletions src/librustc_typeck/check/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down
28 changes: 8 additions & 20 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,25 +277,15 @@ 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,
};

let expr_parent = self.tcx.hir().get_parent_node(*expr_hir_id);
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,
};

Expand Down Expand Up @@ -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;
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
&["<yield_ty>", "<return_ty>", "<witness>"][..]
} else {
Expand Down Expand Up @@ -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 {
Expand Down