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

Add #[must_use] to Option and Result overlap methods #79006

Closed

Conversation

bobrippling
Copy link
Contributor

This helps users avoid the dropping of errors/None without checking, for example Miserlou/Loop/pull/66.

This helps users to avoid dropping errors/`None` without checking,
for example Miserlou/Loop/pull/66.
@bobrippling bobrippling force-pushed the must-use-option-result branch 2 times, most recently from c0cd6ca to babfc2e Compare November 12, 2020 22:52
@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. A-result-option Area: Result and Option combinators labels Nov 12, 2020
@bobrippling bobrippling force-pushed the must-use-option-result branch from babfc2e to 29cbcc6 Compare November 13, 2020 18:09
@m-ou-se
Copy link
Member

m-ou-se commented Nov 13, 2020

Unfortunately, it's quite common now to use .ok() to ignore a Result. Changing this would break start warning in quite a lot of existing code.

See #66116 for discussion.

(By the way, the #[must_use] attributes you added on the Option methods have no effect. They return a Result, which is already #[must_use].)

@bobrippling
Copy link
Contributor Author

Ah, thanks for the explanation and link, I was wondering why I was finding so many discard usages!

@bobrippling bobrippling deleted the must-use-option-result branch November 13, 2020 22:17
@m-ou-se
Copy link
Member

m-ou-se commented Nov 14, 2020

@bobrippling That doesn't mean that this is something we can never change, but it does require some more discussion first. Feel free to join the discussion about this on the issue and share your thoughts there. :)

@bobrippling bobrippling restored the must-use-option-result branch June 27, 2021 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-result-option Area: Result and Option combinators 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.

3 participants