From 7b36047239f50df16e6dbd1ab0ca8575e7db1a55 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 5 Jul 2022 17:42:39 -0700 Subject: [PATCH] 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. --- compiler/rustc_ast_lowering/src/index.rs | 12 +- compiler/rustc_lint/src/types.rs | 98 +++++++------ compiler/rustc_typeck/src/check/demand.rs | 25 ++-- .../ui/lint/lint-attr-everywhere-early.stderr | 20 +-- .../ui/lint/lint-attr-everywhere-late.stderr | 132 +++++++++--------- 5 files changed, 139 insertions(+), 148 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/index.rs b/compiler/rustc_ast_lowering/src/index.rs index 899e0dd5cc3b..2b7431f09905 100644 --- a/compiler/rustc_ast_lowering/src/index.rs +++ b/compiler/rustc_ast_lowering/src/index.rs @@ -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)); diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 5c07afeb7aa8..cafd2c6e6799 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -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, diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index bac0be44aa9a..07046f3f0326 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -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; } }; diff --git a/src/test/ui/lint/lint-attr-everywhere-early.stderr b/src/test/ui/lint/lint-attr-everywhere-early.stderr index f65d811105fa..9b54b033c88a 100644 --- a/src/test/ui/lint/lint-attr-everywhere-early.stderr +++ b/src/test/ui/lint/lint-attr-everywhere-early.stderr @@ -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 diff --git a/src/test/ui/lint/lint-attr-everywhere-late.stderr b/src/test/ui/lint/lint-attr-everywhere-late.stderr index d302798d6500..977843997c6d 100644 --- a/src/test/ui/lint/lint-attr-everywhere-late.stderr +++ b/src/test/ui/lint/lint-attr-everywhere-late.stderr @@ -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) {} - | ^^^^ - | -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::(&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::(&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) {} + | ^^^^ + | +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::(&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::(&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::(&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::(&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