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

in which the unused-must-use lint adapts to the world of impl-Trait #51571

Closed
wants to merge 1 commit into from

Conversation

zackmdavis
Copy link
Member

impl_must_use

The message names the concrete type. If we wanted it to say impl TraitName, that would be a little more work—and would also be potentially misleading, because other implementors of the trait might not be must-use. (Perhaps this is an argument for introducing must-use traits, as was floated in the thread? I guess it depends on whether we want to think of impl Trait as mere syntactic sugar (in which case I think this is fine), or if nothing about the concrete type is supposed to leak into user-visibility.)

Resolves #51560.

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2018
@scottmcm
Copy link
Member

scottmcm commented Jun 15, 2018

I don't know if anything other than fn-level must_use can really work here, since it could be

fn x() -> impl Iterator<Item = i32> {
    std::fs::remove_file("/etc/passwd").unwrap();
    vec![1, 2, 3].into_iter().skip(1)
}

@zackmdavis
Copy link
Member Author

@scottmcm Could you elaborate? I don't see how the fact that some functions could contain malicious lines of code (like the remove_file("/etc/passwd") in your example) has anything to do with this issue.

@scottmcm
Copy link
Member

@zackmdavis Sorry, I should have picked a different filename, since the point was just to have a side-effect, not to be malicious. ensure_db_tables_exist() or similar would also make it reasonable to call the function and ignore the returned iterator.

In some ways, the iterator adapters being lazy is actually an advantage for that, since one is always free to return them after doing something useful since creating them is essentially free.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 15, 2018

@scottmcm you should use let _ = x(); if you want to ignore an iterator.

@zackmdavis I don't like exposing the real type of an impl Trait. Not even if it's just for diagnostics. I'd be fine with treating all impl Trait returning functions as #[must_use] no matter of the real type, but that needs an RFC imo.

@zackmdavis zackmdavis closed this Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants