diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-05-19 18:46:38 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2021-05-19 18:46:38 +0100 |
commit | c7196620abd5e9bab4fbd53388da361f0f6987a1 (patch) | |
tree | 3511921f52d08ea88da3d650cfb835dffef2c8de /crates | |
parent | 1cf0794f5eac5de7a3829fe93a1b99f4d22fd2f0 (diff) | |
parent | e2b1c69f7488b942360bb3c398a1c831510d1afc (diff) |
Merge #8875
8875: fix: false positive "Missing match arm" when an or-pattern has mismatched types r=flodiebold a=iDawer
![Screenshot_20210519_114510](https://user-images.githubusercontent.com/7803845/118768935-19e12c00-b86f-11eb-90c4-1eed3f2bf57f.jpg)
`InferenceResult` now records pattern type mismatches.
Co-authored-by: Dawer <[email protected]>
Diffstat (limited to 'crates')
-rw-r--r-- | crates/hir_ty/src/diagnostics/expr.rs | 22 | ||||
-rw-r--r-- | crates/hir_ty/src/diagnostics/match_check.rs | 12 | ||||
-rw-r--r-- | crates/hir_ty/src/infer.rs | 23 | ||||
-rw-r--r-- | crates/hir_ty/src/infer/expr.rs | 9 | ||||
-rw-r--r-- | crates/hir_ty/src/infer/pat.rs | 7 | ||||
-rw-r--r-- | crates/hir_ty/src/tests.rs | 5 | ||||
-rw-r--r-- | crates/hir_ty/src/tests/patterns.rs | 58 |
7 files changed, 123 insertions, 13 deletions
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 47709c1e8..b9a69b79c 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs | |||
@@ -211,7 +211,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
211 | 211 | ||
212 | // FIXME: Due to shortcomings in the current type system implementation, only emit this | 212 | // FIXME: Due to shortcomings in the current type system implementation, only emit this |
213 | // diagnostic if there are no type mismatches in the containing function. | 213 | // diagnostic if there are no type mismatches in the containing function. |
214 | if self.infer.type_mismatches.iter().next().is_some() { | 214 | if self.infer.expr_type_mismatches().next().is_some() { |
215 | return; | 215 | return; |
216 | } | 216 | } |
217 | 217 | ||
@@ -311,11 +311,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
311 | // necessary. | 311 | // necessary. |
312 | // | 312 | // |
313 | // FIXME we should use the type checker for this. | 313 | // FIXME we should use the type checker for this. |
314 | if pat_ty == match_expr_ty | 314 | if (pat_ty == match_expr_ty |
315 | || match_expr_ty | 315 | || match_expr_ty |
316 | .as_reference() | 316 | .as_reference() |
317 | .map(|(match_expr_ty, ..)| match_expr_ty == pat_ty) | 317 | .map(|(match_expr_ty, ..)| match_expr_ty == pat_ty) |
318 | .unwrap_or(false) | 318 | .unwrap_or(false)) |
319 | && types_of_subpatterns_do_match(pat, &cx.body, &infer) | ||
319 | { | 320 | { |
320 | // If we had a NotUsefulMatchArm diagnostic, we could | 321 | // If we had a NotUsefulMatchArm diagnostic, we could |
321 | // check the usefulness of each pattern as we added it | 322 | // check the usefulness of each pattern as we added it |
@@ -496,6 +497,21 @@ pub fn record_pattern_missing_fields( | |||
496 | Some((variant_def, missed_fields, exhaustive)) | 497 | Some((variant_def, missed_fields, exhaustive)) |
497 | } | 498 | } |
498 | 499 | ||
500 | fn types_of_subpatterns_do_match(pat: PatId, body: &Body, infer: &InferenceResult) -> bool { | ||
501 | fn walk(pat: PatId, body: &Body, infer: &InferenceResult, has_type_mismatches: &mut bool) { | ||
502 | match infer.type_mismatch_for_pat(pat) { | ||
503 | Some(_) => *has_type_mismatches = true, | ||
504 | None => { | ||
505 | body[pat].walk_child_pats(|subpat| walk(subpat, body, infer, has_type_mismatches)) | ||
506 | } | ||
507 | } | ||
508 | } | ||
509 | |||
510 | let mut has_type_mismatches = false; | ||
511 | walk(pat, body, infer, &mut has_type_mismatches); | ||
512 | !has_type_mismatches | ||
513 | } | ||
514 | |||
499 | #[cfg(test)] | 515 | #[cfg(test)] |
500 | mod tests { | 516 | mod tests { |
501 | use crate::diagnostics::tests::check_diagnostics; | 517 | use crate::diagnostics::tests::check_diagnostics; |
diff --git a/crates/hir_ty/src/diagnostics/match_check.rs b/crates/hir_ty/src/diagnostics/match_check.rs index 6ee0529c6..e8dd669bf 100644 --- a/crates/hir_ty/src/diagnostics/match_check.rs +++ b/crates/hir_ty/src/diagnostics/match_check.rs | |||
@@ -1128,6 +1128,18 @@ fn main() { | |||
1128 | } | 1128 | } |
1129 | 1129 | ||
1130 | #[test] | 1130 | #[test] |
1131 | fn mismatched_types_in_or_patterns() { | ||
1132 | check_diagnostics( | ||
1133 | r#" | ||
1134 | fn main() { | ||
1135 | match false { true | () => {} } | ||
1136 | match (false,) { (true | (),) => {} } | ||
1137 | } | ||
1138 | "#, | ||
1139 | ); | ||
1140 | } | ||
1141 | |||
1142 | #[test] | ||
1131 | fn malformed_match_arm_tuple_enum_missing_pattern() { | 1143 | fn malformed_match_arm_tuple_enum_missing_pattern() { |
1132 | // We are testing to be sure we don't panic here when the match | 1144 | // We are testing to be sure we don't panic here when the match |
1133 | // arm `Either::B` is missing its pattern. | 1145 | // arm `Either::B` is missing its pattern. |
diff --git a/crates/hir_ty/src/infer.rs b/crates/hir_ty/src/infer.rs index bf2da2d4a..0ee851a74 100644 --- a/crates/hir_ty/src/infer.rs +++ b/crates/hir_ty/src/infer.rs | |||
@@ -137,8 +137,12 @@ pub struct InferenceResult { | |||
137 | assoc_resolutions: FxHashMap<ExprOrPatId, AssocItemId>, | 137 | assoc_resolutions: FxHashMap<ExprOrPatId, AssocItemId>, |
138 | diagnostics: Vec<InferenceDiagnostic>, | 138 | diagnostics: Vec<InferenceDiagnostic>, |
139 | pub type_of_expr: ArenaMap<ExprId, Ty>, | 139 | pub type_of_expr: ArenaMap<ExprId, Ty>, |
140 | /// For each pattern record the type it resolves to. | ||
141 | /// | ||
142 | /// **Note**: When a pattern type is resolved it may still contain | ||
143 | /// unresolved or missing subpatterns or subpatterns of mismatched types. | ||
140 | pub type_of_pat: ArenaMap<PatId, Ty>, | 144 | pub type_of_pat: ArenaMap<PatId, Ty>, |
141 | pub(super) type_mismatches: ArenaMap<ExprId, TypeMismatch>, | 145 | type_mismatches: FxHashMap<ExprOrPatId, TypeMismatch>, |
142 | /// Interned Unknown to return references to. | 146 | /// Interned Unknown to return references to. |
143 | standard_types: InternedStandardTypes, | 147 | standard_types: InternedStandardTypes, |
144 | } | 148 | } |
@@ -163,7 +167,22 @@ impl InferenceResult { | |||
163 | self.assoc_resolutions.get(&id.into()).copied() | 167 | self.assoc_resolutions.get(&id.into()).copied() |
164 | } | 168 | } |
165 | pub fn type_mismatch_for_expr(&self, expr: ExprId) -> Option<&TypeMismatch> { | 169 | pub fn type_mismatch_for_expr(&self, expr: ExprId) -> Option<&TypeMismatch> { |
166 | self.type_mismatches.get(expr) | 170 | self.type_mismatches.get(&expr.into()) |
171 | } | ||
172 | pub fn type_mismatch_for_pat(&self, pat: PatId) -> Option<&TypeMismatch> { | ||
173 | self.type_mismatches.get(&pat.into()) | ||
174 | } | ||
175 | pub fn expr_type_mismatches(&self) -> impl Iterator<Item = (ExprId, &TypeMismatch)> { | ||
176 | self.type_mismatches.iter().filter_map(|(expr_or_pat, mismatch)| match *expr_or_pat { | ||
177 | ExprOrPatId::ExprId(expr) => Some((expr, mismatch)), | ||
178 | _ => None, | ||
179 | }) | ||
180 | } | ||
181 | pub fn pat_type_mismatches(&self) -> impl Iterator<Item = (PatId, &TypeMismatch)> { | ||
182 | self.type_mismatches.iter().filter_map(|(expr_or_pat, mismatch)| match *expr_or_pat { | ||
183 | ExprOrPatId::PatId(pat) => Some((pat, mismatch)), | ||
184 | _ => None, | ||
185 | }) | ||
167 | } | 186 | } |
168 | pub fn add_diagnostics( | 187 | pub fn add_diagnostics( |
169 | &self, | 188 | &self, |
diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs index b6b5a1b75..7278faeec 100644 --- a/crates/hir_ty/src/infer/expr.rs +++ b/crates/hir_ty/src/infer/expr.rs | |||
@@ -42,7 +42,7 @@ impl<'a> InferenceContext<'a> { | |||
42 | let could_unify = self.unify(&ty, &expected.ty); | 42 | let could_unify = self.unify(&ty, &expected.ty); |
43 | if !could_unify { | 43 | if !could_unify { |
44 | self.result.type_mismatches.insert( | 44 | self.result.type_mismatches.insert( |
45 | tgt_expr, | 45 | tgt_expr.into(), |
46 | TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }, | 46 | TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }, |
47 | ); | 47 | ); |
48 | } | 48 | } |
@@ -54,9 +54,10 @@ impl<'a> InferenceContext<'a> { | |||
54 | pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty { | 54 | pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty { |
55 | let ty = self.infer_expr_inner(expr, &expected); | 55 | let ty = self.infer_expr_inner(expr, &expected); |
56 | let ty = if !self.coerce(&ty, &expected.coercion_target()) { | 56 | let ty = if !self.coerce(&ty, &expected.coercion_target()) { |
57 | self.result | 57 | self.result.type_mismatches.insert( |
58 | .type_mismatches | 58 | expr.into(), |
59 | .insert(expr, TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }); | 59 | TypeMismatch { expected: expected.ty.clone(), actual: ty.clone() }, |
60 | ); | ||
60 | // Return actual type when type mismatch. | 61 | // Return actual type when type mismatch. |
61 | // This is needed for diagnostic when return type mismatch. | 62 | // This is needed for diagnostic when return type mismatch. |
62 | ty | 63 | ty |
diff --git a/crates/hir_ty/src/infer/pat.rs b/crates/hir_ty/src/infer/pat.rs index 60b94a642..b15f4977d 100644 --- a/crates/hir_ty/src/infer/pat.rs +++ b/crates/hir_ty/src/infer/pat.rs | |||
@@ -10,7 +10,7 @@ use hir_def::{ | |||
10 | }; | 10 | }; |
11 | use hir_expand::name::Name; | 11 | use hir_expand::name::Name; |
12 | 12 | ||
13 | use super::{BindingMode, Expectation, InferenceContext}; | 13 | use super::{BindingMode, Expectation, InferenceContext, TypeMismatch}; |
14 | use crate::{ | 14 | use crate::{ |
15 | lower::lower_to_chalk_mutability, static_lifetime, Interner, Substitution, Ty, TyBuilder, | 15 | lower::lower_to_chalk_mutability, static_lifetime, Interner, Substitution, Ty, TyBuilder, |
16 | TyExt, TyKind, | 16 | TyExt, TyKind, |
@@ -266,7 +266,10 @@ impl<'a> InferenceContext<'a> { | |||
266 | // use a new type variable if we got error type here | 266 | // use a new type variable if we got error type here |
267 | let ty = self.insert_type_vars_shallow(ty); | 267 | let ty = self.insert_type_vars_shallow(ty); |
268 | if !self.unify(&ty, expected) { | 268 | if !self.unify(&ty, expected) { |
269 | // FIXME record mismatch, we need to change the type of self.type_mismatches for that | 269 | self.result.type_mismatches.insert( |
270 | pat.into(), | ||
271 | TypeMismatch { expected: expected.clone(), actual: ty.clone() }, | ||
272 | ); | ||
270 | } | 273 | } |
271 | let ty = self.resolve_ty_as_possible(ty); | 274 | let ty = self.resolve_ty_as_possible(ty); |
272 | self.write_pat_ty(pat, ty.clone()); | 275 | self.write_pat_ty(pat, ty.clone()); |
diff --git a/crates/hir_ty/src/tests.rs b/crates/hir_ty/src/tests.rs index ccfb88c52..cc819373c 100644 --- a/crates/hir_ty/src/tests.rs +++ b/crates/hir_ty/src/tests.rs | |||
@@ -130,7 +130,10 @@ fn infer_with_mismatches(content: &str, include_mismatches: bool) -> String { | |||
130 | } | 130 | } |
131 | Err(SyntheticSyntax) => continue, | 131 | Err(SyntheticSyntax) => continue, |
132 | }; | 132 | }; |
133 | types.push((syntax_ptr, ty)); | 133 | types.push((syntax_ptr.clone(), ty)); |
134 | if let Some(mismatch) = inference_result.type_mismatch_for_pat(pat) { | ||
135 | mismatches.push((syntax_ptr, mismatch)); | ||
136 | } | ||
134 | } | 137 | } |
135 | 138 | ||
136 | for (expr, ty) in inference_result.type_of_expr.iter() { | 139 | for (expr, ty) in inference_result.type_of_expr.iter() { |
diff --git a/crates/hir_ty/src/tests/patterns.rs b/crates/hir_ty/src/tests/patterns.rs index 787647e9f..ddbadbe40 100644 --- a/crates/hir_ty/src/tests/patterns.rs +++ b/crates/hir_ty/src/tests/patterns.rs | |||
@@ -546,7 +546,9 @@ fn infer_const_pattern() { | |||
546 | 273..276 'Bar': usize | 546 | 273..276 'Bar': usize |
547 | 280..283 'Bar': usize | 547 | 280..283 'Bar': usize |
548 | 200..223: expected (), got Foo | 548 | 200..223: expected (), got Foo |
549 | 211..214: expected (), got Foo | ||
549 | 262..285: expected (), got usize | 550 | 262..285: expected (), got usize |
551 | 273..276: expected (), got usize | ||
550 | "#]], | 552 | "#]], |
551 | ); | 553 | ); |
552 | } | 554 | } |
@@ -703,7 +705,7 @@ fn box_pattern() { | |||
703 | 705 | ||
704 | #[test] | 706 | #[test] |
705 | fn tuple_ellipsis_pattern() { | 707 | fn tuple_ellipsis_pattern() { |
706 | check_infer( | 708 | check_infer_with_mismatches( |
707 | r#" | 709 | r#" |
708 | fn foo(tuple: (u8, i16, f32)) { | 710 | fn foo(tuple: (u8, i16, f32)) { |
709 | match tuple { | 711 | match tuple { |
@@ -744,6 +746,8 @@ fn foo(tuple: (u8, i16, f32)) { | |||
744 | 186..200 '{/*too long*/}': () | 746 | 186..200 '{/*too long*/}': () |
745 | 209..210 '_': (u8, i16, f32) | 747 | 209..210 '_': (u8, i16, f32) |
746 | 214..216 '{}': () | 748 | 214..216 '{}': () |
749 | 136..142: expected (u8, i16, f32), got (u8, i16) | ||
750 | 170..182: expected (u8, i16, f32), got (u8, i16, f32, _) | ||
747 | "#]], | 751 | "#]], |
748 | ); | 752 | ); |
749 | } | 753 | } |
@@ -851,3 +855,55 @@ fn f(e: Enum) { | |||
851 | "#, | 855 | "#, |
852 | ) | 856 | ) |
853 | } | 857 | } |
858 | |||
859 | #[test] | ||
860 | fn type_mismatch_in_or_pattern() { | ||
861 | check_infer_with_mismatches( | ||
862 | r#" | ||
863 | fn main() { | ||
864 | match (false,) { | ||
865 | (true | (),) => {} | ||
866 | (() | true,) => {} | ||
867 | (_ | (),) => {} | ||
868 | (() | _,) => {} | ||
869 | } | ||
870 | } | ||
871 | "#, | ||
872 | expect![[r#" | ||
873 | 10..142 '{ ... } }': () | ||
874 | 16..140 'match ... }': () | ||
875 | 22..30 '(false,)': (bool,) | ||
876 | 23..28 'false': bool | ||
877 | 41..53 '(true | (),)': (bool,) | ||
878 | 42..46 'true': bool | ||
879 | 42..46 'true': bool | ||
880 | 42..51 'true | ()': bool | ||
881 | 49..51 '()': () | ||
882 | 57..59 '{}': () | ||
883 | 68..80 '(() | true,)': ((),) | ||
884 | 69..71 '()': () | ||
885 | 69..78 '() | true': () | ||
886 | 74..78 'true': bool | ||
887 | 74..78 'true': bool | ||
888 | 84..86 '{}': () | ||
889 | 95..104 '(_ | (),)': (bool,) | ||
890 | 96..97 '_': bool | ||
891 | 96..102 '_ | ()': bool | ||
892 | 100..102 '()': () | ||
893 | 108..110 '{}': () | ||
894 | 119..128 '(() | _,)': ((),) | ||
895 | 120..122 '()': () | ||
896 | 120..126 '() | _': () | ||
897 | 125..126 '_': bool | ||
898 | 132..134 '{}': () | ||
899 | 49..51: expected bool, got () | ||
900 | 68..80: expected (bool,), got ((),) | ||
901 | 69..71: expected bool, got () | ||
902 | 69..78: expected bool, got () | ||
903 | 100..102: expected bool, got () | ||
904 | 119..128: expected (bool,), got ((),) | ||
905 | 120..122: expected bool, got () | ||
906 | 120..126: expected bool, got () | ||
907 | "#]], | ||
908 | ); | ||
909 | } | ||