Auto merge of #117585 - dnbln:feat/move-kw-span, r=cjgillot

Add the `Span` of the `move` keyword to the HIR. This is required to implement a lint like the one described here: rust-lang/rust-clippy#11721
rust-lang · Nov 6, 2023 · 152a4e9 · 152a4e9
2 parents 7a892ab + c077147
commit 152a4e9
Show file tree

Hide file tree

Showing 14 changed files with 49 additions and 20 deletions.
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
@@ -1548,7 +1548,10 @@ pub struct QSelf {
 #[derive(Clone, Copy, PartialEq, Encodable, Decodable, Debug, HashStable_Generic)]
 pub enum CaptureBy {
     /// `move |x| y + x`.
-    Value,
+    Value {
+        /// The span of the `move` keyword.
+        move_kw: Span,
+    },
     /// `move` keyword was not specified.
     Ref,
 }

diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs
@@ -302,6 +302,10 @@ pub trait MutVisitor: Sized {
     fn visit_format_args(&mut self, fmt: &mut FormatArgs) {
         noop_visit_format_args(fmt, self)
     }
+
+    fn visit_capture_by(&mut self, capture_by: &mut CaptureBy) {
+        noop_visit_capture_by(capture_by, self)
+    }
 }
 
 /// Use a map-style function (`FnOnce(T) -> T`) to overwrite a `&mut T`. Useful
@@ -1397,7 +1401,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
         }
         ExprKind::Closure(box Closure {
             binder,
-            capture_clause: _,
+            capture_clause,
             constness,
             asyncness,
             movability: _,
@@ -1409,6 +1413,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
             vis.visit_closure_binder(binder);
             visit_constness(constness, vis);
             vis.visit_asyncness(asyncness);
+            vis.visit_capture_by(capture_clause);
             vis.visit_fn_decl(fn_decl);
             vis.visit_expr(body);
             vis.visit_span(fn_decl_span);
@@ -1562,6 +1567,15 @@ pub fn noop_visit_vis<T: MutVisitor>(visibility: &mut Visibility, vis: &mut T) {
     vis.visit_span(&mut visibility.span);
 }
 
+pub fn noop_visit_capture_by<T: MutVisitor>(capture_by: &mut CaptureBy, vis: &mut T) {
+    match capture_by {
+        CaptureBy::Ref => {}
+        CaptureBy::Value { move_kw } => {
+            vis.visit_span(move_kw);
+        }
+    }
+}
+
 /// Some value for the AST node that is valid but possibly meaningless.
 pub trait DummyAstNode {
     fn dummy() -> Self;

diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs
@@ -251,6 +251,9 @@ pub trait Visitor<'ast>: Sized {
     fn visit_inline_asm_sym(&mut self, sym: &'ast InlineAsmSym) {
         walk_inline_asm_sym(self, sym)
     }
+    fn visit_capture_by(&mut self, _capture_by: &'ast CaptureBy) {
+        // Nothing to do
+    }
 }
 
 #[macro_export]
@@ -857,7 +860,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
         }
         ExprKind::Closure(box Closure {
             binder,
-            capture_clause: _,
+            capture_clause,
             asyncness: _,
             constness: _,
             movability: _,
@@ -866,6 +869,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
             fn_decl_span: _,
             fn_arg_span: _,
         }) => {
+            visitor.visit_capture_by(capture_clause);
             visitor.visit_fn(FnKind::Closure(binder, fn_decl, body), expression.span, expression.id)
         }
         ExprKind::Block(block, opt_label) => {

diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
@@ -1201,7 +1201,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             }
 
             let async_expr = this.make_async_expr(
-                CaptureBy::Value,
+                CaptureBy::Value { move_kw: rustc_span::DUMMY_SP },
                 closure_id,
                 None,
                 body.span,

diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
@@ -673,7 +673,7 @@ impl<'a> State<'a> {
 
     fn print_capture_clause(&mut self, capture_clause: ast::CaptureBy) {
         match capture_clause {
-            ast::CaptureBy::Value => self.word_space("move"),
+            ast::CaptureBy::Value { .. } => self.word_space("move"),
             ast::CaptureBy::Ref => {}
         }
     }

diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs
@@ -2017,7 +2017,7 @@ impl<'a> State<'a> {
 
     fn print_capture_clause(&mut self, capture_clause: hir::CaptureBy) {
         match capture_clause {
-            hir::CaptureBy::Value => self.word_space("move"),
+            hir::CaptureBy::Value { .. } => self.word_space("move"),
             hir::CaptureBy::Ref => {}
         }
     }

diff --git a/compiler/rustc_hir_typeck/src/upvar.rs b/compiler/rustc_hir_typeck/src/upvar.rs
@@ -424,7 +424,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 origin = updated.1;
 
                 let (place, capture_kind) = match capture_clause {
-                    hir::CaptureBy::Value => adjust_for_move_closure(place, capture_kind),
+                    hir::CaptureBy::Value { .. } => adjust_for_move_closure(place, capture_kind),
                     hir::CaptureBy::Ref => adjust_for_non_move_closure(place, capture_kind),
                 };
 
@@ -958,7 +958,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let ty = self.resolve_vars_if_possible(self.node_ty(var_hir_id));
 
         let ty = match closure_clause {
-            hir::CaptureBy::Value => ty, // For move closure the capture kind should be by value
+            hir::CaptureBy::Value { .. } => ty, // For move closure the capture kind should be by value
             hir::CaptureBy::Ref => {
                 // For non move closure the capture kind is the max capture kind of all captures
                 // according to the ordering ImmBorrow < UniqueImmBorrow < MutBorrow < ByValue
@@ -1073,7 +1073,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
             match closure_clause {
                 // Only migrate if closure is a move closure
-                hir::CaptureBy::Value => {
+                hir::CaptureBy::Value { .. } => {
                     let mut diagnostics_info = FxIndexSet::default();
                     let upvars =
                         self.tcx.upvars_mentioned(closure_def_id).expect("must be an upvar");
@@ -1479,10 +1479,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // If the data will be moved out of this place, then the place will be truncated
             // at the first Deref in `adjust_upvar_borrow_kind_for_consume` and then moved into
             // the closure.
-            hir::CaptureBy::Value if !place.deref_tys().any(Ty::is_ref) => {
+            hir::CaptureBy::Value { .. } if !place.deref_tys().any(Ty::is_ref) => {
                 ty::UpvarCapture::ByValue
             }
-            hir::CaptureBy::Value | hir::CaptureBy::Ref => ty::UpvarCapture::ByRef(ty::ImmBorrow),
+            hir::CaptureBy::Value { .. } | hir::CaptureBy::Ref => {
+                ty::UpvarCapture::ByRef(ty::ImmBorrow)
+            }
         }
     }
 

diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
@@ -2303,13 +2303,14 @@ impl<'a> Parser<'a> {
     /// Parses an optional `move` prefix to a closure-like construct.
     fn parse_capture_clause(&mut self) -> PResult<'a, CaptureBy> {
         if self.eat_keyword(kw::Move) {
+            let move_kw_span = self.prev_token.span;
             // Check for `move async` and recover
             if self.check_keyword(kw::Async) {
                 let move_async_span = self.token.span.with_lo(self.prev_token.span.data().lo);
                 Err(errors::AsyncMoveOrderIncorrect { span: move_async_span }
                     .into_diagnostic(&self.sess.span_diagnostic))
             } else {
-                Ok(CaptureBy::Value)
+                Ok(CaptureBy::Value { move_kw: move_kw_span })
             }
         } else {
             Ok(CaptureBy::Ref)

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -2938,7 +2938,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                 else {
                     bug!("expected closure in SizedClosureCapture obligation");
                 };
-                if let hir::CaptureBy::Value = closure.capture_clause
+                if let hir::CaptureBy::Value { .. } = closure.capture_clause
                     && let Some(span) = closure.fn_arg_span
                 {
                     err.span_label(span, "this closure captures all values by move");

diff --git a/src/tools/clippy/clippy_lints/src/utils/author.rs b/src/tools/clippy/clippy_lints/src/utils/author.rs
@@ -7,7 +7,7 @@ use rustc_ast::LitIntType;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_hir::{
-    ArrayLen, BindingAnnotation, Closure, ExprKind, FnRetTy, HirId, Lit, PatKind, QPath, StmtKind, TyKind,
+    ArrayLen, BindingAnnotation, Closure, ExprKind, FnRetTy, HirId, Lit, PatKind, QPath, StmtKind, TyKind, CaptureBy
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::declare_lint_pass;
@@ -479,6 +479,11 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
                 movability,
                 ..
             }) => {
+                let capture_clause = match capture_clause {
+                    CaptureBy::Value { .. } => "Value { .. }",
+                    CaptureBy::Ref => "Ref",
+                };
+
                 let movability = OptionPat::new(movability.map(|m| format!("Movability::{m:?}")));
 
                 let ret_ty = match fn_decl.output {
@@ -487,7 +492,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
                 };
 
                 bind!(self, fn_decl, body_id);
-                kind!("Closure(CaptureBy::{capture_clause:?}, {fn_decl}, {body_id}, _, {movability})");
+                kind!("Closure(CaptureBy::{capture_clause}, {fn_decl}, {body_id}, _, {movability})");
                 chain!(self, "let {ret_ty} = {fn_decl}.output");
                 self.body(body_id);
             },

diff --git a/src/tools/clippy/tests/ui/author/blocks.stdout b/src/tools/clippy/tests/ui/author/blocks.stdout
@@ -40,10 +40,10 @@ if let ExprKind::Block(block, None) = expr.kind
 {
     // report your lint here
 }
-if let ExprKind::Closure(CaptureBy::Value, fn_decl, body_id, _, None) = expr.kind
+if let ExprKind::Closure(CaptureBy::Value { .. }, fn_decl, body_id, _, None) = expr.kind
     && let FnRetTy::DefaultReturn(_) = fn_decl.output
     && expr1 = &cx.tcx.hir().body(body_id).value
-    && let ExprKind::Closure(CaptureBy::Value, fn_decl1, body_id1, _, Some(Movability::Static)) = expr1.kind
+    && let ExprKind::Closure(CaptureBy::Value { .. }, fn_decl1, body_id1, _, Some(Movability::Static)) = expr1.kind
     && let FnRetTy::DefaultReturn(_) = fn_decl1.output
     && expr2 = &cx.tcx.hir().body(body_id1).value
     && let ExprKind::Block(block, None) = expr2.kind

diff --git a/src/tools/rustfmt/src/closures.rs b/src/tools/rustfmt/src/closures.rs
@@ -264,7 +264,7 @@ fn rewrite_closure_fn_decl(
         ""
     };
     let is_async = if asyncness.is_async() { "async " } else { "" };
-    let mover = if capture == ast::CaptureBy::Value {
+    let mover = if matches!(capture, ast::CaptureBy::Value { .. }) {
         "move "
     } else {
         ""

diff --git a/src/tools/rustfmt/src/expr.rs b/src/tools/rustfmt/src/expr.rs
@@ -368,7 +368,7 @@ pub(crate) fn format_expr(
             }
         }
         ast::ExprKind::Gen(capture_by, ref block, ref kind) => {
-            let mover = if capture_by == ast::CaptureBy::Value {
+            let mover = if matches!(capture_by, ast::CaptureBy::Value { .. }) {
                 "move "
             } else {
                 ""

diff --git a/tests/ui-fulldeps/pprust-expr-roundtrip.rs b/tests/ui-fulldeps/pprust-expr-roundtrip.rs
@@ -130,7 +130,7 @@ fn iter_exprs(depth: usize, f: &mut dyn FnMut(P<Expr>)) {
                 iter_exprs(depth - 1, &mut |e| {
                     g(ExprKind::Closure(Box::new(Closure {
                         binder: ClosureBinder::NotPresent,
-                        capture_clause: CaptureBy::Value,
+                        capture_clause: CaptureBy::Value { move_kw: DUMMY_SP },
                         constness: Const::No,
                         asyncness: Async::No,
                         movability: Movability::Movable,