Skip to content
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
67 changes: 67 additions & 0 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
diag.span_label(span, message);
}
}
self.suggest_as_ref_where_appropriate(span, &exp_found, diag);
}

diag.note_expected_found(&"type", expected, found);
Expand All @@ -972,6 +973,72 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.note_error_origin(diag, &cause);
}

/// When encountering a case where `.as_ref()` on a `Result` or `Option` would be appropriate,
/// suggest it.
fn suggest_as_ref_where_appropriate(
Copy link
Contributor

Choose a reason for hiding this comment

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

I like _where_appropriate as part of a function name, but I don't think I've seen it used anywhere else, and I know we have a lot of these "conditionally set a diagnostic" functions ...

&self,
span: Span,
exp_found: &ty::error::ExpectedFound<Ty<'tcx>>,
diag: &mut DiagnosticBuilder<'tcx>,
) {
match (&exp_found.expected.sty, &exp_found.found.sty) {
(TyKind::Adt(exp_def, exp_substs), TyKind::Ref(_, found_ty, _)) => {
if let TyKind::Adt(found_def, found_substs) = found_ty.sty {
let path_str = format!("{:?}", exp_def);
Copy link
Contributor

Choose a reason for hiding this comment

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

Relying on debug output for functionality seems like poor form? (On the grounds that it's meant for debugging convenience and should be subject to change rather than guaranteed stable, even if it happens to be probably-stable in this case.)

if exp_def == &found_def {
let opt_msg = "you can convert from `&Option<T>` to `Option<&T>` using \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let opt_msg = "you can convert from `&Option<T>` to `Option<&T>` using \
let opt_msg = "`&Option<T>` can be converted to `Option<&T>` using \

I lean against the second person in diagnostic messages. (But that's my opinion; we haven't codifed that anywhere.)

`.as_ref()`";
let result_msg = "you can convert from `&Result<T, E>` to \
`Result<&T, &E>` using `.as_ref()`";
let have_as_ref = &[
("std::option::Option", opt_msg),
("core::option::Option", opt_msg),
("std::result::Result", result_msg),
("core::result::Result", result_msg),
];
if let Some(msg) = have_as_ref.iter()
.filter_map(|(path, msg)| if &path_str == path {
Some(msg)
} else {
None
}).next()
{
let mut show_suggestion = true;
for (exp_ty, found_ty) in exp_substs.types().zip(found_substs.types()) {
match exp_ty.sty {
TyKind::Ref(_, exp_ty, _) => {
match (&exp_ty.sty, &found_ty.sty) {
(_, TyKind::Param(_)) |
(_, TyKind::Infer(_)) |
(TyKind::Param(_), _) |
(TyKind::Infer(_), _) => {}
_ if ty::TyS::same_type(exp_ty, found_ty) => {}
_ => show_suggestion = false,
};
}
TyKind::Param(_) | TyKind::Infer(_) => {}
_ => show_suggestion = false,
}
}
if let (Ok(snippet), true) = (
self.tcx.sess.source_map().span_to_snippet(span),
show_suggestion,
) {
diag.span_suggestion_with_applicability(
span,
msg,
format!("{}.as_ref()", snippet),
Applicability::MachineApplicable,
);
}
}
}
}
}
_ => {}
}
}

pub fn report_and_explain_type_error(
&self,
trace: TypeTrace<'tcx>,
Expand Down
28 changes: 14 additions & 14 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,19 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
tcx.needs_drop_raw(param_env.and(self))
}

pub fn same_type(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
match (&a.sty, &b.sty) {
(&Adt(did_a, substs_a), &Adt(did_b, substs_b)) => {
if did_a != did_b {
return false;
}

substs_a.types().zip(substs_b.types()).all(|(a, b)| Self::same_type(a, b))
}
_ => a == b,
}
}

/// Check whether a type is representable. This means it cannot contain unboxed
/// structural recursion. This check is needed for structs and enums.
pub fn is_representable(&'tcx self,
Expand Down Expand Up @@ -730,19 +743,6 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
}
}

fn same_type<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
match (&a.sty, &b.sty) {
(&Adt(did_a, substs_a), &Adt(did_b, substs_b)) => {
if did_a != did_b {
return false;
}

substs_a.types().zip(substs_b.types()).all(|(a, b)| same_type(a, b))
}
_ => a == b,
}
}

