-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
A few cleanups and minor improvements to rustc/infer #54641
Conversation
ljedrz
commented
Sep 28, 2018
- use unwrap_or(_else) where applicable
- convert single-branch matches to if-let
- use to_owned instead of to_string with string literals
- improve vector allocations
- readability improvements
- miscellaneous minor code improvements
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
Whoops, looks like I overdid it somewhere; I'll look into the errors shortly. |
Ok(()) => vec![], | ||
Err(errors) => errors, | ||
}; | ||
let true_errors = fulfill_cx.select_where_possible(self).err().unwrap_or(vec![]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minot nitpick: These are probably better served with .unwrap_or_else(Vec::new)
.
Ok(()) => vec![], | ||
Err(errors) => errors, | ||
}; | ||
let ambig_errors = fulfill_cx.select_all_or_error(self).err().unwrap_or(vec![]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
@@ -100,7 +100,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { | |||
let mut labels = vec![( | |||
span, | |||
if &name == "_" { | |||
"cannot infer type".to_string() | |||
"cannot infer type".to_owned() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no preference nowadays between to_string
and to_owned
other than style as both now have the same performance impact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean when #54613 is merged? At this moment to_string
is not inlined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That change might still not be enough; see this clippy issue.
|
||
dot::Id::new(name()).unwrap_or_else(|_| | ||
bug!("failed to create graphviz node identified by {}", name()) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweak the formatting here, as it makes it look like the closing parenthesis is for the unwrap_or_else
, while it is for Id::new
.
3b19ee4
to
52da886
Compare
@estebank thanks for the review! I addressed your comments and fixed the test errors. |
@bors r+ |
📌 Commit 52da886 has been approved by |
⌛ Testing commit 52da886 with merge 20bb18dff67fd960249cfdac52b987989b8c24ae... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Et tu, homu? |
A few cleanups and minor improvements to rustc/infer - use unwrap_or(_else) where applicable - convert single-branch matches to if-let - use to_owned instead of to_string with string literals - improve vector allocations - readability improvements - miscellaneous minor code improvements
⌛ Testing commit 52da886 with merge 64ce874ff293ab4336b9b5ae6355596fcd08ced2... |
💔 Test failed - status-travis |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors retry An error occurred while generating the build script. |
Rollup of 13 pull requests Successful merges: - #53784 (Document that slices cannot be larger than `isize::MAX` bytes) - #54308 (Better user experience when attempting to call associated functions with dot notation) - #54488 (in which we include attributes in unused `extern crate` suggestion spans) - #54544 (Indicate how to move value out of Box in docs.) - #54623 (Added help message for `impl_trait_in_bindings` feature gate) - #54641 (A few cleanups and minor improvements to rustc/infer) - #54656 (Correct doc for WorkQueue<T>::pop().) - #54674 (update miri) - #54676 (Remove `-Z disable_ast_check_for_mutation_in_guard`) - #54679 (Improve bug! message for impossible case in Relate) - #54681 (Rename sanitizer runtime libraries on OSX) - #54708 (Make ./x.py help <cmd> invoke ./x.py <cmd> -h on its own) - #54713 (Add nightly check for tool_lints warning)