From 5fe220b9873d587188adae63fa205481a9aae9ce Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sat, 29 Feb 2020 15:31:07 +0100 Subject: 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. --- crates/ra_hir_ty/src/infer.rs | 41 ++++++++++++++++++++++---- crates/ra_hir_ty/src/infer/expr.rs | 8 ++--- crates/ra_hir_ty/src/tests/coercion.rs | 31 +++++++++++++++++++ crates/rust-analyzer/src/cli/analysis_stats.rs | 24 +++++++++------ 4 files changed, 86 insertions(+), 18 deletions(-) diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs index 6e1d268de..569d46cc3 100644 --- a/crates/ra_hir_ty/src/infer.rs +++ b/crates/ra_hir_ty/src/infer.rs @@ -583,21 +583,52 @@ impl InferTy { #[derive(Clone, PartialEq, Eq, Debug)] struct Expectation { ty: Ty, - // FIXME: In some cases, we need to be aware whether the expectation is that - // the type match exactly what we passed, or whether it just needs to be - // coercible to the expected type. See Expectation::rvalue_hint in rustc. + /// See the `rvalue_hint` method. + rvalue_hint: bool, } impl Expectation { /// The expectation that the type of the expression needs to equal the given /// type. 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. 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 + } } } diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index 9d5f75625..3db5b2b51 100644 --- a/crates/ra_hir_ty/src/infer/expr.rs +++ b/crates/ra_hir_ty/src/infer/expr.rs @@ -42,14 +42,14 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { /// Return the type after possible coercion. pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty { 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 .type_mismatches .insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }); // Return actual type when type mismatch. // This is needed for diagnostic when return type mismatch. ty - } else if expected.ty == Ty::Unknown { + } else if expected.coercion_target() == &Ty::Unknown { ty } else { 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, // which cannot be coerced } - Expectation::has_type(Ty::clone(exp_inner)) + Expectation::rvalue_hint(Ty::clone(exp_inner)) } else { Expectation::none() }; @@ -542,7 +542,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { let ty = if let Some(expr) = tail { self.infer_expr_coerce(expr, expected) } else { - self.coerce(&Ty::unit(), &expected.ty); + self.coerce(&Ty::unit(), expected.coercion_target()); Ty::unit() }; if diverges { diff --git a/crates/ra_hir_ty/src/tests/coercion.rs b/crates/ra_hir_ty/src/tests/coercion.rs index 60ad6e9be..1e303f5ce 100644 --- a/crates/ra_hir_ty/src/tests/coercion.rs +++ b/crates/ra_hir_ty/src/tests/coercion.rs @@ -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] fn closure_return_coerce() { assert_snapshot!( diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index 4d59db1ee..d70d34bdc 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -4,8 +4,8 @@ use std::{collections::HashSet, fmt::Write, path::Path, time::Instant}; use hir::{ - db::{DefDatabase, HirDatabase}, - AssocItem, Crate, HasSource, HirDisplay, ModuleDef, + db::{AstDatabase, DefDatabase, HirDatabase}, + original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef, }; use hir_def::FunctionId; use hir_ty::{Ty, TypeWalk}; @@ -188,13 +188,19 @@ pub fn analysis_stats( let src = sm.expr_syntax(expr_id); if let Some(src) = src { // FIXME: it might be nice to have a function (on Analysis?) that goes from Source -> (LineCol, LineCol) directly - let original_file = src.file_id.original_file(db); - let path = db.file_relative_path(original_file); - let line_index = host.analysis().file_line_index(original_file).unwrap(); - let text_range = src.value.either( - |it| it.syntax_node_ptr().range(), - |it| it.syntax_node_ptr().range(), - ); + // But also, we should just turn the type mismatches into diagnostics and provide these + let root = db.parse_or_expand(src.file_id).unwrap(); + let node = src.map(|e| { + e.either( + |p| p.to_node(&root).syntax().clone(), + |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) = ( line_index.line_col(text_range.start()), line_index.line_col(text_range.end()), -- cgit v1.2.3