Get rid of check_opaque_type_well_formed

Author: compiler-errors

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -1,18 +1,14 @@
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_errors::ErrorGuaranteed;
-use rustc_hir::OpaqueTyOrigin;
-use rustc_hir::def::DefKind;
 use rustc_hir::def_id::LocalDefId;
 use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin, TyCtxtInferExt as _};
-use rustc_infer::traits::{Obligation, ObligationCause};
 use rustc_macros::extension;
 use rustc_middle::ty::visit::TypeVisitableExt;
 use rustc_middle::ty::{
     self, GenericArgKind, GenericArgs, OpaqueHiddenType, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable,
     TypingMode,
 };
 use rustc_span::Span;
-use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::traits::ObligationCtxt;
 use tracing::{debug, instrument};
 
@@ -303,91 +299,7 @@ impl<'tcx> InferCtxt<'tcx> {
             return Ty::new_error(self.tcx, e);
         }
 
-        // `definition_ty` does not live in of the current inference context,
-        // so lets make sure that we don't accidentally misuse our current `infcx`.
-        match check_opaque_type_well_formed(
-            self.tcx,
-            self.next_trait_solver(),
-            opaque_type_key.def_id,
-            instantiated_ty.span,
-            definition_ty,
-        ) {
-            Ok(hidden_ty) => hidden_ty,
-            Err(guar) => Ty::new_error(self.tcx, guar),
-        }
-    }
-}
-
-/// This logic duplicates most of `check_opaque_meets_bounds`.
-/// FIXME(oli-obk): Also do region checks here and then consider removing
-/// `check_opaque_meets_bounds` entirely.
-fn check_opaque_type_well_formed<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    next_trait_solver: bool,
-    def_id: LocalDefId,
-    definition_span: Span,
-    definition_ty: Ty<'tcx>,
-) -> Result<Ty<'tcx>, ErrorGuaranteed> {
-    // Only check this for TAIT. RPIT already supports `tests/ui/impl-trait/nested-return-type2.rs`
-    // on stable and we'd break that.
-    let opaque_ty_hir = tcx.hir().expect_opaque_ty(def_id);
-    let OpaqueTyOrigin::TyAlias { .. } = opaque_ty_hir.origin else {
-        return Ok(definition_ty);
-    };
-    let param_env = tcx.param_env(def_id);
-
-    let mut parent_def_id = def_id;
-    while tcx.def_kind(parent_def_id) == DefKind::OpaqueTy {
-        parent_def_id = tcx.local_parent(parent_def_id);
-    }
-
-    // FIXME(#132279): This should eventually use the already defined hidden types
-    // instead. Alternatively we'll entirely remove this function given we also check
-    // the opaque in `check_opaque_meets_bounds` later.
-    let infcx = tcx
-        .infer_ctxt()
-        .with_next_trait_solver(next_trait_solver)
-        .build(TypingMode::analysis_in_body(tcx, parent_def_id));
-    let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
-    let identity_args = GenericArgs::identity_for_item(tcx, def_id);
-
-    // Require that the hidden type actually fulfills all the bounds of the opaque type, even without
-    // the bounds that the function supplies.
-    let opaque_ty = Ty::new_opaque(tcx, def_id.to_def_id(), identity_args);
-    ocx.eq(&ObligationCause::misc(definition_span, def_id), param_env, opaque_ty, definition_ty)
-        .map_err(|err| {
-            infcx
-                .err_ctxt()
-                .report_mismatched_types(
-                    &ObligationCause::misc(definition_span, def_id),
-                    param_env,
-                    opaque_ty,
-                    definition_ty,
-                    err,
-                )
-                .emit()
-        })?;
-
-    // Require the hidden type to be well-formed with only the generics of the opaque type.
-    // Defining use functions may have more bounds than the opaque type, which is ok, as long as the
-    // hidden type is well formed even without those bounds.
-    let predicate = ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(
-        definition_ty.into(),
-    )));
-    ocx.register_obligation(Obligation::misc(tcx, definition_span, def_id, param_env, predicate));
-
-    // Check that all obligations are satisfied by the implementation's
-    // version.
-    let errors = ocx.select_all_or_error();
-
-    // This is fishy, but we check it again in `check_opaque_meets_bounds`.
-    // Remove once we can prepopulate with known hidden types.
-    let _ = infcx.take_opaque_types();
-
-    if errors.is_empty() {
-        Ok(definition_ty)
-    } else {
-        Err(infcx.err_ctxt().report_fulfillment_errors(errors))
+        definition_ty
     }
 }
 

