Skip to content

Commit

Permalink
Add visitors for PatField and ExprField.
Browse files Browse the repository at this point in the history
This helps simplify the code. It also fixes it to use the correct parent
when lowering. One consequence is the `non_snake_case` lint needed
to change the way it looked for parent nodes in a struct pattern.

This also includes a small fix to use the correct `Target` for
expression field attribute validation.
  • Loading branch information
ehuss committed Aug 12, 2022
1 parent 6c7cb2b commit dcd5177
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 73 deletions.
12 changes: 7 additions & 5 deletions compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,19 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {

fn visit_pat(&mut self, pat: &'hir Pat<'hir>) {
self.insert(pat.span, pat.hir_id, Node::Pat(pat));
if let PatKind::Struct(_, fields, _) = pat.kind {
for field in fields {
self.insert(field.span, field.hir_id, Node::PatField(field));
}
}

self.with_parent(pat.hir_id, |this| {
intravisit::walk_pat(this, pat);
});
}

fn visit_pat_field(&mut self, field: &'hir PatField<'hir>) {
self.insert(field.span, field.hir_id, Node::PatField(field));
self.with_parent(field.hir_id, |this| {
intravisit::walk_pat_field(this, field);
});
}

fn visit_arm(&mut self, arm: &'hir Arm<'hir>) {
let node = Node::Arm(arm);

Expand Down
30 changes: 20 additions & 10 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ pub trait Visitor<'v>: Sized {
fn visit_pat(&mut self, p: &'v Pat<'v>) {
walk_pat(self, p)
}
fn visit_pat_field(&mut self, f: &'v PatField<'v>) {
walk_pat_field(self, f)
}
fn visit_array_length(&mut self, len: &'v ArrayLen) {
walk_array_len(self, len)
}
Expand All @@ -337,6 +340,9 @@ pub trait Visitor<'v>: Sized {
fn visit_let_expr(&mut self, lex: &'v Let<'v>) {
walk_let_expr(self, lex)
}
fn visit_expr_field(&mut self, field: &'v ExprField<'v>) {
walk_expr_field(self, field)
}
fn visit_ty(&mut self, t: &'v Ty<'v>) {
walk_ty(self, t)
}
Expand Down Expand Up @@ -761,11 +767,7 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat<'v>) {
}
PatKind::Struct(ref qpath, fields, _) => {
visitor.visit_qpath(qpath, pattern.hir_id, pattern.span);
for field in fields {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_pat(&field.pat)
}
walk_list!(visitor, visit_pat_field, fields);
}
PatKind::Or(pats) => walk_list!(visitor, visit_pat, pats),
PatKind::Tuple(tuple_elements, _) => {
Expand All @@ -792,6 +794,12 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat<'v>) {
}
}

pub fn walk_pat_field<'v, V: Visitor<'v>>(visitor: &mut V, field: &'v PatField<'v>) {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_pat(&field.pat)
}

pub fn walk_foreign_item<'v, V: Visitor<'v>>(visitor: &mut V, foreign_item: &'v ForeignItem<'v>) {
visitor.visit_id(foreign_item.hir_id());
visitor.visit_ident(foreign_item.ident);
Expand Down Expand Up @@ -1059,6 +1067,12 @@ pub fn walk_let_expr<'v, V: Visitor<'v>>(visitor: &mut V, let_expr: &'v Let<'v>)
walk_list!(visitor, visit_ty, let_expr.ty);
}

pub fn walk_expr_field<'v, V: Visitor<'v>>(visitor: &mut V, field: &'v ExprField<'v>) {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_expr(&field.expr)
}

pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>) {
visitor.visit_id(expression.hir_id);
match expression.kind {
Expand All @@ -1073,11 +1087,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
}
ExprKind::Struct(ref qpath, fields, ref optional_base) => {
visitor.visit_qpath(qpath, expression.hir_id, expression.span);
for field in fields {
visitor.visit_id(field.hir_id);
visitor.visit_ident(field.ident);
visitor.visit_expr(&field.expr)
}
walk_list!(visitor, visit_expr_field, fields);
walk_list!(visitor, visit_expr, optional_base);
}
ExprKind::Tup(subexpressions) => {
Expand Down
47 changes: 13 additions & 34 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,26 +761,15 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'tcx> {
}

fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) {
match e.kind {
hir::ExprKind::Struct(qpath, fields, base_expr) => {
self.with_lint_attrs(e.hir_id, |builder| {
builder.visit_qpath(qpath, e.hir_id, e.span);
for field in fields {
builder.with_lint_attrs(field.hir_id, |field_builder| {
field_builder.visit_id(field.hir_id);
field_builder.visit_ident(field.ident);
field_builder.visit_expr(field.expr);
});
}
if let Some(base_expr) = base_expr {
builder.visit_expr(base_expr);
}
});
}
_ => self.with_lint_attrs(e.hir_id, |builder| {
intravisit::walk_expr(builder, e);
}),
}
self.with_lint_attrs(e.hir_id, |builder| {
intravisit::walk_expr(builder, e);
})
}

