Skip to content

Commit 43d8935

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#71726 - ldm0:ref2ptr, r=oli-obk
Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability Fixes rust-lang#71676 1. Implement dereference suggestion when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability. 2. Extract the dereference steps into `deref_steps()`, which removes all the `use` and `pub` noise introduced by last PR rust-lang#71540, and makes the code more readable. 3. Use the `remove_prefix()` closure which makes the prefix removal more readable. 4. Introduce `Applicability` as a return value of `check_ref` to suggest `Applicability::Unspecified` suggestion. **Special**: I found it is not possible to genereate `Applicability::MachineApplicable` suggestion for situation like this: ```rust use std::ops::Deref; use std::ops::DerefMut; struct Bar(u8); struct Foo(Bar); struct Emm(Foo); impl Deref for Bar{ type Target = u8; fn deref(&self) -> &Self::Target { &self.0 } } impl Deref for Foo { type Target = Bar; fn deref(&self) -> &Self::Target { &self.0 } } impl Deref for Emm { type Target = Foo; fn deref(&self) -> &Self::Target { &self.0 } } impl DerefMut for Bar{ fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } impl DerefMut for Foo { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } impl DerefMut for Emm { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } } fn main() { let a = Emm(Foo(Bar(0))); let _: *mut u8 = &a; //~ ERROR mismatched types } ``` We may suggest `&mut ***a` here, but the `a` is not declared as mutable variable. And also when processing HIR, it's not possible to check if `a` is declared as a mutable variable (currently we do borrow checking with MIR). So we cannot ensure that suggestion when coercing immutable reference to mutable pointer is always machine applicable. Therefore I added a `Applicability` return value in `check_ref()`. And move the `immutable reference -> mutable pointer` situation into a sperate test file without `run-rustfix`. (It seems that `run-rustfix` will also adopt `Applicability::Unspecified` suggestion, which is strange)
2 parents 18e8b9d + 9a212c1 commit 43d8935

10 files changed

+334
-47
lines changed

‎src/librustc_typeck/check/coercion.rs

+15-3
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode};
7474
use smallvec::{smallvec, SmallVec};
7575
use std::ops::Deref;
7676

