Skip to content

Commit 712608c

Browse files
SchahinRohaniSchahinRohani
authored
Fix clippy::needless_pass_by_value (#1413)
1 parent f5de318 commit 712608c

File tree

14 files changed

+64
-65
lines changed

14 files changed

+64
-65
lines changed

‎.bazelrc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect
4545
build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect
4646

4747
# TODO(aaronmondal): Extend these flags until we can run with clippy::pedantic.
48-
build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone
48+
build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone,-Dclippy::needless_pass_by_value
4949
build --@rules_rust//:clippy.toml=//:clippy.toml
5050

5151
test --@rules_rust//:rustfmt.toml=//:.rustfmt.toml

‎nativelink-error/src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ impl Error {
7676

7777
#[inline]
7878
#[must_use]
79-
pub fn append<S: std::string::ToString>(mut self, msg: S) -> Self {
80-
self.messages.push(msg.to_string());
79+
pub fn append<S: Into<String>>(mut self, msg: S) -> Self {
80+
self.messages.push(msg.into());
8181
self
8282
}
8383

‎nativelink-scheduler/src/default_scheduler_factory.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ fn simple_scheduler_factory(
9999
let task_change_notify = Arc::new(Notify::new());
100100
let awaited_action_db = memory_awaited_action_db_factory(
101101
config.retain_completed_for_s,
102-
task_change_notify.clone(),
102+
&task_change_notify.clone(),
103103
SystemTime::now,
104104
);
105105
let (action_scheduler, worker_scheduler) =
@@ -141,7 +141,7 @@ fn simple_scheduler_factory(
141141

142142
pub fn memory_awaited_action_db_factory<I, NowFn>(
143143
mut retain_completed_for_s: u32,
144-
task_change_notify: Arc<Notify>,
144+
task_change_notify: &Arc<Notify>,
145145
now_fn: NowFn,
146146
) -> MemoryAwaitedActionDb<I, NowFn>
147147
where

‎nativelink-scheduler/src/memory_awaited_action_db.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ impl SortedAwaitedActions {
251251
fn insert_sort_map_for_stage(
252252
&mut self,
253253
stage: &ActionStage,
254-
sorted_awaited_action: SortedAwaitedAction,
254+
sorted_awaited_action: &SortedAwaitedAction,
255255
) -> Result<(), Error> {
256256
let newly_inserted = match stage {
257257
ActionStage::Unknown => self.unknown.insert(sorted_awaited_action.clone()),
@@ -294,7 +294,7 @@ impl SortedAwaitedActions {
294294
));
295295
};
296296

297-
self.insert_sort_map_for_stage(&new_awaited_action.state().stage, sorted_awaited_action)
297+
self.insert_sort_map_for_stage(&new_awaited_action.state().stage, &sorted_awaited_action)
298298
.err_tip(|| "In AwaitedActionDb::update_awaited_action")?;
299299
Ok(())
300300
}
@@ -655,7 +655,7 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
655655
/// cleanup of the other maps on drop.
656656
fn make_client_awaited_action(
657657
&mut self,
658-
operation_id: OperationId,
658+
operation_id: &OperationId,
659659
awaited_action: AwaitedAction,
660660
) -> (Arc<ClientAwaitedAction>, watch::Receiver<AwaitedAction>) {
661661
let (tx, rx) = watch::channel(awaited_action);
@@ -704,7 +704,7 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
704704
let sort_key = awaited_action.sort_key();
705705

706706
let (client_awaited_action, rx) =
707-
self.make_client_awaited_action(operation_id.clone(), awaited_action);
707+
self.make_client_awaited_action(&operation_id.clone(), awaited_action);
708708

709709
event!(
710710
Level::DEBUG,
@@ -736,7 +736,7 @@ impl<I: InstantWrapper, NowFn: Fn() -> I + Clone + Send + Sync> AwaitedActionDbI
736736
self.sorted_action_info_hash_keys
737737
.insert_sort_map_for_stage(
738738
&ActionStage::Queued,
739-
SortedAwaitedAction {
739+
&SortedAwaitedAction {
740740
sort_key,
741741
operation_id,
742742
},

‎nativelink-scheduler/src/store_awaited_action_db.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ impl<S: SchedulerStore> AwaitedActionSubscriber for OperationSubscriber<S> {
133133
}
134134
}
135135

136-
fn awaited_action_decode(version: u64, data: Bytes) -> Result<AwaitedAction, Error> {
137-
let mut awaited_action: AwaitedAction = serde_json::from_slice(&data)
136+
fn awaited_action_decode(version: u64, data: &Bytes) -> Result<AwaitedAction, Error> {
137+
let mut awaited_action: AwaitedAction = serde_json::from_slice(data)
138138
.map_err(|e| make_input_err!("In AwaitedAction::decode - {e:?}"))?;
139139
awaited_action.set_version(version);
140140
Ok(awaited_action)
@@ -165,7 +165,7 @@ impl SchedulerStoreKeyProvider for OperationIdToAwaitedAction<'_> {
165165
impl SchedulerStoreDecodeTo for OperationIdToAwaitedAction<'_> {
166166
type DecodeOutput = AwaitedAction;
167167
fn decode(version: u64, data: Bytes) -> Result<Self::DecodeOutput, Error> {
168-
awaited_action_decode(version, data)
168+
awaited_action_decode(version, &data)
169169
}
170170
}
171171

@@ -200,7 +200,7 @@ impl SchedulerIndexProvider for SearchUniqueQualifierToAwaitedAction<'_> {
200200
impl SchedulerStoreDecodeTo for SearchUniqueQualifierToAwaitedAction<'_> {
201201
type DecodeOutput = AwaitedAction;
202202
fn decode(version: u64, data: Bytes) -> Result<Self::DecodeOutput, Error> {
203-
awaited_action_decode(version, data)
203+
awaited_action_decode(version, &data)
204204
}
205205
}
206206

@@ -216,7 +216,7 @@ impl SchedulerIndexProvider for SearchSortKeyPrefixToAwaitedAction {
216216
impl SchedulerStoreDecodeTo for SearchSortKeyPrefixToAwaitedAction {
217217
type DecodeOutput = AwaitedAction;
218218
fn decode(version: u64, data: Bytes) -> Result<Self::DecodeOutput, Error> {
219-
awaited_action_decode(version, data)
219+
awaited_action_decode(version, &data)
220220
}
221221
}
222222

‎nativelink-scheduler/tests/simple_scheduler_test.rs‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ async fn basic_add_action_with_one_worker_test() -> Result<(), Error> {
163163
&nativelink_config::schedulers::SimpleScheduler::default(),
164164
memory_awaited_action_db_factory(
165165
0,
166-
task_change_notify.clone(),
166+
&task_change_notify.clone(),
167167
MockInstantWrapped::default,
168168
),
169169
|| async move {},
@@ -225,7 +225,7 @@ async fn client_does_not_receive_update_timeout() -> Result<(), Error> {
225225
},
226226
memory_awaited_action_db_factory(
227227
0,
228-
task_change_notify.clone(),
228+
&task_change_notify.clone(),
229229
MockInstantWrapped::default,
230230
),
231231
|| async move {},
@@ -294,7 +294,7 @@ async fn find_executing_action() -> Result<(), Error> {
294294
&nativelink_config::schedulers::SimpleScheduler::default(),
295295
memory_awaited_action_db_factory(
296296
0,
297-
task_change_notify.clone(),
297+
&task_change_notify.clone(),
298298
MockInstantWrapped::default,
299299
),
300300
|| async move {},
@@ -374,7 +374,7 @@ async fn remove_worker_reschedules_multiple_running_job_test() -> Result<(), Err
374374
},
375375
memory_awaited_action_db_factory(
376376
0,
377-
task_change_notify.clone(),
377+
&task_change_notify.clone(),
378378
MockInstantWrapped::default,
379379
),
380380
|| async move {},
@@ -549,7 +549,7 @@ async fn set_drain_worker_pauses_and_resumes_worker_test() -> Result<(), Error>
549549
&nativelink_config::schedulers::SimpleScheduler::default(),
550550
memory_awaited_action_db_factory(
551551
0,
552-
task_change_notify.clone(),
552+
&task_change_notify.clone(),
553553
MockInstantWrapped::default,
554554
),
555555
|| async move {},
@@ -636,7 +636,7 @@ async fn worker_should_not_queue_if_properties_dont_match_test() -> Result<(), E
636636
},
637637
memory_awaited_action_db_factory(
638638
0,
639-
task_change_notify.clone(),
639+
&task_change_notify.clone(),
640640
MockInstantWrapped::default,
641641
),
642642
|| async move {},
@@ -727,7 +727,7 @@ async fn cacheable_items_join_same_action_queued_test() -> Result<(), Error> {
727727
&nativelink_config::schedulers::SimpleScheduler::default(),
728728
memory_awaited_action_db_factory(
729729
0,
730-
task_change_notify.clone(),
730+
&task_change_notify.clone(),
731731
MockInstantWrapped::default,
732732
),
733733
|| async move {},
@@ -828,7 +828,7 @@ async fn worker_disconnects_does_not_schedule_for_execution_test() -> Result<(),
828828
&nativelink_config::schedulers::SimpleScheduler::default(),
829829
memory_awaited_action_db_factory(
830830
0,
831-
task_change_notify.clone(),
831+
&task_change_notify.clone(),
832832
MockInstantWrapped::default,
833833
),
834834
|| async move {},
@@ -1087,7 +1087,7 @@ async fn worker_timesout_reschedules_running_job_test() -> Result<(), Error> {
10871087
},
10881088
memory_awaited_action_db_factory(
10891089
0,
1090-
task_change_notify.clone(),
1090+
&task_change_notify.clone(),
10911091
MockInstantWrapped::default,
10921092
),
10931093
|| async move {},
@@ -1209,7 +1209,7 @@ async fn update_action_sends_completed_result_to_client_test() -> Result<(), Err
12091209
&nativelink_config::schedulers::SimpleScheduler::default(),
12101210
memory_awaited_action_db_factory(
12111211
0,
1212-
task_change_notify.clone(),
1212+
&task_change_notify.clone(),
12131213
MockInstantWrapped::default,
12141214
),
12151215
|| async move {},
@@ -1310,7 +1310,7 @@ async fn update_action_sends_completed_result_after_disconnect() -> Result<(), E
13101310
&nativelink_config::schedulers::SimpleScheduler::default(),
13111311
memory_awaited_action_db_factory(
13121312
0,
1313-
task_change_notify.clone(),
1313+
&task_change_notify.clone(),
13141314
MockInstantWrapped::default,
13151315
),
13161316
|| async move {},
@@ -1428,7 +1428,7 @@ async fn update_action_with_wrong_worker_id_errors_test() -> Result<(), Error> {
14281428
&nativelink_config::schedulers::SimpleScheduler::default(),
14291429
memory_awaited_action_db_factory(
14301430
0,
1431-
task_change_notify.clone(),
1431+
&task_change_notify.clone(),
14321432
MockInstantWrapped::default,
14331433
),
14341434
|| async move {},
@@ -1526,7 +1526,7 @@ async fn does_not_crash_if_operation_joined_then_relaunched() -> Result<(), Erro
15261526
&nativelink_config::schedulers::SimpleScheduler::default(),
15271527
memory_awaited_action_db_factory(
15281528
0,
1529-
task_change_notify.clone(),
1529+
&task_change_notify.clone(),
15301530
MockInstantWrapped::default,
15311531
),
15321532
|| async move {},
@@ -1669,7 +1669,7 @@ async fn run_two_jobs_on_same_worker_with_platform_properties_restrictions() ->
16691669
},
16701670
memory_awaited_action_db_factory(
16711671
0,
1672-
task_change_notify.clone(),
1672+
&task_change_notify.clone(),
16731673
MockInstantWrapped::default,
16741674
),
16751675
|| async move {},
@@ -1831,7 +1831,7 @@ async fn run_jobs_in_the_order_they_were_queued() -> Result<(), Error> {
18311831
},
18321832
memory_awaited_action_db_factory(
18331833
0,
1834-
task_change_notify.clone(),
1834+
&task_change_notify.clone(),
18351835
MockInstantWrapped::default,
18361836
),
18371837
|| async move {},
@@ -1898,7 +1898,7 @@ async fn worker_retries_on_internal_error_and_fails_test() -> Result<(), Error>
18981898
},
18991899
memory_awaited_action_db_factory(
19001900
0,
1901-
task_change_notify.clone(),
1901+
&task_change_notify.clone(),
19021902
MockInstantWrapped::default,
19031903
),
19041904
|| async move {},
@@ -2047,7 +2047,7 @@ async fn ensure_scheduler_drops_inner_spawn() -> Result<(), Error> {
20472047
&nativelink_config::schedulers::SimpleScheduler::default(),
20482048
memory_awaited_action_db_factory(
20492049
0,
2050-
task_change_notify.clone(),
2050+
&task_change_notify.clone(),
20512051
MockInstantWrapped::default,
20522052
),
20532053
move || {
@@ -2080,7 +2080,7 @@ async fn ensure_task_or_worker_change_notification_received_test() -> Result<(),
20802080
&nativelink_config::schedulers::SimpleScheduler::default(),
20812081
memory_awaited_action_db_factory(
20822082
0,
2083-
task_change_notify.clone(),
2083+
&task_change_notify.clone(),
20842084
MockInstantWrapped::default,
20852085
),
20862086
|| async move {},
@@ -2155,7 +2155,7 @@ async fn client_reconnect_keeps_action_alive() -> Result<(), Error> {
21552155
},
21562156
memory_awaited_action_db_factory(
21572157
0,
2158-
task_change_notify.clone(),
2158+
&task_change_notify.clone(),
21592159
MockInstantWrapped::default,
21602160
),
21612161
|| async move {},

‎nativelink-service/src/execution_server.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ impl ExecutionServer {
197197
}
198198

199199
fn to_execute_stream(
200-
nl_client_operation_id: NativelinkOperationId,
200+
nl_client_operation_id: &NativelinkOperationId,
201201
action_listener: Box<dyn ActionStateResult>,
202202
) -> Response<ExecuteStream> {
203203
let client_operation_id = OperationId::from(nl_client_operation_id.to_string());
@@ -277,7 +277,7 @@ impl ExecutionServer {
277277
.err_tip(|| "Failed to schedule task")?;
278278

279279
Ok(Self::to_execute_stream(
280-
NativelinkOperationId::new(
280+
&NativelinkOperationId::new(
281281
instance_name,
282282
action_listener
283283
.as_state()
@@ -315,7 +315,7 @@ impl ExecutionServer {
315315
else {
316316
return Err(Status::not_found("Failed to find existing task"));
317317
};
318-
Ok(Self::to_execute_stream(nl_operation_id, rx))
318+
Ok(Self::to_execute_stream(&nl_operation_id, rx))
319319
}
320320
}
321321

‎nativelink-store/src/compression_store.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ pub struct CompressionStore {
220220

221221
impl CompressionStore {
222222
pub fn new(
223-
compression_config: nativelink_config::stores::CompressionStore,
223+
compression_config: &nativelink_config::stores::CompressionStore,
224224
inner_store: Store,
225225
) -> Result<Arc<Self>, Error> {
226226
let lz4_config = match compression_config.compression_algorithm {

‎nativelink-store/src/default_store_factory.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ pub fn store_factory<'a>(
5959
store_factory(&config.backend, store_manager, None).await?,
6060
),
6161
StoreConfig::compression(config) => CompressionStore::new(
62-
*config.clone(),
62+
&config.clone(),
6363
store_factory(&config.backend, store_manager, None).await?,
6464
)?,
6565
StoreConfig::dedup(config) => DedupStore::new(

‎nativelink-store/src/s3_store.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ where
324324
}))
325325
}
326326

327-
fn make_s3_path(&self, key: StoreKey<'_>) -> String {
327+
fn make_s3_path(&self, key: &StoreKey<'_>) -> String {
328328
format!("{}{}", self.key_prefix, key.as_str(),)
329329
}
330330

@@ -335,7 +335,7 @@ where
335335
.s3_client
336336
.head_object()
337337
.bucket(&self.bucket)
338-
.key(self.make_s3_path(digest.borrow()))
338+
.key(self.make_s3_path(&digest.borrow()))
339339
.send()
340340
.await;
341341

@@ -412,7 +412,7 @@ where
412412
mut reader: DropCloserReadHalf,
413413
upload_size: UploadSizeInfo,
414414
) -> Result<(), Error> {
415-
let s3_path = &self.make_s3_path(digest.borrow());
415+
let s3_path = &self.make_s3_path(&digest.borrow());
416416

417417
let max_size = match upload_size {
418418
UploadSizeInfo::ExactSize(sz) | UploadSizeInfo::MaxSize(sz) => sz,
@@ -687,7 +687,7 @@ where
687687
return Ok(());
688688
}
689689

690-
let s3_path = &self.make_s3_path(key);
690+
let s3_path = &self.make_s3_path(&key);
691691
let end_read_byte = length
692692
.map_or(Some(None), |length| Some(offset.checked_add(length)))
693693
.err_tip(|| "Integer overflow protection triggered")?;

0 commit comments

Comments
 (0)