some cleanup in rustc_session - #159398
Conversation
rustbot
added
S-waiting-on-review
|
r? @eholk rustbot has assigned @eholk. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| let mut v: Vec<&str> = v.unwrap_or_default().split(",").collect(); | ||
| v.sort_unstable(); |
There was a problem hiding this comment.
The new code does not reset the slot to an empty Vec for a None argument. I'm curious to see if that matters.
There was a problem hiding this comment.
That would cause -Zoffload= to no longer override -Zoffload=a,b,c and instead silently get igbored, right? That is inconsistent with other cli flags where the last occurence does win.
There was a problem hiding this comment.
That is true, but at the same time, the empty value is the only one that overrides in this way: -Zoffload=a -Zoffload=b is the same as -Zoffload=a,b not as `-Zoffload=b', so it seems to already be inconsistent.
Anyway, no change was intended, so I'll revert. Thanks for explaining the significance to me!
There was a problem hiding this comment.
-Zoffload=a -Zoffload=b is the same as -Zoffload=a,b not as `-Zoffload=b'
Huh, that is inconsistent with other args that use parse_comma_list.
There was a problem hiding this comment.
Yes, the reset only happens for an empty argument.
There was a problem hiding this comment.
The function just before doesn't do a reset ever. Should it?
pub(crate) fn parse_dump_mono_stats(slot: &mut DumpMonoStatsFormat, v: Option<&str>) -> bool {
match v {
None => true, // hkBst: empty arg, no reset <-----
Some("json") => {
*slot = DumpMonoStatsFormat::Json;
true
}
Some("markdown") => {
*slot = DumpMonoStatsFormat::Markdown;
true
}
Some(_) => false,
}
}
| let mut v: Vec<&str> = v.unwrap_or_default().split(",").collect(); | ||
| v.sort_unstable(); | ||
| for &val in v.iter() { | ||
| v.iter().all(|val| { |
There was a problem hiding this comment.
I don't think using side-effects inside of an iterator method is an improvement.
There was a problem hiding this comment.
I agree that that is not ideal, but I think that disadvantage is outweighed by the advantage of showing this is an all combinator instead of some random for loop that just happens to be a manual implementation of the all combinator.
4 participants
No description provided.