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`

Author: Dylan-DPC

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);
                 }

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)) {

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();
                 }
             }
         }

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,
+                    );
+            }
         }
     }
 

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,

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;

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() {}

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
+

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() {}

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
+

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)
 }
 

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`.