From 5c0eb6ecb7ed5a91275582eb245299fd00e74ca2 Mon Sep 17 00:00:00 2001
From: Ariel Ben-Yehuda <ariel.byd@gmail.com>
Date: Tue, 29 Nov 2016 00:12:07 +0200
Subject: [PATCH] evaluate obligations in LIFO order during closure projection

This is an annoying gotcha with the projection cache's handling of
nested obligations.

Nested projection obligations enter the issue in this case:
```
DEBUG:rustc::traits::project: AssociatedTypeNormalizer: depth=3
normalized
<std::iter::Map<std::ops::Range<i32>,
[closure@not-a-recursion-error.rs:5:30: 5:53]> as
std::iter::IntoIterator>::Item to _#7t with 12 add'l obligations
```

Here the normalization result is the result of the nested impl
`<[closure@not-a-recursion-error.rs:5:30: 5:53] as FnMut(i32)>::Output`,
which is an additional obligation that is a part of "add'l obligations".

By itself, this is proper behaviour - the additional obligation is
returned, and the RFC 447 rules ensure that it is processed before the
output `#_7t` is used in any way.

However, the projection cache breaks this - it caches the
`<std::iter::Map<std::ops::Range<i32>,[closure@not-a-recursion-error.rs:5:30:
5:53]> as std::iter::IntoIterator>::Item = #_7t` resolution. Now
everybody else that attempts to look up the projection will just get
`#_7t` *without* any additional obligations. This obviously causes all
sorts of trouble (here a spurious `EvaluatedToAmbig` results in
specializations not being discarded
[here](https://github.com/rust-lang/rust/blob/9ca50bd4d50b55456e88a8c3ad8fcc9798f57522/src/librustc/traits/select.rs#L1705)).

The compiler works even with this projection cache gotcha because in most
cases during "one-pass evaluation". we tend to process obligations in LIFO
order - after an obligation is added to the cache, we process its nested
obligations before we do anything else (and if we have a cycle, we handle
it specifically) - which makes sure the inference variables are resolved
before they are used.

That "LIFO" order That was not done when projecting out of a closure, so
let's just fix that for the time being.

Fixes #38033.
---
 src/librustc/traits/project.rs   |  2 +-
 src/test/run-pass/issue-38033.rs | 88 ++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 src/test/run-pass/issue-38033.rs

diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs
index 76bead99343a7..24731e8ae7bf4 100644
--- a/src/librustc/traits/project.rs
+++ b/src/librustc/traits/project.rs
@@ -1215,8 +1215,8 @@ fn confirm_closure_candidate<'cx, 'gcx, 'tcx>(
                                obligation,
                                &closure_type.sig,
                                util::TupleArgumentsFlag::No)
-        .with_addl_obligations(obligations)
         .with_addl_obligations(vtable.nested)
+        .with_addl_obligations(obligations)
 }
 
 fn confirm_callable_candidate<'cx, 'gcx, 'tcx>(
diff --git a/src/test/run-pass/issue-38033.rs b/src/test/run-pass/issue-38033.rs
new file mode 100644
index 0000000000000..50549dc8b2380
--- /dev/null
+++ b/src/test/run-pass/issue-38033.rs
@@ -0,0 +1,88 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::marker;
+use std::mem;
+
+fn main() {
+    let workers = (0..0).map(|_| result::<u32, ()>());
+    drop(join_all(workers).poll());
+}
+
+trait Future {
+    type Item;
+    type Error;
+
+    fn poll(&mut self) -> Result<Self::Item, Self::Error>;
+}
+
+trait IntoFuture {
+    type Future: Future<Item=Self::Item, Error=Self::Error>;
+    type Item;
+    type Error;
+
+    fn into_future(self) -> Self::Future;
+}
+
+impl<F: Future> IntoFuture for F {
+    type Future = F;
+    type Item = F::Item;
+    type Error = F::Error;
+
+    fn into_future(self) -> F {
+        self
+    }
+}
+
+struct FutureResult<T, E> {
+    _inner: marker::PhantomData<(T, E)>,
+}
+
+fn result<T, E>() -> FutureResult<T, E> {
+    loop {}
+}
+
+impl<T, E> Future for FutureResult<T, E> {
+    type Item = T;
+    type Error = E;
+
+    fn poll(&mut self) -> Result<T, E> {
+        loop {}
+    }
+}
+
+struct JoinAll<I>
+    where I: IntoIterator,
+          I::Item: IntoFuture,
+{
+    elems: Vec<<I::Item as IntoFuture>::Item>,
+}
+
+fn join_all<I>(_: I) -> JoinAll<I>
+    where I: IntoIterator,
+          I::Item: IntoFuture,
+{
+    JoinAll { elems: vec![] }
+}
+
+impl<I> Future for JoinAll<I>
+    where I: IntoIterator,
+          I::Item: IntoFuture,
+{
+    type Item = Vec<<I::Item as IntoFuture>::Item>;
+    type Error = <I::Item as IntoFuture>::Error;
+
+    fn poll(&mut self) -> Result<Self::Item, Self::Error> {
+        let elems = mem::replace(&mut self.elems, Vec::new());
+        Ok(elems.into_iter().map(|e| {
+            e
+        }).collect::<Vec<_>>())
+    }
+}