Rollup merge of #111853 - compiler-errors:opaque-check, r=oli-obk

Check opaques for mismatch during writeback

Revive #111705.

I realized that we don't need to put any substs in the writeback results since all of the hidden types have already been remapped. See the comment in `compiler/rustc_middle/src/ty/typeck_results.rs`, which should make that clear for other explorers of the codebase.

Additionally, we need to do some diagnostic stashing because the diagnostics we produce during HIR typeck is very poor and we should prefer the diagnostic that comes from MIR, if we have one.

r? `@oli-obk`

12 files changed, 131 insertions, 25 deletions

diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs
index 309f23d9226..e9b5c47ce23 100644
--- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs
+++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs
@@ -152,8 +152,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                     let guar = ty.error_reported().err().unwrap_or_else(|| {
                         prev.report_mismatch(
                             &OpaqueHiddenType { ty, span: concrete_type.span },
+                            opaque_type_key.def_id,
                             infcx.tcx,
                         )
+                        .emit()
                     });
                     prev.ty = infcx.tcx.ty_error(guar);
                 }
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 5a80024f19b..074fbb1322c 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -478,6 +478,7 @@ pub enum StashKey {
     MaybeFruTypo,
     CallAssocMethod,
     TraitMissingMethod,
+    OpaqueHiddenTypeMismatch,
 }
 
 fn default_track_diagnostic(d: &mut Diagnostic, f: &mut dyn FnMut(&mut Diagnostic)) {
diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs
index ca430a5e863..97c6cb491d1 100644
--- a/compiler/rustc_hir_analysis/src/collect/type_of.rs
+++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs
@@ -584,7 +584,8 @@ fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> T
                 debug!(?concrete_type, "found constraint");
                 if let Some(prev) = &mut self.found {
                     if concrete_type.ty != prev.ty && !(concrete_type, prev.ty).references_error() {
-                        let guar = prev.report_mismatch(&concrete_type, self.tcx);
+                        let guar =
+                            prev.report_mismatch(&concrete_type, self.def_id, self.tcx).emit();
                         prev.ty = self.tcx.ty_error(guar);
                     }
                 } else {
@@ -678,10 +679,10 @@ fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> T
     // Only check against typeck if we didn't already error
     if !hidden.ty.references_error() {
         for concrete_type in locator.typeck_types {
-            if tcx.erase_regions(concrete_type.ty) != tcx.erase_regions(hidden.ty)
+            if concrete_type.ty != tcx.erase_regions(hidden.ty)
                 && !(concrete_type, hidden).references_error()
             {
-                hidden.report_mismatch(&concrete_type, tcx);
+                hidden.report_mismatch(&concrete_type, def_id, tcx).emit();
             }
         }
     }
@@ -722,7 +723,7 @@ fn find_opaque_ty_constraints_for_rpit(
                 if concrete_type.ty != self.found.ty
                     && !(concrete_type, self.found).references_error()
                 {
-                    self.found.report_mismatch(&concrete_type, self.tcx);
+                    self.found.report_mismatch(&concrete_type, self.def_id, self.tcx).emit();
                 }
             }
         }
diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs
index 2daec205cfc..0f21fc1e662 100644
--- a/compiler/rustc_hir_typeck/src/writeback.rs
+++ b/compiler/rustc_hir_typeck/src/writeback.rs
@@ -5,7 +5,7 @@
 use crate::FnCtxt;
 use hir::def_id::LocalDefId;
 use rustc_data_structures::fx::FxHashMap;
-use rustc_errors::ErrorGuaranteed;
+use rustc_errors::{ErrorGuaranteed, StashKey};
 use rustc_hir as hir;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282;
@@ -82,10 +82,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         wbcx.typeck_results.treat_byte_string_as_slice =
             mem::take(&mut self.typeck_results.borrow_mut().treat_byte_string_as_slice);
 
-        if let Some(e) = self.tainted_by_errors() {
-            wbcx.typeck_results.tainted_by_errors = Some(e);
-        }
-
         debug!("writeback: typeck results for {:?} are {:#?}", item_def_id, wbcx.typeck_results);
 
         self.tcx.arena.alloc(wbcx.typeck_results)
@@ -118,12 +114,21 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
     ) -> WritebackCx<'cx, 'tcx> {
         let owner = body.id().hir_id.owner;
 
-        WritebackCx {
+        let mut wbcx = WritebackCx {
             fcx,
             typeck_results: ty::TypeckResults::new(owner),
             body,
             rustc_dump_user_substs,
+        };
+
+        // HACK: We specifically don't want the (opaque) error from tainting our
+        // inference context. That'll prevent us from doing opaque type inference
+        // later on in borrowck, which affects diagnostic spans pretty negatively.
+        if let Some(e) = fcx.tainted_by_errors() {
+            wbcx.typeck_results.tainted_by_errors = Some(e);
         }
+
+        wbcx
     }
 
     fn tcx(&self) -> TyCtxt<'tcx> {
@@ -578,13 +583,26 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
                 continue;
             }
 
