|
1 | 1 | #![allow(rustc::diagnostic_outside_of_impl)] |
2 | 2 | #![allow(rustc::untranslatable_diagnostic)] |
3 | 3 |
|
| 4 | +use rustc_data_structures::fx::FxHashSet; |
4 | 5 | use rustc_errors::{Applicability, Diag}; |
5 | 6 | use rustc_hir::intravisit::Visitor; |
6 | | -use rustc_hir::{CaptureBy, ExprKind, HirId, Node}; |
| 7 | +use rustc_hir::{self as hir, CaptureBy, ExprKind, HirId, Node}; |
7 | 8 | use rustc_middle::bug; |
8 | 9 | use rustc_middle::mir::*; |
9 | 10 | use rustc_middle::ty::{self, Ty}; |
@@ -683,48 +684,126 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { |
683 | 684 | } |
684 | 685 |
|
685 | 686 | fn add_move_error_suggestions(&self, err: &mut Diag<'_>, binds_to: &[Local]) { |
686 | | - let mut suggestions: Vec<(Span, String, String)> = Vec::new(); |
| 687 | + /// A HIR visitor to associate each binding with a `&` or `&mut` that could be removed to |
| 688 | + /// make it bind by reference instead (if possible) |
| 689 | + struct BindingFinder<'tcx> { |
| 690 | + typeck_results: &'tcx ty::TypeckResults<'tcx>, |
| 691 | + hir: rustc_middle::hir::map::Map<'tcx>, |
| 692 | + /// Input: the span of the pattern we're finding bindings in |
| 693 | + pat_span: Span, |
| 694 | + /// Input: the spans of the bindings we're providing suggestions for |
| 695 | + binding_spans: Vec<Span>, |
| 696 | + /// Internal state: have we reached the pattern we're finding bindings in? |
| 697 | + found_pat: bool, |
| 698 | + /// Internal state: the innermost `&` or `&mut` "above" the visitor |
| 699 | + ref_pat: Option<&'tcx hir::Pat<'tcx>>, |
| 700 | + /// Internal state: could removing a `&` give bindings unexpected types? |
| 701 | + has_adjustments: bool, |
| 702 | + /// Output: for each input binding, the `&` or `&mut` to remove to make it by-ref |
| 703 | + ref_pat_for_binding: Vec<(Span, Option<&'tcx hir::Pat<'tcx>>)>, |
| 704 | + /// Output: ref patterns that can't be removed straightforwardly |
| 705 | + cannot_remove: FxHashSet<HirId>, |
| 706 | + } |
| 707 | + impl<'tcx> Visitor<'tcx> for BindingFinder<'tcx> { |
| 708 | + type NestedFilter = rustc_middle::hir::nested_filter::OnlyBodies; |
| 709 | + |
| 710 | + fn nested_visit_map(&mut self) -> Self::Map { |
| 711 | + self.hir |
| 712 | + } |
| 713 | + |
| 714 | + fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) -> Self::Result { |
| 715 | + // Don't walk into const patterns or anything else that might confuse this |
| 716 | + if !self.found_pat { |
| 717 | + hir::intravisit::walk_expr(self, ex) |
| 718 | + } |
| 719 | + } |
| 720 | + |
| 721 | + fn visit_pat(&mut self, p: &'tcx hir::Pat<'tcx>) { |
| 722 | + if p.span == self.pat_span { |
| 723 | + self.found_pat = true; |
| 724 | + } |
| 725 | + |
| 726 | + let parent_has_adjustments = self.has_adjustments; |
| 727 | + self.has_adjustments |= |
| 728 | + self.typeck_results.pat_adjustments().contains_key(p.hir_id); |
| 729 | + |
| 730 | + // Track the innermost `&` or `&mut` enclosing bindings, to suggest removing it. |
| 731 | + let parent_ref_pat = self.ref_pat; |
| 732 | + if let hir::PatKind::Ref(..) = p.kind { |
| 733 | + self.ref_pat = Some(p); |
| 734 | + // To avoid edition-dependent logic to figure out how many refs this `&` can |
| 735 | + // peel off, simply don't remove the "parent" `&`. |
| 736 | + self.cannot_remove.extend(parent_ref_pat.map(|r| r.hir_id)); |
| 737 | + if self.has_adjustments { |
| 738 | + // Removing this `&` could give child bindings unexpected types, so don't. |
| 739 | + self.cannot_remove.insert(p.hir_id); |
| 740 | + // As long the `&` stays, child patterns' types should be as expected. |
| 741 | + self.has_adjustments = false; |
| 742 | + } |
| 743 | + } |
| 744 | + |
| 745 | + if let hir::PatKind::Binding(_, _, ident, _) = p.kind { |
| 746 | + // the spans in `binding_spans` encompass both the ident and binding mode |
| 747 | + if let Some(&bind_sp) = |
| 748 | + self.binding_spans.iter().find(|bind_sp| bind_sp.contains(ident.span)) |
| 749 | + { |
| 750 | + self.ref_pat_for_binding.push((bind_sp, self.ref_pat)); |
| 751 | + } else { |
| 752 | + // we've encountered a binding that we're not reporting a move error for. |
| 753 | + // we don't want to change its type, so don't remove the surrounding `&`. |
| 754 | + if let Some(ref_pat) = self.ref_pat { |
| 755 | + self.cannot_remove.insert(ref_pat.hir_id); |
| 756 | + } |
| 757 | + } |
| 758 | + } |
| 759 | + |
| 760 | + hir::intravisit::walk_pat(self, p); |
| 761 | + self.ref_pat = parent_ref_pat; |
| 762 | + self.has_adjustments = parent_has_adjustments; |
| 763 | + } |
| 764 | + } |
| 765 | + let mut pat_span = None; |
| 766 | + let mut binding_spans = Vec::new(); |
687 | 767 | for local in binds_to { |
688 | 768 | let bind_to = &self.body.local_decls[*local]; |
689 | | - if let LocalInfo::User(BindingForm::Var(VarBindingForm { pat_span, .. })) = |
| 769 | + if let LocalInfo::User(BindingForm::Var(VarBindingForm { pat_span: pat_sp, .. })) = |
690 | 770 | *bind_to.local_info() |
691 | 771 | { |
692 | | - let Ok(pat_snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(pat_span) |
693 | | - else { |
694 | | - continue; |
695 | | - }; |
696 | | - let Some(stripped) = pat_snippet.strip_prefix('&') else { |
697 | | - suggestions.push(( |
698 | | - bind_to.source_info.span.shrink_to_lo(), |
699 | | - "consider borrowing the pattern binding".to_string(), |
700 | | - "ref ".to_string(), |
701 | | - )); |
702 | | - continue; |
703 | | - }; |
704 | | - let inner_pat_snippet = stripped.trim_start(); |
705 | | - let (pat_span, suggestion, to_remove) = if inner_pat_snippet.starts_with("mut") |
706 | | - && inner_pat_snippet["mut".len()..].starts_with(rustc_lexer::is_whitespace) |
707 | | - { |
708 | | - let inner_pat_snippet = inner_pat_snippet["mut".len()..].trim_start(); |
709 | | - let pat_span = pat_span.with_hi( |
710 | | - pat_span.lo() |
711 | | - + BytePos((pat_snippet.len() - inner_pat_snippet.len()) as u32), |
712 | | - ); |
713 | | - (pat_span, String::new(), "mutable borrow") |
714 | | - } else { |
715 | | - let pat_span = pat_span.with_hi( |
716 | | - pat_span.lo() |
717 | | - + BytePos( |
718 | | - (pat_snippet.len() - inner_pat_snippet.trim_start().len()) as u32, |
719 | | - ), |
720 | | - ); |
721 | | - (pat_span, String::new(), "borrow") |
722 | | - }; |
723 | | - suggestions.push(( |
724 | | - pat_span, |
725 | | - format!("consider removing the {to_remove}"), |
726 | | - suggestion, |
727 | | - )); |
| 772 | + pat_span = Some(pat_sp); |
| 773 | + binding_spans.push(bind_to.source_info.span); |
| 774 | + } |
| 775 | + } |
| 776 | + let Some(pat_span) = pat_span else { return }; |
| 777 | + |
| 778 | + let hir = self.infcx.tcx.hir(); |
| 779 | + let Some(body) = hir.maybe_body_owned_by(self.mir_def_id()) else { return }; |
| 780 | + let typeck_results = self.infcx.tcx.typeck(self.mir_def_id()); |
| 781 | + let mut finder = BindingFinder { |
| 782 | + typeck_results, |
| 783 | + hir, |
| 784 | + pat_span, |
| 785 | + binding_spans, |
| 786 | + found_pat: false, |
| 787 | + ref_pat: None, |
| 788 | + has_adjustments: false, |
| 789 | + ref_pat_for_binding: Vec::new(), |
| 790 | + cannot_remove: FxHashSet::default(), |
| 791 | + }; |
| 792 | + finder.visit_body(body); |
| 793 | + |
| 794 | + let mut suggestions = Vec::new(); |
| 795 | + for (binding_span, opt_ref_pat) in finder.ref_pat_for_binding { |
| 796 | + if let Some(ref_pat) = opt_ref_pat |
| 797 | + && !finder.cannot_remove.contains(&ref_pat.hir_id) |
| 798 | + && let hir::PatKind::Ref(subpat, mutbl) = ref_pat.kind |
| 799 | + && let Some(ref_span) = ref_pat.span.trim_end(subpat.span) |
| 800 | + { |
| 801 | + let mutable_str = if mutbl.is_mut() { "mutable " } else { "" }; |
| 802 | + let msg = format!("consider removing the {mutable_str}borrow"); |
| 803 | + suggestions.push((ref_span, msg, "".to_string())); |
| 804 | + } else { |
| 805 | + let msg = "consider borrowing the pattern binding".to_string(); |
| 806 | + suggestions.push((binding_span.shrink_to_lo(), msg, "ref ".to_string())); |
728 | 807 | } |
729 | 808 | } |
730 | 809 | suggestions.sort_unstable_by_key(|&(span, _, _)| span); |
|
0 commit comments