77-
pub struct Coerce<'a, 'tcx> {
77+
struct Coerce<'a, 'tcx> {
7878
fcx: &'a FnCtxt<'a, 'tcx>,
7979
cause: ObligationCause<'tcx>,
8080
use_lub: bool,
@@ -126,15 +126,15 @@ fn success<'tcx>(
126126
}
127127

128128
impl<'f, 'tcx> Coerce<'f, 'tcx> {
129-
pub fn new(
129+
fn new(
130130
fcx: &'f FnCtxt<'f, 'tcx>,
131131
cause: ObligationCause<'tcx>,
132132
allow_two_phase: AllowTwoPhase,
133133
) -> Self {
134134
Coerce { fcx, cause, allow_two_phase, use_lub: false }
135135
}
136136

137-
pub fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
137+
fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
138138
debug!("unify(a: {:?}, b: {:?}, use_lub: {})", a, b, self.use_lub);
139139
self.commit_if_ok(|_| {
140140
if self.use_lub {
@@ -841,6 +841,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
841841
self.probe(|_| coerce.coerce(source, target)).is_ok()
842842
}
843843

844+
/// Given a type and a target type, this function will calculate and return
845+
/// how many dereference steps needed to achieve `expr_ty <: target`. If
846+
/// it's not possible, return `None`.
847+
pub fn deref_steps(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> Option<usize> {
848+
let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable);
849+
// We don't ever need two-phase here since we throw out the result of the coercion
850+
let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
851+
coerce
852+
.autoderef(rustc_span::DUMMY_SP, expr_ty)
853+
.find_map(|(ty, steps)| coerce.unify(ty, target).ok().map(|_| steps))
854+
}
855+
844856
/// Given some expressions, their known unified type and another expression,
845857
/// tries to unify the types, potentially inserting coercions on any of the
846858
/// provided expressions and returns their LUB (aka "common supertype").

‎src/librustc_typeck/check/demand.rs

+96-40
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::check::coercion::Coerce;
21
use crate::check::FnCtxt;
32
use rustc_infer::infer::InferOk;
43
use rustc_trait_selection::infer::InferCtxtExt as _;
@@ -9,7 +8,6 @@ use rustc_ast::util::parser::PREC_POSTFIX;
98
use rustc_errors::{Applicability, DiagnosticBuilder};
109
use rustc_hir as hir;
1110
use rustc_hir::{is_range_literal, Node};
12-
use rustc_middle::traits::ObligationCauseCode;
1311
use rustc_middle::ty::adjustment::AllowTwoPhase;
1412
use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut};
1513
use rustc_span::symbol::sym;
@@ -355,6 +353,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
355353
false
356354
}
357355

356+
fn replace_prefix<A, B, C>(&self, s: A, old: B, new: C) -> Option<String>
357+
where
358+
A: AsRef<str>,
359+
B: AsRef<str>,
360+
C: AsRef<str>,
361+
{
362+
let s = s.as_ref();
363+
let old = old.as_ref();
364+
if s.starts_with(old) { Some(new.as_ref().to_owned() + &s[old.len()..]) } else { None }
365+
}
366+
358367
/// This function is used to determine potential "simple" improvements or users' errors and
359368
/// provide them useful help. For example:
360369
///
@@ -376,7 +385,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
376385
expr: &hir::Expr<'_>,
377386
checked_ty: Ty<'tcx>,
378387
expected: Ty<'tcx>,
379-
) -> Option<(Span, &'static str, String)> {
388+
) -> Option<(Span, &'static str, String, Applicability)> {
380389
let sm = self.sess().source_map();
381390
let sp = expr.span;
382391
if sm.is_imported(sp) {
@@ -400,11 +409,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
400409
(&ty::Str, &ty::Array(arr, _) | &ty::Slice(arr)) if arr == self.tcx.types.u8 => {
401410
if let hir::ExprKind::Lit(_) = expr.kind {
402411
if let Ok(src) = sm.span_to_snippet(sp) {
403-
if src.starts_with("b\"") {
412+
if let Some(src) = self.replace_prefix(src, "b\"", "\"") {
404413
return Some((
405414
sp,
406415
"consider removing the leading `b`",
407-
src[1..].to_string(),
416+
src,
417+
Applicability::MachineApplicable,
408418
));
409419
}
410420
}
@@ -413,11 +423,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
413423
(&ty::Array(arr, _) | &ty::Slice(arr), &ty::Str) if arr == self.tcx.types.u8 => {
414424
if let hir::ExprKind::Lit(_) = expr.kind {
415425
if let Ok(src) = sm.span_to_snippet(sp) {
416-
if src.starts_with('"') {
426+
if let Some(src) = self.replace_prefix(src, "\"", "b\"") {
417427
return Some((
418428
sp,
419429
"consider adding a leading `b`",
420-
format!("b{}", src),
430+
src,
431+
Applicability::MachineApplicable,
421432
));
422433
}
423434
}
@@ -470,7 +481,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
470481
let sugg_expr = if needs_parens { format!("({})", src) } else { src };
471482

472483
if let Some(sugg) = self.can_use_as_ref(expr) {
473-
return Some(sugg);
484+
return Some((
485+
sugg.0,
486+
sugg.1,
487+
sugg.2,
488+
Applicability::MachineApplicable,
489+
));
474490
}
475491
let field_name = if is_struct_pat_shorthand_field {
476492
format!("{}: ", sugg_expr)
@@ -495,6 +511,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
495511
"consider dereferencing here to assign to the mutable \
496512
borrowed piece of memory",
497513
format!("*{}", src),
514+
Applicability::MachineApplicable,
498515
));
499516
}
500517
}
@@ -505,11 +522,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
505522
sp,
506523
"consider mutably borrowing here",
507524
format!("{}&mut {}", field_name, sugg_expr),
525+
Applicability::MachineApplicable,
508526
),
509527
hir::Mutability::Not => (
510528
sp,
511529
"consider borrowing here",
512530
format!("{}&{}", field_name, sugg_expr),
531+
Applicability::MachineApplicable,
513532
),
514533
});
515534
}
@@ -526,51 +545,88 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
526545
// We have `&T`, check if what was expected was `T`. If so,
527546
// we may want to suggest removing a `&`.
528547
if sm.is_imported(expr.span) {
529-
if let Ok(code) = sm.span_to_snippet(sp) {
530-
if code.starts_with('&') {
548+
if let Ok(src) = sm.span_to_snippet(sp) {
549+
if let Some(src) = self.replace_prefix(src, "&", "") {
531550
return Some((
532551
sp,
533552
"consider removing the borrow",
534-
code[1..].to_string(),
553+
src,
554+
Applicability::MachineApplicable,
535555
));
536556
}
537557
}
538558
return None;
539559
}
540560
if let Ok(code) = sm.span_to_snippet(expr.span) {
541-
return Some((sp, "consider removing the borrow", code));
561+
return Some((
562+
sp,
563+
"consider removing the borrow",
564+
code,
565+
Applicability::MachineApplicable,
566+
));
542567
}
543568
}
544569
(
545570
_,
546-
&ty::RawPtr(TypeAndMut { ty: _, mutbl: hir::Mutability::Not }),
547-
&ty::Ref(_, _, hir::Mutability::Not),
571+
&ty::RawPtr(TypeAndMut { ty: ty_b, mutbl: mutbl_b }),
572+
&ty::Ref(_, ty_a, mutbl_a),
548573
) => {
549-
let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable);
550-
// We don't ever need two-phase here since we throw out the result of the coercion
551-
let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
552-
553-
if let Some(steps) =
554-
coerce.autoderef(sp, checked_ty).skip(1).find_map(|(referent_ty, steps)| {
555-
coerce
556-
.unify(
557-
coerce.tcx.mk_ptr(ty::TypeAndMut {
558-
mutbl: hir::Mutability::Not,
559-
ty: referent_ty,
560-
}),
561-
expected,
562-
)
563-
.ok()
564-
.map(|_| steps)
565-
})
566-
{
567-
// The pointer type implements `Copy` trait so the suggestion is always valid.
568-
if let Ok(code) = sm.span_to_snippet(sp) {
569-
if code.starts_with('&') {
570-
let derefs = "*".repeat(steps - 1);
571-
let message = "consider dereferencing the reference";
572-
let suggestion = format!("&{}{}", derefs, code[1..].to_string());
573-
return Some((sp, message, suggestion));
574+
if let Some(steps) = self.deref_steps(ty_a, ty_b) {
575+
// Only suggest valid if dereferencing needed.
576+
if steps > 0 {
577+
// The pointer type implements `Copy` trait so the suggestion is always valid.
578+
if let Ok(src) = sm.span_to_snippet(sp) {
579+
let derefs = &"*".repeat(steps);
580+
if let Some((src, applicability)) = match mutbl_b {
581+
hir::Mutability::Mut => {
582+
let new_prefix = "&mut ".to_owned() + derefs;
583+
match mutbl_a {
584+
hir::Mutability::Mut => {
585+
if let Some(s) =
586+
self.replace_prefix(src, "&mut ", new_prefix)
587+
{
588+
Some((s, Applicability::MachineApplicable))
589+
} else {
590+
None
591+
}
592+
}
593+
hir::Mutability::Not => {
594+
if let Some(s) =
595+
self.replace_prefix(src, "&", new_prefix)
596+
{
597+
Some((s, Applicability::Unspecified))
598+
} else {
599+
None
600+
}
601+
}
602+
}
603+
}
604+
hir::Mutability::Not => {
605+
let new_prefix = "&".to_owned() + derefs;
606+
match mutbl_a {
607+
hir::Mutability::Mut => {
608+
if let Some(s) =
609+
self.replace_prefix(src, "&mut ", new_prefix)
610+
{
611+
Some((s, Applicability::MachineApplicable))
612+
} else {
613+
None
614+
}
615+
}
616+
hir::Mutability::Not => {
617+
if let Some(s) =
618+
self.replace_prefix(src, "&", new_prefix)
619+
{
620+
Some((s, Applicability::MachineApplicable))
621+
} else {
622+
None
623+
}
624+
}
625+
}
626+
}
627+
} {
628+
return Some((sp, "consider dereferencing", src, applicability));
629+
}
574630
}
575631
}
576632
}
@@ -616,7 +672,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
616672
} else {
617673
format!("*{}", code)
618674
};
619-
return Some((sp, message, suggestion));
675+
return Some((sp, message, suggestion, Applicability::MachineApplicable));
620676
}
621677
}
622678
}

‎src/librustc_typeck/check/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -5036,8 +5036,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
50365036
expected: Ty<'tcx>,
50375037
found: Ty<'tcx>,
50385038
) {
5039-
if let Some((sp, msg, suggestion)) = self.check_ref(expr, found, expected) {
5040-
err.span_suggestion(sp, msg, suggestion, Applicability::MachineApplicable);
5039+
if let Some((sp, msg, suggestion, applicability)) = self.check_ref(expr, found, expected) {
5040+
err.span_suggestion(sp, msg, suggestion, applicability);
50415041
} else if let (ty::FnDef(def_id, ..), true) =
50425042
(&found.kind, self.suggest_fn_call(err, expr, expected, found))
50435043
{

‎src/test/ui/issues/issue-32122-1.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | let _: *const u8 = &a;
55
| --------- ^^
66
| | |
77
| | expected `u8`, found struct `Foo`
8-
| | help: consider dereferencing the reference: `&*a`
8+
| | help: consider dereferencing: `&*a`
99
| expected due to this
1010
|
1111
= note: expected raw pointer `*const u8`

‎src/test/ui/issues/issue-32122-2.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | let _: *const u8 = &a;
55
| --------- ^^
66
| | |
77
| | expected `u8`, found struct `Emm`
8-
| | help: consider dereferencing the reference: `&***a`
8+
| | help: consider dereferencing: `&***a`
99
| expected due to this
1010
|
1111
= note: expected raw pointer `*const u8`

‎src/test/ui/issues/issue-71676-1.fixed

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// run-rustfix
2+
use std::ops::Deref;
3+
use std::ops::DerefMut;
4+
struct Bar(u8);
5+
struct Foo(Bar);
6+
struct Emm(Foo);
7+
impl Deref for Bar{
8+
type Target = u8;
9+
fn deref(&self) -> &Self::Target {
10+
&self.0
11+
}
12+
}
13+
impl Deref for Foo {
14+
type Target = Bar;
15+
fn deref(&self) -> &Self::Target {
16+
&self.0
17+
}
18+
}
19+
impl Deref for Emm {
20+
type Target = Foo;
21+
fn deref(&self) -> &Self::Target {
22+
&self.0
23+
}
24+
}
25+
impl DerefMut for Bar{
26+
fn deref_mut(&mut self) -> &mut Self::Target {
27+
&mut self.0
28+
}
29+
}
30+
impl DerefMut for Foo {
31+
fn deref_mut(&mut self) -> &mut Self::Target {
32+
&mut self.0
33+
}
34+
}
35+
impl DerefMut for Emm {
36+
fn deref_mut(&mut self) -> &mut Self::Target {
37+
&mut self.0
38+
}
39+
}
40+
fn main() {
41+
// Suggest dereference with arbitrary mutability
42+
let a = Emm(Foo(Bar(0)));
43+
let _: *const u8 = &***a; //~ ERROR mismatched types
44+
45+
let mut a = Emm(Foo(Bar(0)));
46+
let _: *mut u8 = &mut ***a; //~ ERROR mismatched types
47+
48+
let a = Emm(Foo(Bar(0)));
49+
let _: *const u8 = &***a; //~ ERROR mismatched types
50+
51+
let mut a = Emm(Foo(Bar(0)));
52+
let _: *mut u8 = &mut ***a; //~ ERROR mismatched types
53+
}

0 commit comments

Comments
 (0)