-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Plumb inference obligations through librustc/middle and librustc_typeck #31759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
133c9bc
to
9d01f08
Compare
Ok(()) => { } | ||
Ok(RelateOk { obligations, .. }) => | ||
for obligation in obligations { | ||
fulfillment_cx.register_predicate_obligation(&infcx, obligation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These obligations are registered, but the fulfillment context is never drained (nor was it drained earlier in history just prior to these commits). Is it supposed to get drained?
I am confused w.r.t. the Travis failure. It doesn't seem related, and I can't find any test that was run and actually spat out 'failure'. I'm thinking the test harness buggered out, but am unsure. Thoughts? |
@soltanmm Scroll to the end of https://s3.amazonaws.com/archive.travis-ci.org/jobs/110213891/log.txt. |
Huh. I didn't realize those existed. Thanks, @apasel422 . |
} | ||
} | ||
} | ||
fn relate_result_from_iter<'tcx, T, U, I: IntoIterator<Item=RelateResult<'tcx, U>>>(iter: I) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: add at least a short comment explaining what this function does; also some spacing between items :)
OK, I made some minor comments, but then I remembered the way I had intended to write this code. I'm not sure which would be better, actually, but the way I had in mind would certainly involve fewer changes... basically, the idea was to carry an The only way this would get complicated is if there is code that starts a snapshot and "catches" the result to do something other than rollback -- I'm not sure if that's the case, but it'd have to be rewritten to truncate anything added to the vector in the failed branch. Otherwise, the results would stay the same, and we'd just decide at the top-level whether to ignore this vector (if the result is Err) or return its final result (if the result is Ok). Besides being a smaller diff, this seems like it would likely be quite a bit more efficient, since we'd only ever allocate one vector, rather than allocating a lot of small vectors and folding them up together. |
After writing the other one I don't like this one anymore. See #31867. |
Per suggestion, sending this in as a separate PR in advance of further work on lazy normalization.
r? @nikomatsakis
cc @jroesch