-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Removed wrong usage of default
from the Option API
#10854
Conversation
I feel like the arg-swap is a negative. The closure can be a large block of code, which leaves the default value hidden behind it. shrugs |
Shouldn't there be a map_or_else that takes a closure for the default value? Typical use is when the default value needs memory allocations to be constructed, or needs to be retrieved with a system call or a communication mechanism. |
This is a change to some core types which needs to be discussed. |
@bill-myers It's hard to draw the line there - if we just added everything that might be useful we might end up with a lot of maintenance burden for functions no one ever uses - in this case In any case, just like EDIT: With this I don't mean to discourage adding such a function though, there are a few API holes like this, for example an |
This is on the next meeting agenda |
We decided in today's meeting that the renaming is a good idea, but the reordering of parameters is not as favorable. Would you mind removing the commit with the reordering of the parameters? |
Ah, I should link to the notes as well: https://github.com/mozilla/rust/wiki/Meeting-weekly-2014-01-07 |
…lexcrichton This implements parts of the changes to `Option` I proposed and discussed in this thread: https://mail.mozilla.org/pipermail/rust-dev/2013-November/006254.html, and on IRC. In short, the string "default" should not be used in any context that has nothing to do with the `std::default::Default` trait. This PR consists of this change: - Renamed `map_default -> map_or` and `mutate_default -> mutate_or_set`.
Language change: rust-lang/rust#10854
Language change: rust-lang/rust#10854
Language change: rust-lang/rust#10854
Fixes rust-lang#10854. Co-authored-by: Alejandra González <blyxyas@gmail.com>
…t-closure-for-method-calls, r=blyxyas [fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules Fixes rust-lang#10854. Currently, `redundant_closure_for_method_calls` suggest incorrect paths when a method defined on a struct within inline mod is referenced (see the description in the aforementioned issue for an example; also see [this playground link](https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=f7d3c5b2663c9bd3ab7abdb0bd38ee43) for the current-version output for the test cases added in this PR). It will now try to construct a relative path path to the module and suggest it instead. changelog: [`redundant_closure_for_method_calls`] Fix incorrect path suggestions for types within local modules
This implements parts of the changes to
Option
I proposed and discussed in this thread: https://mail.mozilla.org/pipermail/rust-dev/2013-November/006254.html, and on IRC.In short, the string "default" should not be used in any context that has nothing to do with the
std::default::Default
trait.This PR consists of this change:
map_default -> map_or
andmutate_default -> mutate_or_set
.