-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Simplify some nested if
statements
#130235
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
rustbot has assigned @michaelwoerister. Use |
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
if ocx.eq(&cause, self.param_env, a, b).is_ok() && ocx.select_all_or_error().is_empty() { | ||
// All good. | ||
return true; | ||
} | ||
return false; |
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.
I mean at this point we should just make this return ocx.eq(&cause, self.param_env, a, b).is_ok() && ocx.select_all_or_error().is_empty()
, I think. I was using two if
to make the sequencing more explicit.
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.
right, I missed the trivial body here.
For what it's worth, I actually tend to write this like two separate if statements returning false since those are the "exceptional" path.
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.
Feel free to change this to the usual style used with infctx, I don't usually interact much with that type so I don't know the conventions.^^
if let (Some(const_span), Some(fn_sig)) = (const_span, fn_sig) | ||
&& fn_sig.header.abi != Abi::RustIntrinsic | ||
&& !fn_sig.header.is_const() | ||
&& (!self.in_trait_impl || !self.tcx.is_const_fn_raw(def_id.to_def_id())) |
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.
this was previously:
~A \/ (A /\ ~B)
which can be simplified to:
~A \/ ~B
if ocx.eq(&cause, self.param_env, a, b).is_ok() && ocx.select_all_or_error().is_empty() { | ||
// All good. | ||
return true; | ||
} | ||
return false; |
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.
right, I missed the trivial body here.
For what it's worth, I actually tend to write this like two separate if statements returning false since those are the "exceptional" path.
b236487
to
954419a
Compare
Thanks, @compiler-errors, looks good to me! |
Rollup of 8 pull requests Successful merges: - rust-lang#125060 (Expand documentation of PathBuf, discussing lack of sanitization) - rust-lang#129367 (Fix default/minimum deployment target for Aarch64 simulator targets) - rust-lang#130156 (Add test for S_OBJNAME & update test for LF_BUILDINFO cl and cmd) - rust-lang#130160 (Fix `slice::first_mut` docs) - rust-lang#130235 (Simplify some nested `if` statements) - rust-lang#130250 (Fix `clippy::useless_conversion`) - rust-lang#130252 (Properly report error on `const gen fn`) - rust-lang#130256 (Re-run coverage tests if `coverage-dump` was modified) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130235 - compiler-errors:nested-if, r=michaelwoerister Simplify some nested `if` statements Applies some but not all instances of `clippy::collapsible_if`. Some ended up looking worse afterwards, though, so I left those out. Also applies instances of `clippy::collapsible_else_if` Review with whitespace disabled please.
Applies some but not all instances of
clippy::collapsible_if
. Some ended up looking worse afterwards, though, so I left those out. Also applies instances ofclippy::collapsible_else_if
Review with whitespace disabled please.