Skip to content

Commit

Permalink
Rollup merge of #134244 - Enselic:no-mut-hint-for-raw-ref, r=jieyouxu
Browse files Browse the repository at this point in the history
rustc_borrowck: Stop suggesting the invalid syntax `&mut raw const`

A legitimate suggestion would be to change from

    &raw const val

to

    &raw mut val

But until we have figured out how to make that happen we should at least
stop suggesting invalid syntax.

I recommend review commit-by-commit.

Part of #127562
  • Loading branch information
matthiaskrgr authored Dec 14, 2024
2 parents a53a3cc + f6cb227 commit 34e6075
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 16 deletions.
69 changes: 53 additions & 16 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1100,12 +1100,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}
let decl_span = local_decl.source_info.span;

let label = match *local_decl.local_info() {
let amp_mut_sugg = match *local_decl.local_info() {
LocalInfo::User(mir::BindingForm::ImplicitSelf(_)) => {
let suggestion = suggest_ampmut_self(self.infcx.tcx, decl_span);
let additional =
local_trait.map(|span| (span, suggest_ampmut_self(self.infcx.tcx, span)));
Some((true, decl_span, suggestion, additional))
Some(AmpMutSugg { has_sugg: true, span: decl_span, suggestion, additional })
}

LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm {
Expand Down Expand Up @@ -1150,7 +1150,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
None
}
None => {
let (has_sugg, decl_span, sugg) = if name != kw::SelfLower {
if name != kw::SelfLower {
suggest_ampmut(
self.infcx.tcx,
local_decl.ty,
Expand All @@ -1165,7 +1165,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
..
})) => {
let sugg = suggest_ampmut_self(self.infcx.tcx, decl_span);
(true, decl_span, sugg)
Some(AmpMutSugg {
has_sugg: true,
span: decl_span,
suggestion: sugg,
additional: None,
})
}
// explicit self (eg `self: &'a Self`)
_ => suggest_ampmut(
Expand All @@ -1176,8 +1181,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
opt_ty_info,
),
}
};
Some((has_sugg, decl_span, sugg, None))
}
}
}
}
Expand All @@ -1187,15 +1191,24 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
..
})) => {
let pattern_span: Span = local_decl.source_info.span;
suggest_ref_mut(self.infcx.tcx, pattern_span)
.map(|span| (true, span, "mut ".to_owned(), None))
suggest_ref_mut(self.infcx.tcx, pattern_span).map(|span| AmpMutSugg {
has_sugg: true,
span,
suggestion: "mut ".to_owned(),
additional: None,
})
}

_ => unreachable!(),
};

match label {
Some((true, err_help_span, suggested_code, additional)) => {
match amp_mut_sugg {
Some(AmpMutSugg {
has_sugg: true,
span: err_help_span,
suggestion: suggested_code,
additional,
}) => {
let mut sugg = vec![(err_help_span, suggested_code)];
if let Some(s) = additional {
sugg.push(s);
Expand All @@ -1217,7 +1230,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
);
}
}
Some((false, err_label_span, message, _)) => {
Some(AmpMutSugg {
has_sugg: false, span: err_label_span, suggestion: message, ..
}) => {
let def_id = self.body.source.def_id();
let hir_id = if let Some(local_def_id) = def_id.as_local()
&& let Some(body) = self.infcx.tcx.hir().maybe_body_owned_by(local_def_id)
Expand Down Expand Up @@ -1422,6 +1437,13 @@ fn suggest_ampmut_self<'tcx>(tcx: TyCtxt<'tcx>, span: Span) -> String {
}
}

struct AmpMutSugg {
has_sugg: bool,
span: Span,
suggestion: String,
additional: Option<(Span, String)>,
}

// When we want to suggest a user change a local variable to be a `&mut`, there
// are three potential "obvious" things to highlight:
//
Expand All @@ -1443,7 +1465,7 @@ fn suggest_ampmut<'tcx>(
decl_span: Span,
opt_assignment_rhs_span: Option<Span>,
opt_ty_info: Option<Span>,
) -> (bool, Span, String) {
) -> Option<AmpMutSugg> {
// 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
Expand All @@ -1456,6 +1478,11 @@ fn suggest_ampmut<'tcx>(
&& let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span)
&& let Some(stripped) = src.strip_prefix('&')
{
let is_raw_ref = stripped.trim_start().starts_with("raw ");
// We don't support raw refs yet
if is_raw_ref {
return None;
}
let is_mut = if let Some(rest) = stripped.trim_start().strip_prefix("mut") {
match rest.chars().next() {
// e.g. `&mut x`
Expand All @@ -1479,7 +1506,12 @@ fn suggest_ampmut<'tcx>(

// FIXME(Ezrashaw): returning is bad because we still might want to
// update the annotated type, see #106857.
return (true, span, "mut ".to_owned());
return Some(AmpMutSugg {
has_sugg: true,
span,
suggestion: "mut ".to_owned(),
additional: None,
});
}
}

Expand All @@ -1504,18 +1536,23 @@ fn suggest_ampmut<'tcx>(
&& let Some(ws_pos) = src.find(char::is_whitespace)
{
let span = span.with_lo(span.lo() + BytePos(ws_pos as u32)).shrink_to_lo();
(true, span, " mut".to_owned())
Some(AmpMutSugg { has_sugg: true, span, suggestion: " mut".to_owned(), additional: None })
// 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())
Some(AmpMutSugg { has_sugg: true, span, suggestion: "mut ".to_owned(), additional: None })
} else {
// otherwise, suggest that the user annotates the binding; we provide the
// type of the local.
let ty = decl_ty.builtin_deref(true).unwrap();

(false, span, format!("{}mut {}", if decl_ty.is_ref() { "&" } else { "*" }, ty))
Some(AmpMutSugg {
has_sugg: false,
span,
suggestion: format!("{}mut {}", if decl_ty.is_ref() { "&" } else { "*" }, ty),
additional: None,
})
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//! Regression test for invalid suggestion for `&raw const expr` reported in
//! <https://github.com/rust-lang/rust/issues/127562>.
fn main() {
let val = 2;
let ptr = &raw const val;
unsafe { *ptr = 3; } //~ ERROR cannot assign to `*ptr`, which is behind a `*const` pointer
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0594]: cannot assign to `*ptr`, which is behind a `*const` pointer
--> $DIR/no-invalid-mut-suggestion-for-raw-pointer-issue-127562.rs:7:14
|
LL | unsafe { *ptr = 3; }
| ^^^^^^^^ `ptr` is a `*const` pointer, so the data it refers to cannot be written

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0594`.

0 comments on commit 34e6075

Please sign in to comment.