about summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authorkennytm <kennytm@gmail.com>2018-10-26 18:24:59 +0800
committerGitHub <noreply@github.com>2018-10-26 18:24:59 +0800
commit46f504543d47e18484a65251918a937147bb8a45 (patch)
tree98a73c2cd1ecea9180a658ec02b08993c3889e41 /src
parentd83376c70511b2fc12506bb22c3bb02353e028d1 (diff)
parent4f2624cac90ee9145175d6e4b9c59b9ab7875a9c (diff)
downloadrust-46f504543d47e18484a65251918a937147bb8a45.tar.gz
rust-46f504543d47e18484a65251918a937147bb8a45.zip
Rollup merge of #55258 - Aaron1011:fix/rustdoc-blanket, r=GuillaumeGomez
Fix Rustdoc ICE when checking blanket impls

Fixes #55001, #54744

Previously, SelectionContext would unconditionally cache the selection
result for an obligation. This worked fine for most users of
SelectionContext, but it caused an issue when used by Rustdoc's blanket
impl finder.

The issue occured when SelectionContext chose a ParamCandidate which
contained inference variables. Since inference variables can change
between calls to select(), it's not safe to cache the selection result -
the chosen candidate might not be applicable for future results, leading
to an ICE when we try to run confirmation.

This commit prevents SelectionContext from caching any ParamCandidate
that contains inference variables. This should always be completely
safe, as trait selection should never depend on a particular result
being cached.

I've also added some extra debug!() statements, which I found helpful in
tracking down this bug.
Diffstat (limited to 'src')
-rw-r--r--src/librustc/infer/combine.rs10
-rw-r--r--src/librustc/infer/equate.rs3
-rw-r--r--src/librustc/traits/select.rs35
-rw-r--r--src/librustdoc/clean/blanket_impl.rs11
-rw-r--r--src/test/rustdoc/issue-55001.rs31
5 files changed, 87 insertions, 3 deletions
diff --git a/src/librustc/infer/combine.rs b/src/librustc/infer/combine.rs
index de8f57ee796..0ee03bc4c6e 100644
--- a/src/librustc/infer/combine.rs
+++ b/src/librustc/infer/combine.rs
@@ -251,6 +251,7 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> {
                   dir: RelationDir)
                   -> RelateResult<'tcx, Generalization<'tcx>>
     {
+        debug!("generalize(ty={:?}, for_vid={:?}, dir={:?}", ty, for_vid, dir);
         // Determine the ambient variance within which `ty` appears.
         // The surrounding equation is:
         //
@@ -273,8 +274,15 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> {
             root_ty: ty,
         };
 
-        let ty = generalize.relate(&ty, &ty)?;
+        let ty = match generalize.relate(&ty, &ty) {
+            Ok(ty) => ty,
+            Err(e) => {
+                debug!("generalize: failure {:?}", e);
+                return Err(e);
+            }
+        };
         let needs_wf = generalize.needs_wf;
+        debug!("generalize: success {{ {:?}, {:?} }}", ty, needs_wf);
         Ok(Generalization { ty, needs_wf })
     }
 }
diff --git a/src/librustc/infer/equate.rs b/src/librustc/infer/equate.rs
index 854960492c9..c7b5ddb8341 100644
--- a/src/librustc/infer/equate.rs
+++ b/src/librustc/infer/equate.rs
@@ -74,6 +74,9 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
         let infcx = self.fields.infcx;
         let a = infcx.type_variables.borrow_mut().replace_if_possible(a);
         let b = infcx.type_variables.borrow_mut().replace_if_possible(b);
