Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[beta] back out #53793 - stabilize outlives requirements #54877

Merged
merged 2 commits into from
Oct 9, 2018
Merged

[beta] back out #53793 - stabilize outlives requirements #54877

merged 2 commits into from
Oct 9, 2018

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Oct 6, 2018

Fixes #54467 for beta, looks like a less risky fix than #54701

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2018
@Mark-Simulacrum
Copy link
Member

cc @Centril @aturon -- outlives requirements are a part of 2018 I believe which makes this worrisome. Do we need to prioritize resources to finding the underlying cause? (Not exactly sure what the status is here)

@Centril
Copy link
Contributor

Centril commented Oct 7, 2018 •
edited
Loading

@Mark-Simulacrum They are; but we could also drop it from 2018 if it can't be done. However, that would be quite unfortunate and make 2018 less exciting. As for finding the underlying cause I can't help out myself, but I do think it's worth prioritizing this. I'd wait for @aturon's input tho.

@arielb1
Copy link
Contributor Author

arielb1 commented Oct 8, 2018 •
edited
Loading

@Centril

We have a fix for #54467 on nightly. I just don't want to push it to beta. If you think that getting more experience for stable outlives requirements is a priority, there's no reason not to backport #54467.

@Centril
Copy link
Contributor

Centril commented Oct 8, 2018

@arielb1 I'm quite unsure what to do here so I'll defer to someone else (e.g. @aturon).

@nikomatsakis
Copy link
Contributor

Perhaps I am missing something. This PR merely removes the stabilization from the current beta — this change will still be considered stable on nightly and in RC2 when that branch breaks, and hence it will still be part of Rust 2018. It's just arriving a bit later to avoid a more complex backport.

The only real reason I can see to land this change earlier would be to help us flush out more bugs, but finding those bugs on stable is usually not the sort of thing we try to do (rather, we tend to prefer to give a patch more time for testing on nightly and beta).

@nikomatsakis
Copy link
Contributor

r? @nikomatsakis -- stealing this from @eddyb

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Oct 8, 2018
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 8, 2018

📌 Commit e954510 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 8, 2018
@nikomatsakis
Copy link
Contributor

After some brief conversation on Discord, it seems clear we can move forward with this.

@pietroalbini
Copy link
Member

@bors p=1 (beta backport)

@bors
Copy link
Contributor

bors commented Oct 9, 2018

⌛ Testing commit e954510 with merge 1320d21...

bors added a commit that referenced this pull request Oct 9, 2018
@bors
Auto merge of #54877 - arielb1:destabilize-outlives, r=nikomatsakis
1320d21 
[beta] back out #53793 - stabilize outlives requirements

Fixes #54467 for beta, looks like a less risky fix than #54701
@bors
Copy link
Contributor

bors commented Oct 9, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 1320d21 to beta...

@bors bors merged commit e954510 into rust-lang:beta Oct 9, 2018
@pnkfelix pnkfelix mentioned this pull request Oct 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@arielb1 @rust-highfive @Mark-Simulacrum @Centril @nikomatsakis @bors @pietroalbini @eddyb