Rollup merge of rust-lang#89895 - camsteffen:for-loop-head-span, r=davidtwco

JohnTitor · web-flow · commit 91fb223f5913 · 2021-10-22T19:42:45.000+09:00
Don't mark for loop iter expression as desugared We typically don't mark spans of lowered things as desugared. This helps Clippy rightly discern when code is (not) from expansion. This was discovered by ``@flip1995`` at rust-lang/rust-clippy#7789 (comment).
diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs
@@ -1332,15 +1332,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
         body: &Block,
         opt_label: Option<Label>,
     ) -> hir::Expr<'hir> {
-        let orig_head_span = head.span;
         // expand <head>
-        let mut head = self.lower_expr_mut(head);
-        let desugared_span = self.mark_span_with_reason(
-            DesugaringKind::ForLoop(ForLoopLoc::Head),
-            orig_head_span,
-            None,
-        );
-        head.span = self.lower_span(desugared_span);
+        let head = self.lower_expr_mut(head);
+        let desugared_span =
+            self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), head.span, None);
+        let e_span = self.lower_span(e.span);
 
         let iter = Ident::with_dummy_span(sym::iter);
 
@@ -1354,23 +1350,24 @@ impl<'hir> LoweringContext<'_, 'hir> {
         // `::std::option::Option::Some(val) => __next = val`
         let pat_arm = {
             let val_ident = Ident::with_dummy_span(sym::val);
-            let (val_pat, val_pat_hid) = self.pat_ident(pat.span, val_ident);
-            let val_expr = self.expr_ident(pat.span, val_ident, val_pat_hid);
-            let next_expr = self.expr_ident(pat.span, next_ident, next_pat_hid);
+            let pat_span = self.lower_span(pat.span);
+            let (val_pat, val_pat_hid) = self.pat_ident(pat_span, val_ident);
+            let val_expr = self.expr_ident(pat_span, val_ident, val_pat_hid);
+            let next_expr = self.expr_ident(pat_span, next_ident, next_pat_hid);
             let assign = self.arena.alloc(self.expr(
-                pat.span,
-                hir::ExprKind::Assign(next_expr, val_expr, self.lower_span(pat.span)),
+                pat_span,
+                hir::ExprKind::Assign(next_expr, val_expr, self.lower_span(pat_span)),
                 ThinVec::new(),
             ));
-            let some_pat = self.pat_some(pat.span, val_pat);
+            let some_pat = self.pat_some(pat_span, val_pat);
             self.arm(some_pat, assign)
         };
 
         // `::std::option::Option::None => break`
         let break_arm = {
             let break_expr =
-                self.with_loop_scope(e.id, |this| this.expr_break_alloc(e.span, ThinVec::new()));
-            let pat = self.pat_none(e.span);
+                self.with_loop_scope(e.id, |this| this.expr_break_alloc(e_span, ThinVec::new()));
+            let pat = self.pat_none(e_span);
             self.arm(pat, break_expr)
         };
 
@@ -1416,10 +1413,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
         let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false));
         let body_expr = self.expr_block(body_block, ThinVec::new());
-        let body_stmt = self.stmt_expr(body.span, body_expr);
+        let body_stmt = self.stmt_expr(body_block.span, body_expr);
 
         let loop_block = self.block_all(
-            e.span,
+            e_span,
             arena_vec![self; next_let, match_stmt, pat_let, body_stmt],
             None,
         );
@@ -1429,7 +1426,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             loop_block,
             self.lower_label(opt_label),
             hir::LoopSource::ForLoop,
-            self.lower_span(e.span.with_hi(orig_head_span.hi())),
+            self.lower_span(e_span.with_hi(head.span.hi())),
         );
         let loop_expr = self.arena.alloc(hir::Expr {
             hir_id: self.lower_node_id(e.id),
@@ -1442,7 +1439,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
         let into_iter_span = self.mark_span_with_reason(
             DesugaringKind::ForLoop(ForLoopLoc::IntoIter),
-            orig_head_span,
+            head.span,
             None,
         );
 
@@ -1458,7 +1455,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         // #82462: to correctly diagnose borrow errors, the block that contains
         // the iter expr needs to have a span that covers the loop body.
         let desugared_full_span =
-            self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), e.span, None);
+            self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), e_span, None);
 
         let match_expr = self.arena.alloc(self.expr_match(
             desugared_full_span,
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -11,7 +11,6 @@ use rustc_middle::mir::{
 };
 use rustc_middle::ty::{self, suggest_constraining_type_param, Ty};
 use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex};
-use rustc_span::source_map::DesugaringKind;
 use rustc_span::symbol::sym;
 use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP};
 use rustc_trait_selection::infer::InferCtxtExt;