-            let hidden_type = hidden_type.remap_generic_params_to_declaration_params(
-                opaque_type_key,
-                self.fcx.infcx.tcx,
-                true,
-            );
-
-            self.typeck_results.concrete_opaque_types.insert(opaque_type_key.def_id, hidden_type);
+            let hidden_type =
+                self.tcx().erase_regions(hidden_type.remap_generic_params_to_declaration_params(
+                    opaque_type_key,
+                    self.tcx(),
+                    true,
+                ));
+
+            if let Some(last_opaque_ty) = self
+                .typeck_results
+                .concrete_opaque_types
+                .insert(opaque_type_key.def_id, hidden_type)
+                && last_opaque_ty.ty != hidden_type.ty
+            {
+                hidden_type
+                    .report_mismatch(&last_opaque_ty, opaque_type_key.def_id, self.tcx())
+                    .stash(
+                        self.tcx().def_span(opaque_type_key.def_id),
+                        StashKey::OpaqueHiddenTypeMismatch,
+                    );
+            }
         }
     }
 
diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
index ecb191676c2..c9cd644fab0 100644
--- a/compiler/rustc_middle/src/ty/mod.rs
+++ b/compiler/rustc_middle/src/ty/mod.rs
@@ -37,7 +37,7 @@ use rustc_data_structures::intern::Interned;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::steal::Steal;
 use rustc_data_structures::tagged_ptr::CopyTaggedPtr;
-use rustc_errors::ErrorGuaranteed;
+use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed, StashKey};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, DocLinkResMap, LifetimeRes, Res};
 use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, LocalDefIdMap};
