Skip to content

Remove lifetime bounds in higher-ranked lifetimes#62

Merged
bors[bot] merged 1 commit into
rustgd:masterfrom
RalfJung:master
Mar 9, 2018
Merged

Remove lifetime bounds in higher-ranked lifetimes#62
bors[bot] merged 1 commit into
rustgd:masterfrom
RalfJung:master

Conversation

@RalfJung

@RalfJung RalfJung commented Mar 9, 2018

Copy link
Copy Markdown
Contributor

Those bounds are ignored by the compiler, and will trigger an error once rust-lang/rust#48326 lands. According to crater, this is the only crate using these bounds. This patch should make the crate work on nightly again.

However, I have been unable to compile the crate even with nightly 2018-02-25, which does not include the change mentioned above. So, I couldn't compile-test this PR.


This change is Reviewable

Those bounds are ignored by the compiler, and will soon trigger an error.
According to crater, this is the only crate using these bounds.
@Rhuagh

Rhuagh commented Mar 9, 2018

Copy link
Copy Markdown
Collaborator

Thanks!

Bors r+

bors Bot added a commit that referenced this pull request Mar 9, 2018
62: Remove lifetime bounds in higher-ranked lifetimes r=Rhuagh a=RalfJung

Those bounds are ignored by the compiler, and will trigger an error once rust-lang/rust#48326 lands. According to crater, this is the only crate using these bounds. This patch should make the crate work on nightly again.

However, I have been unable to compile the crate even with nightly 2018-02-25, which does not include the change mentioned above. So, I couldn't compile-test this PR.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/rustgd/rhusics/62)
<!-- Reviewable:end -->
@Rhuagh

Rhuagh commented Mar 9, 2018

Copy link
Copy Markdown
Collaborator

We'll see if it builds on CI :)

@bors

bors Bot commented Mar 9, 2018

Copy link
Copy Markdown
Contributor

Build succeeded

@bors
bors Bot merged commit 2727673 into rustgd:master Mar 9, 2018
@Rhuagh

Rhuagh commented Mar 9, 2018

Copy link
Copy Markdown
Collaborator

Looks like it works there atleast

@RalfJung

RalfJung commented Mar 9, 2018

Copy link
Copy Markdown
Contributor Author

The build failure was caused by rust-lang/rust#48551. That's fixed, but my branch where I develop rust-lang/rust#48326 does not include the fix yet, so I can't test whether this will keep working when my patch lands. However, these are all the occurrences of higher-ranked lifetime bounds that crater found.

Thanks for merging so quickly. :) Please release a new minor version of the create before the patch hits stable (which will take ~9 weeks).

@Rhuagh

Rhuagh commented Mar 9, 2018

Copy link
Copy Markdown
Collaborator

We will probably release a new major version before that (we have some breaking changes).

@RalfJung

RalfJung commented Mar 9, 2018

Copy link
Copy Markdown
Contributor Author

AFAIK a new minor version with the fix would be necessary for reverse dependencies that will just do cargo update -- they won't get the new major version automatically, so they would fail to build.

@Rhuagh

Rhuagh commented Mar 9, 2018

Copy link
Copy Markdown
Collaborator

True, I'll get that out tonight.

@RalfJung

RalfJung commented Mar 9, 2018

Copy link
Copy Markdown
Contributor Author

I compiled rustc with my patch merged into master, and can confirm that this can build current rhusics master (while it fails on the version without my patch, as expected).

So, looking good :)

@Rhuagh

Rhuagh commented Mar 13, 2018

Copy link
Copy Markdown
Collaborator

The breaking changes had not been merged to master yet, so this was easier than I thought it would :) Anyways, new minors have been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants