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

Replaced some unwrap_or and map_or with lazy variants #82456

Merged
merged 3 commits into from
Feb 26, 2021

Conversation

klensy
Copy link
Contributor

@klensy klensy commented Feb 23, 2021

Replaced some unwrap_or and map_or with unwrap_or_else and map_or_else.

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 23, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@klensy
Copy link
Contributor Author

klensy commented Feb 23, 2021

There's a bug in rust-log-analyzer, it shows:

-            .self_desc
-            .clone()
-            .clone()
-            .map_or_else(|| String::new(), |ty| format!(" for type `{}`", ty))
+        overlap.self_desc.clone().map_or_else(|| String::new(), |ty| format!(" for type `{}`", ty))

but here actually:

-        overlap
-            .self_desc
-            .clone()
-            .map_or_else(|| String::new(), |ty| format!(" for type `{}`", ty))
+        overlap.self_desc.clone().map_or_else(|| String::new(), |ty| format!(" for type `{}`", ty))

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

You can always pass any callable to a place that expects a closure, as long as the number of arguments matches. I'm surprised we don't lint about this.

@@ -2372,7 +2372,7 @@ fn compute_type_parameters(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>) -> &'ll DIAr
fn get_parameter_names(cx: &CodegenCx<'_, '_>, generics: &ty::Generics) -> Vec<Symbol> {
let mut names = generics
.parent
.map_or(vec![], |def_id| get_parameter_names(cx, cx.tcx.generics_of(def_id)));
.map_or_else(|| vec![], |def_id| get_parameter_names(cx, cx.tcx.generics_of(def_id)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map_or_else(|| vec![], |def_id| get_parameter_names(cx, cx.tcx.generics_of(def_id)));
.map_or_else(Vec::new, |def_id| get_parameter_names(cx, cx.tcx.generics_of(def_id)));

.parent
.map_or(vec![], |def_id| get_parameter_names(cx, cx.tcx.generics_of(def_id)));
let mut names = generics.parent.map_or_else(
|| vec![],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|| vec![],
Vec::new,

let name = name_buf.map_or(
String::new(), // We got a NULL ptr, ignore `name_len`.
let name = name_buf.map_or_else(
|| String::new(), // We got a NULL ptr, ignore `name_len`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|| String::new(), // We got a NULL ptr, ignore `name_len`.
String::new, // We got a NULL ptr, ignore `name_len`.

@@ -50,7 +50,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(

let name =
with_no_trimmed_paths(|| ty::tls::with(|tcx| tcx.def_path_str(cid.instance.def_id())));
let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p));
let prom = cid.promoted.map_or_else(|| String::new(), |p| format!("::promoted[{:?}]", p));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let prom = cid.promoted.map_or_else(|| String::new(), |p| format!("::promoted[{:?}]", p));
let prom = cid.promoted.map_or_else(String::new, |p| format!("::promoted[{:?}]", p));

@@ -223,7 +223,7 @@ impl<'a> Parser<'a> {
fn tokens_to_string(tokens: &[TokenType]) -> String {
let mut i = tokens.iter();
// This might be a sign we need a connect method on `Iterator`.
let b = i.next().map_or(String::new(), |t| t.to_string());
let b = i.next().map_or_else(|| String::new(), |t| t.to_string());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let b = i.next().map_or_else(|| String::new(), |t| t.to_string());
let b = i.next().map_or_else(String::new, |t| t.to_string());

overlap.self_desc.map_or(String::new(), |ty| format!(" for `{}`", ty))
overlap
.self_desc
.map_or_else(|| String::new(), |ty| format!(" for `{}`", ty))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map_or_else(|| String::new(), |ty| format!(" for `{}`", ty))
.map_or_else(String::new, |ty| format!(" for `{}`", ty))

@@ -1695,7 +1695,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
} else {
self.fcx
.associated_item(def_id, name, Namespace::ValueNS)
.map_or(Vec::new(), |x| vec![x])
.map_or_else(|| Vec::new(), |x| vec![x])
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map_or_else(|| Vec::new(), |x| vec![x])
.map_or_else(Vec::new, |x| vec![x])

tcx.sess
.source_map()
.span_to_snippet(span)
.map_or_else(|_| String::new(), |s| format!(" `{}`", s)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map_or_else(|_| String::new(), |s| format!(" `{}`", s)),
.map_or_else(String::new, |s| format!(" `{}`", s)),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as long as the number of arguments matches

You catched me here and with next 2 suggestions )

@@ -879,7 +879,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let sm = tcx.sess.source_map();
let path_str = sm
.span_to_snippet(sm.span_until_char(pat.span, '('))
.map_or(String::new(), |s| format!(" `{}`", s.trim_end()));
.map_or_else(|_| String::new(), |s| format!(" `{}`", s.trim_end()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.map_or_else(|_| String::new(), |s| format!(" `{}`", s.trim_end()));
.map_or_else(String::new, |s| format!(" `{}`", s.trim_end()));

compiler/rustc_typeck/src/collect.rs Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 25, 2021

📌 Commit 08b1e80 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 25, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 26, 2021
Replaced some unwrap_or and map_or with lazy variants

Replaced some `unwrap_or` and `map_or` with `unwrap_or_else` and `map_or_else`.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 26, 2021
Replaced some unwrap_or and map_or with lazy variants

Replaced some `unwrap_or` and `map_or` with `unwrap_or_else` and `map_or_else`.
This was referenced Feb 26, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 26, 2021
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#81940 (Stabilize str_split_once)
 - rust-lang#82165 (Reword labels on E0308 involving async fn return type)
 - rust-lang#82456 (Replaced some unwrap_or and map_or with lazy variants)
 - rust-lang#82491 (Consider inexpensive inlining criteria first)
 - rust-lang#82506 (Properly account for non-shorthand pattern field in unused variable lint)
 - rust-lang#82535 (Set codegen thread names)
 - rust-lang#82545 (rustdoc: add optional woff2 versions of FiraSans.)
 - rust-lang#82549 (Revert "Update normalize.css to 8.0.1")

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 039b1b6 into rust-lang:master Feb 26, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants