diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-02-29 15:27:53 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2020-02-29 15:27:53 +0000 |
commit | 0ae7054210b0bbc48ea51c3672be640d3096cfdd (patch) | |
tree | 51599b3664e16cd5f0ef288dac924330c5cd0af0 /crates | |
parent | 0ec7f760fcd8bb9c2273e004468faa2a8cbeb29d (diff) | |
parent | 5fe220b9873d587188adae63fa205481a9aae9ce (diff) |
Merge #3376
3376: Fix a common false-positive type mismatch r=matklad a=flodiebold
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.
Co-authored-by: Florian Diebold <[email protected]>
Diffstat (limited to 'crates')
-rw-r--r-- | crates/ra_hir_ty/src/infer.rs | 41 | ||||
-rw-r--r-- | crates/ra_hir_ty/src/infer/expr.rs | 8 | ||||
-rw-r--r-- | crates/ra_hir_ty/src/tests/coercion.rs | 31 | ||||
-rw-r--r-- | 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 { | |||
583 | #[derive(Clone, PartialEq, Eq, Debug)] | 583 | #[derive(Clone, PartialEq, Eq, Debug)] |
584 | struct Expectation { | 584 | struct Expectation { |
585 | ty: Ty, | 585 | ty: Ty, |
586 | // FIXME: In some cases, we need to be aware whether the expectation is that | 586 | /// See the `rvalue_hint` method. |
587 | // the type match exactly what we passed, or whether it just needs to be | 587 | rvalue_hint: bool, |
588 | // coercible to the expected type. See Expectation::rvalue_hint in rustc. | ||
589 | } | 588 | } |
590 | 589 | ||
591 | impl Expectation { | 590 | impl Expectation { |
592 | /// The expectation that the type of the expression needs to equal the given | 591 | /// The expectation that the type of the expression needs to equal the given |
593 | /// type. | 592 | /// type. |
594 | fn has_type(ty: Ty) -> Self { | 593 | fn has_type(ty: Ty) -> Self { |
595 | Expectation { ty } | 594 | Expectation { ty, rvalue_hint: false } |
595 | } | ||
596 | |||
597 | /// The following explanation is copied straight from rustc: | ||
598 | /// Provides an expectation for an rvalue expression given an *optional* | ||
599 | /// hint, which is not required for type safety (the resulting type might | ||
600 | /// be checked higher up, as is the case with `&expr` and `box expr`), but | ||
601 | /// is useful in determining the concrete type. | ||
602 | /// | ||
603 | /// The primary use case is where the expected type is a fat pointer, | ||
604 | /// like `&[isize]`. For example, consider the following statement: | ||
605 | /// | ||
606 | /// let x: &[isize] = &[1, 2, 3]; | ||
607 | /// | ||
608 | /// In this case, the expected type for the `&[1, 2, 3]` expression is | ||
609 | /// `&[isize]`. If however we were to say that `[1, 2, 3]` has the | ||
610 | /// expectation `ExpectHasType([isize])`, that would be too strong -- | ||
611 | /// `[1, 2, 3]` does not have the type `[isize]` but rather `[isize; 3]`. | ||
612 | /// It is only the `&[1, 2, 3]` expression as a whole that can be coerced | ||
613 | /// to the type `&[isize]`. Therefore, we propagate this more limited hint, | ||
614 | /// which still is useful, because it informs integer literals and the like. | ||
615 | /// See the test case `test/ui/coerce-expect-unsized.rs` and #20169 | ||
616 | /// for examples of where this comes up,. | ||
617 | fn rvalue_hint(ty: Ty) -> Self { | ||
618 | Expectation { ty, rvalue_hint: true } | ||
596 | } | 619 | } |
597 | 620 | ||
598 | /// This expresses no expectation on the type. | 621 | /// This expresses no expectation on the type. |
599 | fn none() -> Self { | 622 | fn none() -> Self { |
600 | Expectation { ty: Ty::Unknown } | 623 | Expectation { ty: Ty::Unknown, rvalue_hint: false } |
624 | } | ||
625 | |||
626 | fn coercion_target(&self) -> &Ty { | ||
627 | if self.rvalue_hint { | ||
628 | &Ty::Unknown | ||
629 | } else { | ||
630 | &self.ty | ||
631 | } | ||
601 | } | 632 | } |
602 | } | 633 | } |
603 | 634 | ||
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> { | |||
42 | /// Return the type after possible coercion. | 42 | /// Return the type after possible coercion. |
43 | pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty { | 43 | pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty { |
44 | let ty = self.infer_expr_inner(expr, &expected); | 44 | let ty = self.infer_expr_inner(expr, &expected); |
45 | let ty = if !self.coerce(&ty, &expected.ty) { | 45 | let ty = if !self.coerce(&ty, &expected.coercion_target()) { |
46 | self.result | 46 | self.result |
47 | .type_mismatches | 47 | .type_mismatches |
48 | .insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }); | 48 | .insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }); |
49 | // Return actual type when type mismatch. | 49 | // Return actual type when type mismatch. |
50 | // This is needed for diagnostic when return type mismatch. | 50 | // This is needed for diagnostic when return type mismatch. |
51 | ty | 51 | ty |
52 | } else if expected.ty == Ty::Unknown { | 52 | } else if expected.coercion_target() == &Ty::Unknown { |
53 | ty | 53 | ty |
54 | } else { | 54 | } else { |
55 | expected.ty.clone() | 55 | expected.ty.clone() |
@@ -297,7 +297,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
297 | // FIXME: throw type error - expected mut reference but found shared ref, | 297 | // FIXME: throw type error - expected mut reference but found shared ref, |
298 | // which cannot be coerced | 298 | // which cannot be coerced |
299 | } | 299 | } |
300 | Expectation::has_type(Ty::clone(exp_inner)) | 300 | Expectation::rvalue_hint(Ty::clone(exp_inner)) |
301 | } else { | 301 | } else { |
302 | Expectation::none() | 302 | Expectation::none() |
303 | }; | 303 | }; |
@@ -542,7 +542,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
542 | let ty = if let Some(expr) = tail { | 542 | let ty = if let Some(expr) = tail { |
543 | self.infer_expr_coerce(expr, expected) | 543 | self.infer_expr_coerce(expr, expected) |
544 | } else { | 544 | } else { |
545 | self.coerce(&Ty::unit(), &expected.ty); | 545 | self.coerce(&Ty::unit(), expected.coercion_target()); |
546 | Ty::unit() | 546 | Ty::unit() |
547 | }; | 547 | }; |
548 | if diverges { | 548 | 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 | |||
@@ -458,6 +458,37 @@ fn test() { | |||
458 | } | 458 | } |
459 | 459 | ||
460 | #[test] | 460 | #[test] |
461 | fn coerce_autoderef_block() { | ||
462 | assert_snapshot!( | ||
463 | infer_with_mismatches(r#" | ||
464 | struct String {} | ||
465 | #[lang = "deref"] | ||
466 | trait Deref { type Target; } | ||
467 | impl Deref for String { type Target = str; } | ||
468 | fn takes_ref_str(x: &str) {} | ||
469 | fn returns_string() -> String { loop {} } | ||
470 | fn test() { | ||
471 | takes_ref_str(&{ returns_string() }); | ||
472 | } | ||
473 | "#, true), | ||
474 | @r###" | ||
475 | [127; 128) 'x': &str | ||
476 | [136; 138) '{}': () | ||
477 | [169; 180) '{ loop {} }': String | ||
478 | [171; 178) 'loop {}': ! | ||
479 | [176; 178) '{}': () | ||
480 | [191; 236) '{ ... }); }': () | ||
481 | [197; 210) 'takes_ref_str': fn takes_ref_str(&str) -> () | ||
482 | [197; 233) 'takes_...g() })': () | ||
483 | [211; 232) '&{ ret...ng() }': &String | ||
484 | [212; 232) '{ retu...ng() }': String | ||
485 | [214; 228) 'returns_string': fn returns_string() -> String | ||
486 | [214; 230) 'return...ring()': String | ||
487 | "### | ||
488 | ); | ||
489 | } | ||
490 | |||
491 | #[test] | ||
461 | fn closure_return_coerce() { | 492 | fn closure_return_coerce() { |
462 | assert_snapshot!( | 493 | assert_snapshot!( |
463 | infer_with_mismatches(r#" | 494 | infer_with_mismatches(r#" |
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 @@ | |||
4 | use std::{collections::HashSet, fmt::Write, path::Path, time::Instant}; | 4 | use std::{collections::HashSet, fmt::Write, path::Path, time::Instant}; |
5 | 5 | ||
6 | use hir::{ | 6 | use hir::{ |
7 | db::{DefDatabase, HirDatabase}, | 7 | db::{AstDatabase, DefDatabase, HirDatabase}, |
8 | AssocItem, Crate, HasSource, HirDisplay, ModuleDef, | 8 | original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef, |
9 | }; | 9 | }; |
10 | use hir_def::FunctionId; | 10 | use hir_def::FunctionId; |
11 | use hir_ty::{Ty, TypeWalk}; | 11 | use hir_ty::{Ty, TypeWalk}; |
@@ -188,13 +188,19 @@ pub fn analysis_stats( | |||
188 | let src = sm.expr_syntax(expr_id); | 188 | let src = sm.expr_syntax(expr_id); |
189 | if let Some(src) = src { | 189 | if let Some(src) = src { |
190 | // FIXME: it might be nice to have a function (on Analysis?) that goes from Source<T> -> (LineCol, LineCol) directly | 190 | // FIXME: it might be nice to have a function (on Analysis?) that goes from Source<T> -> (LineCol, LineCol) directly |
191 | let original_file = src.file_id.original_file(db); | 191 | // But also, we should just turn the type mismatches into diagnostics and provide these |
192 | let path = db.file_relative_path(original_file); | 192 | let root = db.parse_or_expand(src.file_id).unwrap(); |
193 | let line_index = host.analysis().file_line_index(original_file).unwrap(); | 193 | let node = src.map(|e| { |
194 | let text_range = src.value.either( | 194 | e.either( |
195 | |it| it.syntax_node_ptr().range(), | 195 | |p| p.to_node(&root).syntax().clone(), |
196 | |it| it.syntax_node_ptr().range(), | 196 | |p| p.to_node(&root).syntax().clone(), |
197 | ); | 197 | ) |
198 | }); | ||
199 | let original_range = original_range(db, node.as_ref()); | ||
200 | let path = db.file_relative_path(original_range.file_id); | ||
201 | let line_index = | ||
202 | host.analysis().file_line_index(original_range.file_id).unwrap(); | ||
203 | let text_range = original_range.range; | ||
198 | let (start, end) = ( | 204 | let (start, end) = ( |
199 | line_index.line_col(text_range.start()), | 205 | line_index.line_col(text_range.start()), |
200 | line_index.line_col(text_range.end()), | 206 | line_index.line_col(text_range.end()), |