Skip to content

Suggest deref when coercing ty::Ref to ty::RawPtr #71540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode};
use smallvec::{smallvec, SmallVec};
use std::ops::Deref;

struct Coerce<'a, 'tcx> {
pub struct Coerce<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
cause: ObligationCause<'tcx>,
use_lub: bool,
Expand Down Expand Up @@ -124,15 +124,15 @@ fn success<'tcx>(
}

impl<'f, 'tcx> Coerce<'f, 'tcx> {
fn new(
pub fn new(
fcx: &'f FnCtxt<'f, 'tcx>,
cause: ObligationCause<'tcx>,
allow_two_phase: AllowTwoPhase,
) -> Self {
Coerce { fcx, cause, allow_two_phase, use_lub: false }
}

fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
pub fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
self.commit_if_ok(|_| {
if self.use_lub {
self.at(&self.cause, self.fcx.param_env).lub(b, a)
Expand Down Expand Up @@ -771,10 +771,10 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
ty::RawPtr(mt) => (false, mt),
_ => return self.unify_and(a, b, identity),
};
coerce_mutbls(mt_a.mutbl, mutbl_b)?;

// Check that the types which they point at are compatible.
let a_unsafe = self.tcx.mk_ptr(ty::TypeAndMut { mutbl: mutbl_b, ty: mt_a.ty });
coerce_mutbls(mt_a.mutbl, mutbl_b)?;
// Although references and unsafe ptrs have the same
// representation, we still register an Adjust::DerefRef so that
// regionck knows that the region for `a` must be valid here.
Expand Down
40 changes: 38 additions & 2 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::check::coercion::Coerce;
use crate::check::FnCtxt;
use rustc_infer::infer::InferOk;
use rustc_trait_selection::infer::InferCtxtExt as _;
Expand All @@ -8,8 +9,9 @@ use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::{is_range_literal, Node};
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::adjustment::AllowTwoPhase;
use rustc_middle::ty::{self, AssocItem, Ty};
use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut};
use rustc_span::symbol::sym;
use rustc_span::Span;

Expand All @@ -25,7 +27,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) {
self.annotate_expected_due_to_let_ty(err, expr);
self.suggest_compatible_variants(err, expr, expected, expr_ty);
self.suggest_ref_or_into(err, expr, expected, expr_ty);
self.suggest_deref_ref_or_into(err, expr, expected, expr_ty);
if self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) {
return;
}
Expand Down Expand Up @@ -539,6 +541,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return Some((sp, "consider removing the borrow", code));
}
}
(
_,
&ty::RawPtr(TypeAndMut { ty: _, mutbl: hir::Mutability::Not }),
&ty::Ref(_, _, hir::Mutability::Not),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I believe we could also do something for mutable references (both for staying in the same mutability and moving to const), this requires some care to get right without false positives, so maybe just leave a FIXME or open an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I missed the mutable references situation. I'll open another issue.

) => {
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);

if let Some(steps) =
coerce.autoderef(sp, checked_ty).skip(1).find_map(|(referent_ty, steps)| {
coerce
.unify(
coerce.tcx.mk_ptr(ty::TypeAndMut {
mutbl: hir::Mutability::Not,
ty: referent_ty,
}),
expected,
)
.ok()
.map(|_| steps)
})
{
// The pointer type implements `Copy` trait so the suggestion is always valid.
if let Ok(code) = sm.span_to_snippet(sp) {
if code.starts_with('&') {
let derefs = "*".repeat(steps - 1);
let message = "consider dereferencing the reference";
let suggestion = format!("&{}{}", derefs, code[1..].to_string());
return Some((sp, message, suggestion));
}
}
}
}
_ if sp == expr.span && !is_macro => {
// Check for `Deref` implementations by constructing a predicate to
// prove: `<T as Deref>::Output == U`
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

if let Some(mut err) = self.demand_suptype_diag(expr.span, expected_ty, ty) {
let expr = expr.peel_drop_temps();
self.suggest_ref_or_into(&mut err, expr, expected_ty, ty);
self.suggest_deref_ref_or_into(&mut err, expr, expected_ty, ty);
extend_err(&mut err);
// Error possibly reported in `check_assign` so avoid emitting error again.
err.emit_unless(self.is_assign_to_bool(expr, expected_ty));
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5029,7 +5029,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
}

pub fn suggest_ref_or_into(
pub fn suggest_deref_ref_or_into(
&self,
err: &mut DiagnosticBuilder<'_>,
expr: &hir::Expr<'_>,
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/issues/issue-32122-1.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-rustfix
use std::ops::Deref;

struct Foo(u8);

impl Deref for Foo {
type Target = u8;
fn deref(&self) -> &Self::Target {
&self.0
}
}

fn main() {
let a = Foo(0);
// Should suggest `&*` when coercing &ty to *const ty
let _: *const u8 = &*a; //~ ERROR mismatched types
}
17 changes: 17 additions & 0 deletions src/test/ui/issues/issue-32122-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-rustfix
use std::ops::Deref;

struct Foo(u8);

impl Deref for Foo {
type Target = u8;
fn deref(&self) -> &Self::Target {
&self.0
}
}

fn main() {
let a = Foo(0);
// Should suggest `&*` when coercing &ty to *const ty
let _: *const u8 = &a; //~ ERROR mismatched types
}
16 changes: 16 additions & 0 deletions src/test/ui/issues/issue-32122-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0308]: mismatched types
--> $DIR/issue-32122-1.rs:16:24
|
LL | let _: *const u8 = &a;
| --------- ^^
| | |
| | expected `u8`, found struct `Foo`
| | help: consider dereferencing the reference: `&*a`
| expected due to this
|
= note: expected raw pointer `*const u8`
found reference `&Foo`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
28 changes: 28 additions & 0 deletions src/test/ui/issues/issue-32122-2.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// run-rustfix
use std::ops::Deref;
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
}
}
fn main() {
let a = Emm(Foo(Bar(0)));
// Should suggest `&***` even when deref is pretty deep
let _: *const u8 = &***a; //~ ERROR mismatched types
}
28 changes: 28 additions & 0 deletions src/test/ui/issues/issue-32122-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// run-rustfix
use std::ops::Deref;
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
}
}
fn main() {
let a = Emm(Foo(Bar(0)));
// Should suggest `&***` even when deref is pretty deep
let _: *const u8 = &a; //~ ERROR mismatched types
}
16 changes: 16 additions & 0 deletions src/test/ui/issues/issue-32122-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0308]: mismatched types
--> $DIR/issue-32122-2.rs:27:24
|
LL | let _: *const u8 = &a;
| --------- ^^
| | |
| | expected `u8`, found struct `Emm`
| | help: consider dereferencing the reference: `&***a`
| expected due to this
|
= note: expected raw pointer `*const u8`
found reference `&Emm`

error: aborting due to previous error

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