Auto merge of #145244 - lcnr:handle-opaque-types-before-region-inference, r=BoxyUwU

support non-defining uses of opaques in borrowck

Reimplements the first part of #139587, but limited to only the new solver. To do so I also entirely rewrite the way we handle opaque types in borrowck even on stable. This should not impact behavior however.

We now support revealing uses during MIR borrowck with the new solver:
```rust
fn foo<'a>(x: &'a u32) -> impl Sized + use<'a> {
    let local = 1;
    foo::<'_>(&local);
    x
}
```

### How do opaque types work right now

Whenever we use an opaque type during type checking, we remember this use in the `opaque_type_storage` of the `InferCtxt`.

Right now, we collect all *member constraints* at the end of MIR type check by looking at all uses from the `opaque_type_storage`. We then apply these constraints while computing the region values for each SCC. This does not add actual region constraints but directly updates the final region values.

This means we need to manually handle any constraints from member constraints for diagnostics. We do this by separately tracking `applied_member_constraints` in the `RegionInferenceContext`.

After we've finished computing the region values, it is now immutable and we check whether all member constraints hold. If not, we error.

We now map the hidden types of our defining uses to the defining scope. This assumes that all member constraints apply. To handle non-member regions, we simply map any region in the hidden type we fail to map to a choice region to `'erased` https://github.com/rust-lang/rust/blob/b1b26b834d85e84b46aa8f8f3ce210a1627aa85f/compiler/rustc_borrowck/src/region_infer/opaque_types.rs#L126-L132

### How do we handle opaque types with this PR

MIR type check still works the same by populating the `opaque_type_storage` whenever we use an opaque type.

We now have a new step `fn handle_opaque_type_uses` which happens between MIR type check and `compute_regions`.

This step looks at all opaque type uses in the storage and first checks whether they are defining: are the arguments of the `opaque_type_key` unique region parameters. *With the new solver we silently ignore any *non-defining* uses here. The old solver emits an errors.*

`fn compute_concrete_opaque_types`: We then collect all member constraints for the defining uses and apply them just like we did before. However, we do it on a temporary region graph which is only used while computing the concrete opaque types. We then use this region graph to compute the concrete type which we then store in the `root_cx`.

`fn apply_computed_concrete_opaque_types`: Now that we know the final concrete type of each opaque type and have mapped them to the definition of the opaque. We iterate over all opaque type uses and equate their hidden type with the instantiated final concrete type. This is the step which actually mutates the region graph.

The actual region checking can now entirely ignores opaque types (outside of the `ConstraintCategory` from checking the opaque type uses).

### Diagnostics issue (chill)

Because we now simply use type equality to "apply member constraints" we get ordinary `OutlivesConstraint`s, even if the regions were already related to another.

This is generally not an issue, expect that it can *hide* the actual region constraints which resulted in the final value of the opaque. The constraints we get from checking against the final opaque type definition relies on constraints we used to compute that definition.

I mostly handle this by updating `find_constraint_path_between_regions` to first ignore member constraints in its search and only if that does not find a path, retry while considering member constraints.

### Diagnostics issue (not chill)

A separate issue is that `find_constraint_paths_between_regions` currently looks up member constraints by their **scc**, not by region value:
https://github.com/rust-lang/rust/blob/2c1ac85679678dfe5cce7ea8037735b0349ceaf3/compiler/rustc_borrowck/src/region_infer/mod.rs#L1768-L1775

This means that in the `borrowck-4` test, the resulting constraint path is currently
```
('?2: '?5) due to Single(bb0[5]) (None) (Boring) (ConstraintSccIndex(1): ConstraintSccIndex(1)),
('?5: '?3) due to Single(bb0[6]) (None) (Boring) (ConstraintSccIndex(1): ConstraintSccIndex(1)),
('?3: '?0) due to All(src/main.rs:15:5: 15:6 (#0)) (None) (OpaqueType) (ConstraintSccIndex(1): ConstraintSccIndex(1))
```
Here `'?3` is equal to `'?4`, but the reason why it's in the opaque is that it's related to `'?4`. With my PR this will be correctly tracked so we end up with
```
('?2: '?5) due to Single(bb0[5]) (None) (Boring) (ConstraintSccIndex(1): ConstraintSccIndex(1)),
('?5: '?3) due to Single(bb0[6]) (None) (Boring) (ConstraintSccIndex(1): ConstraintSccIndex(1)),
('?3: '?4) due to Single(bb0[6]) (None) (Assignment) (ConstraintSccIndex(1): ConstraintSccIndex(1)),
('?4: '?0) due to All(src/main.rs:15:5: 15:6 (#0)) (None) (OpaqueType) (ConstraintSccIndex(1): ConstraintSccIndex(1)),
```
This additional `Assignment` step then worsens the error message as we stop talking about the fact that the closures is returned from the function. Fixing this is hard. I've looked into this and it's making me sad :< Properly handling this requires some deeper changes to MIR borrowck diagnostics and that seems like too much for this PR. Given that this only impacts a single test, it seems acceptable to me.