compiler/rustc_hir_analysis/src/check/check.rs

@@ -5,13 +5,14 @@ use rustc_abi::FieldIdx;
 use rustc_data_structures::unord::{UnordMap, UnordSet};
 use rustc_errors::MultiSpan;
 use rustc_errors::codes::*;
-use rustc_hir::Node;
 use rustc_hir::def::{CtorKind, DefKind};
+use rustc_hir::{Node, intravisit};
 use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
 use rustc_infer::traits::Obligation;
 use rustc_lint_defs::builtin::{
     REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS, UNSUPPORTED_FN_PTR_CALLING_CONVENTIONS,
 };
+use rustc_middle::hir::nested_filter;
 use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
 use rustc_middle::middle::stability::EvalResult;
 use rustc_middle::span_bug;
@@ -190,7 +191,7 @@ fn check_static_inhabited(tcx: TyCtxt<'_>, def_id: LocalDefId) {
 /// Checks that an opaque type does not contain cycles and does not use `Self` or `T::Foo`
 /// projections that would result in "inheriting lifetimes".
 fn check_opaque(tcx: TyCtxt<'_>, def_id: LocalDefId) {
-    let hir::OpaqueTy { origin, .. } = tcx.hir().expect_opaque_ty(def_id);
+    let hir::OpaqueTy { origin, .. } = *tcx.hir().expect_opaque_ty(def_id);
 
     // HACK(jynelson): trying to infer the type of `impl trait` breaks documenting
     // `async-std` (and `pub async fn` in general).
@@ -200,23 +201,20 @@ fn check_opaque(tcx: TyCtxt<'_>, def_id: LocalDefId) {
         return;
     }
 
-    let span = tcx.def_span(def_id);
-
     if tcx.type_of(def_id).instantiate_identity().references_error() {
         return;
     }
-    if check_opaque_for_cycles(tcx, def_id, span).is_err() {
+    if check_opaque_for_cycles(tcx, def_id).is_err() {
         return;
     }
 
-    let _ = check_opaque_meets_bounds(tcx, def_id, span, origin);
+    let _ = check_opaque_meets_bounds(tcx, def_id, origin);
 }
 
 /// Checks that an opaque type does not contain cycles.
 pub(super) fn check_opaque_for_cycles<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
-    span: Span,
 ) -> Result<(), ErrorGuaranteed> {
     let args = GenericArgs::identity_for_item(tcx, def_id);
 
@@ -233,7 +231,7 @@ pub(super) fn check_opaque_for_cycles<'tcx>(
             .try_expand_impl_trait_type(def_id.to_def_id(), args, InspectCoroutineFields::No)
             .is_err()
         {
-            let reported = opaque_type_cycle_error(tcx, def_id, span);
+            let reported = opaque_type_cycle_error(tcx, def_id);
             return Err(reported);
         }
 
@@ -267,10 +265,11 @@ pub(super) fn check_opaque_for_cycles<'tcx>(
 fn check_opaque_meets_bounds<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
-    span: Span,
-    origin: &hir::OpaqueTyOrigin<LocalDefId>,
+    origin: hir::OpaqueTyOrigin<LocalDefId>,
 ) -> Result<(), ErrorGuaranteed> {
-    let defining_use_anchor = match *origin {
+    let span = span_of_opaque(tcx, def_id, origin).unwrap_or_else(|| tcx.def_span(def_id));
+
+    let defining_use_anchor = match origin {
         hir::OpaqueTyOrigin::FnReturn { parent, .. }
         | hir::OpaqueTyOrigin::AsyncFn { parent, .. }
         | hir::OpaqueTyOrigin::TyAlias { parent, .. } => parent,
@@ -281,7 +280,7 @@ fn check_opaque_meets_bounds<'tcx>(
     let infcx = tcx.infer_ctxt().build(TypingMode::analysis_in_body(tcx, defining_use_anchor));
     let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
 
-    let args = match *origin {
+    let args = match origin {
         hir::OpaqueTyOrigin::FnReturn { parent, .. }
         | hir::OpaqueTyOrigin::AsyncFn { parent, .. }
         | hir::OpaqueTyOrigin::TyAlias { parent, .. } => GenericArgs::identity_for_item(
@@ -308,6 +307,7 @@ fn check_opaque_meets_bounds<'tcx>(
 
     let misc_cause = traits::ObligationCause::misc(span, def_id);
 
+    // FIXME: We should just register the item bounds here, rather than equating.
     match ocx.eq(&misc_cause, param_env, opaque_ty, hidden_ty) {
         Ok(()) => {}
         Err(ty_err) => {
@@ -364,6 +364,97 @@ fn check_opaque_meets_bounds<'tcx>(
     }
 }
 
+fn span_of_opaque<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    opaque_def_id: LocalDefId,
+    origin: hir::OpaqueTyOrigin<LocalDefId>,
+) -> Option<Span> {
+    struct TaitConstraintLocator<'tcx> {
+        opaque_def_id: LocalDefId,
+        tcx: TyCtxt<'tcx>,
+    }
+    impl<'tcx> TaitConstraintLocator<'tcx> {
+        fn check(&self, item_def_id: LocalDefId) -> ControlFlow<Span> {
+            if !self.tcx.has_typeck_results(item_def_id) {
+                return ControlFlow::Continue(());
+            }
+
+            if let Some(hidden_ty) =
+                self.tcx.mir_borrowck(item_def_id).concrete_opaque_types.get(&self.opaque_def_id)
+            {
+                ControlFlow::Break(hidden_ty.span)
+            } else {
+                ControlFlow::Continue(())
+            }
+        }
+    }
+    impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> {
+        type NestedFilter = nested_filter::All;
+        type Result = ControlFlow<Span>;
+        fn nested_visit_map(&mut self) -> Self::Map {
+            self.tcx.hir()
+        }
+        fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) -> Self::Result {
+            if let hir::ExprKind::Closure(closure) = ex.kind {
+                self.check(closure.def_id)?;
+            }
+            intravisit::walk_expr(self, ex)
+        }
+        fn visit_item(&mut self, it: &'tcx hir::Item<'tcx>) -> Self::Result {
+            self.check(it.owner_id.def_id)?;
+            intravisit::walk_item(self, it)
+        }
+        fn visit_impl_item(&mut self, it: &'tcx hir::ImplItem<'tcx>) -> Self::Result {
+            self.check(it.owner_id.def_id)?;
+            intravisit::walk_impl_item(self, it)
+        }
+        fn visit_trait_item(&mut self, it: &'tcx hir::TraitItem<'tcx>) -> Self::Result {
+            self.check(it.owner_id.def_id)?;
+            intravisit::walk_trait_item(self, it)
+        }
+        fn visit_foreign_item(&mut self, it: &'tcx hir::ForeignItem<'tcx>) -> Self::Result {
+            intravisit::walk_foreign_item(self, it)
+        }
+    }
+
+    let mut locator = TaitConstraintLocator { tcx, opaque_def_id };
+    match origin {
+        hir::OpaqueTyOrigin::FnReturn { parent, .. }
+        | hir::OpaqueTyOrigin::AsyncFn { parent, .. } => locator.check(parent).break_value(),
+        hir::OpaqueTyOrigin::TyAlias { parent, in_assoc_ty: true } => {
+            let impl_def_id = tcx.local_parent(parent);
+            for assoc in tcx.associated_items(impl_def_id).in_definition_order() {
+                match assoc.kind {
+                    ty::AssocKind::Const | ty::AssocKind::Fn => {
+                        if let ControlFlow::Break(span) = locator.check(assoc.def_id.expect_local())
+                        {
+                            return Some(span);
+                        }
+                    }
+                    ty::AssocKind::Type => {}
+                }
+            }
+
+            None
+        }
+        hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: false, .. } => {
+            let scope = tcx.hir().get_defining_scope(tcx.local_def_id_to_hir_id(opaque_def_id));
+            let found = if scope == hir::CRATE_HIR_ID {
+                tcx.hir().walk_toplevel_module(&mut locator)
+            } else {
+                match tcx.hir_node(scope) {
+                    Node::Item(it) => locator.visit_item(it),
+                    Node::ImplItem(it) => locator.visit_impl_item(it),
+                    Node::TraitItem(it) => locator.visit_trait_item(it),
+                    Node::ForeignItem(it) => locator.visit_foreign_item(it),
+                    other => bug!("{:?} is not a valid scope for an opaque type item", other),
+                }
+            };
+            found.break_value()
+        }
+    }
+}
+
 fn sanity_check_found_hidden_type<'tcx>(
     tcx: TyCtxt<'tcx>,
     key: ty::OpaqueTypeKey<'tcx>,
@@ -1535,11 +1626,8 @@ fn check_type_alias_type_params_are_used<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalD
 ///
 /// If all the return expressions evaluate to `!`, then we explain that the error will go away
 /// after changing it. This can happen when a user uses `panic!()` or similar as a placeholder.
-fn opaque_type_cycle_error(
-    tcx: TyCtxt<'_>,
-    opaque_def_id: LocalDefId,
-    span: Span,
-) -> ErrorGuaranteed {
+fn opaque_type_cycle_error(tcx: TyCtxt<'_>, opaque_def_id: LocalDefId) -> ErrorGuaranteed {
+    let span = tcx.def_span(opaque_def_id);
     let mut err = struct_span_code_err!(tcx.dcx(), span, E0720, "cannot resolve opaque type");
 
     let mut label = false;

tests/ui/async-await/issue-70935-complex-spans.rs

@@ -14,8 +14,8 @@ async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
 
 fn foo(x: NotSync) -> impl Future + Send {
     //~^ ERROR `*mut ()` cannot be shared between threads safely
-    //~| ERROR `*mut ()` cannot be shared between threads safely
     async move {
+    //~^ ERROR `*mut ()` cannot be shared between threads safely
         baz(|| async {
             foo(x.clone());
         }).await;

tests/ui/async-await/issue-70935-complex-spans.stderr

@@ -1,8 +1,13 @@
 error[E0277]: `*mut ()` cannot be shared between threads safely
-  --> $DIR/issue-70935-complex-spans.rs:15:23
+  --> $DIR/issue-70935-complex-spans.rs:17:5
    |
-LL | fn foo(x: NotSync) -> impl Future + Send {
-   |                       ^^^^^^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
+LL | /     async move {
+LL | |
+LL | |         baz(|| async {
+LL | |             foo(x.clone());
+LL | |         }).await;
+LL | |     }
+   | |_____^ `*mut ()` cannot be shared between threads safely
    |
    = help: within `NotSync`, the trait `Sync` is not implemented for `*mut ()`
 note: required because it appears within the type `PhantomData<*mut ()>`
@@ -26,7 +31,7 @@ LL |   async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
 LL | | }
    | |_^
 note: required because it's used within this `async` block
-  --> $DIR/issue-70935-complex-spans.rs:18:5
+  --> $DIR/issue-70935-complex-spans.rs:17:5
    |
 LL |     async move {
    |     ^^^^^^^^^^
@@ -59,11 +64,10 @@ LL |   async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
 LL | | }
    | |_^
 note: required because it's used within this `async` block
-  --> $DIR/issue-70935-complex-spans.rs:18:5
+  --> $DIR/issue-70935-complex-spans.rs:17:5
    |
 LL |     async move {
    |     ^^^^^^^^^^
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
 error: aborting due to 2 previous errors