Rollup merge of #71726 - ldm0:ref2ptr, r=oli-obk

Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability

Fixes #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 #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)
This commit is contained in:
Dylan DPC 2020-05-03 18:34:46 +02:00 committed by GitHub
commit 44e678bf9e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 334 additions and 47 deletions

View File

@ -74,7 +74,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode};
use smallvec::{smallvec, SmallVec}; use smallvec::{smallvec, SmallVec};
use std::ops::Deref; use std::ops::Deref;
pub struct Coerce<'a, 'tcx> { struct Coerce<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>, fcx: &'a FnCtxt<'a, 'tcx>,
cause: ObligationCause<'tcx>, cause: ObligationCause<'tcx>,
use_lub: bool, use_lub: bool,
@ -126,7 +126,7 @@ fn success<'tcx>(
} }
impl<'f, 'tcx> Coerce<'f, 'tcx> { impl<'f, 'tcx> Coerce<'f, 'tcx> {
pub fn new( fn new(
fcx: &'f FnCtxt<'f, 'tcx>, fcx: &'f FnCtxt<'f, 'tcx>,
cause: ObligationCause<'tcx>, cause: ObligationCause<'tcx>,
allow_two_phase: AllowTwoPhase, allow_two_phase: AllowTwoPhase,
@ -134,7 +134,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
Coerce { fcx, cause, allow_two_phase, use_lub: false } Coerce { fcx, cause, allow_two_phase, use_lub: false }
} }
pub fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> { fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
debug!("unify(a: {:?}, b: {:?}, use_lub: {})", a, b, self.use_lub); debug!("unify(a: {:?}, b: {:?}, use_lub: {})", a, b, self.use_lub);
self.commit_if_ok(|_| { self.commit_if_ok(|_| {
if self.use_lub { if self.use_lub {
@ -841,6 +841,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.probe(|_| coerce.coerce(source, target)).is_ok() self.probe(|_| coerce.coerce(source, target)).is_ok()
} }
/// Given a type and a target type, this function will calculate and return
/// how many dereference steps needed to achieve `expr_ty <: target`. If
/// it's not possible, return `None`.
pub fn deref_steps(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> Option<usize> {
let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable);
// We don't ever need two-phase here since we throw out the result of the coercion
let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
coerce
.autoderef(rustc_span::DUMMY_SP, expr_ty)
.find_map(|(ty, steps)| coerce.unify(ty, target).ok().map(|_| steps))
}
/// Given some expressions, their known unified type and another expression, /// Given some expressions, their known unified type and another expression,
/// tries to unify the types, potentially inserting coercions on any of the /// tries to unify the types, potentially inserting coercions on any of the
/// provided expressions and returns their LUB (aka "common supertype"). /// provided expressions and returns their LUB (aka "common supertype").

View File

@ -1,4 +1,3 @@
use crate::check::coercion::Coerce;
use crate::check::FnCtxt; use crate::check::FnCtxt;
use rustc_infer::infer::InferOk; use rustc_infer::infer::InferOk;
use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::infer::InferCtxtExt as _;
@ -9,7 +8,6 @@ use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::{is_range_literal, Node}; use rustc_hir::{is_range_literal, Node};
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::adjustment::AllowTwoPhase; use rustc_middle::ty::adjustment::AllowTwoPhase;
use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut}; use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut};
use rustc_span::symbol::sym; use rustc_span::symbol::sym;
@ -355,6 +353,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false false
} }
fn replace_prefix<A, B, C>(&self, s: A, old: B, new: C) -> Option<String>
where
A: AsRef<str>,
B: AsRef<str>,
C: AsRef<str>,
{
let s = s.as_ref();
let old = old.as_ref();
if s.starts_with(old) { Some(new.as_ref().to_owned() + &s[old.len()..]) } else { None }
}
/// This function is used to determine potential "simple" improvements or users' errors and /// This function is used to determine potential "simple" improvements or users' errors and
/// provide them useful help. For example: /// provide them useful help. For example:
/// ///
@ -376,7 +385,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &hir::Expr<'_>, expr: &hir::Expr<'_>,
checked_ty: Ty<'tcx>, checked_ty: Ty<'tcx>,
expected: Ty<'tcx>, expected: Ty<'tcx>,
) -> Option<(Span, &'static str, String)> { ) -> Option<(Span, &'static str, String, Applicability)> {
let sm = self.sess().source_map(); let sm = self.sess().source_map();
let sp = expr.span; let sp = expr.span;
if sm.is_imported(sp) { if sm.is_imported(sp) {
@ -400,11 +409,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(&ty::Str, &ty::Array(arr, _) | &ty::Slice(arr)) if arr == self.tcx.types.u8 => { (&ty::Str, &ty::Array(arr, _) | &ty::Slice(arr)) if arr == self.tcx.types.u8 => {
if let hir::ExprKind::Lit(_) = expr.kind { if let hir::ExprKind::Lit(_) = expr.kind {
if let Ok(src) = sm.span_to_snippet(sp) { if let Ok(src) = sm.span_to_snippet(sp) {
if src.starts_with("b\"") { if let Some(src) = self.replace_prefix(src, "b\"", "\"") {
return Some(( return Some((
sp, sp,
"consider removing the leading `b`", "consider removing the leading `b`",
src[1..].to_string(), src,
Applicability::MachineApplicable,
)); ));
} }
} }
@ -413,11 +423,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(&ty::Array(arr, _) | &ty::Slice(arr), &ty::Str) if arr == self.tcx.types.u8 => { (&ty::Array(arr, _) | &ty::Slice(arr), &ty::Str) if arr == self.tcx.types.u8 => {
if let hir::ExprKind::Lit(_) = expr.kind { if let hir::ExprKind::Lit(_) = expr.kind {
if let Ok(src) = sm.span_to_snippet(sp) { if let Ok(src) = sm.span_to_snippet(sp) {
if src.starts_with('"') { if let Some(src) = self.replace_prefix(src, "\"", "b\"") {
return Some(( return Some((
sp, sp,
"consider adding a leading `b`", "consider adding a leading `b`",
format!("b{}", src), src,
Applicability::MachineApplicable,
)); ));
} }
} }
@ -470,7 +481,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let sugg_expr = if needs_parens { format!("({})", src) } else { src }; let sugg_expr = if needs_parens { format!("({})", src) } else { src };
if let Some(sugg) = self.can_use_as_ref(expr) { if let Some(sugg) = self.can_use_as_ref(expr) {
return Some(sugg); return Some((
sugg.0,
sugg.1,
sugg.2,
Applicability::MachineApplicable,
));
} }
let field_name = if is_struct_pat_shorthand_field { let field_name = if is_struct_pat_shorthand_field {
format!("{}: ", sugg_expr) format!("{}: ", sugg_expr)
@ -495,6 +511,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"consider dereferencing here to assign to the mutable \ "consider dereferencing here to assign to the mutable \
borrowed piece of memory", borrowed piece of memory",
format!("*{}", src), format!("*{}", src),
Applicability::MachineApplicable,
)); ));
} }
} }
@ -505,11 +522,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
sp, sp,
"consider mutably borrowing here", "consider mutably borrowing here",
format!("{}&mut {}", field_name, sugg_expr), format!("{}&mut {}", field_name, sugg_expr),
Applicability::MachineApplicable,
), ),
hir::Mutability::Not => ( hir::Mutability::Not => (
sp, sp,
"consider borrowing here", "consider borrowing here",
format!("{}&{}", field_name, sugg_expr), format!("{}&{}", field_name, sugg_expr),
Applicability::MachineApplicable,
), ),
}); });
} }
@ -526,51 +545,88 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// We have `&T`, check if what was expected was `T`. If so, // We have `&T`, check if what was expected was `T`. If so,
// we may want to suggest removing a `&`. // we may want to suggest removing a `&`.
if sm.is_imported(expr.span) { if sm.is_imported(expr.span) {
if let Ok(code) = sm.span_to_snippet(sp) { if let Ok(src) = sm.span_to_snippet(sp) {
if code.starts_with('&') { if let Some(src) = self.replace_prefix(src, "&", "") {
return Some(( return Some((
sp, sp,
"consider removing the borrow", "consider removing the borrow",
code[1..].to_string(), src,
Applicability::MachineApplicable,
)); ));
} }
} }
return None; return None;
} }
if let Ok(code) = sm.span_to_snippet(expr.span) { if let Ok(code) = sm.span_to_snippet(expr.span) {
return Some((sp, "consider removing the borrow", code)); return Some((
sp,
"consider removing the borrow",
code,
Applicability::MachineApplicable,
));
} }
} }
( (
_, _,
&ty::RawPtr(TypeAndMut { ty: _, mutbl: hir::Mutability::Not }), &ty::RawPtr(TypeAndMut { ty: ty_b, mutbl: mutbl_b }),
&ty::Ref(_, _, hir::Mutability::Not), &ty::Ref(_, ty_a, mutbl_a),
) => { ) => {
let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable); if let Some(steps) = self.deref_steps(ty_a, ty_b) {
// We don't ever need two-phase here since we throw out the result of the coercion // Only suggest valid if dereferencing needed.
let coerce = Coerce::new(self, cause, AllowTwoPhase::No); if steps > 0 {
// The pointer type implements `Copy` trait so the suggestion is always valid.
if let Some(steps) = if let Ok(src) = sm.span_to_snippet(sp) {
coerce.autoderef(sp, checked_ty).skip(1).find_map(|(referent_ty, steps)| { let derefs = &"*".repeat(steps);
coerce if let Some((src, applicability)) = match mutbl_b {
.unify( hir::Mutability::Mut => {
coerce.tcx.mk_ptr(ty::TypeAndMut { let new_prefix = "&mut ".to_owned() + derefs;
mutbl: hir::Mutability::Not, match mutbl_a {
ty: referent_ty, hir::Mutability::Mut => {
}), if let Some(s) =
expected, self.replace_prefix(src, "&mut ", new_prefix)
) {
.ok() Some((s, Applicability::MachineApplicable))
.map(|_| steps) } else {
}) None
{ }
// The pointer type implements `Copy` trait so the suggestion is always valid. }
if let Ok(code) = sm.span_to_snippet(sp) { hir::Mutability::Not => {
if code.starts_with('&') { if let Some(s) =
let derefs = "*".repeat(steps - 1); self.replace_prefix(src, "&", new_prefix)
let message = "consider dereferencing the reference"; {
let suggestion = format!("&{}{}", derefs, code[1..].to_string()); Some((s, Applicability::Unspecified))
return Some((sp, message, suggestion)); } else {
None
}
}
}
}
hir::Mutability::Not => {
let new_prefix = "&".to_owned() + derefs;
match mutbl_a {
hir::Mutability::Mut => {
if let Some(s) =
self.replace_prefix(src, "&mut ", new_prefix)
{
Some((s, Applicability::MachineApplicable))
} else {
None
}
}
hir::Mutability::Not => {
if let Some(s) =
self.replace_prefix(src, "&", new_prefix)
{
Some((s, Applicability::MachineApplicable))
} else {
None
}
}
}
}
} {
return Some((sp, "consider dereferencing", src, applicability));
}
} }
} }
} }
@ -616,7 +672,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else { } else {
format!("*{}", code) format!("*{}", code)
}; };
return Some((sp, message, suggestion)); return Some((sp, message, suggestion, Applicability::MachineApplicable));
} }
} }
} }

