about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEric Huss <eric@huss.org>2022-07-05 17:42:39 -0700
committerEric Huss <eric@huss.org>2022-08-11 21:48:39 -0700
commit7b36047239f50df16e6dbd1ab0ca8575e7db1a55 (patch)
treeae336c76d2a17514dba4530659057041068282ca
parentdcd5177fd43d5fdd9b89a7700fe3c283a9a52a48 (diff)
downloadrust-7b36047239f50df16e6dbd1ab0ca8575e7db1a55.tar.gz
rust-7b36047239f50df16e6dbd1ab0ca8575e7db1a55.zip
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.
-rw-r--r--compiler/rustc_ast_lowering/src/index.rs12
-rw-r--r--compiler/rustc_lint/src/types.rs98
-rw-r--r--compiler/rustc_typeck/src/check/demand.rs25
-rw-r--r--src/test/ui/lint/lint-attr-everywhere-early.stderr20
-rw-r--r--src/test/ui/lint/lint-attr-everywhere-late.stderr132
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 899e0dd5cc3..2b7431f0990 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 5c07afeb7aa..cafd2c6e679 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 bac0be44aa9..07046f3f032 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 f65d811105f..9b54b033c88 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 d302798d650..977843997c6 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<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