@@ -247,6 +246,36 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                         place_name, partially_str, loop_message
                                     ),
                                 );
+                                let sess = self.infcx.tcx.sess;
+                                let ty = used_place.ty(self.body, self.infcx.tcx).ty;
+                                // If we have a `&mut` ref, we need to reborrow.
+                                if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() {
+                                    // If we are in a loop this will be suggested later.
+                                    if !is_loop_move {
+                                        err.span_suggestion_verbose(
+                                            move_span.shrink_to_lo(),
+                                            &format!(
+                                                "consider creating a fresh reborrow of {} here",
+                                                self.describe_place(moved_place.as_ref())
+                                                    .map(|n| format!("`{}`", n))
+                                                    .unwrap_or_else(
+                                                        || "the mutable reference".to_string()
+                                                    ),
+                                            ),
+                                            "&mut *".to_string(),
+                                            Applicability::MachineApplicable,
+                                        );
+                                    }
+                                } else if let Ok(snippet) =
+                                    sess.source_map().span_to_snippet(move_span)
+                                {
+                                    err.span_suggestion(
+                                        move_span,
+                                        "consider borrowing to avoid moving into the for loop",
+                                        format!("&{}", snippet),
+                                        Applicability::MaybeIncorrect,
+                                    );
+                                }
                             } else {
                                 err.span_label(
                                     fn_call_span,
@@ -315,35 +344,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                         in_pattern = true;
                     }
                 }
-
-                if let Some(DesugaringKind::ForLoop(_)) = move_span.desugaring_kind() {
-                    let sess = self.infcx.tcx.sess;
-                    let ty = used_place.ty(self.body, self.infcx.tcx).ty;
-                    // If we have a `&mut` ref, we need to reborrow.
-                    if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() {
-                        // If we are in a loop this will be suggested later.
-                        if !is_loop_move {
-                            err.span_suggestion_verbose(
-                                move_span.shrink_to_lo(),
-                                &format!(
-                                    "consider creating a fresh reborrow of {} here",
-                                    self.describe_place(moved_place.as_ref())
-                                        .map(|n| format!("`{}`", n))
-                                        .unwrap_or_else(|| "the mutable reference".to_string()),
-                                ),
-                                "&mut *".to_string(),
-                                Applicability::MachineApplicable,
-                            );
-                        }
-                    } else if let Ok(snippet) = sess.source_map().span_to_snippet(move_span) {
-                        err.span_suggestion(
-                            move_span,
-                            "consider borrowing to avoid moving into the for loop",
-                            format!("&{}", snippet),
-                            Applicability::MaybeIncorrect,
-                        );
-                    }
-                }
             }
 
             use_spans.var_span_label_path_only(
diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
@@ -5,11 +5,10 @@ use rustc_middle::ty;
 use rustc_mir_dataflow::move_paths::{
     IllegalMoveOrigin, IllegalMoveOriginKind, LookupResult, MoveError, MovePathIndex,
 };
-use rustc_span::source_map::DesugaringKind;
 use rustc_span::{sym, Span, DUMMY_SP};
 use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;
 
-use crate::diagnostics::UseSpans;
+use crate::diagnostics::{FnSelfUseKind, UseSpans};
 use crate::prefixes::PrefixSet;
 use crate::MirBorrowckCtxt;
 
@@ -400,19 +399,21 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
             | ty::Opaque(def_id, _) => def_id,
             _ => return err,
         };
