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.
This commit is contained in:
Nilstrieb
2023-08-03 21:43:17 +02:00
parent ff27f9095f
commit ed0dfed24f
33 changed files with 42 additions and 42 deletions
+1 -1
View File
@@ -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,
+1 -1
View File
@@ -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,
}
}
+1 -1
View File
@@ -254,7 +254,7 @@ fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
// Checking for slice indexing
let parent_id = map.parent_id(expr.hir_id);
if let Some(hir::Node::Expr(parent_expr)) = map.find(parent_id);
if let hir::ExprKind::Index(_, index_expr) = parent_expr.kind;
if let hir::ExprKind::Index(_, index_expr, _) = parent_expr.kind;
if let Some(Constant::Int(index_value)) = constant(cx, cx.typeck_results(), index_expr);
if let Ok(index_value) = index_value.try_into();
if index_value < max_suggested_slice;
+1 -1
View File
@@ -103,7 +103,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
return;
}
if let ExprKind::Index(array, index) = &expr.kind {
if let ExprKind::Index(array, index, _) = &expr.kind {
let note = "the suggestion might not be applicable in constant blocks";
let ty = cx.typeck_results().expr_ty(array).peel_refs();
if let Some(range) = higher::Range::hir(index) {
+2 -2
View File
@@ -60,8 +60,8 @@ pub(super) fn check<'tcx>(
o.and_then(|(lhs, rhs)| {
let rhs = fetch_cloned_expr(rhs);
if_chain! {
if let ExprKind::Index(base_left, idx_left) = lhs.kind;
if let ExprKind::Index(base_right, idx_right) = rhs.kind;
if let ExprKind::Index(base_left, idx_left, _) = lhs.kind;
if let ExprKind::Index(base_right, idx_right, _) = rhs.kind;
if let Some(ty) = get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_left));
if get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_right)).is_some();
if let Some((start_left, offset_left)) = get_details_from_idx(cx, idx_left, &starts);
@@ -319,7 +319,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if_chain! {
// an index op
if let ExprKind::Index(seqexpr, idx) = expr.kind;
if let ExprKind::Index(seqexpr, idx, _) = expr.kind;
if !self.check(idx, seqexpr, expr);
then {
return;
+1 -1
View File
@@ -162,7 +162,7 @@ fn never_loop_expr<'tcx>(
ExprKind::Binary(_, e1, e2)
| ExprKind::Assign(e1, e2, _)
| ExprKind::AssignOp(_, e1, e2)
| ExprKind::Index(e1, e2) => never_loop_expr_all(cx, &mut [e1, e2].iter().copied(), ignore_ids, main_loop_id),
| ExprKind::Index(e1, e2, _) => never_loop_expr_all(cx, &mut [e1, e2].iter().copied(), ignore_ids, main_loop_id),
ExprKind::Loop(b, _, _, _) => {
// Break can come from the inner loop so remove them.
absorb_break(never_loop_block(cx, b, ignore_ids, main_loop_id))
@@ -113,7 +113,7 @@ fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExp
// Shouldn't have side effects, but there's no way to trace which field is used. So forget which fields have
// already been seen.
ExprKind::Index(base, idx) if !idx.can_have_side_effects() => {
ExprKind::Index(base, idx, _) if !idx.can_have_side_effects() => {
can_move = false;
fields.clear();
e = base;
+1 -1
View File
@@ -204,7 +204,7 @@ fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
if_chain! {
if is_ref_str(self.cx, ex);
let unref = peel_ref(ex);
if let ExprKind::Index(indexed, index) = &unref.kind;
if let ExprKind::Index(indexed, index, _) = &unref.kind;
if let Some(higher::Range { start, end, .. }) = higher::Range::hir(index);
if let ExprKind::Path(path) = &indexed.kind;
if self.cx.qpath_res(path, ex.hir_id) == self.target;
@@ -12,7 +12,7 @@
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>) {
if_chain! {
if let Some(idx_expr) = is_vec_indexing(cx, scrutinee);
if let ExprKind::Index(vec, idx) = idx_expr.kind;
if let ExprKind::Index(vec, idx, _) = idx_expr.kind;
then {
// FIXME: could be improved to suggest surrounding every pattern with Some(_),
@@ -36,7 +36,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>) {
fn is_vec_indexing<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if_chain! {
if let ExprKind::Index(array, index) = expr.kind;
if let ExprKind::Index(array, index, _) = expr.kind;
if is_vector(cx, array);
if !is_full_range(cx, index);
+1 -1
View File
@@ -12,7 +12,7 @@
fn path_to_local(expr: &hir::Expr<'_>) -> Option<hir::HirId> {
match expr.kind {
hir::ExprKind::Field(f, _) => path_to_local(f),
hir::ExprKind::Index(recv, _) => path_to_local(recv),
hir::ExprKind::Index(recv, _, _) => path_to_local(recv),
hir::ExprKind::Path(hir::QPath::Resolved(
_,
hir::Path {
+1 -1
View File
@@ -27,7 +27,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, cal
if derefs_to_slice(cx, caller_expr, cx.typeck_results().expr_ty(caller_expr)).is_some() {
// caller is a Slice
if_chain! {
if let hir::ExprKind::Index(caller_var, index_expr) = &caller_expr.kind;
if let hir::ExprKind::Index(caller_var, index_expr, _) = &caller_expr.kind;
if let Some(higher::Range { start: Some(start_expr), end: None, limits: ast::RangeLimits::HalfOpen })
= higher::Range::hir(index_expr);
if let hir::ExprKind::Lit(start_lit) = &start_expr.kind;
@@ -239,7 +239,7 @@ fn check_expr<'tcx>(vis: &mut ReadVisitor<'_, 'tcx>, expr: &'tcx Expr<'_>) -> St
| ExprKind::MethodCall(..)
| ExprKind::Call(_, _)
| ExprKind::Assign(..)
| ExprKind::Index(_, _)
| ExprKind::Index(..)
| ExprKind::Repeat(_, _)
| ExprKind::Struct(_, _, _) => {
walk_expr(vis, expr);
+1 -1
View File
@@ -119,7 +119,7 @@ fn condition_needs_parentheses(e: &Expr<'_>) -> bool {
| ExprKind::Call(i, _)
| ExprKind::Cast(i, _)
| ExprKind::Type(i, _)
| ExprKind::Index(i, _) = inner.kind
| ExprKind::Index(i, _, _) = inner.kind
{
if matches!(
i.kind,
+2 -2
View File
@@ -160,7 +160,7 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
match peel_blocks(expr).kind {
ExprKind::Lit(..) | ExprKind::Closure { .. } => true,
ExprKind::Path(..) => !has_drop(cx, cx.typeck_results().expr_ty(expr)),
ExprKind::Index(a, b) | ExprKind::Binary(_, a, b) => has_no_effect(cx, a) && has_no_effect(cx, b),
ExprKind::Index(a, b, _) | ExprKind::Binary(_, a, b) => has_no_effect(cx, a) && has_no_effect(cx, b),
ExprKind::Array(v) | ExprKind::Tup(v) => v.iter().all(|val| has_no_effect(cx, val)),
ExprKind::Repeat(inner, _)
| ExprKind::Cast(inner, _)
@@ -263,7 +263,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec
return None;
}
match expr.kind {
ExprKind::Index(a, b) => Some(vec![a, b]),
ExprKind::Index(a, b, _) => Some(vec![a, b]),
ExprKind::Binary(ref binop, a, b) if binop.node != BinOpKind::And && binop.node != BinOpKind::Or => {
Some(vec![a, b])
},
+1 -1
View File
@@ -438,7 +438,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
dereferenced_expr = parent_expr;
},
ExprKind::Index(e, _) if ptr::eq(&**e, cur_expr) => {
ExprKind::Index(e, _, _) if ptr::eq(&**e, cur_expr) => {
// `e[i]` => desugared to `*Index::index(&e, i)`,
// meaning `e` must be referenced.
// no need to go further up since a method call is involved now.
+1 -1
View File
@@ -695,7 +695,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
}
},
// Indexing is fine for currently supported types.
ExprKind::Index(e, _) if e.hir_id == child_id => (),
ExprKind::Index(e, _, _) if e.hir_id == child_id => (),
_ => set_skip_flag(),
},
_ => set_skip_flag(),
+1 -1
View File
@@ -81,7 +81,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, addressee) = expr.kind;
if addressee.span.ctxt() == ctxt;
if let ExprKind::Index(indexed, range) = addressee.kind;
if let ExprKind::Index(indexed, range, _) = addressee.kind;
if is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), LangItem::RangeFull);
then {
let (expr_ty, expr_ref_count) = peel_mid_ty_refs(cx.typeck_results().expr_ty(expr));
+2 -2
View File
@@ -190,7 +190,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
);
}
},
ExprKind::Index(target, _idx) => {
ExprKind::Index(target, _idx, _) => {
let e_ty = cx.typeck_results().expr_ty(target).peel_refs();
if e_ty.is_str() || is_type_lang_item(cx, e_ty, LangItem::String) {
span_lint(
@@ -262,7 +262,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
// Find string::as_bytes
if let ExprKind::AddrOf(BorrowKind::Ref, _, args) = args[0].kind;
if let ExprKind::Index(left, right) = args.kind;
if let ExprKind::Index(left, right, _) = args.kind;
let (method_names, expressions, _) = method_calls(left, 1);
if method_names.len() == 1;
if expressions.len() == 1;
@@ -572,7 +572,7 @@ fn ident_difference_expr_with_base_location(
| (AddrOf(_, _, _), AddrOf(_, _, _))
| (Path(_, _), Path(_, _))
| (Range(_, _, _), Range(_, _, _))
| (Index(_, _), Index(_, _))
| (Index(_, _, _), Index(_, _, _))
| (Field(_, _), Field(_, _))
| (AssignOp(_, _, _), AssignOp(_, _, _))
| (Assign(_, _, _), Assign(_, _, _))
+2 -2
View File
@@ -86,8 +86,8 @@ fn generate_swap_warning(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>, spa
let mut applicability = Applicability::MachineApplicable;
if !can_mut_borrow_both(cx, e1, e2) {
if let ExprKind::Index(lhs1, idx1) = e1.kind
&& let ExprKind::Index(lhs2, idx2) = e2.kind
if let ExprKind::Index(lhs1, idx1, _) = e1.kind
&& let ExprKind::Index(lhs2, idx2, _) = e2.kind
&& eq_expr_value(cx, lhs1, lhs2)
&& e1.span.ctxt() == ctxt
&& e2.span.ctxt() == ctxt
+1 -1
View File
@@ -33,7 +33,7 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryAssignment {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Assign(target, ..) = &expr.kind {
let mut base = target;
while let ExprKind::Field(f, _) | ExprKind::Index(f, _) = &base.kind {
while let ExprKind::Field(f, _) | ExprKind::Index(f, _, _) = &base.kind {
base = f;
}
if is_temporary(base) && !is_adjusted(cx, base) {
+2 -2
View File
@@ -103,11 +103,11 @@ fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &
// Fix #11100
&& tys.iter().all_equal()
&& let Some(locals) = (match first.kind {
ExprKind::Index(_, _) => elements
ExprKind::Index(..) => elements
.iter()
.enumerate()
.map(|(i, i_expr)| -> Option<&'tcx Expr<'tcx>> {
if let ExprKind::Index(lhs, index) = i_expr.kind
if let ExprKind::Index(lhs, index, _) = i_expr.kind
&& let ExprKind::Lit(lit) = index.kind
&& let LitKind::Int(val, _) = lit.node
{
+1 -1
View File
@@ -526,7 +526,7 @@ macro_rules! kind {
self.ident(field_name);
self.expr(object);
},
ExprKind::Index(object, index) => {
ExprKind::Index(object, index, _) => {
bind!(self, object, index);
kind!("Index({object}, {index})");
self.expr(object);
+1 -1
View File
@@ -88,7 +88,7 @@ fn display_err(&self, cx: &LateContext<'_>) {
let mut last_place = parent;
while let Some(parent) = get_parent_expr(cx, last_place) {
if matches!(parent.kind, ExprKind::Unary(UnOp::Deref, _) | ExprKind::Field(..))
|| matches!(parent.kind, ExprKind::Index(e, _) if e.hir_id == last_place.hir_id)
|| matches!(parent.kind, ExprKind::Index(e, _, _) if e.hir_id == last_place.hir_id)
{
last_place = parent;
} else {
+1 -1
View File
@@ -178,7 +178,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool {
(Yield(l), Yield(r)) | (Ret(l), Ret(r)) => eq_expr_opt(l, r),
(Break(ll, le), Break(rl, re)) => eq_label(ll, rl) && eq_expr_opt(le, re),
(Continue(ll), Continue(rl)) => eq_label(ll, rl),
(Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2), Index(r1, r2)) => eq_expr(l1, r1) && eq_expr(l2, r2),
(Assign(l1, l2, _), Assign(r1, r2, _)) | (Index(l1, l2, _), Index(r1, r2, _)) => eq_expr(l1, r1) && eq_expr(l2, r2),
(AssignOp(lo, lp, lv), AssignOp(ro, rp, rv)) => lo.node == ro.node && eq_expr(lp, rp) && eq_expr(lv, rv),
(Field(lp, lf), Field(rp, rf)) => eq_id(*lf, *rf) && eq_expr(lp, rp),
(Match(ls, la), Match(rs, ra)) => eq_expr(ls, rs) && over(la, ra, eq_arm),
+1 -1
View File
@@ -163,7 +163,7 @@ fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) {
) => (Pat::Str("unsafe"), Pat::Str("}")),
ExprKind::Block(_, None) => (Pat::Str("{"), Pat::Str("}")),
ExprKind::Field(e, name) => (expr_search_pat(tcx, e).0, Pat::Sym(name.name)),
ExprKind::Index(e, _) => (expr_search_pat(tcx, e).0, Pat::Str("]")),
ExprKind::Index(e, _, _) => (expr_search_pat(tcx, e).0, Pat::Str("]")),
ExprKind::Path(ref path) => qpath_search_pat(path),
ExprKind::AddrOf(_, _, e) => (Pat::Str("&"), expr_search_pat(tcx, e).1),
ExprKind::Break(Destination { label: None, .. }, None) => (Pat::Str("break"), Pat::Str("break")),
+1 -1
View File
@@ -394,7 +394,7 @@ pub fn expr(&mut self, e: &Expr<'_>) -> Option<Constant<'tcx>> {
}
}
},
ExprKind::Index(arr, index) => self.index(arr, index),
ExprKind::Index(arr, index, _) => self.index(arr, index),
ExprKind::AddrOf(_, _, inner) => self.expr(inner).map(|r| Constant::Ref(Box::new(r))),
ExprKind::Field(local_expr, ref field) => {
let result = self.expr(local_expr);
+1 -1
View File
@@ -185,7 +185,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
.type_dependent_def_id(e.hir_id)
.map_or(Lazy, |id| fn_eagerness(self.cx, id, name.ident.name, true));
},
ExprKind::Index(_, e) => {
ExprKind::Index(_, e, _) => {
let ty = self.cx.typeck_results().expr_ty_adjusted(e);
if is_copy(self.cx, ty) && !ty.is_ref() {
self.eagerness |= NoChange;
+2 -2
View File
@@ -299,7 +299,7 @@ pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool {
(&ExprKind::Field(l_f_exp, ref l_f_ident), &ExprKind::Field(r_f_exp, ref r_f_ident)) => {
l_f_ident.name == r_f_ident.name && self.eq_expr(l_f_exp, r_f_exp)
},
(&ExprKind::Index(la, li), &ExprKind::Index(ra, ri)) => self.eq_expr(la, ra) && self.eq_expr(li, ri),
(&ExprKind::Index(la, li, _), &ExprKind::Index(ra, ri, _)) => self.eq_expr(la, ra) && self.eq_expr(li, ri),
(&ExprKind::If(lc, lt, ref le), &ExprKind::If(rc, rt, ref re)) => {
self.eq_expr(lc, rc) && self.eq_expr(lt, rt) && both(le, re, |l, r| self.eq_expr(l, r))
},
@@ -730,7 +730,7 @@ pub fn hash_expr(&mut self, e: &Expr<'_>) {
self.hash_expr(e);
self.hash_name(f.name);
},
ExprKind::Index(a, i) => {
ExprKind::Index(a, i, _) => {
self.hash_expr(a);
self.hash_expr(i);
},
+2 -2
View File
@@ -735,7 +735,7 @@ fn projection_stack<'a, 'hir>(mut e: &'a Expr<'hir>) -> (Vec<&'a Expr<'hir>>, &'
let mut result = vec![];
let root = loop {
match e.kind {
ExprKind::Index(ep, _) | ExprKind::Field(ep, _) => {
ExprKind::Index(ep, _, _) | ExprKind::Field(ep, _) => {
result.push(e);
e = ep;
},
@@ -782,7 +782,7 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
return true;
}
},
(ExprKind::Index(_, i1), ExprKind::Index(_, i2)) => {
(ExprKind::Index(_, i1, _), ExprKind::Index(_, i2, _)) => {
if !eq_expr_value(cx, i1, i2) {
return false;
}
+1 -1
View File
@@ -31,7 +31,7 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>) -> Certainty {
let certainty = match &expr.kind {
ExprKind::Unary(_, expr)
| ExprKind::Field(expr, _)
| ExprKind::Index(expr, _)
| ExprKind::Index(expr, _, _)
| ExprKind::AddrOf(_, _, expr) => expr_type_certainty(cx, expr),
ExprKind::Array(exprs) => join(exprs.iter().map(|expr| expr_type_certainty(cx, expr))),
+2 -2
View File
@@ -329,7 +329,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
&& self.cx.typeck_results().expr_ty(rhs).peel_refs().is_primitive_ty() => {},
ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty(e).is_ref() => (),
ExprKind::Unary(_, e) if self.cx.typeck_results().expr_ty(e).peel_refs().is_primitive_ty() => (),
ExprKind::Index(base, _)
ExprKind::Index(base, _, _)
if matches!(
self.cx.typeck_results().expr_ty(base).peel_refs().kind(),
ty::Slice(_) | ty::Array(..)
@@ -629,7 +629,7 @@ fn helper<'tcx, B>(
helper(typeck, true, arg, f)?;
}
},
ExprKind::Index(borrowed, consumed)
ExprKind::Index(borrowed, consumed, _)
| ExprKind::Assign(borrowed, consumed, _)
| ExprKind::AssignOp(_, borrowed, consumed) => {
helper(typeck, false, borrowed, f)?;