Skip to content

New lint for statement with no effect #422

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 1 commit into from
Oct 29, 2015
Merged

New lint for statement with no effect #422

merged 1 commit into from
Oct 29, 2015

Conversation

sanxiyn
Copy link
Member

@sanxiyn sanxiyn commented Oct 28, 2015

This is an implementation of rust-lang/rust#16170.

Many extensions are possible (tuples, for example), but this implements just struct/enum constructors.

@Ryman
Copy link

Ryman commented Oct 28, 2015

Theoretically, if any of these unit structs implemented Drop it could have an effect?

@sanxiyn
Copy link
Member Author

sanxiyn commented Oct 28, 2015

Yes, Drop should be considered. On the other hand, HasDrop; is still worth warning, because the destructor will run immediately, which is usually not the intention. You need to use let to run the destructor at the end of the block.

@llogiq
Copy link
Contributor

llogiq commented Oct 28, 2015

I too think that at least the lint message should be different when the constructed type implements Drop. Perhaps even give it another lint specifier – what about naked_drop?

@Manishearth
Copy link
Member

Personal preference is that we should not have an extra lint -- I'm okay with both warning and not warning on Drop.

@Manishearth
Copy link
Member

As far as the rest of it goes, we should add a macro check (in_macro), otherwise LGTM

@sanxiyn
Copy link
Member Author

sanxiyn commented Oct 29, 2015

Added the macro check. Leaving warning on Drop as-is, we can open a follwup issue for that. See also discussion on Rust's built-in lint re Drop at rust-lang/rust#21775.

@llogiq
Copy link
Contributor

llogiq commented Oct 29, 2015

LGTM.

llogiq added a commit that referenced this pull request Oct 29, 2015
New lint for statement with no effect
@llogiq llogiq merged commit 555328c into rust-lang:master Oct 29, 2015
@sanxiyn sanxiyn deleted the no-effect branch October 29, 2015 11:43
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