Rollup merge of rust-lang#73335 - Aaron1011:fix/rustdoc-overflow, r=e…

…stebank

Ignore overflow when finding auto-trait impls in Rustdoc

In rust-lang#72936 (comment),
it was determined that some unusual code could cause rustc to overflow
when evaluating a predicate of the form `T: AutoTrait`. Even if this is
a bug, it will still be possible to cause overflow through writing
explicit impls of auto traits, just like any other type of impl.

In rustdoc, this overflow can happen simply as a result of defining
certain types, since we will automatically generate and evaluate
auto-trait predicates when generating documentation.

For now, we just ignore overflow during selection if it occurs in
rustdoc. We should probably come up with a better way to handle this -
e.g. rendering some kind of error in the generated documentation.
However, this is a very unusual corner case, and this PR is sufficient
to unblock landing a Chalk update in PR rust-lang#72936

This adds additional hacks to `librustc_trait_selection`. The
auto-trait-finding code should probably be completely rewritten, but I
think this is good enough for the time being.

src/librustc_trait_selection/traits/auto_trait.rs

@@ -5,6 +5,7 @@ use super::*;
 
 use crate::infer::region_constraints::{Constraint, RegionConstraintData};
 use crate::infer::InferCtxt;
+use rustc_middle::infer::canonical::OriginalQueryValues;
 use rustc_middle::ty::fold::TypeFolder;
 use rustc_middle::ty::{Region, RegionVid};
 