// Does the type `ty` directly (without indirection through a pointer)
// contain any types on stack `seen`?
fn is_type_structurally_recursive<'a, 'tcx>(
Expand Down Expand Up @@ -807,7 +807,7 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
// struct Foo { Option<Option<Foo>> }

for &seen_type in iter {
if same_type(ty, seen_type) {
if ty::TyS::same_type(ty, seen_type) {
debug!("ContainsRecursive: {:?} contains {:?}",
seen_type,
ty);
Expand Down
47 changes: 0 additions & 47 deletions src/test/ui/as-ref.stderr

This file was deleted.

10 changes: 10 additions & 0 deletions src/test/ui/as-ref.rs → src/test/ui/suggestions/as-ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,14 @@ fn main() {
//~^ ERROR mismatched types [E0308]
opt.and_then(|arg| Ok(takes_ref(arg)));
//~^ ERROR mismatched types [E0308]
let x: &Option<usize> = &Some(3);
let y: Option<&usize> = x;
//~^ ERROR mismatched types [E0308]
let x: &Result<usize, usize> = &Ok(3);
let y: Result<&usize, &usize> = x;
//~^ ERROR mismatched types [E0308]
// note: do not suggest because of `E: usize`
let x: &Result<usize, usize> = &Ok(3);
let y: Result<&usize, usize> = x;
//~^ ERROR mismatched types [E0308]
}
81 changes: 81 additions & 0 deletions src/test/ui/suggestions/as-ref.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
error[E0308]: mismatched types
--> $DIR/as-ref.rs:6:27
|
LL | opt.map(|arg| takes_ref(arg));
| - ^^^ expected &Foo, found struct `Foo`
| |
| help: consider using `as_ref` instead: `as_ref().`
Copy link
Contributor

Choose a reason for hiding this comment

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

as_ref(). (trailing dot!) as the suggestion text faked me out for a moment; our insertion suggestions can look kind of awkward sometimes. (But making the span cover opt.map and suggesting opt.as_ref().map is ugly in its own way.)

In any case, the code that sets this was actually introduced in #51100, not here, so it doesn't affect the outcome of this review.

|
= note: expected type `&Foo`
found type `Foo`

error[E0308]: mismatched types
--> $DIR/as-ref.rs:8:37
|
LL | opt.and_then(|arg| Some(takes_ref(arg)));
| - ^^^ expected &Foo, found struct `Foo`
| |
| help: consider using `as_ref` instead: `as_ref().`
|
= note: expected type `&Foo`
found type `Foo`

error[E0308]: mismatched types
--> $DIR/as-ref.rs:11:27
|
LL | opt.map(|arg| takes_ref(arg));
| - ^^^ expected &Foo, found struct `Foo`
| |
| help: consider using `as_ref` instead: `as_ref().`
|
= note: expected type `&Foo`
found type `Foo`

error[E0308]: mismatched types
--> $DIR/as-ref.rs:13:35
|
LL | opt.and_then(|arg| Ok(takes_ref(arg)));
| - ^^^ expected &Foo, found struct `Foo`
| |
| help: consider using `as_ref` instead: `as_ref().`
|
= note: expected type `&Foo`
found type `Foo`

error[E0308]: mismatched types
--> $DIR/as-ref.rs:16:27
|
LL | let y: Option<&usize> = x;
| ^
| |
| expected enum `std::option::Option`, found reference
| help: you can convert from `&Option<T>` to `Option<&T>` using `.as_ref()`: `x.as_ref()`
|
= note: expected type `std::option::Option<&usize>`
found type `&std::option::Option<usize>`

error[E0308]: mismatched types
--> $DIR/as-ref.rs:19:35
|
LL | let y: Result<&usize, &usize> = x;
| ^ expected enum `std::result::Result`, found reference
|
= note: expected type `std::result::Result<&usize, &usize>`
found type `&std::result::Result<usize, usize>`
help: you can convert from `&Result<T, E>` to `Result<&T, &E>` using `.as_ref()`
|
LL | let y: Result<&usize, &usize> = x.as_ref();
| ^^^^^^^^^^

error[E0308]: mismatched types
--> $DIR/as-ref.rs:23:34
|
LL | let y: Result<&usize, usize> = x;
| ^ expected enum `std::result::Result`, found reference
|
= note: expected type `std::result::Result<&usize, usize>`
found type `&std::result::Result<usize, usize>`

error: aborting due to 7 previous errors

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