From f86fe3d891ab295e9e394a1338da86524a6205d3 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 5 Dec 2019 23:02:31 +0100 Subject: [PATCH] Don't unify within a reference If we are expecting a `&Foo` and get a `&something`, when checking the `something`, we are *expecting* a `Foo`, but we shouldn't try to unify whatever we get with that expectation, because it could actually be a `&Foo`, and `&&Foo` coerces to `&Foo`. So this fixes quite a few false type mismatches. --- crates/ra_hir_ty/src/infer/expr.rs | 21 +++++++-------- crates/ra_hir_ty/src/tests.rs | 26 +++++++++++++++++++ crates/ra_hir_ty/src/tests/coercion.rs | 36 ++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index 1e78f6efd4..b8df277063 100644 --- a/crates/ra_hir_ty/src/infer/expr.rs +++ b/crates/ra_hir_ty/src/infer/expr.rs @@ -201,7 +201,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { } Expr::Return { expr } => { if let Some(expr) = expr { - self.infer_expr(*expr, &Expectation::has_type(self.return_ty.clone())); + self.infer_expr_coerce(*expr, &Expectation::has_type(self.return_ty.clone())); } Ty::simple(TypeCtor::Never) } @@ -245,7 +245,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { ty } Expr::Field { expr, name } => { - let receiver_ty = self.infer_expr(*expr, &Expectation::none()); + let receiver_ty = self.infer_expr_inner(*expr, &Expectation::none()); let canonicalized = self.canonicalizer().canonicalize_ty(receiver_ty); let ty = autoderef::autoderef( self.db, @@ -280,7 +280,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { self.normalize_associated_types_in(ty) } Expr::Await { expr } => { - let inner_ty = self.infer_expr(*expr, &Expectation::none()); + let inner_ty = self.infer_expr_inner(*expr, &Expectation::none()); let ty = match self.resolve_future_future_output() { Some(future_future_output_alias) => { let ty = self.table.new_type_var(); @@ -299,7 +299,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { ty } Expr::Try { expr } => { - let inner_ty = self.infer_expr(*expr, &Expectation::none()); + let inner_ty = self.infer_expr_inner(*expr, &Expectation::none()); let ty = match self.resolve_ops_try_ok() { Some(ops_try_ok_alias) => { let ty = self.table.new_type_var(); @@ -318,7 +318,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { ty } Expr::Cast { expr, type_ref } => { - let _inner_ty = self.infer_expr(*expr, &Expectation::none()); + let _inner_ty = self.infer_expr_inner(*expr, &Expectation::none()); let cast_ty = self.make_ty(type_ref); // FIXME check the cast... cast_ty @@ -334,12 +334,11 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { } else { Expectation::none() }; - // FIXME reference coercions etc. - let inner_ty = self.infer_expr(*expr, &expectation); + let inner_ty = self.infer_expr_inner(*expr, &expectation); Ty::apply_one(TypeCtor::Ref(*mutability), inner_ty) } Expr::Box { expr } => { - let inner_ty = self.infer_expr(*expr, &Expectation::none()); + let inner_ty = self.infer_expr_inner(*expr, &Expectation::none()); if let Some(box_) = self.resolve_boxed_box() { Ty::apply_one(TypeCtor::Adt(box_), inner_ty) } else { @@ -347,7 +346,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { } } Expr::UnaryOp { expr, op } => { - let inner_ty = self.infer_expr(*expr, &Expectation::none()); + let inner_ty = self.infer_expr_inner(*expr, &Expectation::none()); match op { UnaryOp::Deref => match self.resolver.krate() { Some(krate) => { @@ -417,7 +416,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { _ => Ty::Unknown, }, Expr::Range { lhs, rhs, range_type } => { - let lhs_ty = lhs.map(|e| self.infer_expr(e, &Expectation::none())); + let lhs_ty = lhs.map(|e| self.infer_expr_inner(e, &Expectation::none())); let rhs_expect = lhs_ty .as_ref() .map_or_else(Expectation::none, |ty| Expectation::has_type(ty.clone())); @@ -455,7 +454,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { } } Expr::Index { base, index } => { - let _base_ty = self.infer_expr(*base, &Expectation::none()); + let _base_ty = self.infer_expr_inner(*base, &Expectation::none()); let _index_ty = self.infer_expr(*index, &Expectation::none()); // FIXME: use `std::ops::Index::Output` to figure out the real return type Ty::Unknown diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 9f373a8f41..f1b67555f2 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -50,6 +50,10 @@ fn type_at(content: &str) -> String { } fn infer(content: &str) -> String { + infer_with_mismatches(content, false) +} + +fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String { let (db, file_id) = TestDB::with_single_file(content); let mut acc = String::new(); @@ -57,6 +61,7 @@ fn infer(content: &str) -> String { let mut infer_def = |inference_result: Arc, body_source_map: Arc| { let mut types = Vec::new(); + let mut mismatches = Vec::new(); for (pat, ty) in inference_result.type_of_pat.iter() { let syntax_ptr = match body_source_map.pat_syntax(pat) { @@ -76,6 +81,9 @@ fn infer(content: &str) -> String { None => continue, }; types.push((syntax_ptr, ty)); + if let Some(mismatch) = inference_result.type_mismatch_for_expr(expr) { + mismatches.push((syntax_ptr, mismatch)); + } } // sort ranges for consistency @@ -101,6 +109,24 @@ fn infer(content: &str) -> String { ) .unwrap(); } + if include_mismatches { + mismatches.sort_by_key(|(src_ptr, _)| { + (src_ptr.value.range().start(), src_ptr.value.range().end()) + }); + for (src_ptr, mismatch) in &mismatches { + let range = src_ptr.value.range(); + let macro_prefix = if src_ptr.file_id != file_id.into() { "!" } else { "" }; + write!( + acc, + "{}{}: expected {}, got {}\n", + macro_prefix, + range, + mismatch.expected.display(&db), + mismatch.actual.display(&db), + ) + .unwrap(); + } + } }; let module = db.module_for_file(file_id); diff --git a/crates/ra_hir_ty/src/tests/coercion.rs b/crates/ra_hir_ty/src/tests/coercion.rs index 1530fcc637..58b22396fc 100644 --- a/crates/ra_hir_ty/src/tests/coercion.rs +++ b/crates/ra_hir_ty/src/tests/coercion.rs @@ -1,3 +1,4 @@ +use super::infer_with_mismatches; use insta::assert_snapshot; use test_utils::covers; @@ -367,3 +368,38 @@ fn test() { "### ); } + +#[test] +fn coerce_autoderef() { + assert_snapshot!( + infer_with_mismatches(r#" +struct Foo; +fn takes_ref_foo(x: &Foo) {} +fn test() { + takes_ref_foo(&Foo); + takes_ref_foo(&&Foo); + takes_ref_foo(&&&Foo); +} +"#, true), + @r###" + [30; 31) 'x': &Foo + [39; 41) '{}': () + [52; 133) '{ ...oo); }': () + [58; 71) 'takes_ref_foo': fn takes_ref_foo(&Foo) -> () + [58; 77) 'takes_...(&Foo)': () + [72; 76) '&Foo': &Foo + [73; 76) 'Foo': Foo + [83; 96) 'takes_ref_foo': fn takes_ref_foo(&Foo) -> () + [83; 103) 'takes_...&&Foo)': () + [97; 102) '&&Foo': &&Foo + [98; 102) '&Foo': &Foo + [99; 102) 'Foo': Foo + [109; 122) 'takes_ref_foo': fn takes_ref_foo(&Foo) -> () + [109; 130) 'takes_...&&Foo)': () + [123; 129) '&&&Foo': &&&Foo + [124; 129) '&&Foo': &&Foo + [125; 129) '&Foo': &Foo + [126; 129) 'Foo': Foo + "### + ); +}