+
+        debug!("{}.tys: replacements ({:?}, {:?})", self.tag(), a, b);
+
         match (&a.sty, &b.sty) {
             (&ty::Infer(TyVar(a_id)), &ty::Infer(TyVar(b_id))) => {
                 infcx.type_variables.borrow_mut().equate(a_id, b_id);
diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs
index 49f37179354..2ea16823cc6 100644
--- a/src/librustc/traits/select.rs
+++ b/src/librustc/traits/select.rs
@@ -1522,6 +1522,33 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             .map(|v| v.get(tcx))
     }
 
+    /// Determines whether can we safely cache the result
+    /// of selecting an obligation. This is almost always 'true',
+    /// except when dealing with certain ParamCandidates.
+    ///
+    /// Ordinarily, a ParamCandidate will contain no inference variables,
+    /// since it was usually produced directly from a DefId. However,
+    /// certain cases (currently only librustdoc's blanket impl finder),
+    /// a ParamEnv may be explicitly constructed with inference types.
+    /// When this is the case, we do *not* want to cache the resulting selection
+    /// candidate. This is due to the fact that it might not always be possible
+    /// to equate the obligation's trait ref and the candidate's trait ref,
+    /// if more constraints end up getting added to an inference variable.
+    ///
+    /// Because of this, we always want to re-run the full selection
+    /// process for our obligation the next time we see it, since
+    /// we might end up picking a different SelectionCandidate (or none at all)
+    fn can_cache_candidate(&self,
+        result: &SelectionResult<'tcx, SelectionCandidate<'tcx>>
+     ) -> bool {
+        match result {
+            Ok(Some(SelectionCandidate::ParamCandidate(trait_ref))) => {
+                !trait_ref.skip_binder().input_types().any(|t| t.walk().any(|t_| t_.is_ty_infer()))
+            },
+            _ => true
+        }
+    }
+
     fn insert_candidate_cache(
         &mut self,
         param_env: ty::ParamEnv<'tcx>,
@@ -1531,6 +1558,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     ) {
         let tcx = self.tcx();
         let trait_ref = cache_fresh_trait_pred.skip_binder().trait_ref;
+
+        if !self.can_cache_candidate(&candidate) {
+            debug!("insert_candidate_cache(trait_ref={:?}, candidate={:?} -\
+                    candidate is not cacheable", trait_ref, candidate);
+            return;
+
+        }
+
         if self.can_use_global_caches(param_env) {
             if let Err(Overflow) = candidate {
                 // Don't cache overflow globally; we only produce this
diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs
index b6bc8d603d5..57c7e2afd62 100644
--- a/src/librustdoc/clean/blanket_impl.rs
+++ b/src/librustdoc/clean/blanket_impl.rs
@@ -50,6 +50,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> BlanketImplFinder <'a, 'tcx, 'rcx, 'cstore> {
         name: Option<String>,
     ) -> Vec<Item>
     where F: Fn(DefId) -> Def {
+        debug!("get_blanket_impls(def_id={:?}, ...)", def_id);
         let mut impls = Vec::new();
         if self.cx
             .tcx
@@ -78,6 +79,8 @@ impl<'a, 'tcx, 'rcx, 'cstore> BlanketImplFinder <'a, 'tcx, 'rcx, 'cstore> {
             }
             self.cx.tcx.for_each_relevant_impl(trait_def_id, ty, |impl_def_id| {
                 self.cx.tcx.infer_ctxt().enter(|infcx| {
+                    debug!("get_blanet_impls: Considering impl for trait '{:?}' {:?}",
+                           trait_def_id, impl_def_id);
                     let t_generics = infcx.tcx.generics_of(impl_def_id);
                     let trait_ref = infcx.tcx.impl_trait_ref(impl_def_id)
                                              .expect("Cannot get impl trait");
@@ -104,8 +107,8 @@ impl<'a, 'tcx, 'rcx, 'cstore> BlanketImplFinder <'a, 'tcx, 'rcx, 'cstore> {
                         drop(obligations);
 
                         debug!(
-                            "invoking predicate_may_hold: {:?}",
-                            trait_ref,
+                            "invoking predicate_may_hold: param_env={:?}, trait_ref={:?}, ty={:?}",
+                             param_env, trait_ref, ty
                         );
                         let may_apply = match infcx.evaluate_obligation(
                             &traits::Obligation::new(
@@ -117,6 +120,10 @@ impl<'a, 'tcx, 'rcx, 'cstore> BlanketImplFinder <'a, 'tcx, 'rcx, 'cstore> {
                             Ok(eval_result) => eval_result.may_apply(),
                             Err(traits::OverflowError) => true, // overflow doesn't mean yes *or* no
                         };
+                        debug!("get_blanket_impls: found applicable impl: {}\
+                               for trait_ref={:?}, ty={:?}",
+                               may_apply, trait_ref, ty);
+
                         if !may_apply {
                             return
                         }
diff --git a/src/test/rustdoc/issue-55001.rs b/src/test/rustdoc/issue-55001.rs
new file mode 100644
index 00000000000..f6c7f9a3d08
--- /dev/null
+++ b/src/test/rustdoc/issue-55001.rs
@@ -0,0 +1,31 @@
+// Regression test for issue #55001. Previously, we would incorrectly
+// cache certain trait selection results when checking for blanket impls,
+// resulting in an ICE when we tried to confirm the cached ParamCandidate
+// against an obligation.
+
+pub struct DefaultAllocator;
+pub struct Standard;
+pub struct Inner;
+
+pub trait Rand {}
+
+pub trait Distribution<T> {}
+pub trait Allocator<N> {}
+
+impl<T> Rand for T where Standard: Distribution<T> {}
+
+impl<A> Distribution<Point<A>> for Standard
+where
+DefaultAllocator: Allocator<A>,
+Standard: Distribution<A> {}
+
+impl Distribution<Inner> for Standard {}
+
+
+pub struct Point<N>
+where DefaultAllocator: Allocator<N>
+{
+    field: N
+}
+
+fn main() {}