-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
misc cleanups #131623
misc cleanups #131623
Conversation
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
This comment has been minimized.
This comment has been minimized.
3d11954
to
9f7fed5
Compare
compiler/rustc_middle/src/mir/mod.rs
Outdated
scope_data | ||
.inlined_parent_scope | ||
.map(|inlined_scope| source_scopes[inlined_scope].inlined.unwrap().0) |
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 have mixed feelings about manual_map
in general, but this particular pattern of replacing only the tail of an if-chain with a map
seems like a clear downgrade to me.
Now the final condition (using map
) is inconsistent with the prior conditions (which are still using if let Some
followed by Some
), for no real benefit.
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.
+1 I find the replacing only the tail with a map
makes it quite hard to read
9f7fed5
to
3da4363
Compare
if let Some(ac) = a.reduce_and(tcx, c) { | ||
Some(ac.and(tcx, b)) | ||
} else if let Some(bc) = b.reduce_and(tcx, c) { | ||
Some(Self::And(tcx.arena.alloc([a, bc]))) | ||
} else { | ||
None | ||
b.reduce_and(tcx, c).map(|bc| Self::And(tcx.arena.alloc([a, bc]))) | ||
} |
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 agree with the others, this feels less legible now than before. I won't block this though if you feel that's an improvement
☔ The latest upstream changes (presumably #132027) made this pull request unmergeable. Please resolve the merge conflicts. |
double_parens filter_map_identity needless_question_mark redundant_guards
3da4363
to
d84d659
Compare
r=me when CI is green |
@bors r=Nadrieril rollup |
Rollup of 10 pull requests Successful merges: - rust-lang#130225 (Rename Receiver -> LegacyReceiver) - rust-lang#131169 (Fix `target_vendor` in QNX Neutrino targets) - rust-lang#131623 (misc cleanups) - rust-lang#131756 (Deeply normalize `TypeTrace` when reporting type error in new solver) - rust-lang#131898 (minor `*dyn` cast cleanup) - rust-lang#131909 (Prevent overflowing enum cast from ICEing) - rust-lang#131930 (Don't allow test revisions that conflict with built in cfgs) - rust-lang#131956 (coverage: Pass coverage mappings to LLVM as separate structs) - rust-lang#132076 (HashStable for rustc_feature::Features: stop hashing compile-time constant) - rust-lang#132088 (Print safety correctly in extern static items) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#131623 - matthiaskrgr:clippy_sat, r=Nadrieril misc cleanups
No description provided.