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.

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

Embark standard lints v0.4 - require Rust 1.52 #66

Merged
merged 5 commits into from
Jun 16, 2021
Merged

Conversation

repi
Copy link
Contributor

@repi repi commented Jun 15, 2021

With these changes our standard lint set now requires Rust 1.52, and builds on Rust 1.53 / beta / nightly without warnings about renamed/changed lints.

Removals

Additions

Enables the following lints that we've been testing with in Ark for a few months. Many of them are somewhat low impact but low cost so just nice to have enabled.

Part of #59

@repi repi changed the title Standard lints v0.4 Embark standard lints v0.4 Jun 15, 2021
@repi
Copy link
Contributor Author

repi commented Jun 16, 2021

@khyperia can you test using this draft set in rust-gpu and see if there are any new lints that are problematic in it? If it is serious we could exclude it from here, or otherwise just override in that repro itself.

These do work fine in ark and our modules.

@repi repi changed the title Embark standard lints v0.4 Embark standard lints v0.4 - require Rust 1.52 Jun 16, 2021
@repi repi marked this pull request as ready for review June 16, 2021 08:32
@khyperia
Copy link

@khyperia can you test using this draft set in rust-gpu and see if there are any new lints that are problematic in it? If it is serious we could exclude it from here, or otherwise just override in that repro itself.

Works great in rust-gpu! Found a couple silly/small issues, too. Only thing is that this change still says END - Embark standard lints **v0.3** instead of v0.4 :P

Copy link
Member

@Jake-Shadle Jake-Shadle left a comment

Choose a reason for hiding this comment

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

LGTM!

@repi
Copy link
Contributor Author

repi commented Jun 16, 2021

@khyperia thanks for testing! oops had forgotten to push the latest that fixed the ending and included two new lints (implicit_clone and semicolon_if_nothing_returned).

Think we can get this in today then. I did take a look at the new clippy lints in Rust 1.53 but of the 4 new none of them were that interesting for now, so think fine to settle on 1.52 as requirement for this

@repi repi merged commit b0176cc into main Jun 16, 2021
@repi repi deleted the standard-lints-v0.4 branch June 16, 2021 19:54
@repi repi mentioned this pull request Jun 16, 2021
@MarijnS95
Copy link

Interesting choice! I've been advocating this the other way around to guarantee (by the compiler) that the return types (usually void, ()) match up. If the inner function suddenly changes return signature but lacks a #[must_use] this is now easily caught instead of going unnoticed.

Other than that another nice set of lints! ptr_as_ptr is quite some churn for us but was planned anyway and it's great to have it enabled now!

boulos added a commit to boulos/tame-oauth that referenced this pull request Jul 29, 2021
Following EmbarkStudios/rust-ecosystem#66,
tame-oauth also needed to update.
boulos added a commit to boulos/tame-oauth that referenced this pull request Jul 30, 2021
Following EmbarkStudios/rust-ecosystem#66,
tame-oauth also needed to update.
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.

4 participants