1
Fork 0

Rollup merge of #110583 - Ezrashaw:tweak-make-mut-spans, r=estebank

tweak "make mut" spans when assigning to locals

Work towards fixing #106857

This PR just cleans up a lot of spans which is helpful before properly fixing the issues. Best reviewed commit-by-commit.

r? `@estebank`
This commit is contained in:
Dylan DPC 2023-05-09 12:33:45 +05:30 committed by GitHub
commit ff30b8cb7b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
33 changed files with 338 additions and 323 deletions

View file

@ -1,4 +1,4 @@
use rustc_errors::{Applicability, Diagnostic};
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_hir::intravisit::Visitor;
use rustc_hir::Node;
@ -478,179 +478,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
match self.local_names[local] {
Some(name) if !local_decl.from_compiler_desugaring() => {
let label = match *local_decl.local_info() {
LocalInfo::User(mir::BindingForm::ImplicitSelf(_)) => {
let (span, suggestion) =
suggest_ampmut_self(self.infcx.tcx, local_decl);
Some((true, span, suggestion))
}
LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByValue(_),
opt_ty_info,
..
})) => {
// check if the RHS is from desugaring
let opt_assignment_rhs_span =
self.body.find_assignments(local).first().map(|&location| {
if let Some(mir::Statement {
source_info: _,
kind:
mir::StatementKind::Assign(box (
_,
mir::Rvalue::Use(mir::Operand::Copy(place)),
)),
}) = self.body[location.block]
.statements
.get(location.statement_index)
{
self.body.local_decls[place.local].source_info.span
} else {
self.body.source_info(location).span
}
});
match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) {
// on for loops, RHS points to the iterator part
Some(DesugaringKind::ForLoop) => {
self.suggest_similar_mut_method_for_for_loop(&mut err);
err.span_label(opt_assignment_rhs_span.unwrap(), format!(
"this iterator yields `{pointer_sigil}` {pointer_desc}s",
));
None
}
// don't create labels for compiler-generated spans
Some(_) => None,
None => {
let label = if name != kw::SelfLower {
suggest_ampmut(
self.infcx.tcx,
local_decl,
opt_assignment_rhs_span,
opt_ty_info,
)
} else {
match local_decl.local_info() {
LocalInfo::User(mir::BindingForm::Var(
mir::VarBindingForm {
opt_ty_info: None, ..
},
)) => {
let (span, sugg) = suggest_ampmut_self(
self.infcx.tcx,
local_decl,
);
(true, span, sugg)
}
// explicit self (eg `self: &'a Self`)
_ => suggest_ampmut(
self.infcx.tcx,
local_decl,
opt_assignment_rhs_span,
opt_ty_info,
),
}
};
Some(label)
}
}
}
LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByReference(_),
..
})) => {
let pattern_span = local_decl.source_info.span;
suggest_ref_mut(self.infcx.tcx, pattern_span)
.map(|replacement| (true, pattern_span, replacement))
}
_ => unreachable!(),
};
match label {
Some((true, err_help_span, suggested_code)) => {
let (is_trait_sig, local_trait) = self.is_error_in_trait(local);
if !is_trait_sig {
err.span_suggestion_verbose(
err_help_span,
format!(
"consider changing this to be a mutable {pointer_desc}"
),
suggested_code,
Applicability::MachineApplicable,
);
} else if let Some(x) = local_trait {
err.span_suggestion_verbose(
x,
format!(
"consider changing that to be a mutable {pointer_desc}"
),
suggested_code,
Applicability::MachineApplicable,
);
}
}
Some((false, err_label_span, message)) => {
struct BindingFinder {
span: Span,
hir_id: Option<hir::HirId>,
}
impl<'tcx> Visitor<'tcx> for BindingFinder {
fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) {
if let hir::StmtKind::Local(local) = s.kind {
if local.pat.span == self.span {
self.hir_id = Some(local.hir_id);
}
}
hir::intravisit::walk_stmt(self, s);
}
}
let hir_map = self.infcx.tcx.hir();
let def_id = self.body.source.def_id();
let hir_id = hir_map.local_def_id_to_hir_id(def_id.expect_local());
let node = hir_map.find(hir_id);
let hir_id = if let Some(hir::Node::Item(item)) = node
&& let hir::ItemKind::Fn(.., body_id) = item.kind
{
let body = hir_map.body(body_id);
let mut v = BindingFinder {
span: err_label_span,
hir_id: None,
};
v.visit_body(body);
v.hir_id
} else {
None
};
if let Some(hir_id) = hir_id
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
{
let (changing, span, sugg) = match local.ty {
Some(ty) => ("changing", ty.span, message),
None => (
"specifying",
local.pat.span.shrink_to_hi(),
format!(": {message}"),
),
};
err.span_suggestion_verbose(
span,
format!("consider {changing} this binding's type"),
sugg,
Applicability::HasPlaceholders,
);
} else {
err.span_label(
err_label_span,
format!(
"consider changing this binding's type to be: `{message}`"
),
);
}
}
None => {}
}
err.span_label(
span,
format!(
@ -658,6 +485,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
so the data it refers to cannot be {acted_on}",
),
);
self.suggest_make_local_mut(&mut err, local, name);
}
_ => {
err.span_label(
@ -1131,6 +960,184 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
}
}
fn suggest_make_local_mut(
&self,
err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>,
local: Local,
name: Symbol,
) {
let local_decl = &self.body.local_decls[local];
let (pointer_sigil, pointer_desc) =
if local_decl.ty.is_ref() { ("&", "reference") } else { ("*const", "pointer") };
let (is_trait_sig, local_trait) = self.is_error_in_trait(local);
if is_trait_sig && local_trait.is_none() {
return;
}
let decl_span = match local_trait {
Some(span) => span,
None => local_decl.source_info.span,
};
let label = match *local_decl.local_info() {
LocalInfo::User(mir::BindingForm::ImplicitSelf(_)) => {
let suggestion = suggest_ampmut_self(self.infcx.tcx, decl_span);
Some((true, decl_span, suggestion))
}
LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByValue(_),
opt_ty_info,
..
})) => {
// check if the RHS is from desugaring
let opt_assignment_rhs_span =
self.body.find_assignments(local).first().map(|&location| {
if let Some(mir::Statement {
source_info: _,
kind:
mir::StatementKind::Assign(box (
_,
mir::Rvalue::Use(mir::Operand::Copy(place)),
)),
}) = self.body[location.block].statements.get(location.statement_index)
{
self.body.local_decls[place.local].source_info.span
} else {
self.body.source_info(location).span
}
});
match opt_assignment_rhs_span.and_then(|s| s.desugaring_kind()) {
// on for loops, RHS points to the iterator part
Some(DesugaringKind::ForLoop) => {
self.suggest_similar_mut_method_for_for_loop(err);
err.span_label(
opt_assignment_rhs_span.unwrap(),
format!("this iterator yields `{pointer_sigil}` {pointer_desc}s",),
);
None
}
// don't create labels for compiler-generated spans
Some(_) => None,
None => {
let label = if name != kw::SelfLower {
suggest_ampmut(
self.infcx.tcx,
local_decl.ty,
decl_span,
opt_assignment_rhs_span,
opt_ty_info,
)
} else {
match local_decl.local_info() {
LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm {
opt_ty_info: None,
..
})) => {
let sugg = suggest_ampmut_self(self.infcx.tcx, decl_span);
(true, decl_span, sugg)
}
// explicit self (eg `self: &'a Self`)
_ => suggest_ampmut(
self.infcx.tcx,
local_decl.ty,
decl_span,
opt_assignment_rhs_span,
opt_ty_info,
),
}
};
Some(label)
}
}
}
LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByReference(_),
..
})) => {
let pattern_span: Span = local_decl.source_info.span;
suggest_ref_mut(self.infcx.tcx, pattern_span)
.map(|span| (true, span, "mut ".to_owned()))
}
_ => unreachable!(),
};
match label {
Some((true, err_help_span, suggested_code)) => {
err.span_suggestion_verbose(
err_help_span,
format!("consider changing this to be a mutable {pointer_desc}"),
suggested_code,
Applicability::MachineApplicable,
);
}
Some((false, err_label_span, message)) => {
struct BindingFinder {
span: Span,
hir_id: Option<hir::HirId>,
}
impl<'tcx> Visitor<'tcx> for BindingFinder {
fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) {
if let hir::StmtKind::Local(local) = s.kind {
if local.pat.span == self.span {
self.hir_id = Some(local.hir_id);
}
}
hir::intravisit::walk_stmt(self, s);
}
}
let hir_map = self.infcx.tcx.hir();
let def_id = self.body.source.def_id();
let hir_id = hir_map.local_def_id_to_hir_id(def_id.expect_local());
let node = hir_map.find(hir_id);
let hir_id = if let Some(hir::Node::Item(item)) = node
&& let hir::ItemKind::Fn(.., body_id) = item.kind
{
let body = hir_map.body(body_id);
let mut v = BindingFinder {
span: err_label_span,
hir_id: None,
};
v.visit_body(body);
v.hir_id
} else {
None
};
if let Some(hir_id) = hir_id
&& let Some(hir::Node::Local(local)) = hir_map.find(hir_id)
{
let (changing, span, sugg) = match local.ty {
Some(ty) => ("changing", ty.span, message),
None => (
"specifying",
local.pat.span.shrink_to_hi(),
format!(": {message}"),
),
};
err.span_suggestion_verbose(
span,
format!("consider {changing} this binding's type"),
sugg,
Applicability::HasPlaceholders,
);
} else {
err.span_label(
err_label_span,
format!(
"consider changing this binding's type to be: `{message}`"
),
);
}
}
None => {}
}
}
}
pub fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option<Symbol>) -> bool {
@ -1160,25 +1167,18 @@ pub fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option<
}
}
fn suggest_ampmut_self<'tcx>(
tcx: TyCtxt<'tcx>,
local_decl: &mir::LocalDecl<'tcx>,
) -> (Span, String) {
let sp = local_decl.source_info.span;
(
sp,
match tcx.sess.source_map().span_to_snippet(sp) {
Ok(snippet) => {
let lt_pos = snippet.find('\'');
if let Some(lt_pos) = lt_pos {
format!("&{}mut self", &snippet[lt_pos..snippet.len() - 4])
} else {
"&mut self".to_string()
}
fn suggest_ampmut_self<'tcx>(tcx: TyCtxt<'tcx>, span: Span) -> String {
match tcx.sess.source_map().span_to_snippet(span) {
Ok(snippet) => {
let lt_pos = snippet.find('\'');
if let Some(lt_pos) = lt_pos {
format!("&{}mut self", &snippet[lt_pos..snippet.len() - 4])
} else {
"&mut self".to_string()
}
_ => "&mut self".to_string(),
},
)
}
_ => "&mut self".to_string(),
}
}
// When we want to suggest a user change a local variable to be a `&mut`, there
@ -1198,72 +1198,89 @@ fn suggest_ampmut_self<'tcx>(
// by trying (3.), then (2.) and finally falling back on (1.).
fn suggest_ampmut<'tcx>(
tcx: TyCtxt<'tcx>,
local_decl: &mir::LocalDecl<'tcx>,
decl_ty: Ty<'tcx>,
decl_span: Span,
opt_assignment_rhs_span: Option<Span>,
opt_ty_info: Option<Span>,
) -> (bool, Span, String) {
// if there is a RHS and it starts with a `&` from it, then check if it is
// mutable, and if not, put suggest putting `mut ` to make it mutable.
// we don't have to worry about lifetime annotations here because they are
// not valid when taking a reference. For example, the following is not valid Rust:
//
// let x: &i32 = &'a 5;
// ^^ lifetime annotation not allowed
//
if let Some(assignment_rhs_span) = opt_assignment_rhs_span
&& let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span)
&& let Some(stripped) = src.strip_prefix('&')
{
let is_mutbl = |ty: &str| -> bool {
if let Some(rest) = ty.strip_prefix("mut") {
match rest.chars().next() {
// e.g. `&mut x`
Some(c) if c.is_whitespace() => true,
// e.g. `&mut(x)`
Some('(') => true,
// e.g. `&mut{x}`
Some('{') => true,
// e.g. `&mutablevar`
_ => false,
}
} else {
false
let is_mut = if let Some(rest) = stripped.trim_start().strip_prefix("mut") {
match rest.chars().next() {
// e.g. `&mut x`
Some(c) if c.is_whitespace() => true,
// e.g. `&mut(x)`
Some('(') => true,
// e.g. `&mut{x}`
Some('{') => true,
// e.g. `&mutablevar`
_ => false,
}
} else {
false
};
if let (true, Some(ws_pos)) = (src.starts_with("&'"), src.find(char::is_whitespace)) {
let lt_name = &src[1..ws_pos];
let ty = src[ws_pos..].trim_start();
if !is_mutbl(ty) {
return (true, assignment_rhs_span, format!("&{lt_name} mut {ty}"));
}
} else if let Some(stripped) = src.strip_prefix('&') {
let stripped = stripped.trim_start();
if !is_mutbl(stripped) {
return (true, assignment_rhs_span, format!("&mut {stripped}"));
}
// if the reference is already mutable then there is nothing we can do
// here.
if !is_mut {
let span = assignment_rhs_span;
// shrink the span to just after the `&` in `&variable`
let span = span.with_lo(span.lo() + BytePos(1)).shrink_to_lo();
// FIXME(Ezrashaw): returning is bad because we still might want to
// update the annotated type, see #106857.
return (true, span, "mut ".to_owned());
}
}
let (suggestibility, highlight_span) = match opt_ty_info {
let (binding_exists, span) = match opt_ty_info {
// if this is a variable binding with an explicit type,
// try to highlight that for the suggestion.
// then we will suggest changing it to be mutable.
// this is `Applicability::MachineApplicable`.
Some(ty_span) => (true, ty_span),
// otherwise, just highlight the span associated with
// the (MIR) LocalDecl.
None => (false, local_decl.source_info.span),
// otherwise, we'll suggest *adding* an annotated type, we'll suggest
// the RHS's type for that.
// this is `Applicability::HasPlaceholders`.
None => (false, decl_span),
};
if let Ok(src) = tcx.sess.source_map().span_to_snippet(highlight_span)
&& let (true, Some(ws_pos)) = (src.starts_with("&'"), src.find(char::is_whitespace))
// if the binding already exists and is a reference with a explicit
// lifetime, then we can suggest adding ` mut`. this is special-cased from
// the path without a explicit lifetime.
if let Ok(src) = tcx.sess.source_map().span_to_snippet(span)
&& src.starts_with("&'")
// note that `& 'a T` is invalid so this is correct.
&& let Some(ws_pos) = src.find(char::is_whitespace)
{
let lt_name = &src[1..ws_pos];
let ty = &src[ws_pos..];
return (true, highlight_span, format!("&{lt_name} mut{ty}"));
}
let span = span.with_lo(span.lo() + BytePos(ws_pos as u32)).shrink_to_lo();
(true, span, " mut".to_owned())
// if there is already a binding, we modify it to be `mut`
} else if binding_exists {
// shrink the span to just after the `&` in `&variable`
let span = span.with_lo(span.lo() + BytePos(1)).shrink_to_lo();
(true, span, "mut ".to_owned())
} else {
// otherwise, suggest that the user annotates the binding; we provide the
// type of the local.
let ty_mut = decl_ty.builtin_deref(true).unwrap();
assert_eq!(ty_mut.mutbl, hir::Mutability::Not);
let ty_mut = local_decl.ty.builtin_deref(true).unwrap();
assert_eq!(ty_mut.mutbl, hir::Mutability::Not);
(
suggestibility,
highlight_span,
if local_decl.ty.is_ref() {
format!("&mut {}", ty_mut.ty)
} else {
format!("*mut {}", ty_mut.ty)
},
)
(
false,
span,
format!("{}mut {}", if decl_ty.is_ref() {"&"} else {"*"}, ty_mut.ty)
)
}
}
fn is_closure_or_generator(ty: Ty<'_>) -> bool {
@ -1300,11 +1317,13 @@ fn get_mut_span_in_struct_field<'tcx>(
}
/// If possible, suggest replacing `ref` with `ref mut`.
fn suggest_ref_mut(tcx: TyCtxt<'_>, binding_span: Span) -> Option<String> {
let hi_src = tcx.sess.source_map().span_to_snippet(binding_span).ok()?;
if hi_src.starts_with("ref") && hi_src["ref".len()..].starts_with(rustc_lexer::is_whitespace) {
let replacement = format!("ref mut{}", &hi_src["ref".len()..]);
Some(replacement)
fn suggest_ref_mut(tcx: TyCtxt<'_>, span: Span) -> Option<Span> {
let pattern_str = tcx.sess.source_map().span_to_snippet(span).ok()?;
if pattern_str.starts_with("ref")
&& pattern_str["ref".len()..].starts_with(rustc_lexer::is_whitespace)
{
let span = span.with_lo(span.lo() + BytePos(4)).shrink_to_lo();
Some(span)
} else {
None
}