r? `@ghost`

Author: bors

compiler/rustc_borrowck/src/dataflow.rs

@@ -1,7 +1,6 @@
 use std::fmt;
 
 use rustc_data_structures::fx::FxIndexMap;
-use rustc_data_structures::graph;
 use rustc_index::bit_set::{DenseBitSet, MixedBitSet};
 use rustc_middle::mir::{
     self, BasicBlock, Body, CallReturnPlaces, Location, Place, TerminatorEdges,
@@ -317,9 +316,8 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
             loans_out_of_scope_at_location: FxIndexMap::default(),
         };
         for (loan_idx, loan_data) in borrow_set.iter_enumerated() {
-            let issuing_region = loan_data.region;
             let loan_issued_at = loan_data.reserve_location;
-            prec.precompute_loans_out_of_scope(loan_idx, issuing_region, loan_issued_at);
+            prec.precompute_loans_out_of_scope(loan_idx, loan_issued_at);
         }
 
         prec.loans_out_of_scope_at_location
@@ -328,45 +326,7 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
     /// Loans are in scope while they are live: whether they are contained within any live region.
     /// In the location-insensitive analysis, a loan will be contained in a region if the issuing
     /// region can reach it in the subset graph. So this is a reachability problem.
-    fn precompute_loans_out_of_scope(
-        &mut self,
-        loan_idx: BorrowIndex,
-        issuing_region: RegionVid,
-        loan_issued_at: Location,
-    ) {
-        let sccs = self.regioncx.constraint_sccs();
-        let universal_regions = self.regioncx.universal_regions();
-
-        // The loop below was useful for the location-insensitive analysis but shouldn't be
-        // impactful in the location-sensitive case. It seems that it does, however, as without it a
-        // handful of tests fail. That likely means some liveness or outlives data related to choice
-        // regions is missing
-        // FIXME: investigate the impact of loans traversing applied member constraints and why some
-        // tests fail otherwise.
-        //
-        // We first handle the cases where the loan doesn't go out of scope, depending on the
-        // issuing region's successors.
-        for successor in graph::depth_first_search(&self.regioncx.region_graph(), issuing_region) {
-            // Via applied member constraints
-            //
-            // The issuing region can flow into the choice regions, and they are either:
-            // - placeholders or free regions themselves,
-            // - or also transitively outlive a free region.
-            //
-            // That is to say, if there are applied member constraints here, the loan escapes the
-            // function and cannot go out of scope. We could early return here.
-            //
-            // For additional insurance via fuzzing and crater, we verify that the constraint's min
-            // choice indeed escapes the function. In the future, we could e.g. turn this check into
-            // a debug assert and early return as an optimization.
-            let scc = sccs.scc(successor);
-            for constraint in self.regioncx.applied_member_constraints(scc) {
-                if universal_regions.is_universal_region(constraint.min_choice) {
-                    return;
-                }
-            }
-        }
-
+    fn precompute_loans_out_of_scope(&mut self, loan_idx: BorrowIndex, loan_issued_at: Location) {
         let first_block = loan_issued_at.block;
         let first_bb_data = &self.body.basic_blocks[first_block];
 

compiler/rustc_borrowck/src/diagnostics/opaque_types.rs

@@ -13,6 +13,8 @@ use rustc_middle::mir::{self, ConstraintCategory, Location};
 use rustc_middle::ty::{
     self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
 };
+use rustc_span::Span;
+use rustc_trait_selection::error_reporting::infer::region::unexpected_hidden_region_diagnostic;
 use rustc_trait_selection::errors::impl_trait_overcapture_suggestion;
 
 use crate::MirBorrowckCtxt;
@@ -26,13 +28,61 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         if errors.is_empty() {
             return;
         }
+
+        let infcx = self.infcx;
         let mut guar = None;
+        let mut last_unexpected_hidden_region: Option<(Span, Ty<'_>, ty::OpaqueTypeKey<'tcx>)> =
+            None;
         for error in errors {
             guar = Some(match error {
-                DeferredOpaqueTypeError::InvalidOpaqueTypeArgs(err) => err.report(self.infcx),
+                DeferredOpaqueTypeError::InvalidOpaqueTypeArgs(err) => err.report(infcx),
                 DeferredOpaqueTypeError::LifetimeMismatchOpaqueParam(err) => {
-                    self.infcx.dcx().emit_err(err)
+                    infcx.dcx().emit_err(err)
+                }
+                DeferredOpaqueTypeError::UnexpectedHiddenRegion {
+                    opaque_type_key,
+                    hidden_type,
+                    member_region,
+                } => {
+                    let named_ty =
+                        self.regioncx.name_regions_for_member_constraint(infcx.tcx, hidden_type.ty);
+                    let named_key = self
+                        .regioncx
+                        .name_regions_for_member_constraint(infcx.tcx, opaque_type_key);
+                    let named_region =
+                        self.regioncx.name_regions_for_member_constraint(infcx.tcx, member_region);
+                    let diag = unexpected_hidden_region_diagnostic(
+                        infcx,
+                        self.mir_def_id(),
+                        hidden_type.span,
+                        named_ty,
+                        named_region,
+                        named_key,
+                    );
+                    if last_unexpected_hidden_region
+                        != Some((hidden_type.span, named_ty, named_key))
+                    {
+                        last_unexpected_hidden_region =
+                            Some((hidden_type.span, named_ty, named_key));
+                        diag.emit()
+                    } else {
+                        diag.delay_as_bug()
+                    }
                 }
+                DeferredOpaqueTypeError::NonDefiningUseInDefiningScope {
+                    span,
+                    opaque_type_key,
+                } => infcx.dcx().span_err(
+                    span,
+                    format!(
+                        "non-defining use of `{}` in the defining scope",
+                        Ty::new_opaque(
+                            infcx.tcx,
+                            opaque_type_key.def_id.to_def_id(),
+                            opaque_type_key.args
+                        )
+                    ),
+                ),
             });
         }
         let guar = guar.unwrap();
@@ -194,7 +244,7 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for FindOpaqueRegion<'_, 'tcx> {
 
                 // Find a path between the borrow region and our opaque capture.
                 if let Some((path, _)) =
-                    self.regioncx.find_constraint_paths_between_regions(self.borrow_region, |r| {
+                    self.regioncx.find_constraint_path_between_regions(self.borrow_region, |r| {
                         r == opaque_region_vid
                     })
                 {

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -23,7 +23,6 @@ use rustc_trait_selection::error_reporting::infer::nice_region_error::{
     self, HirTraitObjectVisitor, NiceRegionError, TraitObjectVisitor, find_anon_type,
     find_param_with_region, suggest_adding_lifetime_params,
 };
-use rustc_trait_selection::error_reporting::infer::region::unexpected_hidden_region_diagnostic;
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::{Obligation, ObligationCtxt};
 use tracing::{debug, instrument, trace};
@@ -105,18 +104,6 @@ pub(crate) enum RegionErrorKind<'tcx> {
     /// A generic bound failure for a type test (`T: 'a`).
     TypeTestError { type_test: TypeTest<'tcx> },
 
-    /// An unexpected hidden region for an opaque type.
-    UnexpectedHiddenRegion {
-        /// The span for the member constraint.
-        span: Span,
-        /// The hidden type.
-        hidden_ty: Ty<'tcx>,
-        /// The opaque type.
-        key: ty::OpaqueTypeKey<'tcx>,
-        /// The unexpected region.
-        member_region: ty::Region<'tcx>,
-    },
-
     /// Higher-ranked subtyping error.
     BoundUniversalRegionError {
         /// The placeholder free region.
@@ -323,11 +310,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     pub(crate) fn report_region_errors(&mut self, nll_errors: RegionErrors<'tcx>) {
         // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are
         // buffered in the `MirBorrowckCtxt`.
-
         let mut outlives_suggestion = OutlivesSuggestionBuilder::default();
-        let mut last_unexpected_hidden_region: Option<(Span, Ty<'_>, ty::OpaqueTypeKey<'tcx>)> =
-            None;
-
         for (nll_error, _) in nll_errors.into_iter() {
             match nll_error {
                 RegionErrorKind::TypeTestError { type_test } => {
@@ -378,30 +361,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     }
                 }
 
-                RegionErrorKind::UnexpectedHiddenRegion { span, hidden_ty, key, member_region } => {
-                    let named_ty =
-                        self.regioncx.name_regions_for_member_constraint(self.infcx.tcx, hidden_ty);
-                    let named_key =
-                        self.regioncx.name_regions_for_member_constraint(self.infcx.tcx, key);
-                    let named_region = self
-                        .regioncx
-                        .name_regions_for_member_constraint(self.infcx.tcx, member_region);
-                    let diag = unexpected_hidden_region_diagnostic(
-                        self.infcx,
-                        self.mir_def_id(),
-                        span,
-                        named_ty,
-                        named_region,
-                        named_key,
-                    );
-                    if last_unexpected_hidden_region != Some((span, named_ty, named_key)) {
-                        self.buffer_error(diag);
-                        last_unexpected_hidden_region = Some((span, named_ty, named_key));
-                    } else {
-                        diag.delay_as_bug();
-                    }
-                }
-
                 RegionErrorKind::BoundUniversalRegionError {
                     longer_fr,
                     placeholder,

compiler/rustc_borrowck/src/handle_placeholders.rs

@@ -15,7 +15,6 @@ use tracing::debug;
 use crate::constraints::{ConstraintSccIndex, OutlivesConstraintSet};
 use crate::consumers::OutlivesConstraint;
 use crate::diagnostics::UniverseInfo;
-use crate::member_constraints::MemberConstraintSet;
 use crate::region_infer::values::{LivenessValues, PlaceholderIndices};
 use crate::region_infer::{ConstraintSccs, RegionDefinition, Representative, TypeTest};
 use crate::ty::VarianceDiagInfo;
@@ -30,7 +29,6 @@ pub(crate) struct LoweredConstraints<'tcx> {
     pub(crate) constraint_sccs: Sccs<RegionVid, ConstraintSccIndex>,
     pub(crate) definitions: Frozen<IndexVec<RegionVid, RegionDefinition<'tcx>>>,
     pub(crate) scc_annotations: IndexVec<ConstraintSccIndex, RegionTracker>,
-    pub(crate) member_constraints: MemberConstraintSet<'tcx, RegionVid>,
     pub(crate) outlives_constraints: Frozen<OutlivesConstraintSet<'tcx>>,
     pub(crate) type_tests: Vec<TypeTest<'tcx>>,
     pub(crate) liveness_constraints: LivenessValues,
@@ -147,9 +145,10 @@ impl scc::Annotation for RegionTracker {
 
 /// Determines if the region variable definitions contain
 /// placeholders, and compute them for later use.
-fn region_definitions<'tcx>(
-    universal_regions: &UniversalRegions<'tcx>,
+// FIXME: This is also used by opaque type handling. Move it to a separate file.
+pub(super) fn region_definitions<'tcx>(
     infcx: &BorrowckInferCtxt<'tcx>,
+    universal_regions: &UniversalRegions<'tcx>,
 ) -> (Frozen<IndexVec<RegionVid, RegionDefinition<'tcx>>>, bool) {
     let var_infos = infcx.get_region_var_infos();
     // Create a RegionDefinition for each inference variable. This happens here because
@@ -213,14 +212,13 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>(
     infcx: &BorrowckInferCtxt<'tcx>,
 ) -> LoweredConstraints<'tcx> {
     let universal_regions = &universal_region_relations.universal_regions;
-    let (definitions, has_placeholders) = region_definitions(universal_regions, infcx);
+    let (definitions, has_placeholders) = region_definitions(infcx, universal_regions);
 
     let MirTypeckRegionConstraints {
         placeholder_indices,
         placeholder_index_to_region: _,
         liveness_constraints,
         mut outlives_constraints,
-        member_constraints,
         universe_causes,
         type_tests,
     } = constraints;
@@ -246,7 +244,6 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>(
 
         return LoweredConstraints {
             type_tests,
-            member_constraints,
             constraint_sccs,
             scc_annotations: scc_annotations.scc_to_annotation,
             definitions,
@@ -283,7 +280,6 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>(
         constraint_sccs,
         definitions,
         scc_annotations,
-        member_constraints,
         outlives_constraints: Frozen::freeze(outlives_constraints),
         type_tests,
         liveness_constraints,

compiler/rustc_borrowck/src/lib.rs

@@ -78,7 +78,6 @@ mod dataflow;
 mod def_use;
 mod diagnostics;
 mod handle_placeholders;
-mod member_constraints;
 mod nll;
 mod path_utils;
 mod place_ext;
@@ -335,9 +334,10 @@ fn do_mir_borrowck<'tcx>(
 
     // Run the MIR type-checker.
     let MirTypeckResults {
-        constraints,
+        mut constraints,
         universal_region_relations,
-        opaque_type_values,
+        region_bound_pairs,
+        known_type_outlives_obligations,
         polonius_context,
     } = type_check::type_check(
         root_cx,
@@ -352,6 +352,17 @@ fn do_mir_borrowck<'tcx>(
         Rc::clone(&location_map),
     );
 
+    let opaque_type_errors = region_infer::opaque_types::handle_opaque_type_uses(
+        root_cx,
+        &infcx,
+        &body,
+        &universal_region_relations,
+        &region_bound_pairs,
+        &known_type_outlives_obligations,
+        &location_map,
+        &mut constraints,
+    );
+
     // Compute non-lexical lifetimes using the constraints computed
     // by typechecking the MIR body.
     let nll::NllOutput {
@@ -375,8 +386,6 @@ fn do_mir_borrowck<'tcx>(
         polonius_context,
     );
 
-    let opaque_type_errors = regioncx.infer_opaque_types(root_cx, &infcx, opaque_type_values);
-
     // Dump MIR results into a file, if that is enabled. This lets us
     // write unit-tests, as well as helping with debugging.
     nll::dump_nll_mir(&infcx, body, &regioncx, &opt_closure_req, &borrow_set);