@@ -1439,14 +1439,26 @@ pub struct OpaqueHiddenType<'tcx> {
 }
 
 impl<'tcx> OpaqueHiddenType<'tcx> {
-    pub fn report_mismatch(&self, other: &Self, tcx: TyCtxt<'tcx>) -> ErrorGuaranteed {
+    pub fn report_mismatch(
+        &self,
+        other: &Self,
+        opaque_def_id: LocalDefId,
+        tcx: TyCtxt<'tcx>,
+    ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
+        if let Some(diag) = tcx
+            .sess
+            .diagnostic()
+            .steal_diagnostic(tcx.def_span(opaque_def_id), StashKey::OpaqueHiddenTypeMismatch)
+        {
+            diag.cancel();
+        }
         // Found different concrete types for the opaque type.
         let sub_diag = if self.span == other.span {
             TypeMismatchReason::ConflictType { span: self.span }
         } else {
             TypeMismatchReason::PreviousUse { span: self.span }
         };
-        tcx.sess.emit_err(OpaqueHiddenTypeMismatch {
+        tcx.sess.create_err(OpaqueHiddenTypeMismatch {
             self_ty: self.ty,
             other_ty: other.ty,
             other_span: other.span,
diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs
index 5a0571f4bb7..e04dbbff9a7 100644
--- a/compiler/rustc_middle/src/ty/typeck_results.rs
+++ b/compiler/rustc_middle/src/ty/typeck_results.rs
@@ -151,10 +151,14 @@ pub struct TypeckResults<'tcx> {
     /// this field will be set to `Some(ErrorGuaranteed)`.
     pub tainted_by_errors: Option<ErrorGuaranteed>,
 
-    /// All the opaque types that have hidden types set
-    /// by this function. We also store the
-    /// type here, so that mir-borrowck can use it as a hint for figuring out hidden types,
-    /// even if they are only set in dead code (which doesn't show up in MIR).
+    /// All the opaque types that have hidden types set by this function.
+    /// We also store the type here, so that the compiler can use it as a hint
+    /// for figuring out hidden types, even if they are only set in dead code
+    /// (which doesn't show up in MIR).
+    ///
+    /// These types are mapped back to the opaque's identity substitutions
+    /// (with erased regions), which is why we don't associated substs with any
+    /// of these usages.
     pub concrete_opaque_types: FxIndexMap<LocalDefId, ty::OpaqueHiddenType<'tcx>>,
 
     /// Tracks the minimum captures required for a closure;
diff --git a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.rs b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.rs
new file mode 100644
index 00000000000..e5bfbfdae91
--- /dev/null
+++ b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.rs
@@ -0,0 +1,15 @@
+#![feature(type_alias_impl_trait)]
+
+type Tait<'a> = impl Sized + 'a;
+
+fn foo<'a, 'b>() {
+    if false {
+        if { return } {
+            let y: Tait<'b> = 1i32;
+            //~^ ERROR concrete type differs from previous defining opaque type use
+        }
+    }
+    let x: Tait<'a> = ();
+}
+
+fn main() {}
diff --git a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.stderr b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.stderr
new file mode 100644
index 00000000000..f2eb7bc4dc7
--- /dev/null
+++ b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.stderr
@@ -0,0 +1,14 @@
+error: concrete type differs from previous defining opaque type use
+  --> $DIR/different_defining_uses_never_type-2.rs:8:31
+   |
+LL |             let y: Tait<'b> = 1i32;
+   |                               ^^^^ expected `()`, got `i32`
+   |
+note: previous use here
+  --> $DIR/different_defining_uses_never_type-2.rs:12:23
+   |
+LL |     let x: Tait<'a> = ();
+   |                       ^^
+
+error: aborting due to previous error
+
diff --git a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-3.rs b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-3.rs
new file mode 100644
index 00000000000..2b30a9cd57c
--- /dev/null
+++ b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-3.rs
@@ -0,0 +1,15 @@
+#![feature(type_alias_impl_trait)]
+
+type Tait<T> = impl Sized;
+
+fn foo<T, U>() {
+    if false {
+        if { return } {
+            let y: Tait<U> = 1i32;
+            //~^ ERROR concrete type differs from previous defining opaque type use
+        }
+    }
+    let x: Tait<T> = ();
+}
+
+fn main() {}
diff --git a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-3.stderr b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-3.stderr
new file mode 100644
index 00000000000..8fc2e22848c
--- /dev/null
+++ b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-3.stderr
@@ -0,0 +1,14 @@
+error: concrete type differs from previous defining opaque type use
+  --> $DIR/different_defining_uses_never_type-3.rs:8:30
+   |
+LL |             let y: Tait<U> = 1i32;
+   |                              ^^^^ expected `()`, got `i32`
+   |
+note: previous use here
+  --> $DIR/different_defining_uses_never_type-3.rs:12:22
+   |
+LL |     let x: Tait<T> = ();
+   |                      ^^
+
+error: aborting due to previous error
+
diff --git a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.rs b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.rs
index da845e86147..9ae2c34b935 100644
--- a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.rs
+++ b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.rs
@@ -8,6 +8,7 @@ type X<A, B> = impl Into<&'static A>;
 
 fn f<A, B: 'static>(a: &'static A, b: B) -> (X<A, B>, X<B, A>) {
     //~^ ERROR the trait bound `&'static B: From<&A>` is not satisfied
+    //~| ERROR concrete type differs from previous defining opaque type use
     (a, a)
 }
 
diff --git a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr
index 66a6b0bbf74..0d24d42ba62 100644
--- a/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr
+++ b/tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn.stderr
@@ -10,6 +10,15 @@ help: consider introducing a `where` clause, but there might be an alternative b
 LL | fn f<A, B: 'static>(a: &'static A, b: B) -> (X<A, B>, X<B, A>) where &'static B: From<&A> {
    |                                                                ++++++++++++++++++++++++++
 
-error: aborting due to previous error
+error: concrete type differs from previous defining opaque type use
+  --> $DIR/multiple-def-uses-in-one-fn.rs:9:45
+   |
+LL | fn f<A, B: 'static>(a: &'static A, b: B) -> (X<A, B>, X<B, A>) {
+   |                                             ^^^^^^^^^^^^^^^^^^
+   |                                             |
+   |                                             expected `&B`, got `&A`
+   |                                             this expression supplies two conflicting concrete types for the same opaque type
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0277`.