fix wrong suggestions for boxed trait objects instead of impl trait

rust-lang · Aug 7, 2022 · 82f2c08 · 82f2c08
1 parent fc43bd6
commit 82f2c08
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 31 deletions.
diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs
@@ -94,7 +94,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             let arm_ty = self.check_expr_with_expectation(&arm.body, expected);
             all_arms_diverge &= self.diverges.get();
 
-            let opt_suggest_box_span = self.opt_suggest_box_span(arm_ty, orig_expected);
+            let opt_suggest_box_span = prior_arm.and_then(|(_, prior_arm_ty, _)| {
+                self.opt_suggest_box_span(prior_arm_ty, arm_ty, orig_expected)
+            });
 
             let (arm_block_id, arm_span) = if let hir::ExprKind::Block(blk, _) = arm.body.kind {
                 (Some(blk.hir_id), self.find_block_span(blk))
@@ -473,43 +475,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     // provide a structured suggestion in that case.
     pub(crate) fn opt_suggest_box_span(
         &self,
-        outer_ty: Ty<'tcx>,
+        first_ty: Ty<'tcx>,
+        second_ty: Ty<'tcx>,
         orig_expected: Expectation<'tcx>,
     ) -> Option<Span> {
         match orig_expected {
             Expectation::ExpectHasType(expected)
                 if self.in_tail_expr
                     && self.return_type_has_opaque
-                    && self.can_coerce(outer_ty, expected) =>
+                    && self.can_coerce(first_ty, expected)
+                    && self.can_coerce(second_ty, expected) =>
             {
                 let obligations = self.fulfillment_cx.borrow().pending_obligations();
                 let mut suggest_box = !obligations.is_empty();
-                for o in obligations {
-                    match o.predicate.kind().skip_binder() {
-                        ty::PredicateKind::Trait(t) => {
-                            let pred =
-                                ty::Binder::dummy(ty::PredicateKind::Trait(ty::TraitPredicate {
-                                    trait_ref: ty::TraitRef {
-                                        def_id: t.def_id(),
-                                        substs: self.tcx.mk_substs_trait(outer_ty, &[]),
+                'outer: for o in obligations {
+                    for outer_ty in &[first_ty, second_ty] {
+                        match o.predicate.kind().skip_binder() {
+                            ty::PredicateKind::Trait(t) => {
+                                let pred = ty::Binder::dummy(ty::PredicateKind::Trait(
+                                    ty::TraitPredicate {
+                                        trait_ref: ty::TraitRef {
+                                            def_id: t.def_id(),
+                                            substs: self.tcx.mk_substs_trait(*outer_ty, &[]),
+                                        },
+                                        constness: t.constness,
+                                        polarity: t.polarity,
                                     },
-                                    constness: t.constness,
-                                    polarity: t.polarity,
-                                }));
-                            let obl = Obligation::new(
-                                o.cause.clone(),
-                                self.param_env,
-                                pred.to_predicate(self.tcx),
-                            );
-                            suggest_box &= self.predicate_must_hold_modulo_regions(&obl);
-                            if !suggest_box {
-                                // We've encountered some obligation that didn't hold, so the
-                                // return expression can't just be boxed. We don't need to
-                                // evaluate the rest of the obligations.
-                                break;
+                                ));
+                                let obl = Obligation::new(
+                                    o.cause.clone(),
+                                    self.param_env,
+                                    pred.to_predicate(self.tcx),
+                                );
+                                suggest_box &= self.predicate_must_hold_modulo_regions(&obl);
+                                if !suggest_box {
+                                    // We've encountered some obligation that didn't hold, so the
+                                    // return expression can't just be boxed. We don't need to
+                                    // evaluate the rest of the obligations.
+                                    break 'outer;
+                                }
                             }
+                            _ => {}
                         }
-                        _ => {}
                     }
                 }
                 // If all the obligations hold (or there are no obligations) the tail expression

diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs
@@ -1002,7 +1002,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             let else_ty = self.check_expr_with_expectation(else_expr, expected);
             let else_diverges = self.diverges.get();
 
-            let opt_suggest_box_span = self.opt_suggest_box_span(else_ty, orig_expected);
+            let opt_suggest_box_span = self.opt_suggest_box_span(then_ty, else_ty, orig_expected);
             let if_cause = self.if_cause(
                 sp,
                 cond_expr.span,

diff --git a/src/test/ui/mismatched_types/do-not-suggest-boxed-trait-objects-instead-of-impl-trait.rs b/src/test/ui/mismatched_types/do-not-suggest-boxed-trait-objects-instead-of-impl-trait.rs
@@ -0,0 +1,23 @@
+struct S;
+struct Y;
+
+trait Trait {}
+
+impl Trait for Y {}
+
+fn foo() -> impl Trait {
+    if true {
+        S
+    } else {
+        Y //~ ERROR `if` and `else` have incompatible types
+    }
+}
+
+fn bar() -> impl Trait {
+    match true {
+        true => S,
+        false => Y, //~ ERROR `match` arms have incompatible types
+    }
+}
+
+fn main() {}
diff --git a/src/test/ui/mismatched_types/do-not-suggest-boxed-trait-objects-instead-of-impl-trait.stderr b/src/test/ui/mismatched_types/do-not-suggest-boxed-trait-objects-instead-of-impl-trait.stderr
@@ -0,0 +1,26 @@
+error[E0308]: `if` and `else` have incompatible types
+  --> $DIR/do-not-suggest-boxed-trait-objects-instead-of-impl-trait.rs:12:9
+   |
+LL | /     if true {
+LL | |         S
+   | |         - expected because of this
+LL | |     } else {
+LL | |         Y
+   | |         ^ expected struct `S`, found struct `Y`
+LL | |     }
+   | |_____- `if` and `else` have incompatible types
+
+error[E0308]: `match` arms have incompatible types
+  --> $DIR/do-not-suggest-boxed-trait-objects-instead-of-impl-trait.rs:19:18
+   |
+LL | /     match true {
+LL | |         true => S,
+   | |                 - this is found to be of type `S`
+LL | |         false => Y,
+   | |                  ^ expected struct `S`, found struct `Y`
+LL | |     }
+   | |_____- `match` arms have incompatible types
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0308`.
diff --git a/src/test/ui/mismatched_types/suggest-boxed-trait-objects-instead-of-impl-trait.fixed b/src/test/ui/mismatched_types/suggest-boxed-trait-objects-instead-of-impl-trait.fixed
@@ -10,12 +10,19 @@ trait Trait {}
 impl Trait for S {}
 impl Trait for Y {}
 
-fn baz() -> Box<dyn Trait> {
+fn foo() -> Box<dyn Trait> {
     if true {
         Box::new(S)
     } else {
         Box::new(Y) //~ ERROR `if` and `else` have incompatible types
     }
 }
 
+fn bar() -> Box<dyn Trait> {
+    match true {
+        true => Box::new(S),
+        false => Box::new(Y), //~ ERROR `match` arms have incompatible types
+    }
+}
+
 fn main() {}
diff --git a/src/test/ui/mismatched_types/suggest-boxed-trait-objects-instead-of-impl-trait.rs b/src/test/ui/mismatched_types/suggest-boxed-trait-objects-instead-of-impl-trait.rs
@@ -10,12 +10,19 @@ trait Trait {}
 impl Trait for S {}
 impl Trait for Y {}
 
-fn baz() -> impl Trait {
+fn foo() -> impl Trait {
     if true {
         S
     } else {
         Y //~ ERROR `if` and `else` have incompatible types
     }
 }
 
+fn bar() -> impl Trait {
+    match true {
+        true => S,
+        false => Y, //~ ERROR `match` arms have incompatible types
+    }
+}
+
 fn main() {}
diff --git a/src/test/ui/mismatched_types/suggest-boxed-trait-objects-instead-of-impl-trait.stderr b/src/test/ui/mismatched_types/suggest-boxed-trait-objects-instead-of-impl-trait.stderr
@@ -12,7 +12,7 @@ LL | |     }
    |
 help: you could change the return type to be a boxed trait object
    |
-LL | fn baz() -> Box<dyn Trait> {
+LL | fn foo() -> Box<dyn Trait> {
    |             ~~~~~~~      +
 help: if you change the return type to expect trait objects, box the returned expressions
    |
@@ -21,6 +21,27 @@ LL |     } else {
 LL ~         Box::new(Y)
    |
 
-error: aborting due to previous error
+error[E0308]: `match` arms have incompatible types
+  --> $DIR/suggest-boxed-trait-objects-instead-of-impl-trait.rs:24:18
+   |
+LL | /     match true {
+LL | |         true => S,
+   | |                 - this is found to be of type `S`
+LL | |         false => Y,
+   | |                  ^ expected struct `S`, found struct `Y`
+LL | |     }
+   | |_____- `match` arms have incompatible types
+   |
+help: you could change the return type to be a boxed trait object
+   |
+LL | fn bar() -> Box<dyn Trait> {
+   |             ~~~~~~~      +
+help: if you change the return type to expect trait objects, box the returned expressions
+   |
+LL ~         true => Box::new(S),
+LL ~         false => Box::new(Y),
+   |
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0308`.