fn visit_expr_field(&mut self, field: &'tcx hir::ExprField<'tcx>) {
self.with_lint_attrs(field.hir_id, |builder| {
intravisit::walk_expr_field(builder, field);
})
}

fn visit_field_def(&mut self, s: &'tcx hir::FieldDef<'tcx>) {
Expand Down Expand Up @@ -819,20 +808,10 @@ impl<'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'tcx> {
});
}

fn visit_pat(&mut self, p: &'tcx hir::Pat<'tcx>) {
match &p.kind {
hir::PatKind::Struct(qpath, fields, _) => {
self.visit_qpath(&qpath, p.hir_id, p.span);
for field in *fields {
self.with_lint_attrs(field.hir_id, |builder| {
builder.visit_id(field.hir_id);
builder.visit_ident(field.ident);
builder.visit_pat(field.pat);
})
}
}
_ => intravisit::walk_pat(self, p),
}
fn visit_pat_field(&mut self, field: &'tcx hir::PatField<'tcx>) {
self.with_lint_attrs(field.hir_id, |builder| {
intravisit::walk_pat_field(builder, field);
})
}

fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) {
Expand Down
17 changes: 6 additions & 11 deletions compiler/rustc_lint/src/nonstandard_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,19 +437,14 @@ impl<'tcx> LateLintPass<'tcx> for NonSnakeCase {

fn check_pat(&mut self, cx: &LateContext<'_>, p: &hir::Pat<'_>) {
if let PatKind::Binding(_, hid, ident, _) = p.kind {
if let hir::Node::Pat(parent_pat) = cx.tcx.hir().get(cx.tcx.hir().get_parent_node(hid))
if let hir::Node::PatField(field) = cx.tcx.hir().get(cx.tcx.hir().get_parent_node(hid))
{
if let PatKind::Struct(_, field_pats, _) = &parent_pat.kind {
if field_pats
.iter()
.any(|field| !field.is_shorthand && field.pat.hir_id == p.hir_id)
{
// Only check if a new name has been introduced, to avoid warning
// on both the struct definition and this pattern.
self.check_snake_case(cx, "variable", &ident);
}
return;
if !field.is_shorthand {
// Only check if a new name has been introduced, to avoid warning
// on both the struct definition and this pattern.
self.check_snake_case(cx, "variable", &ident);
}
return;
}
self.check_snake_case(cx, "variable", &ident);
}
Expand Down
20 changes: 8 additions & 12 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2065,14 +2065,14 @@ impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
};

self.check_attributes(expr.hir_id, expr.span, target, None);
if let hir::ExprKind::Struct(_, fields, _) = expr.kind {
for field in fields {
self.check_attributes(field.hir_id, field.span, Target::PatField, None);
}
}
intravisit::walk_expr(self, expr)
}

fn visit_expr_field(&mut self, field: &'tcx hir::ExprField<'tcx>) {
self.check_attributes(field.hir_id, field.span, Target::ExprField, None);
intravisit::walk_expr_field(self, field)
}

fn visit_variant(&mut self, variant: &'tcx hir::Variant<'tcx>) {
self.check_attributes(variant.id, variant.span, Target::Variant, None);
intravisit::walk_variant(self, variant)
Expand All @@ -2084,13 +2084,9 @@ impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
intravisit::walk_param(self, param);
}

fn visit_pat(&mut self, p: &'tcx hir::Pat<'tcx>) {
if let hir::PatKind::Struct(_, fields, _) = p.kind {
for field in fields {
self.check_attributes(field.hir_id, field.span, Target::PatField, None);
}
}
intravisit::walk_pat(self, p);
fn visit_pat_field(&mut self, field: &'tcx hir::PatField<'tcx>) {
self.check_attributes(field.hir_id, field.span, Target::PatField, None);
intravisit::walk_pat_field(self, field);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lint/unused/unused_attributes-must_use.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ error: `#[must_use]` has no effect when applied to an match arm
LL | #[must_use]
| ^^^^^^^^^^^

error: `#[must_use]` has no effect when applied to a pattern field
error: `#[must_use]` has no effect when applied to a struct field
--> $DIR/unused_attributes-must_use.rs:129:28
|
LL | let s = PatternField { #[must_use] foo: 123 };
Expand Down

0 comments on commit dcd5177

Please sign in to comment.