Skip to content

Commit da58373

Browse files
mkeetermkeeter
authored
Simplify matches (oxidecomputer#1490)
Drive-by cleanup to simplify match statements: - Use `..` instead of `foobar: _` to ignore fields - Consolidate `match` statement with identical branches (I don't think we're ever deliberately destructuring objects to check their shape, but let me know if that's an incorrect assumption!)
1 parent aad43cf commit da58373

File tree

7 files changed

+61
-98
lines changed

7 files changed

+61
-98
lines changed

‎crucible-client-types/src/lib.rs

+6-13
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,12 @@ pub enum VolumeConstructionRequest {
4141
impl VolumeConstructionRequest {
4242
pub fn targets(&self) -> Vec<SocketAddr> {
4343
match self {
44-
VolumeConstructionRequest::Volume {
45-
id: _,
46-
block_size: _,
47-
sub_volumes,
48-
read_only_parent: _,
49-
} => sub_volumes.iter().flat_map(|s| s.targets()).collect(),
50-
VolumeConstructionRequest::Region {
51-
block_size: _,
52-
blocks_per_extent: _,
53-
extent_count: _,
54-
opts,
55-
gen: _,
56-
} => opts.target.clone(),
44+
VolumeConstructionRequest::Volume { sub_volumes, .. } => {
45+
sub_volumes.iter().flat_map(|s| s.targets()).collect()
46+
}
47+
VolumeConstructionRequest::Region { opts, .. } => {
48+
opts.target.clone()
49+
}
5750
_ => vec![],
5851
}
5952
}

‎downstairs/src/lib.rs

+2-11
Original file line numberDiff line numberDiff line change
@@ -1316,9 +1316,9 @@ impl ActiveConnection {
13161316
Message::ExtentFlush {
13171317
repair_id,
13181318
extent_id,
1319-
client_id: _,
13201319
flush_number,
13211320
gen_number,
1321+
..
13221322
} => {
13231323
let msg = {
13241324
debug!(
@@ -3680,16 +3680,7 @@ mod test {
36803680
assert!(work.completed.is_complete(dep));
36813681
}
36823682

3683-
matches!(
3684-
job,
3685-
IOop::Flush {
3686-
dependencies: _,
3687-
flush_number: _,
3688-
gen_number: _,
3689-
snapshot_details: _,
3690-
extent_limit: _,
3691-
}
3692-
)
3683+
matches!(job, IOop::Flush { .. })
36933684
};
36943685

36953686
if is_flush {

‎protocol/src/lib.rs

+45-43
Original file line numberDiff line numberDiff line change
@@ -592,53 +592,55 @@ impl Message {
592592
/// Return true if this message contains an Error result
593593
pub fn err(&self) -> Option<&CrucibleError> {
594594
match self {
595-
Message::HereIAm { .. } => None,
596-
Message::YesItsMe { .. } => None,
597-
Message::VersionMismatch { .. } => None,
598-
Message::ReadOnlyMismatch { .. } => None,
599-
Message::EncryptedMismatch { .. } => None,
600-
Message::PromoteToActive { .. } => None,
601-
Message::YouAreNowActive { .. } => None,
602-
Message::YouAreNoLongerActive { .. } => None,
603-
Message::UuidMismatch { .. } => None,
604-
Message::Ruok { .. } => None,
605-
Message::Imok { .. } => None,
606-
Message::ExtentClose { .. } => None,
607-
Message::ExtentReopen { .. } => None,
608-
Message::ExtentFlush { .. } => None,
609-
Message::ExtentRepair { .. } => None,
610-
Message::RepairAckId { .. } => None,
611-
Message::RegionInfoPlease { .. } => None,
612-
Message::RegionInfo { .. } => None,
613-
Message::ExtentVersionsPlease { .. } => None,
614-
Message::ExtentVersions { .. } => None,
615-
Message::LastFlush { .. } => None,
616-
Message::LastFlushAck { .. } => None,
617-
Message::Write { .. } => None,
618-
Message::ExtentLiveClose { .. } => None,
619-
Message::ExtentLiveFlushClose { .. } => None,
620-
Message::ExtentLiveRepair { .. } => None,
621-
Message::ExtentLiveReopen { .. } => None,
622-
Message::ExtentLiveNoOp { .. } => None,
623-
Message::Flush { .. } => None,
624-
Message::ReadRequest { .. } => None,
625-
Message::WriteUnwritten { .. } => None,
626-
Message::Unknown(..) => None,
627-
628-
Message::ExtentError { error, .. } => Some(error),
629-
Message::ErrorReport { error, .. } => Some(error),
595+
Message::HereIAm { .. }
596+
| Message::YesItsMe { .. }
597+
| Message::VersionMismatch { .. }
598+
| Message::ReadOnlyMismatch { .. }
599+
| Message::EncryptedMismatch { .. }
600+
| Message::PromoteToActive { .. }
601+
| Message::YouAreNowActive { .. }
602+
| Message::YouAreNoLongerActive { .. }
603+
| Message::UuidMismatch { .. }
604+
| Message::Ruok { .. }
605+
| Message::Imok { .. }
606+
| Message::ExtentClose { .. }
607+
| Message::ExtentReopen { .. }
608+
| Message::ExtentFlush { .. }
609+
| Message::ExtentRepair { .. }
610+
| Message::RepairAckId { .. }
611+
| Message::RegionInfoPlease { .. }
612+
| Message::RegionInfo { .. }
613+
| Message::ExtentVersionsPlease { .. }
614+
| Message::ExtentVersions { .. }
615+
| Message::LastFlush { .. }
616+
| Message::LastFlushAck { .. }
617+
| Message::Write { .. }
618+
| Message::ExtentLiveClose { .. }
619+
| Message::ExtentLiveFlushClose { .. }
620+
| Message::ExtentLiveRepair { .. }
621+
| Message::ExtentLiveReopen { .. }
622+
| Message::ExtentLiveNoOp { .. }
623+
| Message::Flush { .. }
624+
| Message::ReadRequest { .. }
625+
| Message::WriteUnwritten { .. }
626+
| Message::Unknown(..) => None,
627+
628+
Message::ExtentError { error, .. }
629+
| Message::ErrorReport { error, .. } => Some(error),
630630

631631
Message::ExtentLiveCloseAck { result, .. } => result.as_ref().err(),
632-
Message::ExtentLiveRepairAckId { result, .. } => {
632+
633+
Message::ExtentLiveRepairAckId { result, .. }
634+
| Message::ExtentLiveAckId { result, .. }
635+
| Message::WriteAck { result, .. }
636+
| Message::FlushAck { result, .. }
637+
| Message::WriteUnwrittenAck { result, .. } => {
633638
result.as_ref().err()
634639
}
635-
Message::ExtentLiveAckId { result, .. } => result.as_ref().err(),
636-
Message::WriteAck { result, .. } => result.as_ref().err(),
637-
Message::FlushAck { result, .. } => result.as_ref().err(),
640+
638641
Message::ReadResponse { header, .. } => {
639642
header.blocks.as_ref().err()
640643
}
641-
Message::WriteUnwrittenAck { result, .. } => result.as_ref().err(),
642644
}
643645
}
644646
}
@@ -658,7 +660,7 @@ impl std::fmt::Display for Message {
658660
start,
659661
..
660662
},
661-
data: _,
663+
..
662664
} => f
663665
.debug_struct("Message::Write")
664666
.field("upstairs_id", &upstairs_id)
@@ -678,7 +680,7 @@ impl std::fmt::Display for Message {
678680
start,
679681
..
680682
},
681-
data: _,
683+
..
682684
} => f
683685
.debug_struct("Message::WriteUnwritten")
684686
.field("upstairs_id", &upstairs_id)
@@ -696,7 +698,7 @@ impl std::fmt::Display for Message {
696698
job_id,
697699
blocks,
698700
},
699-
data: _,
701+
..
700702
} => f
701703
.debug_struct("Message::ReadResponse")
702704
.field("upstairs_id", &upstairs_id)

‎upstairs/src/downstairs.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -1077,10 +1077,7 @@ impl Downstairs {
10771077
repair.state = LiveRepairState::Noop { noop_id, reopen_id };
10781078
self.create_and_enqueue_noop_io(gw, vec![repair_id], noop_id);
10791079
}
1080-
LiveRepairState::Noop {
1081-
noop_id: _,
1082-
reopen_id,
1083-
} => {
1080+
LiveRepairState::Noop { reopen_id, .. } => {
10841081
info!(
10851082
self.log,
10861083
"RE:{} Wait for result from reopen command {}",
@@ -7975,7 +7972,7 @@ pub(crate) mod test {
79757972

79767973
println!("repair op: {:?}", repair_op);
79777974
match repair_op {
7978-
IOop::ExtentLiveNoOp { dependencies: _ } => {}
7975+
IOop::ExtentLiveNoOp { .. } => {}
79797976
x => {
79807977
panic!("Incorrect work type returned: {:?}", x);
79817978
}
@@ -8009,11 +8006,10 @@ pub(crate) mod test {
80098006

80108007
match repair_op {
80118008
IOop::ExtentLiveRepair {
8012-
dependencies: _,
80138009
extent,
80148010
source_downstairs,
8015-
source_repair_address: _,
80168011
repair_downstairs,
8012+
..
80178013
} => {
80188014
assert_eq!(extent, ExtentId(0));
80198015
assert_eq!(source_downstairs, source);

‎upstairs/src/dummy_downstairs_tests.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,7 @@ impl DownstairsHandle {
131131
pub async fn negotiate_start(&mut self) {
132132
let packet = self.recv().await.unwrap();
133133
if let Message::HereIAm {
134-
version,
135-
upstairs_id: _,
136-
session_id: _,
137-
gen: _,
138-
read_only,
139-
encrypted: _,
140-
alternate_versions: _,
134+
version, read_only, ..
141135
} = &packet
142136
{
143137
info!(

‎upstairs/src/lib.rs

+3-13
Original file line numberDiff line numberDiff line change
@@ -1248,32 +1248,22 @@ impl IOop {
12481248
let job_type = "WriteU".to_string();
12491249
(job_type, blocks.len(), dependencies.clone())
12501250
}
1251-
IOop::Flush {
1252-
dependencies,
1253-
flush_number: _flush_number,
1254-
gen_number: _gen_number,
1255-
snapshot_details: _,
1256-
extent_limit: _,
1257-
} => {
1251+
IOop::Flush { dependencies, .. } => {
12581252
let job_type = "Flush".to_string();
12591253
(job_type, 0, dependencies.clone())
12601254
}
12611255
IOop::ExtentFlushClose {
12621256
dependencies,
12631257
extent,
1264-
flush_number: _,
1265-
gen_number: _,
1266-
repair_downstairs: _,
1258+
..
12671259
} => {
12681260
let job_type = "FClose".to_string();
12691261
(job_type, extent.0 as usize, dependencies.clone())
12701262
}
12711263
IOop::ExtentLiveRepair {
12721264
dependencies,
12731265
extent,
1274-
source_downstairs: _,
1275-
source_repair_address: _,
1276-
repair_downstairs: _,
1266+
..
12771267
} => {
12781268
let job_type = "Repair".to_string();
12791269
(job_type, extent.0 as usize, dependencies.clone())

‎upstairs/src/volume.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -1335,10 +1335,7 @@ impl Volume {
13351335
// Beyond this point zero or one target changes where found
13361336

13371337
match &compare_result {
1338-
CompareResult::Volume {
1339-
sub_compares,
1340-
read_only_parent_compare: _,
1341-
} => {
1338+
CompareResult::Volume { sub_compares, .. } => {
13421339
// Walk the cases. As some deltas are okay for a read only
13431340
// parent that are not for a sub_volume, we need to look at all
13441341
// the possible permutations Nexus supports.

0 commit comments

Comments
 (0)