View File

@ -5036,8 +5036,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>, expected: Ty<'tcx>,
found: Ty<'tcx>, found: Ty<'tcx>,
) { ) {
if let Some((sp, msg, suggestion)) = self.check_ref(expr, found, expected) { if let Some((sp, msg, suggestion, applicability)) = self.check_ref(expr, found, expected) {
err.span_suggestion(sp, msg, suggestion, Applicability::MachineApplicable); err.span_suggestion(sp, msg, suggestion, applicability);
} else if let (ty::FnDef(def_id, ..), true) = } else if let (ty::FnDef(def_id, ..), true) =
(&found.kind, self.suggest_fn_call(err, expr, expected, found)) (&found.kind, self.suggest_fn_call(err, expr, expected, found))
{ {

View File

@ -5,7 +5,7 @@ LL | let _: *const u8 = &a;
| --------- ^^ | --------- ^^
| | | | | |
| | expected `u8`, found struct `Foo` | | expected `u8`, found struct `Foo`
| | help: consider dereferencing the reference: `&*a` | | help: consider dereferencing: `&*a`
| expected due to this | expected due to this
| |
= note: expected raw pointer `*const u8` = note: expected raw pointer `*const u8`

View File

@ -5,7 +5,7 @@ LL | let _: *const u8 = &a;
| --------- ^^ | --------- ^^
| | | | | |
| | expected `u8`, found struct `Emm` | | expected `u8`, found struct `Emm`
| | help: consider dereferencing the reference: `&***a` | | help: consider dereferencing: `&***a`
| expected due to this | expected due to this
| |
= note: expected raw pointer `*const u8` = note: expected raw pointer `*const u8`

View File

@ -0,0 +1,53 @@
// run-rustfix
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() {
// Suggest dereference with arbitrary mutability
let a = Emm(Foo(Bar(0)));
let _: *const u8 = &***a; //~ ERROR mismatched types
let mut a = Emm(Foo(Bar(0)));
let _: *mut u8 = &mut ***a; //~ ERROR mismatched types
let a = Emm(Foo(Bar(0)));
let _: *const u8 = &***a; //~ ERROR mismatched types
let mut a = Emm(Foo(Bar(0)));
let _: *mut u8 = &mut ***a; //~ ERROR mismatched types
}

View File

@ -0,0 +1,53 @@
// run-rustfix
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() {
// Suggest dereference with arbitrary mutability
let a = Emm(Foo(Bar(0)));
let _: *const u8 = &a; //~ ERROR mismatched types
let mut a = Emm(Foo(Bar(0)));
let _: *mut u8 = &a; //~ ERROR mismatched types
let a = Emm(Foo(Bar(0)));
let _: *const u8 = &mut a; //~ ERROR mismatched types
let mut a = Emm(Foo(Bar(0)));
let _: *mut u8 = &mut a; //~ ERROR mismatched types
}

View File

@ -0,0 +1,55 @@
error[E0308]: mismatched types
--> $DIR/issue-71676-1.rs:43:24
|
LL | let _: *const u8 = &a;
| --------- ^^
| | |
| | expected `u8`, found struct `Emm`
| | help: consider dereferencing: `&***a`
| expected due to this
|
= note: expected raw pointer `*const u8`
found reference `&Emm`
error[E0308]: mismatched types
--> $DIR/issue-71676-1.rs:46:22
|
LL | let _: *mut u8 = &a;
| ------- ^^
| | |
| | types differ in mutability
| | help: consider dereferencing: `&mut ***a`
| expected due to this
|
= note: expected raw pointer `*mut u8`
found reference `&Emm`
error[E0308]: mismatched types
--> $DIR/issue-71676-1.rs:49:24
|
LL | let _: *const u8 = &mut a;
| --------- ^^^^^^
| | |
| | expected `u8`, found struct `Emm`
| | help: consider dereferencing: `&***a`
| expected due to this
|
= note: expected raw pointer `*const u8`
found mutable reference `&mut Emm`
error[E0308]: mismatched types
--> $DIR/issue-71676-1.rs:52:22
|
LL | let _: *mut u8 = &mut a;
| ------- ^^^^^^
| | |
| | expected `u8`, found struct `Emm`
| | help: consider dereferencing: `&mut ***a`
| expected due to this
|
= note: expected raw pointer `*mut u8`
found mutable reference `&mut Emm`
error: aborting due to 4 previous errors
For more information about this error, try `rustc --explain E0308`.

View File

@ -0,0 +1,42 @@
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
}

View File

@ -0,0 +1,16 @@
error[E0308]: mismatched types
--> $DIR/issue-71676-2.rs:41:22
|
LL | let _: *mut u8 = &a;
| ------- ^^
| | |
| | types differ in mutability
| | help: consider dereferencing: `&mut ***a`
| expected due to this
|
= note: expected raw pointer `*mut u8`
found reference `&Emm`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0308`.