@@ -88,7 +89,7 @@ impl<'tcx> AutoTraitFinder<'tcx> {
         let trait_pred = ty::Binder::bind(trait_ref);
 
         let bail_out = tcx.infer_ctxt().enter(|infcx| {
-            let mut selcx = SelectionContext::with_negative(&infcx, true);
+            let mut selcx = SelectionContext::with_negative_and_force_overflow(&infcx);
             let result = selcx.select(&Obligation::new(
                 ObligationCause::dummy(),
                 orig_env,
@@ -186,11 +187,26 @@ impl<'tcx> AutoTraitFinder<'tcx> {
             // At this point, we already have all of the bounds we need. FulfillmentContext is used
             // to store all of the necessary region/lifetime bounds in the InferContext, as well as
             // an additional sanity check.
-            let mut fulfill = FulfillmentContext::new();
+            let mut fulfill = FulfillmentContext::new_rustdoc();
             fulfill.register_bound(&infcx, full_env, ty, trait_did, ObligationCause::dummy());
-            fulfill.select_all_or_error(&infcx).unwrap_or_else(|e| {
-                panic!("Unable to fulfill trait {:?} for '{:?}': {:?}", trait_did, ty, e)
-            });
+            if let Err(errs) = fulfill.select_all_or_error(&infcx) {
+                if errs.iter().all(|err| {
+                    matches!(
+                        err.code,
+                        FulfillmentErrorCode::CodeSelectionError(SelectionError::Overflow)
+                    )
+                }) {
+                    info!(
+                        "rustdoc: overflow occured when processing auto-trait {:?} for ty {:?}",
+                        trait_did, ty
+                    );
+                } else {
+                    panic!(
+                        "Failed to fulfill: {:?} {:?} {:?} : errors {:?}",
+                        ty, trait_did, orig_env, errs
+                    );
+                }
+            }
 
             let body_id_map: FxHashMap<_, _> = infcx
                 .inner
@@ -268,9 +284,10 @@ impl AutoTraitFinder<'tcx> {
         fresh_preds: &mut FxHashSet<ty::Predicate<'tcx>>,
         only_projections: bool,
     ) -> Option<(ty::ParamEnv<'tcx>, ty::ParamEnv<'tcx>)> {
+        debug!("evaluate_predicates: trait_did={:?} ty={:?}", trait_did, ty);
         let tcx = infcx.tcx;
 
-        let mut select = SelectionContext::with_negative(&infcx, true);
+        let mut select = SelectionContext::with_negative_and_force_overflow(&infcx);
 
         let mut already_visited = FxHashSet::default();
         let mut predicates = VecDeque::new();
@@ -290,9 +307,21 @@ impl AutoTraitFinder<'tcx> {
         while let Some(pred) = predicates.pop_front() {
             infcx.clear_caches();
 
-            if !already_visited.insert(pred) {
+            let mut _orig_values = OriginalQueryValues::default();
+            // Canonicalize the predicate before inserting it into our map.
+            // In some cases, we may repeatedly generate a predicate with fresh
+            // inference variables which is otherwise identical to an existing predicate.
+            // To avoid infinitely looping, we treat these predicates as equivalent
+            // for the purposes of determining when to stop.
+            //
+            // We check that our computed `ParamEnv` is sufficient using `FulfillmentContext`,
+            // so we should get an error if we skip over a necessary predicate.
+            let canonical_pred = infcx.canonicalize_query(&pred, &mut _orig_values);
+
+            if !already_visited.insert(canonical_pred) {
                 continue;
             }
+            debug!("evaluate_predicates: predicate={:?}", pred);
 
             // Call `infcx.resolve_vars_if_possible` to see if we can
             // get rid of any inference variables.
@@ -341,7 +370,7 @@ impl AutoTraitFinder<'tcx> {
                 &Ok(None) => {}
                 &Err(SelectionError::Unimplemented) => {
                     if self.is_param_no_infer(pred.skip_binder().trait_ref.substs) {
-                        already_visited.remove(&pred);
+                        already_visited.remove(&canonical_pred);
                         self.add_user_pred(
                             &mut user_computed_preds,
                             ty::PredicateKind::Trait(pred, hir::Constness::NotConst)

src/librustc_trait_selection/traits/fulfill.rs

@@ -70,6 +70,9 @@ pub struct FulfillmentContext<'tcx> {
     // a snapshot (they don't *straddle* a snapshot, so there
     // is no trouble there).
     usable_in_snapshot: bool,
+
+    // See `SelectionContext::force_propagate_overflow`
+    rustdoc: bool,
 }
 
 #[derive(Clone, Debug)]
@@ -93,6 +96,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
             predicates: ObligationForest::new(),
             register_region_obligations: true,
             usable_in_snapshot: false,
+            rustdoc: false,
         }
     }
 
@@ -101,6 +105,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
             predicates: ObligationForest::new(),
             register_region_obligations: true,
             usable_in_snapshot: true,
+            rustdoc: false,
         }
     }
 
@@ -109,6 +114,16 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
             predicates: ObligationForest::new(),
             register_region_obligations: false,
             usable_in_snapshot: false,
+            rustdoc: false,
+        }
+    }
+
+    pub fn new_rustdoc() -> FulfillmentContext<'tcx> {
+        FulfillmentContext {
+            predicates: ObligationForest::new(),
+            register_region_obligations: false,
+            usable_in_snapshot: false,
+            rustdoc: true,
         }
     }
 
@@ -176,7 +191,11 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
 
         // FIXME(#20304) -- cache
 
-        let mut selcx = SelectionContext::new(infcx);
+        let mut selcx = if self.rustdoc {
+            SelectionContext::with_negative_and_force_overflow(infcx)
+        } else {
+            SelectionContext::new(infcx)
+        };
         let mut obligations = vec![];
         let normalized_ty = project::normalize_projection_type(
             &mut selcx,
@@ -229,7 +248,11 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
     ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
-        let mut selcx = SelectionContext::new(infcx);
+        let mut selcx = if self.rustdoc {
+            SelectionContext::with_negative_and_force_overflow(infcx)
+        } else {
+            SelectionContext::new(infcx)
+        };
         self.select(&mut selcx)
     }
 

src/librustc_trait_selection/traits/select/mod.rs

@@ -82,6 +82,11 @@ pub struct SelectionContext<'cx, 'tcx> {
     /// and a negative impl
     allow_negative_impls: bool,
 
+    /// If `true`, propagate `OverflowError` instead of calling `report_overflow_error`,
+    /// regardless of `TraitQueryMode.` This is a hack used by `rustdoc` to avoid
+    /// aborting the compilation session when overflow occurs.
+    force_propagate_overflow: bool,
+
     /// The mode that trait queries run in, which informs our error handling
     /// policy. In essence, canonicalized queries need their errors propagated
     /// rather than immediately reported because we do not have accurate spans.
@@ -174,6 +179,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
             query_mode: TraitQueryMode::Standard,
+            force_propagate_overflow: false,
         }
     }
 
@@ -185,21 +191,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
             query_mode: TraitQueryMode::Standard,
+            force_propagate_overflow: false,
         }
     }
 
