Improve spans for indexing expressions

Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location. This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR.
rust-lang · Aug 4, 2023 · 5706be1 · 5706be1
1 parent fcf3006
commit 5706be1
Show file tree

Hide file tree

Showing 82 changed files with 192 additions and 149 deletions.
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
@@ -1462,7 +1462,8 @@ pub enum ExprKind {
     /// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct field.
     Field(P<Expr>, Ident),
     /// An indexing operation (e.g., `foo[2]`).
-    Index(P<Expr>, P<Expr>),
+    /// The span represents the span of the `[2]`, including brackets.
+    Index(P<Expr>, P<Expr>, Span),
     /// A range (e.g., `1..2`, `1..`, `..2`, `1..=2`, `..=2`; and `..` in destructuring assignment).
     Range(Option<P<Expr>>, Option<P<Expr>>, RangeLimits),
     /// An underscore, used in destructuring assignment to ignore a value.

diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs
@@ -1400,14 +1400,15 @@ pub fn noop_visit_expr<T: MutVisitor>(
             fn_decl,
             body,
             fn_decl_span,
-            fn_arg_span: _,
+            fn_arg_span,
         }) => {
             vis.visit_closure_binder(binder);
             visit_constness(constness, vis);
             vis.visit_asyncness(asyncness);
             vis.visit_fn_decl(fn_decl);
             vis.visit_expr(body);
             vis.visit_span(fn_decl_span);
+            vis.visit_span(fn_arg_span);
         }
         ExprKind::Block(blk, label) => {
             vis.visit_block(blk);
@@ -1420,9 +1421,10 @@ pub fn noop_visit_expr<T: MutVisitor>(
             vis.visit_expr(expr);
             vis.visit_span(await_kw_span);
         }
-        ExprKind::Assign(el, er, _) => {
+        ExprKind::Assign(el, er, span) => {
             vis.visit_expr(el);
             vis.visit_expr(er);
+            vis.visit_span(span);
         }
         ExprKind::AssignOp(_op, el, er) => {
             vis.visit_expr(el);
@@ -1432,9 +1434,10 @@ pub fn noop_visit_expr<T: MutVisitor>(
             vis.visit_expr(el);
             vis.visit_ident(ident);
         }
-        ExprKind::Index(el, er) => {
+        ExprKind::Index(el, er, brackets_span) => {
             vis.visit_expr(el);
             vis.visit_expr(er);
+            vis.visit_span(brackets_span);
         }
         ExprKind::Range(e1, e2, _lim) => {
             visit_opt(e1, |e1| vis.visit_expr(e1));

diff --git a/compiler/rustc_ast/src/util/parser.rs b/compiler/rustc_ast/src/util/parser.rs
@@ -390,7 +390,7 @@ pub fn contains_exterior_struct_lit(value: &ast::Expr) -> bool {
         | ast::ExprKind::Cast(x, _)
         | ast::ExprKind::Type(x, _)
         | ast::ExprKind::Field(x, _)
-        | ast::ExprKind::Index(x, _) => {
+        | ast::ExprKind::Index(x, _, _) => {
             // &X { y: 1 }, X { y: 1 }.y
             contains_exterior_struct_lit(x)
         }

diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs
@@ -885,7 +885,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
             visitor.visit_expr(subexpression);
             visitor.visit_ident(*ident);
         }
-        ExprKind::Index(main_expression, index_expression) => {
+        ExprKind::Index(main_expression, index_expression, _) => {
             visitor.visit_expr(main_expression);
             visitor.visit_expr(index_expression)
         }

diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs
@@ -240,8 +240,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 ExprKind::Field(el, ident) => {
                     hir::ExprKind::Field(self.lower_expr(el), self.lower_ident(*ident))
                 }
-                ExprKind::Index(el, er) => {
-                    hir::ExprKind::Index(self.lower_expr(el), self.lower_expr(er))
+                ExprKind::Index(el, er, brackets_span) => {
+                    hir::ExprKind::Index(self.lower_expr(el), self.lower_expr(er), *brackets_span)
                 }
                 ExprKind::Range(Some(e1), Some(e2), RangeLimits::Closed) => {
                     self.lower_expr_range_closed(e.span, e1, e2)

diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
@@ -477,7 +477,7 @@ impl<'a> State<'a> {
                 self.word(".");
                 self.print_ident(*ident);
             }
-            ast::ExprKind::Index(expr, index) => {
+            ast::ExprKind::Index(expr, index, _) => {
                 self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
                 self.word("[");
                 self.print_expr(index);

diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
@@ -79,7 +79,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
                         | hir::ExprKind::Unary(hir::UnOp::Deref, inner)
                         | hir::ExprKind::Field(inner, _)
                         | hir::ExprKind::MethodCall(_, inner, _, _)
-                        | hir::ExprKind::Index(inner, _) = &expr.kind
+                        | hir::ExprKind::Index(inner, _, _) = &expr.kind
                     {
                         expr = inner;
                     }

diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
@@ -567,7 +567,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                         }
                     };
                     if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind
-                        && let hir::ExprKind::Index(val, index) = place.kind
+                        && let hir::ExprKind::Index(val, index, _) = place.kind
                         && (expr.span == self.assign_span || place.span == self.assign_span)
                     {
                         // val[index] = rv;
@@ -620,7 +620,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                         );
                         self.suggested = true;
                     } else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind
-                        && let hir::ExprKind::Index(val, index) = receiver.kind
+                        && let hir::ExprKind::Index(val, index, _) = receiver.kind
                         && expr.span == self.assign_span
                     {
                         // val[index].path(args..);

diff --git a/compiler/rustc_builtin_macros/src/assert/context.rs b/compiler/rustc_builtin_macros/src/assert/context.rs
@@ -237,7 +237,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
             ExprKind::If(local_expr, _, _) => {
                 self.manage_cond_expr(local_expr);
             }
-            ExprKind::Index(prefix, suffix) => {
+            ExprKind::Index(prefix, suffix, _) => {
                 self.manage_cond_expr(prefix);
                 self.manage_cond_expr(suffix);
             }

diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -1754,7 +1754,7 @@ impl Expr<'_> {
 
             ExprKind::Unary(UnOp::Deref, _) => true,
 
-            ExprKind::Field(ref base, _) | ExprKind::Index(ref base, _) => {
+            ExprKind::Field(ref base, _) | ExprKind::Index(ref base, _, _) => {
                 allow_projections_from(base) || base.is_place_expr(allow_projections_from)
             }
 
@@ -1831,7 +1831,7 @@ impl Expr<'_> {
             ExprKind::Type(base, _)
             | ExprKind::Unary(_, base)
             | ExprKind::Field(base, _)
-            | ExprKind::Index(base, _)
+            | ExprKind::Index(base, _, _)
             | ExprKind::AddrOf(.., base)
             | ExprKind::Cast(base, _) => {
                 // This isn't exactly true for `Index` and all `Unary`, but we are using this
@@ -2015,7 +2015,9 @@ pub enum ExprKind<'hir> {
     /// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct or tuple field.
     Field(&'hir Expr<'hir>, Ident),
     /// An indexing operation (`foo[2]`).
-    Index(&'hir Expr<'hir>, &'hir Expr<'hir>),
+    /// Similar to [`ExprKind::MethodCall`], the final `Span` represents the span of the brackets
+    /// and index.
+    Index(&'hir Expr<'hir>, &'hir Expr<'hir>, Span),
 
     /// Path to a definition, possibly containing lifetime or type parameters.
     Path(QPath<'hir>),

diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs
@@ -780,7 +780,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
             visitor.visit_expr(subexpression);
             visitor.visit_ident(ident);
         }
-        ExprKind::Index(ref main_expression, ref index_expression) => {
+        ExprKind::Index(ref main_expression, ref index_expression, _) => {
             visitor.visit_expr(main_expression);
             visitor.visit_expr(index_expression)
         }

diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs
@@ -1526,7 +1526,7 @@ impl<'a> State<'a> {
                 self.word(".");
                 self.print_ident(ident);
             }
-            hir::ExprKind::Index(expr, index) => {
+            hir::ExprKind::Index(expr, index, _) => {
                 self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
                 self.word("[");
                 self.print_expr(index);
@@ -2419,7 +2419,7 @@ fn contains_exterior_struct_lit(value: &hir::Expr<'_>) -> bool {
         | hir::ExprKind::Cast(x, _)
         | hir::ExprKind::Type(x, _)
         | hir::ExprKind::Field(x, _)
-        | hir::ExprKind::Index(x, _) => {
+        | hir::ExprKind::Index(x, _, _) => {
             // `&X { y: 1 }, X { y: 1 }.y`
             contains_exterior_struct_lit(x)
         }

diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs
@@ -345,7 +345,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 self.check_expr_struct(expr, expected, qpath, fields, base_expr)
             }
             ExprKind::Field(base, field) => self.check_field(expr, &base, field, expected),
-            ExprKind::Index(base, idx) => self.check_expr_index(base, idx, expr),
+            ExprKind::Index(base, idx, brackets_span) => {
+                self.check_expr_index(base, idx, expr, brackets_span)
+            }
             ExprKind::Yield(value, ref src) => self.check_expr_yield(value, expr, src),
             hir::ExprKind::Err(guar) => Ty::new_error(tcx, guar),
         }
@@ -2840,6 +2842,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         base: &'tcx hir::Expr<'tcx>,
         idx: &'tcx hir::Expr<'tcx>,
         expr: &'tcx hir::Expr<'tcx>,
+        brackets_span: Span,
     ) -> Ty<'tcx> {
         let base_t = self.check_expr(&base);
         let idx_t = self.check_expr(&idx);
@@ -2873,7 +2876,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
                     let mut err = type_error_struct!(
                         self.tcx.sess,
-                        expr.span,
+                        brackets_span,
                         base_t,
                         E0608,
                         "cannot index into a value of type `{base_t}`",
@@ -2887,16 +2890,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             && let ast::LitKind::Int(i, ast::LitIntType::Unsuffixed) = lit.node
                             && i < types.len().try_into().expect("expected tuple index to be < usize length")
                         {
-                            let snip = self.tcx.sess.source_map().span_to_snippet(base.span);
-                            if let Ok(snip) = snip {
-                                err.span_suggestion(
-                                    expr.span,
-                                    "to access tuple elements, use",
-                                    format!("{snip}.{i}"),
-                                    Applicability::MachineApplicable,
-                                );
-                                needs_note = false;
-                            }
+
+                            err.span_suggestion(
+                                brackets_span,
+                                "to access tuple elements, use",
+                                format!(".{i}"),
+                                Applicability::MachineApplicable,
+                            );
+                            needs_note = false;
                         } else if let ExprKind::Path(..) = idx.peel_borrows().kind {
                             err.span_label(idx.span, "cannot access tuple elements at a variable index");
                         }

diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs
@@ -211,7 +211,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
                 self.select_from_expr(base);
             }
 
-            hir::ExprKind::Index(lhs, rhs) => {
+            hir::ExprKind::Index(lhs, rhs, _) => {
                 // lhs[rhs]
                 self.select_from_expr(lhs);
                 self.consume_expr(rhs);

diff --git a/compiler/rustc_hir_typeck/src/mem_categorization.rs b/compiler/rustc_hir_typeck/src/mem_categorization.rs
@@ -336,7 +336,7 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> {
                 ))
             }
 
-            hir::ExprKind::Index(ref base, _) => {
+            hir::ExprKind::Index(ref base, _, _) => {
                 if self.typeck_results.is_method_call(expr) {
                     // If this is an index implemented by a method call, then it
                     // will include an implicit deref of the result.

diff --git a/compiler/rustc_hir_typeck/src/place_op.rs b/compiler/rustc_hir_typeck/src/place_op.rs
@@ -284,7 +284,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let mut exprs = vec![expr];
 
         while let hir::ExprKind::Field(ref expr, _)
-        | hir::ExprKind::Index(ref expr, _)
+        | hir::ExprKind::Index(ref expr, _, _)
         | hir::ExprKind::Unary(hir::UnOp::Deref, ref expr) = exprs.last().unwrap().kind
         {
             exprs.push(expr);

diff --git a/compiler/rustc_hir_typeck/src/rvalue_scopes.rs b/compiler/rustc_hir_typeck/src/rvalue_scopes.rs
@@ -40,7 +40,7 @@ fn record_rvalue_scope_rec(
             hir::ExprKind::AddrOf(_, _, subexpr)
             | hir::ExprKind::Unary(hir::UnOp::Deref, subexpr)
             | hir::ExprKind::Field(subexpr, _)
-            | hir::ExprKind::Index(subexpr, _) => {
+            | hir::ExprKind::Index(subexpr, _, _) => {
                 expr = subexpr;
             }
             _ => {

diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs
@@ -210,7 +210,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
     // to use builtin indexing because the index type is known to be
     // usize-ish
     fn fix_index_builtin_expr(&mut self, e: &hir::Expr<'_>) {
-        if let hir::ExprKind::Index(ref base, ref index) = e.kind {
+        if let hir::ExprKind::Index(ref base, ref index, _) = e.kind {
             // All valid indexing looks like this; might encounter non-valid indexes at this point.
             let base_ty = self.typeck_results.expr_ty_adjusted_opt(base);
             if base_ty.is_none() {

diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs
@@ -653,7 +653,7 @@ trait UnusedDelimLint {
                     ExprKind::Call(fn_, _params) => fn_,
                     ExprKind::Cast(expr, _ty) => expr,
                     ExprKind::Type(expr, _ty) => expr,
-                    ExprKind::Index(base, _subscript) => base,
+                    ExprKind::Index(base, _subscript, _) => base,
                     _ => break,
                 };
                 if !classify::expr_requires_semi_to_be_stmt(innermost) {
@@ -830,7 +830,7 @@ trait UnusedDelimLint {
                 (value, UnusedDelimsCtx::ReturnValue, false, Some(left), None, true)
             }
 
-            Index(_, ref value) => (value, UnusedDelimsCtx::IndexExpr, false, None, None, false),
+            Index(_, ref value, _) => (value, UnusedDelimsCtx::IndexExpr, false, None, None, false),
 
             Assign(_, ref value, _) | AssignOp(.., ref value) => {
                 (value, UnusedDelimsCtx::AssignedValue, false, None, None, false)

diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs
@@ -469,11 +469,17 @@ impl<'tcx> Cx<'tcx> {
                 }
             }
 
-            hir::ExprKind::Index(ref lhs, ref index) => {
+            hir::ExprKind::Index(ref lhs, ref index, brackets_span) => {
                 if self.typeck_results().is_method_call(expr) {
                     let lhs = self.mirror_expr(lhs);
                     let index = self.mirror_expr(index);
-                    self.overloaded_place(expr, expr_ty, None, Box::new([lhs, index]), expr.span)
+                    self.overloaded_place(
+                        expr,
+                        expr_ty,
+                        None,
+                        Box::new([lhs, index]),
+                        brackets_span,
+                    )
                 } else {
                     ExprKind::Index { lhs: self.mirror_expr(lhs), index: self.mirror_expr(index) }
                 }

diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
@@ -857,7 +857,7 @@ impl<'a> Parser<'a> {
             let msg = format!(
                 "cast cannot be followed by {}",
                 match with_postfix.kind {
-                    ExprKind::Index(_, _) => "indexing",
+                    ExprKind::Index(..) => "indexing",
                     ExprKind::Try(_) => "`?`",
                     ExprKind::Field(_, _) => "a field access",
                     ExprKind::MethodCall(_) => "a method call",
@@ -1304,7 +1304,10 @@ impl<'a> Parser<'a> {
         let index = self.parse_expr()?;
         self.suggest_missing_semicolon_before_array(prev_span, open_delim_span)?;
         self.expect(&token::CloseDelim(Delimiter::Bracket))?;
-        Ok(self.mk_expr(lo.to(self.prev_token.span), self.mk_index(base, index)))
+        Ok(self.mk_expr(
+            lo.to(self.prev_token.span),
+            self.mk_index(base, index, open_delim_span.to(self.prev_token.span)),
+        ))
     }
 
     /// Assuming we have just parsed `.`, continue parsing into an expression.
@@ -3366,8 +3369,8 @@ impl<'a> Parser<'a> {
         ExprKind::Binary(binop, lhs, rhs)
     }
 
-    fn mk_index(&self, expr: P<Expr>, idx: P<Expr>) -> ExprKind {
-        ExprKind::Index(expr, idx)
+    fn mk_index(&self, expr: P<Expr>, idx: P<Expr>, brackets_span: Span) -> ExprKind {
+        ExprKind::Index(expr, idx, brackets_span)
     }
 
     fn mk_call(&self, f: P<Expr>, args: ThinVec<P<Expr>>) -> ExprKind {

diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs
@@ -1061,7 +1061,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
                 self.propagate_through_expr(&l, ln)
             }
 
-            hir::ExprKind::Index(ref l, ref r) | hir::ExprKind::Binary(_, ref l, ref r) => {
+            hir::ExprKind::Index(ref l, ref r, _) | hir::ExprKind::Binary(_, ref l, ref r) => {
                 let r_succ = self.propagate_through_expr(&r, succ);
                 self.propagate_through_expr(&l, r_succ)
             }

diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -4269,7 +4269,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
             ExprKind::ConstBlock(ref ct) => {
                 self.resolve_anon_const(ct, AnonConstKind::InlineConst);
             }
-            ExprKind::Index(ref elem, ref idx) => {
+            ExprKind::Index(ref elem, ref idx, _) => {
                 self.resolve_expr(elem, Some(expr));
                 self.visit_expr(idx);
             }

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -2854,7 +2854,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                         err.note("all local variables must have a statically known size");
                     }
                     Some(Node::Local(hir::Local {
-                        init: Some(hir::Expr { kind: hir::ExprKind::Index(_, _), span, .. }),
+                        init: Some(hir::Expr { kind: hir::ExprKind::Index(..), span, .. }),
                         ..
                     })) => {
                         // When encountering an assignment of an unsized trait, like

diff --git a/src/tools/clippy/clippy_lints/src/dereference.rs b/src/tools/clippy/clippy_lints/src/dereference.rs
@@ -800,7 +800,7 @@ fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> boo
         && parent.span.ctxt() == e.span.ctxt()
     {
         match parent.kind {
-            ExprKind::Call(child, _) | ExprKind::MethodCall(_, child, _, _) | ExprKind::Index(child, _)
+            ExprKind::Call(child, _) | ExprKind::MethodCall(_, child, _, _) | ExprKind::Index(child, _, _)
                 if child.hir_id == e.hir_id => true,
             ExprKind::Field(_, _) | ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) => true,
             _ => false,

diff --git a/src/tools/clippy/clippy_lints/src/functions/must_use.rs b/src/tools/clippy/clippy_lints/src/functions/must_use.rs
@@ -221,7 +221,7 @@ fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
     match e.kind {
         Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
         Path(_) => true,
-        Field(inner, _) | Index(inner, _) => is_mutated_static(inner),
+        Field(inner, _) | Index(inner, _, _) => is_mutated_static(inner),
         _ => false,
     }
 }