Skip to content

use param_env on the trait_cache key #44741

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

Merged
merged 5 commits into from
Sep 26, 2017

Conversation

qmx
Copy link
Member

@qmx qmx commented Sep 21, 2017

We bailed from making trans_fulfill_obligation return Option or Result, just made it less prone to crashing outside trans

r? @nikomatsakis

@arielb1
Copy link
Contributor

arielb1 commented Sep 21, 2017

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 21, 2017

📌 Commit 03d9056 has been approved by arielb1

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit.

@@ -33,22 +33,22 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {
/// obligations *could be* resolved if we wanted to.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can we extend this comment to say "/// Assumes that this is run after the entire crate has been successfully type-checked."

@alexcrichton
Copy link
Member

@bors: r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Sep 21, 2017

📌 Commit a7e302a has been approved by nikomatsakis

@alexcrichton alexcrichton added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 21, 2017
@bors
Copy link
Collaborator

bors commented Sep 23, 2017

⌛ Testing commit a7e302a33a634fdbadd01b71f97d964b3a6b31f4 with merge b88c670c31bd47a19d5cd788a1e5d7ab8e54d6f8...

@bors
Copy link
Collaborator

bors commented Sep 23, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Sep 23, 2017

Needs to submit a PR to miri, and set miri's toolstate to "Broken". (cc @oli-obk @RalfJung @eddyb)

[00:47:34] error[E0061]: this function takes 3 parameters but 2 parameters were supplied
[00:47:34]     --> /checkout/src/tools/miri/src/librustc_mir/interpret/eval_context.rs:2419:45
[00:47:34]      |
[00:47:34] 2419 |     let vtbl = tcx.trans_fulfill_obligation(DUMMY_SP, ty::Binder(trait_ref));
[00:47:34]      |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 3 parameters
[00:47:34] 
[00:47:35] error: aborting due to previous error
[00:47:35] 
[00:47:35] error: Could not compile `rustc_miri`.

@qmx qmx force-pushed the trans_fulfill_obligation_should_not_crash branch from f7da45f to 9d52cb2 Compare September 25, 2017 13:52
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 25, 2017

📌 Commit 9d52cb2 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Sep 26, 2017

⌛ Testing commit 9d52cb2 with merge 930d3b1...

bors added a commit that referenced this pull request Sep 26, 2017
… r=nikomatsakis

use param_env on the trait_cache key

We bailed from making trans_fulfill_obligation return `Option` or `Result`, just made it less prone to crashing outside trans

r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Sep 26, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 930d3b1 to master...

@bors bors merged commit 9d52cb2 into rust-lang:master Sep 26, 2017
@qmx qmx deleted the trans_fulfill_obligation_should_not_crash branch September 28, 2017 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants