Make Node::ExprField a child of Node::Expr.

This was incorrectly inserting the ExprField as a sibling of the struct
expression.

This required adjusting various parts which were looking at parent node
of a field expression to find the struct.
This commit is contained in:
Eric Huss 2022-07-05 17:42:39 -07:00
parent dcd5177fd4
commit 7b36047239
5 changed files with 139 additions and 148 deletions

View File

@ -226,17 +226,19 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
fn visit_expr(&mut self, expr: &'hir Expr<'hir>) {
self.insert(expr.span, expr.hir_id, Node::Expr(expr));
if let ExprKind::Struct(_, fields, _) = expr.kind {
for field in fields {
self.insert(field.span, field.hir_id, Node::ExprField(field));
}
}
self.with_parent(expr.hir_id, |this| {
intravisit::walk_expr(this, expr);
});
}
fn visit_expr_field(&mut self, field: &'hir ExprField<'hir>) {
self.insert(field.span, field.hir_id, Node::ExprField(field));
self.with_parent(field.hir_id, |this| {
intravisit::walk_expr_field(this, field);
});
}
fn visit_stmt(&mut self, stmt: &'hir Stmt<'hir>) {
self.insert(stmt.span, stmt.hir_id, Node::Stmt(stmt));

View File

@ -125,45 +125,51 @@ fn lint_overflowing_range_endpoint<'tcx>(
lit_val: u128,
max: u128,
expr: &'tcx hir::Expr<'tcx>,
parent_expr: &'tcx hir::Expr<'tcx>,
ty: &str,
) -> bool {
// We only want to handle exclusive (`..`) ranges,
// which are represented as `ExprKind::Struct`.
let par_id = cx.tcx.hir().get_parent_node(expr.hir_id);
let Node::ExprField(field) = cx.tcx.hir().get(par_id) else { return false };
let field_par_id = cx.tcx.hir().get_parent_node(field.hir_id);
let Node::Expr(struct_expr) = cx.tcx.hir().get(field_par_id) else { return false };
if !is_range_literal(struct_expr) {
return false;
};
let ExprKind::Struct(_, eps, _) = &struct_expr.kind else { return false };
if eps.len() != 2 {
return false;
}
let mut overwritten = false;
if let ExprKind::Struct(_, eps, _) = &parent_expr.kind {
if eps.len() != 2 {
return false;
}
// We can suggest using an inclusive range
// (`..=`) instead only if it is the `end` that is
// overflowing and only by 1.
if eps[1].expr.hir_id == expr.hir_id && lit_val - 1 == max {
cx.struct_span_lint(OVERFLOWING_LITERALS, parent_expr.span, |lint| {
let mut err = lint.build(fluent::lint::range_endpoint_out_of_range);
err.set_arg("ty", ty);
if let Ok(start) = cx.sess().source_map().span_to_snippet(eps[0].span) {
use ast::{LitIntType, LitKind};
// We need to preserve the literal's suffix,
// as it may determine typing information.
let suffix = match lit.node {
LitKind::Int(_, LitIntType::Signed(s)) => s.name_str(),
LitKind::Int(_, LitIntType::Unsigned(s)) => s.name_str(),
LitKind::Int(_, LitIntType::Unsuffixed) => "",
_ => bug!(),
};
let suggestion = format!("{}..={}{}", start, lit_val - 1, suffix);
err.span_suggestion(
parent_expr.span,
fluent::lint::suggestion,
suggestion,
Applicability::MachineApplicable,
);
err.emit();
overwritten = true;
}
});
}
// We can suggest using an inclusive range
// (`..=`) instead only if it is the `end` that is
// overflowing and only by 1.
if eps[1].expr.hir_id == expr.hir_id && lit_val - 1 == max {
cx.struct_span_lint(OVERFLOWING_LITERALS, struct_expr.span, |lint| {
let mut err = lint.build(fluent::lint::range_endpoint_out_of_range);
err.set_arg("ty", ty);
if let Ok(start) = cx.sess().source_map().span_to_snippet(eps[0].span) {
use ast::{LitIntType, LitKind};
// We need to preserve the literal's suffix,
// as it may determine typing information.
let suffix = match lit.node {
LitKind::Int(_, LitIntType::Signed(s)) => s.name_str(),
LitKind::Int(_, LitIntType::Unsigned(s)) => s.name_str(),
LitKind::Int(_, LitIntType::Unsuffixed) => "",
_ => bug!(),
};
let suggestion = format!("{}..={}{}", start, lit_val - 1, suffix);
err.span_suggestion(
struct_expr.span,
fluent::lint::suggestion,
suggestion,
Applicability::MachineApplicable,
);
err.emit();
overwritten = true;
}
});
}
overwritten
}
@ -339,16 +345,9 @@ fn lint_int_literal<'tcx>(
return;
}
let par_id = cx.tcx.hir().get_parent_node(e.hir_id);
if let Node::Expr(par_e) = cx.tcx.hir().get(par_id) {
if let hir::ExprKind::Struct(..) = par_e.kind {
if is_range_literal(par_e)
&& lint_overflowing_range_endpoint(cx, lit, v, max, e, par_e, t.name_str())
{
// The overflowing literal lint was overridden.
return;
}
}
if lint_overflowing_range_endpoint(cx, lit, v, max, e, t.name_str()) {
// The overflowing literal lint was overridden.
return;
}
cx.struct_span_lint(OVERFLOWING_LITERALS, e.span, |lint| {
@ -408,16 +407,13 @@ fn lint_uint_literal<'tcx>(
return;
}
}
hir::ExprKind::Struct(..) if is_range_literal(par_e) => {
let t = t.name_str();
if lint_overflowing_range_endpoint(cx, lit, lit_val, max, e, par_e, t) {
// The overflowing literal lint was overridden.
return;
}
}
_ => {}
}
}
if lint_overflowing_range_endpoint(cx, lit, lit_val, max, e, t.name_str()) {
// The overflowing literal lint was overridden.
return;
}
if let Some(repr_str) = get_bin_hex_repr(cx, lit) {
report_bin_hex_error(
cx,

View File

@ -638,11 +638,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}?;
match hir.find(hir.get_parent_node(expr.hir_id))? {
Node::Expr(hir::Expr { kind: hir::ExprKind::Struct(_, fields, ..), .. }) => {
for field in *fields {
if field.ident.name == local.name && field.is_shorthand {
return Some(local.name);
}
Node::ExprField(field) => {
if field.ident.name == local.name && field.is_shorthand {
return Some(local.name);
}
}
_ => {}
@ -1073,21 +1071,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut sugg = vec![];
if let Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Struct(_, fields, _), ..
})) = self.tcx.hir().find(self.tcx.hir().get_parent_node(expr.hir_id))
if let Some(hir::Node::ExprField(field)) =
self.tcx.hir().find(self.tcx.hir().get_parent_node(expr.hir_id))
{
// `expr` is a literal field for a struct, only suggest if appropriate
match (*fields)
.iter()
.find(|field| field.expr.hir_id == expr.hir_id && field.is_shorthand)
{
if field.is_shorthand {
// This is a field literal
Some(field) => {
sugg.push((field.ident.span.shrink_to_lo(), format!("{}: ", field.ident)));
}
sugg.push((field.ident.span.shrink_to_lo(), format!("{}: ", field.ident)));
} else {
// Likely a field was meant, but this field wasn't found. Do not suggest anything.
None => return false,
return false;
}
};

View File

@ -25,7 +25,7 @@ help: remove these parentheses
|
LL - type BareFnPtr = fn(#[deny(unused_parens)](i32));
LL + type BareFnPtr = fn(#[deny(unused_parens)]i32);
|
|
error: type `ITEM_OUTER` should have an upper camel case name
--> $DIR/lint-attr-everywhere-early.rs:30:8
@ -198,7 +198,7 @@ help: remove these parentheses
|
LL - type assoc_type = (i32);
LL + type assoc_type = i32;
|
|
error: unnecessary parentheses around type
--> $DIR/lint-attr-everywhere-early.rs:87:31
@ -215,7 +215,7 @@ help: remove these parentheses
|
LL - #[deny(unused_parens)]f1: (i32),
LL + #[deny(unused_parens)]f1: i32,
|
|
error: unnecessary parentheses around type
--> $DIR/lint-attr-everywhere-early.rs:89:42
@ -232,7 +232,7 @@ help: remove these parentheses
|
LL - struct StructTuple(#[deny(unused_parens)](i32));
LL + struct StructTuple(#[deny(unused_parens)]i32);
|
|
error: variant `VARIANT_CAMEL` should have an upper camel case name
--> $DIR/lint-attr-everywhere-early.rs:93:5
@ -261,7 +261,7 @@ help: remove these parentheses
|
LL - fn foreign_denied_from_inner(x: (i32));
LL + fn foreign_denied_from_inner(x: i32);
|
|
error: unnecessary parentheses around type
--> $DIR/lint-attr-everywhere-early.rs:104:37
@ -278,7 +278,7 @@ help: remove these parentheses
|
LL - fn foreign_denied_from_outer(x: (i32));
LL + fn foreign_denied_from_outer(x: i32);
|
|
error: unnecessary parentheses around type
--> $DIR/lint-attr-everywhere-early.rs:107:43
@ -295,7 +295,7 @@ help: remove these parentheses
|
LL - fn function(#[deny(unused_parens)] param: (i32)) {}
LL + fn function(#[deny(unused_parens)] param: i32) {}
|
|
error: type parameter `foo` should have an upper camel case name
--> $DIR/lint-attr-everywhere-early.rs:109:42
@ -324,7 +324,7 @@ help: remove these parentheses
|
LL - let x = (1);
LL + let x = 1;
|
|
error: unnecessary parentheses around type
--> $DIR/lint-attr-everywhere-early.rs:121:50
@ -341,7 +341,7 @@ help: remove these parentheses
|
LL - let closure = |#[deny(unused_parens)] param: (i32)| {};
LL + let closure = |#[deny(unused_parens)] param: i32| {};
|
|
error: unnecessary parentheses around block return value
--> $DIR/lint-attr-everywhere-early.rs:125:46
@ -358,7 +358,7 @@ help: remove these parentheses
|
LL - let f = Match{#[deny(unused_parens)]f1: {(123)}};
LL + let f = Match{#[deny(unused_parens)]f1: {123}};
|
|
error: usage of an `unsafe` block
--> $DIR/lint-attr-everywhere-early.rs:132:13

View File

@ -142,35 +142,6 @@ note: the lint level is defined here
LL | #[deny(missing_docs)]
| ^^^^^^^^^^^^
error: types that do not implement `Drop` can still have drop glue, consider instead using `std::mem::needs_drop` to detect whether a type is trivially dropped
--> $DIR/lint-attr-everywhere-late.rs:93:38
|
LL | fn denied_from_inner(_x: Box<dyn Drop>) {}
| ^^^^
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:91:13
|
LL | #![deny(dyn_drop)]
| ^^^^^^^^
error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
--> $DIR/lint-attr-everywhere-late.rs:96:21
|
LL | fn assoc_fn() { discriminant::<i32>(&123); }
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:95:12
|
LL | #[deny(enum_intrinsics_non_enums)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum.
--> $DIR/lint-attr-everywhere-late.rs:96:41
|
LL | fn assoc_fn() { discriminant::<i32>(&123); }
| ^^^^
error: missing documentation for a variant
--> $DIR/lint-attr-everywhere-late.rs:112:5
|
@ -217,6 +188,60 @@ LL | #[deny(clashing_extern_declarations)]
= note: expected `unsafe extern "C" fn()`
found `unsafe extern "C" fn(i32)`
error: types that do not implement `Drop` can still have drop glue, consider instead using `std::mem::needs_drop` to detect whether a type is trivially dropped
--> $DIR/lint-attr-everywhere-late.rs:93:38
|
LL | fn denied_from_inner(_x: Box<dyn Drop>) {}
| ^^^^
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:91:13
|
LL | #![deny(dyn_drop)]
| ^^^^^^^^
error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
--> $DIR/lint-attr-everywhere-late.rs:96:21
|
LL | fn assoc_fn() { discriminant::<i32>(&123); }
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:95:12
|
LL | #[deny(enum_intrinsics_non_enums)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `i32`, which is not an enum.
--> $DIR/lint-attr-everywhere-late.rs:96:41
|
LL | fn assoc_fn() { discriminant::<i32>(&123); }
| ^^^^
error: literal out of range for `u8`
--> $DIR/lint-attr-everywhere-late.rs:98:59
|
LL | #[deny(overflowing_literals)] const ASSOC_CONST: u8 = 1000;
| ^^^^
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:98:12
|
LL | #[deny(overflowing_literals)] const ASSOC_CONST: u8 = 1000;
| ^^^^^^^^^^^^^^^^^^^^
= note: the literal `1000` does not fit into the type `u8` whose range is `0..=255`
error: variable `PARAM` should have a snake case name
--> $DIR/lint-attr-everywhere-late.rs:131:37
|
LL | fn function(#[deny(non_snake_case)] PARAM: i32) {}
| ^^^^^ help: convert the identifier to snake case: `param`
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:131:20
|
LL | fn function(#[deny(non_snake_case)] PARAM: i32) {}
| ^^^^^^^^^^^^^^
error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
--> $DIR/lint-attr-everywhere-late.rs:139:13
|
@ -234,6 +259,18 @@ note: the argument to `discriminant` should be a reference to an enum, but it wa
LL | let _ = discriminant::<i32>(&123);
| ^^^^
error: variable `PARAM` should have a snake case name
--> $DIR/lint-attr-everywhere-late.rs:145:44
|
LL | let closure = |#[deny(non_snake_case)] PARAM: i32| {};
| ^^^^^ help: convert the identifier to snake case: `param`
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:145:27
|
LL | let closure = |#[deny(non_snake_case)] PARAM: i32| {};
| ^^^^^^^^^^^^^^
error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
--> $DIR/lint-attr-everywhere-late.rs:155:13
|
@ -387,42 +424,5 @@ note: the argument to `discriminant` should be a reference to an enum, but it wa
LL | TupleStruct(#[deny(enum_intrinsics_non_enums)] discriminant::<i32>(&123));
| ^^^^
error: literal out of range for `u8`
--> $DIR/lint-attr-everywhere-late.rs:98:59
|
LL | #[deny(overflowing_literals)] const ASSOC_CONST: u8 = 1000;
| ^^^^
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:98:12
|
LL | #[deny(overflowing_literals)] const ASSOC_CONST: u8 = 1000;
| ^^^^^^^^^^^^^^^^^^^^
= note: the literal `1000` does not fit into the type `u8` whose range is `0..=255`
error: variable `PARAM` should have a snake case name
--> $DIR/lint-attr-everywhere-late.rs:131:37
|
LL | fn function(#[deny(non_snake_case)] PARAM: i32) {}
| ^^^^^ help: convert the identifier to snake case: `param`
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:131:20
|
LL | fn function(#[deny(non_snake_case)] PARAM: i32) {}
| ^^^^^^^^^^^^^^
error: variable `PARAM` should have a snake case name
--> $DIR/lint-attr-everywhere-late.rs:145:44
|
LL | let closure = |#[deny(non_snake_case)] PARAM: i32| {};
| ^^^^^ help: convert the identifier to snake case: `param`
|
note: the lint level is defined here
--> $DIR/lint-attr-everywhere-late.rs:145:27
|
LL | let closure = |#[deny(non_snake_case)] PARAM: i32| {};
| ^^^^^^^^^^^^^^
error: aborting due to 31 previous errors