-        let is_option = self.infcx.tcx.is_diagnostic_item(sym::Option, def_id);
-        let is_result = self.infcx.tcx.is_diagnostic_item(sym::Result, def_id);
-        if (is_option || is_result) && use_spans.map_or(true, |v| !v.for_closure()) {
+        let diag_name = self.infcx.tcx.get_diagnostic_name(def_id);
+        if matches!(diag_name, Some(sym::Option | sym::Result))
+            && use_spans.map_or(true, |v| !v.for_closure())
+        {
             err.span_suggestion_verbose(
                 span.shrink_to_hi(),
-                &format!(
-                    "consider borrowing the `{}`'s content",
-                    if is_option { "Option" } else { "Result" }
-                ),
+                &format!("consider borrowing the `{}`'s content", diag_name.unwrap()),
                 ".as_ref()".to_string(),
                 Applicability::MaybeIncorrect,
             );
-        } else if matches!(span.desugaring_kind(), Some(DesugaringKind::ForLoop(_))) {
+        } else if let Some(UseSpans::FnSelfUse {
+            kind: FnSelfUseKind::Normal { implicit_into_iter: true, .. },
+            ..
+        }) = use_spans
+        {
             let suggest = match self.infcx.tcx.get_diagnostic_item(sym::IntoIterator) {
                 Some(def_id) => self.infcx.tcx.infer_ctxt().enter(|infcx| {
                     type_known_to_meet_bound_modulo_regions(
diff --git a/compiler/rustc_lint/src/array_into_iter.rs b/compiler/rustc_lint/src/array_into_iter.rs
@@ -134,9 +134,8 @@ impl<'tcx> LateLintPass<'tcx> for ArrayIntoIter {
                     Applicability::MachineApplicable,
                 );
                 if self.for_expr_span == expr.span {
-                    let expr_span = expr.span.ctxt().outer_expn_data().call_site;
                     diag.span_suggestion(
-                        receiver_arg.span.shrink_to_hi().to(expr_span.shrink_to_hi()),
+                        receiver_arg.span.shrink_to_hi().to(expr.span.shrink_to_hi()),
                         "or remove `.into_iter()` to iterate by value",
                         String::new(),
                         Applicability::MaybeIncorrect,
diff --git a/src/test/incremental/hashes/for_loops.rs b/src/test/incremental/hashes/for_loops.rs
@@ -83,7 +83,7 @@ pub fn change_iteration_variable_pattern() {
 #[cfg(not(any(cfail1,cfail4)))]
 #[rustc_clean(cfg="cfail2", except="hir_owner_nodes, optimized_mir, typeck")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir, typeck, promoted_mir")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir, typeck")]
 #[rustc_clean(cfg="cfail6")]
 pub fn change_iteration_variable_pattern() {
     let mut _x = 0;
@@ -108,7 +108,7 @@ pub fn change_iterable() {
 #[cfg(not(any(cfail1,cfail4)))]
 #[rustc_clean(cfg="cfail2", except="hir_owner_nodes, promoted_mir")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, promoted_mir, optimized_mir")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, promoted_mir")]
 #[rustc_clean(cfg="cfail6")]
 pub fn change_iterable() {
     let mut _x = 0;
@@ -183,7 +183,7 @@ pub fn add_loop_label_to_break() {
 #[cfg(not(any(cfail1,cfail4)))]
 #[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
 #[rustc_clean(cfg="cfail6")]
 pub fn add_loop_label_to_break() {
     let mut _x = 0;
@@ -237,7 +237,7 @@ pub fn add_loop_label_to_continue() {
 #[cfg(not(any(cfail1,cfail4)))]
 #[rustc_clean(cfg="cfail2", except="hir_owner_nodes")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")]
 #[rustc_clean(cfg="cfail6")]
 pub fn add_loop_label_to_continue() {
     let mut _x = 0;
diff --git a/src/tools/clippy/clippy_lints/src/vec.rs b/src/tools/clippy/clippy_lints/src/vec.rs
@@ -63,13 +63,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
             if is_copy(cx, vec_type(cx.typeck_results().expr_ty_adjusted(arg)));
             then {
                 // report the error around the `vec!` not inside `<std macros>:`
-                let span = arg.span
-                    .ctxt()
-                    .outer_expn_data()
-                    .call_site
-                    .ctxt()
-                    .outer_expn_data()
-                    .call_site;
+                let span = arg.span.ctxt().outer_expn_data().call_site;
                 self.check_vec_macro(cx, &vec_args, Mutability::Not, span);
             }
         }
