evaluate obligations in LIFO order during closure projection #38059
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arielb1
added
the
beta-nominated
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 rust-lang#38033.
arielb1
force-pushed
the
no-mere-overflow
branch
from
0131fe6
to
5c0eb6e
Compare
|
@bors r+ |
|
📌 Commit 5c0eb6e has been approved by |
nikomatsakis
added
the
beta-accepted
|
Marking as beta-accepted because this is such a tiny delta and fixes a regression. cc @rust-lang/compiler |
bors
added a commit
that referenced
this pull request
Dec 3, 2016
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. Beta-nominating because regression. r? @nikomatsakis
alexcrichton
removed
the
beta-nominated
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an annoying gotcha with the projection cache's handling of
nested obligations.
Nested projection obligations enter the issue in this case:
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
#_7tis 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 = #_7tresolution. Noweverybody else that attempts to look up the projection will just get
#_7twithout any additional obligations. This obviously causes allsorts of trouble (here a spurious
EvaluatedToAmbigresults inspecializations not being discarded
here).
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.
Beta-nominating because regression.
r? @nikomatsakis