Fix a common false-positive type mismatch

E.g. for `&{ some_string() }` in a context where a `&str` is expected, we
reported a mismatch inside the block. The problem is that we're passing an
expectation of `str` down, but the expectation is more of a hint in this case.
There's a long comment in rustc about this, which I just copied.

Also, fix reported location for type mismatches in macros.
This commit is contained in:
Florian Diebold 2020-02-29 15:31:07 +01:00
parent 0ec7f760fc
commit 5fe220b987
4 changed files with 86 additions and 18 deletions

View file

@ -583,21 +583,52 @@ impl InferTy {
#[derive(Clone, PartialEq, Eq, Debug)] #[derive(Clone, PartialEq, Eq, Debug)]
struct Expectation { struct Expectation {
ty: Ty, ty: Ty,
// FIXME: In some cases, we need to be aware whether the expectation is that /// See the `rvalue_hint` method.
// the type match exactly what we passed, or whether it just needs to be rvalue_hint: bool,
// coercible to the expected type. See Expectation::rvalue_hint in rustc.
} }
impl Expectation { impl Expectation {
/// The expectation that the type of the expression needs to equal the given /// The expectation that the type of the expression needs to equal the given
/// type. /// type.
fn has_type(ty: Ty) -> Self { fn has_type(ty: Ty) -> Self {
Expectation { ty } Expectation { ty, rvalue_hint: false }
}
/// The following explanation is copied straight from rustc:
/// Provides an expectation for an rvalue expression given an *optional*
/// hint, which is not required for type safety (the resulting type might
/// be checked higher up, as is the case with `&expr` and `box expr`), but
/// is useful in determining the concrete type.
///
/// The primary use case is where the expected type is a fat pointer,
/// like `&[isize]`. For example, consider the following statement:
///
/// let x: &[isize] = &[1, 2, 3];
///
/// In this case, the expected type for the `&[1, 2, 3]` expression is
/// `&[isize]`. If however we were to say that `[1, 2, 3]` has the
/// expectation `ExpectHasType([isize])`, that would be too strong --
/// `[1, 2, 3]` does not have the type `[isize]` but rather `[isize; 3]`.
/// It is only the `&[1, 2, 3]` expression as a whole that can be coerced
/// to the type `&[isize]`. Therefore, we propagate this more limited hint,
/// which still is useful, because it informs integer literals and the like.
/// See the test case `test/ui/coerce-expect-unsized.rs` and #20169
/// for examples of where this comes up,.
fn rvalue_hint(ty: Ty) -> Self {
Expectation { ty, rvalue_hint: true }
} }
/// This expresses no expectation on the type. /// This expresses no expectation on the type.
fn none() -> Self { fn none() -> Self {
Expectation { ty: Ty::Unknown } Expectation { ty: Ty::Unknown, rvalue_hint: false }
}
fn coercion_target(&self) -> &Ty {
if self.rvalue_hint {
&Ty::Unknown
} else {
&self.ty
}
} }
} }

View file

@ -42,14 +42,14 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
/// Return the type after possible coercion. /// Return the type after possible coercion.
pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty { pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty {
let ty = self.infer_expr_inner(expr, &expected); let ty = self.infer_expr_inner(expr, &expected);
let ty = if !self.coerce(&ty, &expected.ty) { let ty = if !self.coerce(&ty, &expected.coercion_target()) {
self.result self.result
.type_mismatches .type_mismatches
.insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }); .insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() });
// Return actual type when type mismatch. // Return actual type when type mismatch.
// This is needed for diagnostic when return type mismatch. // This is needed for diagnostic when return type mismatch.
ty ty
} else if expected.ty == Ty::Unknown { } else if expected.coercion_target() == &Ty::Unknown {
ty ty
} else { } else {
expected.ty.clone() expected.ty.clone()
@ -297,7 +297,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
// FIXME: throw type error - expected mut reference but found shared ref, // FIXME: throw type error - expected mut reference but found shared ref,
// which cannot be coerced // which cannot be coerced
} }
Expectation::has_type(Ty::clone(exp_inner)) Expectation::rvalue_hint(Ty::clone(exp_inner))
} else { } else {
Expectation::none() Expectation::none()
}; };
@ -542,7 +542,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
let ty = if let Some(expr) = tail { let ty = if let Some(expr) = tail {
self.infer_expr_coerce(expr, expected) self.infer_expr_coerce(expr, expected)
} else { } else {
self.coerce(&Ty::unit(), &expected.ty); self.coerce(&Ty::unit(), expected.coercion_target());
Ty::unit() Ty::unit()
}; };
if diverges { if diverges {

View file

@ -457,6 +457,37 @@ fn test() {
); );
} }
#[test]
fn coerce_autoderef_block() {
assert_snapshot!(
infer_with_mismatches(r#"
struct String {}
#[lang = "deref"]
trait Deref { type Target; }
impl Deref for String { type Target = str; }
fn takes_ref_str(x: &str) {}
fn returns_string() -> String { loop {} }
fn test() {
takes_ref_str(&{ returns_string() });
}
"#, true),
@r###"
[127; 128) 'x': &str
[136; 138) '{}': ()
[169; 180) '{ loop {} }': String
[171; 178) 'loop {}': !
[176; 178) '{}': ()
[191; 236) '{ ... }); }': ()
[197; 210) 'takes_ref_str': fn takes_ref_str(&str) -> ()
[197; 233) 'takes_...g() })': ()
[211; 232) '&{ ret...ng() }': &String
[212; 232) '{ retu...ng() }': String
[214; 228) 'returns_string': fn returns_string() -> String
[214; 230) 'return...ring()': String
"###
);
}
#[test] #[test]
fn closure_return_coerce() { fn closure_return_coerce() {
assert_snapshot!( assert_snapshot!(

View file

@ -4,8 +4,8 @@
use std::{collections::HashSet, fmt::Write, path::Path, time::Instant}; use std::{collections::HashSet, fmt::Write, path::Path, time::Instant};
use hir::{ use hir::{
db::{DefDatabase, HirDatabase}, db::{AstDatabase, DefDatabase, HirDatabase},
AssocItem, Crate, HasSource, HirDisplay, ModuleDef, original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
}; };
use hir_def::FunctionId; use hir_def::FunctionId;
use hir_ty::{Ty, TypeWalk}; use hir_ty::{Ty, TypeWalk};
@ -188,13 +188,19 @@ pub fn analysis_stats(
let src = sm.expr_syntax(expr_id); let src = sm.expr_syntax(expr_id);
if let Some(src) = src { if let Some(src) = src {
// FIXME: it might be nice to have a function (on Analysis?) that goes from Source<T> -> (LineCol, LineCol) directly // FIXME: it might be nice to have a function (on Analysis?) that goes from Source<T> -> (LineCol, LineCol) directly
let original_file = src.file_id.original_file(db); // But also, we should just turn the type mismatches into diagnostics and provide these
let path = db.file_relative_path(original_file); let root = db.parse_or_expand(src.file_id).unwrap();
let line_index = host.analysis().file_line_index(original_file).unwrap(); let node = src.map(|e| {
let text_range = src.value.either( e.either(
|it| it.syntax_node_ptr().range(), |p| p.to_node(&root).syntax().clone(),
|it| it.syntax_node_ptr().range(), |p| p.to_node(&root).syntax().clone(),
); )
});
let original_range = original_range(db, node.as_ref());
let path = db.file_relative_path(original_range.file_id);
let line_index =
host.analysis().file_line_index(original_range.file_id).unwrap();
let text_range = original_range.range;
let (start, end) = ( let (start, end) = (
line_index.line_col(text_range.start()), line_index.line_col(text_range.start()),
line_index.line_col(text_range.end()), line_index.line_col(text_range.end()),