diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-06-12 22:05:12 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2021-06-12 22:05:12 +0100 |
commit | d6737e55fb49d286b5e646f57975b27b2c95ce92 (patch) | |
tree | 8d79fe8e63f8154e2e331660442aee83d99904a4 /crates/hir_ty | |
parent | 7bbb3e34d5e2ca25b6987bf679cb9962bd0c52b6 (diff) | |
parent | 20487a1b4a7c2fdffdb1de61c7837ee6f673f21a (diff) |
Merge #9239
9239: fix: Fix coercion in match with expected type r=flodiebold a=flodiebold
Plus add infrastructure to test type mismatches without expect.
CC #8961
Co-authored-by: Florian Diebold <[email protected]>
Diffstat (limited to 'crates/hir_ty')
-rw-r--r-- | crates/hir_ty/src/infer.rs | 32 | ||||
-rw-r--r-- | crates/hir_ty/src/infer/expr.rs | 7 | ||||
-rw-r--r-- | crates/hir_ty/src/tests.rs | 98 | ||||
-rw-r--r-- | crates/hir_ty/src/tests/coercion.rs | 43 | ||||
-rw-r--r-- | crates/hir_ty/src/tests/patterns.rs | 57 |
5 files changed, 183 insertions, 54 deletions
diff --git a/crates/hir_ty/src/infer.rs b/crates/hir_ty/src/infer.rs index 2c667da25..f023c1fb7 100644 --- a/crates/hir_ty/src/infer.rs +++ b/crates/hir_ty/src/infer.rs | |||
@@ -761,6 +761,38 @@ impl Expectation { | |||
761 | Expectation::RValueLikeUnsized(_) | Expectation::None => None, | 761 | Expectation::RValueLikeUnsized(_) | Expectation::None => None, |
762 | } | 762 | } |
763 | } | 763 | } |
764 | |||
765 | /// Comment copied from rustc: | ||
766 | /// Disregard "castable to" expectations because they | ||
767 | /// can lead us astray. Consider for example `if cond | ||
768 | /// {22} else {c} as u8` -- if we propagate the | ||
769 | /// "castable to u8" constraint to 22, it will pick the | ||
770 | /// type 22u8, which is overly constrained (c might not | ||
771 | /// be a u8). In effect, the problem is that the | ||
772 | /// "castable to" expectation is not the tightest thing | ||
773 | /// we can say, so we want to drop it in this case. | ||
774 | /// The tightest thing we can say is "must unify with | ||
775 | /// else branch". Note that in the case of a "has type" | ||
776 | /// constraint, this limitation does not hold. | ||
777 | /// | ||
778 | /// If the expected type is just a type variable, then don't use | ||
779 | /// an expected type. Otherwise, we might write parts of the type | ||
780 | /// when checking the 'then' block which are incompatible with the | ||
781 | /// 'else' branch. | ||
782 | fn adjust_for_branches(&self, table: &mut unify::InferenceTable) -> Expectation { | ||
783 | match self { | ||
784 | Expectation::HasType(ety) => { | ||
785 | let ety = table.resolve_ty_shallow(&ety); | ||
786 | if !ety.is_ty_var() { | ||
787 | Expectation::HasType(ety) | ||
788 | } else { | ||
789 | Expectation::None | ||
790 | } | ||
791 | } | ||
792 | Expectation::RValueLikeUnsized(ety) => Expectation::RValueLikeUnsized(ety.clone()), | ||
793 | _ => Expectation::None, | ||
794 | } | ||
795 | } | ||
764 | } | 796 | } |
765 | 797 | ||
766 | #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] | 798 | #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] |
diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs index f73bf43b2..e34f194ff 100644 --- a/crates/hir_ty/src/infer/expr.rs +++ b/crates/hir_ty/src/infer/expr.rs | |||
@@ -337,10 +337,15 @@ impl<'a> InferenceContext<'a> { | |||
337 | Expr::Match { expr, arms } => { | 337 | Expr::Match { expr, arms } => { |
338 | let input_ty = self.infer_expr(*expr, &Expectation::none()); | 338 | let input_ty = self.infer_expr(*expr, &Expectation::none()); |
339 | 339 | ||
340 | let expected = expected.adjust_for_branches(&mut self.table); | ||
341 | |||
340 | let mut result_ty = if arms.is_empty() { | 342 | let mut result_ty = if arms.is_empty() { |
341 | TyKind::Never.intern(&Interner) | 343 | TyKind::Never.intern(&Interner) |
342 | } else { | 344 | } else { |
343 | self.table.new_type_var() | 345 | match &expected { |
346 | Expectation::HasType(ty) => ty.clone(), | ||
347 | _ => self.table.new_type_var(), | ||
348 | } | ||
344 | }; | 349 | }; |
345 | 350 | ||
346 | let matchee_diverges = self.diverges; | 351 | let matchee_diverges = self.diverges; |
diff --git a/crates/hir_ty/src/tests.rs b/crates/hir_ty/src/tests.rs index 9d726b024..b873585c4 100644 --- a/crates/hir_ty/src/tests.rs +++ b/crates/hir_ty/src/tests.rs | |||
@@ -9,7 +9,7 @@ mod macros; | |||
9 | mod display_source_code; | 9 | mod display_source_code; |
10 | mod incremental; | 10 | mod incremental; |
11 | 11 | ||
12 | use std::{env, sync::Arc}; | 12 | use std::{collections::HashMap, env, sync::Arc}; |
13 | 13 | ||
14 | use base_db::{fixture::WithFixture, FileRange, SourceDatabase, SourceDatabaseExt}; | 14 | use base_db::{fixture::WithFixture, FileRange, SourceDatabase, SourceDatabaseExt}; |
15 | use expect_test::Expect; | 15 | use expect_test::Expect; |
@@ -83,9 +83,105 @@ fn check_types_impl(ra_fixture: &str, display_source: bool) { | |||
83 | checked_one = true; | 83 | checked_one = true; |
84 | } | 84 | } |
85 | } | 85 | } |
86 | |||
86 | assert!(checked_one, "no `//^` annotations found"); | 87 | assert!(checked_one, "no `//^` annotations found"); |
87 | } | 88 | } |
88 | 89 | ||
90 | fn check_no_mismatches(ra_fixture: &str) { | ||
91 | check_mismatches_impl(ra_fixture, true) | ||
92 | } | ||
93 | |||
94 | #[allow(unused)] | ||
95 | fn check_mismatches(ra_fixture: &str) { | ||
96 | check_mismatches_impl(ra_fixture, false) | ||
97 | } | ||
98 | |||
99 | fn check_mismatches_impl(ra_fixture: &str, allow_none: bool) { | ||
100 | let _tracing = setup_tracing(); | ||
101 | let (db, file_id) = TestDB::with_single_file(ra_fixture); | ||
102 | let module = db.module_for_file(file_id); | ||
103 | let def_map = module.def_map(&db); | ||
104 | |||
105 | let mut defs: Vec<DefWithBodyId> = Vec::new(); | ||
106 | visit_module(&db, &def_map, module.local_id, &mut |it| defs.push(it)); | ||
107 | defs.sort_by_key(|def| match def { | ||
108 | DefWithBodyId::FunctionId(it) => { | ||
109 | let loc = it.lookup(&db); | ||
110 | loc.source(&db).value.syntax().text_range().start() | ||
111 | } | ||
112 | DefWithBodyId::ConstId(it) => { | ||
113 | let loc = it.lookup(&db); | ||
114 | loc.source(&db).value.syntax().text_range().start() | ||
115 | } | ||
116 | DefWithBodyId::StaticId(it) => { | ||
117 | let loc = it.lookup(&db); | ||
118 | loc.source(&db).value.syntax().text_range().start() | ||
119 | } | ||
120 | }); | ||
121 | let mut mismatches = HashMap::new(); | ||
122 | let mut push_mismatch = |src_ptr: InFile<SyntaxNode>, mismatch: TypeMismatch| { | ||
123 | let range = src_ptr.value.text_range(); | ||
124 | if src_ptr.file_id.call_node(&db).is_some() { | ||
125 | panic!("type mismatch in macro expansion"); | ||
126 | } | ||
127 | let file_range = FileRange { file_id: src_ptr.file_id.original_file(&db), range }; | ||
128 | let actual = format!( | ||
129 | "expected {}, got {}", | ||
130 | mismatch.expected.display_test(&db), | ||
131 | mismatch.actual.display_test(&db) | ||
132 | ); | ||
133 | mismatches.insert(file_range, actual); | ||
134 | }; | ||
135 | for def in defs { | ||
136 | let (_body, body_source_map) = db.body_with_source_map(def); | ||
137 | let inference_result = db.infer(def); | ||
138 | for (pat, mismatch) in inference_result.pat_type_mismatches() { | ||
139 | let syntax_ptr = match body_source_map.pat_syntax(pat) { | ||
140 | Ok(sp) => { | ||
141 | let root = db.parse_or_expand(sp.file_id).unwrap(); | ||
142 | sp.map(|ptr| { | ||
143 | ptr.either( | ||
144 | |it| it.to_node(&root).syntax().clone(), | ||
145 | |it| it.to_node(&root).syntax().clone(), | ||
146 | ) | ||
147 | }) | ||
148 | } | ||
149 | Err(SyntheticSyntax) => continue, | ||
150 | }; | ||
151 | push_mismatch(syntax_ptr, mismatch.clone()); | ||
152 | } | ||
153 | for (expr, mismatch) in inference_result.expr_type_mismatches() { | ||
154 | let node = match body_source_map.expr_syntax(expr) { | ||
155 | Ok(sp) => { | ||
156 | let root = db.parse_or_expand(sp.file_id).unwrap(); | ||
157 | sp.map(|ptr| ptr.to_node(&root).syntax().clone()) | ||
158 | } | ||
159 | Err(SyntheticSyntax) => continue, | ||
160 | }; | ||
161 | push_mismatch(node, mismatch.clone()); | ||
162 | } | ||
163 | } | ||
164 | let mut checked_one = false; | ||
165 | for (file_id, annotations) in db.extract_annotations() { | ||
166 | for (range, expected) in annotations { | ||
167 | let file_range = FileRange { file_id, range }; | ||
168 | if let Some(mismatch) = mismatches.remove(&file_range) { | ||
169 | assert_eq!(mismatch, expected); | ||
170 | } else { | ||
171 | assert!(false, "Expected mismatch not encountered: {}\n", expected); | ||
172 | } | ||
173 | checked_one = true; | ||
174 | } | ||
175 | } | ||
176 | let mut buf = String::new(); | ||
177 | for (range, mismatch) in mismatches { | ||
178 | format_to!(buf, "{:?}: {}\n", range.range, mismatch,); | ||
179 | } | ||
180 | assert!(buf.is_empty(), "Unexpected type mismatches:\n{}", buf); | ||
181 | |||
182 | assert!(checked_one || allow_none, "no `//^` annotations found"); | ||
183 | } | ||
184 | |||
89 | fn type_at_range(db: &TestDB, pos: FileRange) -> Ty { | 185 | fn type_at_range(db: &TestDB, pos: FileRange) -> Ty { |
90 | let file = db.parse(pos.file_id).ok().unwrap(); | 186 | let file = db.parse(pos.file_id).ok().unwrap(); |
91 | let expr = algo::find_node_at_range::<ast::Expr>(file.syntax(), pos.range).unwrap(); | 187 | let expr = algo::find_node_at_range::<ast::Expr>(file.syntax(), pos.range).unwrap(); |
diff --git a/crates/hir_ty/src/tests/coercion.rs b/crates/hir_ty/src/tests/coercion.rs index 6dac7e103..71047703d 100644 --- a/crates/hir_ty/src/tests/coercion.rs +++ b/crates/hir_ty/src/tests/coercion.rs | |||
@@ -1,6 +1,6 @@ | |||
1 | use expect_test::expect; | 1 | use expect_test::expect; |
2 | 2 | ||
3 | use super::{check_infer, check_infer_with_mismatches, check_types}; | 3 | use super::{check_infer, check_infer_with_mismatches, check_no_mismatches, check_types}; |
4 | 4 | ||
5 | #[test] | 5 | #[test] |
6 | fn infer_block_expr_type_mismatch() { | 6 | fn infer_block_expr_type_mismatch() { |
@@ -963,7 +963,7 @@ fn test() -> i32 { | |||
963 | 963 | ||
964 | #[test] | 964 | #[test] |
965 | fn panic_macro() { | 965 | fn panic_macro() { |
966 | check_infer_with_mismatches( | 966 | check_no_mismatches( |
967 | r#" | 967 | r#" |
968 | mod panic { | 968 | mod panic { |
969 | #[macro_export] | 969 | #[macro_export] |
@@ -991,15 +991,34 @@ fn main() { | |||
991 | panic!() | 991 | panic!() |
992 | } | 992 | } |
993 | "#, | 993 | "#, |
994 | expect![[r#" | 994 | ); |
995 | 174..185 '{ loop {} }': ! | 995 | } |
996 | 176..183 'loop {}': ! | 996 | |
997 | 181..183 '{}': () | 997 | #[test] |
998 | !0..24 '$crate...:panic': fn panic() -> ! | 998 | fn coerce_unsize_expected_type() { |
999 | !0..26 '$crate...anic()': ! | 999 | check_no_mismatches( |
1000 | !0..26 '$crate...anic()': ! | 1000 | r#" |
1001 | !0..28 '$crate...015!()': ! | 1001 | #[lang = "sized"] |
1002 | 454..470 '{ ...c!() }': () | 1002 | pub trait Sized {} |
1003 | "#]], | 1003 | #[lang = "unsize"] |
1004 | pub trait Unsize<T> {} | ||
1005 | #[lang = "coerce_unsized"] | ||
1006 | pub trait CoerceUnsized<T> {} | ||
1007 | |||
1008 | impl<T: Unsize<U>, U> CoerceUnsized<&U> for &T {} | ||
1009 | |||
1010 | fn main() { | ||
1011 | let foo: &[u32] = &[1, 2]; | ||
1012 | let foo: &[u32] = match true { | ||
1013 | true => &[1, 2], | ||
1014 | false => &[1, 2, 3], | ||
1015 | }; | ||
1016 | let foo: &[u32] = if true { | ||
1017 | &[1, 2] | ||
1018 | } else { | ||
1019 | &[1, 2, 3] | ||
1020 | }; | ||
1021 | } | ||
1022 | "#, | ||
1004 | ); | 1023 | ); |
1005 | } | 1024 | } |
diff --git a/crates/hir_ty/src/tests/patterns.rs b/crates/hir_ty/src/tests/patterns.rs index 7d00cee9b..aa513c56d 100644 --- a/crates/hir_ty/src/tests/patterns.rs +++ b/crates/hir_ty/src/tests/patterns.rs | |||
@@ -1,6 +1,6 @@ | |||
1 | use expect_test::expect; | 1 | use expect_test::expect; |
2 | 2 | ||
3 | use super::{check_infer, check_infer_with_mismatches, check_types}; | 3 | use super::{check_infer, check_infer_with_mismatches, check_mismatches, check_types}; |
4 | 4 | ||
5 | #[test] | 5 | #[test] |
6 | fn infer_pattern() { | 6 | fn infer_pattern() { |
@@ -518,47 +518,24 @@ fn infer_generics_in_patterns() { | |||
518 | 518 | ||
519 | #[test] | 519 | #[test] |
520 | fn infer_const_pattern() { | 520 | fn infer_const_pattern() { |
521 | check_infer_with_mismatches( | 521 | check_mismatches( |
522 | r#" | 522 | r#" |
523 | enum Option<T> { None } | 523 | enum Option<T> { None } |
524 | use Option::None; | 524 | use Option::None; |
525 | struct Foo; | 525 | struct Foo; |
526 | const Bar: usize = 1; | 526 | const Bar: usize = 1; |
527 | 527 | ||
528 | fn test() { | 528 | fn test() { |
529 | let a: Option<u32> = None; | 529 | let a: Option<u32> = None; |
530 | let b: Option<i64> = match a { | 530 | let b: Option<i64> = match a { |
531 | None => None, | 531 | None => None, |
532 | }; | 532 | }; |
533 | let _: () = match () { Foo => Foo }; // Expected mismatch | 533 | let _: () = match () { Foo => () }; |
534 | let _: () = match () { Bar => Bar }; // Expected mismatch | 534 | // ^^^ expected (), got Foo |
535 | } | 535 | let _: () = match () { Bar => () }; |
536 | // ^^^ expected (), got usize | ||
537 | } | ||
536 | "#, | 538 | "#, |
537 | expect![[r#" | ||
538 | 73..74 '1': usize | ||
539 | 87..309 '{ ...atch }': () | ||
540 | 97..98 'a': Option<u32> | ||
541 | 114..118 'None': Option<u32> | ||
542 | 128..129 'b': Option<i64> | ||
543 | 145..182 'match ... }': Option<i64> | ||
544 | 151..152 'a': Option<u32> | ||
545 | 163..167 'None': Option<u32> | ||
546 | 171..175 'None': Option<i64> | ||
547 | 192..193 '_': () | ||
548 | 200..223 'match ... Foo }': Foo | ||
549 | 206..208 '()': () | ||
550 | 211..214 'Foo': Foo | ||
551 | 218..221 'Foo': Foo | ||
552 | 254..255 '_': () | ||
553 | 262..285 'match ... Bar }': usize | ||
554 | 268..270 '()': () | ||
555 | 273..276 'Bar': usize | ||
556 | 280..283 'Bar': usize | ||
557 | 200..223: expected (), got Foo | ||
558 | 211..214: expected (), got Foo | ||
559 | 262..285: expected (), got usize | ||
560 | 273..276: expected (), got usize | ||
561 | "#]], | ||
562 | ); | 539 | ); |
563 | } | 540 | } |
564 | 541 | ||