-    pub fn with_negative(
+    /// Only for use by rustdoc
+    pub fn with_negative_and_force_overflow(
         infcx: &'cx InferCtxt<'cx, 'tcx>,
-        allow_negative_impls: bool,
     ) -> SelectionContext<'cx, 'tcx> {
-        debug!("with_negative({:?})", allow_negative_impls);
+        debug!("with_negative_and_force_overflow()");
         SelectionContext {
             infcx,
             freshener: infcx.freshener(),
             intercrate: false,
             intercrate_ambiguity_causes: None,
-            allow_negative_impls,
+            allow_negative_impls: true,
             query_mode: TraitQueryMode::Standard,
+            force_propagate_overflow: true,
         }
     }
 
@@ -215,6 +223,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             intercrate_ambiguity_causes: None,
             allow_negative_impls: false,
             query_mode,
+            force_propagate_overflow: false,
         }
     }
 
@@ -281,7 +290,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             Err(SelectionError::Overflow) => {
                 // In standard mode, overflow must have been caught and reported
                 // earlier.
-                assert!(self.query_mode == TraitQueryMode::Canonical);
+                assert!(
+                    self.query_mode == TraitQueryMode::Canonical || self.force_propagate_overflow
+                );
                 return Err(SelectionError::Overflow);
             }
             Err(e) => {
@@ -295,7 +306,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         match self.confirm_candidate(obligation, candidate) {
             Err(SelectionError::Overflow) => {
-                assert!(self.query_mode == TraitQueryMode::Canonical);
+                assert!(
+                    self.query_mode == TraitQueryMode::Canonical || self.force_propagate_overflow
+                );
                 Err(SelectionError::Overflow)
             }
             Err(e) => Err(e),
@@ -908,6 +921,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         if !self.infcx.tcx.sess.recursion_limit().value_within_limit(obligation.recursion_depth) {
             match self.query_mode {
                 TraitQueryMode::Standard => {
+                    if self.force_propagate_overflow {
+                        return Err(OverflowError);
+                    }
                     self.infcx().report_overflow_error(error_obligation, true);
                 }
                 TraitQueryMode::Canonical => {

src/test/rustdoc/synthetic_auto/overflow.rs

@@ -0,0 +1,35 @@
+// Tests that we don't fail with an overflow error for certain
+// strange types
+// See https://github.com/rust-lang/rust/pull/72936#issuecomment-643676915
+
+pub trait Interner {
+    type InternedType;
+}
+
+struct RustInterner<'tcx> {
+    foo: &'tcx ()
+}
+
+impl<'tcx> Interner for RustInterner<'tcx> {
+    type InternedType = Box<TyData<Self>>;
+}
+
+enum TyData<I: Interner> {
+    FnDef(I::InternedType)
+}
+
+struct VariableKind<I: Interner>(I::InternedType);
+
+// @has overflow/struct.BoundVarsCollector.html
+// @has - '//code' "impl<'tcx> Send for BoundVarsCollector<'tcx>"
+pub struct BoundVarsCollector<'tcx> {
+    val: VariableKind<RustInterner<'tcx>>
+}
+
+fn is_send<T: Send>() {}
+
+struct MyInterner<'tcx> {
+    val: &'tcx ()
+}
+
+fn main() {}