Skip to content

Commit 5fe220b

Browse files
committed
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.
1 parent 0ec7f76 commit 5fe220b

File tree

4 files changed

+86
-18
lines changed

4 files changed

+86
-18
lines changed

crates/ra_hir_ty/src/infer.rs

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -583,21 +583,52 @@ impl InferTy {
583583
#[derive(Clone, PartialEq, Eq, Debug)]
584584
struct Expectation {
585585
ty: Ty,
586-
// FIXME: In some cases, we need to be aware whether the expectation is that
587-
// the type match exactly what we passed, or whether it just needs to be
588-
// coercible to the expected type. See Expectation::rvalue_hint in rustc.
586+
/// See the `rvalue_hint` method.
587+
rvalue_hint: bool,
589588
}
590589

591590
impl Expectation {
592591
/// The expectation that the type of the expression needs to equal the given
593592
/// type.
594593
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 }
596619
}
597620

598621
/// This expresses no expectation on the type.
599622
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+
}
601632
}
602633
}
603634

crates/ra_hir_ty/src/infer/expr.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
4242
/// Return the type after possible coercion.
4343
pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty {
4444
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()) {
4646
self.result
4747
.type_mismatches
4848
.insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() });
4949
// Return actual type when type mismatch.
5050
// This is needed for diagnostic when return type mismatch.
5151
ty
52-
} else if expected.ty == Ty::Unknown {
52+
} else if expected.coercion_target() == &Ty::Unknown {
5353
ty
5454
} else {
5555
expected.ty.clone()
@@ -297,7 +297,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
297297
// FIXME: throw type error - expected mut reference but found shared ref,
298298
// which cannot be coerced
299299
}
300-
Expectation::has_type(Ty::clone(exp_inner))
300+
Expectation::rvalue_hint(Ty::clone(exp_inner))
301301
} else {
302302
Expectation::none()
303303
};
@@ -542,7 +542,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
542542
let ty = if let Some(expr) = tail {
543543
self.infer_expr_coerce(expr, expected)
544544
} else {
545-
self.coerce(&Ty::unit(), &expected.ty);
545+
self.coerce(&Ty::unit(), expected.coercion_target());
546546
Ty::unit()
547547
};
548548
if diverges {

crates/ra_hir_ty/src/tests/coercion.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,37 @@ fn test() {
457457
);
458458
}
459459

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+
460491
#[test]
461492
fn closure_return_coerce() {
462493
assert_snapshot!(

crates/rust-analyzer/src/cli/analysis_stats.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
use std::{collections::HashSet, fmt::Write, path::Path, time::Instant};
55

66
use hir::{
7-
db::{DefDatabase, HirDatabase},
8-
AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
7+
db::{AstDatabase, DefDatabase, HirDatabase},
8+
original_range, AssocItem, Crate, HasSource, HirDisplay, ModuleDef,
99
};
1010
use hir_def::FunctionId;
1111
use hir_ty::{Ty, TypeWalk};
@@ -188,13 +188,19 @@ pub fn analysis_stats(
188188
let src = sm.expr_syntax(expr_id);
189189
if let Some(src) = src {
190190
// 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);
192-
let path = db.file_relative_path(original_file);
193-
let line_index = host.analysis().file_line_index(original_file).unwrap();
194-
let text_range = src.value.either(
195-
|it| it.syntax_node_ptr().range(),
196-
|it| it.syntax_node_ptr().range(),
197-
);
191+
// But also, we should just turn the type mismatches into diagnostics and provide these
192+
let root = db.parse_or_expand(src.file_id).unwrap();
193+
let node = src.map(|e| {
194+
e.either(
195+
|p| p.to_node(&root).syntax().clone(),
196+
|p| p.to_node(&root).syntax().clone(),
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;
198204
let (start, end) = (
199205
line_index.line_col(text_range.start()),
200206
line_index.line_col(text_range.end()),

0 commit comments

Comments
 (0)