From f451550545f701e5a86f7afe89ca51a67f74deec Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Sun, 20 Nov 2022 12:51:23 +0000 Subject: [PATCH 01/30] mark unsafe methods --- capnp/src/message.rs | 16 +++++++++------- capnp/src/private/arena.rs | 12 +++++++----- capnp/src/private/layout.rs | 11 ++++++----- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/capnp/src/message.rs b/capnp/src/message.rs index 4415d1ae1..10677c06d 100644 --- a/capnp/src/message.rs +++ b/capnp/src/message.rs @@ -377,7 +377,7 @@ pub unsafe trait Allocator { /// `word_size` is the length of the segment in words, as returned from `allocate_segment()`. /// `words_used` is always less than or equal to `word_size`, and indicates how many /// words (contiguous from the start of the segment) were possibly written with non-zero values. - fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, words_used: u32); + unsafe fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, words_used: u32); } /// A container used to build a message. @@ -674,7 +674,7 @@ unsafe impl Allocator for HeapAllocator { (ptr, size) } - fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, _words_used: u32) { + unsafe fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, _words_used: u32) { unsafe { alloc::alloc::dealloc( ptr, @@ -703,9 +703,11 @@ fn test_allocate_max() { assert_eq!(s2, allocator.max_segment_words); assert_eq!(s3, allocator.max_segment_words); - allocator.deallocate_segment(a1, s1, 0); - allocator.deallocate_segment(a2, s2, 0); - allocator.deallocate_segment(a3, s3, 0); + unsafe { + allocator.deallocate_segment(a1, s1, 0); + allocator.deallocate_segment(a2, s2, 0); + allocator.deallocate_segment(a3, s3, 0); + } } impl Builder { @@ -793,7 +795,7 @@ unsafe impl<'a> Allocator for ScratchSpaceHeapAllocator<'a> { } } - fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, words_used: u32) { + unsafe fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, words_used: u32) { if ptr == self.scratch_space.as_mut_ptr() { // Rezero the slice to allow reuse of the allocator. We only need to write // words that we know might contain nonzero values. @@ -816,7 +818,7 @@ where (*self).allocate_segment(minimum_size) } - fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, words_used: u32) { + unsafe fn deallocate_segment(&mut self, ptr: *mut u8, word_size: u32, words_used: u32) { (*self).deallocate_segment(ptr, word_size, words_used) } } diff --git a/capnp/src/private/arena.rs b/capnp/src/private/arena.rs index 93845a032..a9495f43e 100644 --- a/capnp/src/private/arena.rs +++ b/capnp/src/private/arena.rs @@ -36,7 +36,7 @@ pub trait ReaderArena { // return pointer to start of segment, and number of words in that segment fn get_segment(&self, id: u32) -> Result<(*const u8, u32)>; - fn check_offset( + unsafe fn check_offset( &self, segment_id: u32, start: *const u8, @@ -107,7 +107,7 @@ where } } - fn check_offset( + unsafe fn check_offset( &self, segment_id: u32, start: *const u8, @@ -261,7 +261,7 @@ where Ok((seg.ptr, seg.allocated)) } - fn check_offset( + unsafe fn check_offset( &self, _segment_id: u32, start: *const u8, @@ -334,7 +334,9 @@ where fn deallocate_all(&mut self) { if let Some(ref mut a) = self.allocator { for seg in &self.segments { - a.deallocate_segment(seg.ptr, seg.capacity, seg.allocated); + unsafe { + a.deallocate_segment(seg.ptr, seg.capacity, seg.allocated); + } } } } @@ -382,7 +384,7 @@ impl ReaderArena for NullArena { Err(Error::failed(String::from("tried to read from null arena"))) } - fn check_offset( + unsafe fn check_offset( &self, _segment_id: u32, start: *const u8, diff --git a/capnp/src/private/layout.rs b/capnp/src/private/layout.rs index 7d9271805..20c088291 100644 --- a/capnp/src/private/layout.rs +++ b/capnp/src/private/layout.rs @@ -172,7 +172,7 @@ impl WirePointer { ) -> Result<*const u8> { let this_addr: *const u8 = self as *const _ as *const _; let offset = 1 + ((self.offset_and_kind.get() as i32) >> 2); - arena.check_offset(segment_id, this_addr, offset) + unsafe { arena.check_offset(segment_id, this_addr, offset) } } #[inline] @@ -3057,9 +3057,10 @@ impl<'a> PointerReader<'a> { Ok(result && data_trunc && ptr_trunc) } } - PointerType::List => self - .get_list_any_size(ptr::null())? - .is_canonical(read_head, self.pointer), + PointerType::List => unsafe { + self.get_list_any_size(ptr::null())? + .is_canonical(read_head, self.pointer) + }, PointerType::Capability => Ok(false), } } @@ -3823,7 +3824,7 @@ impl<'a> ListReader<'a> { } } - pub fn is_canonical( + pub unsafe fn is_canonical( &self, read_head: &Cell<*const u8>, reff: *const WirePointer, From 15442db7ed47e85319449b514d6b6e3fe2ba5f28 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Sun, 20 Nov 2022 12:55:48 +0000 Subject: [PATCH 02/30] use 'std::mem::take' --- capnp-rpc/src/local.rs | 3 +-- capnp-rpc/src/rpc.rs | 2 +- capnp-rpc/src/sender_queue.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/capnp-rpc/src/local.rs b/capnp-rpc/src/local.rs index 107ba306f..a86d16a26 100644 --- a/capnp-rpc/src/local.rs +++ b/capnp-rpc/src/local.rs @@ -30,7 +30,6 @@ use futures::channel::oneshot; use futures::TryFutureExt; use std::cell::RefCell; -use std::mem; use std::rc::Rc; pub trait ResultsDoneHook { @@ -103,7 +102,7 @@ impl Drop for Results { if let (Some(message), Some(fulfiller)) = (self.message.take(), self.results_done_fulfiller.take()) { - let cap_table = mem::replace(&mut self.cap_table, Vec::new()); + let cap_table = ::std::mem::take(&mut self.cap_table); let _ = fulfiller.send(Box::new(ResultsDone::new(message, cap_table))); } else { unreachable!() diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index ff0c1be6a..831bfb0b1 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -806,7 +806,7 @@ impl ConnectionState { answer.received_finish.set(true); if finish.get_release_result_caps() { - exports_to_release = mem::replace(&mut answer.result_exports, Vec::new()); + exports_to_release = ::std::mem::take(&mut answer.result_exports); } // If the pipeline has not been cloned, the following two lines cancel the call. diff --git a/capnp-rpc/src/sender_queue.rs b/capnp-rpc/src/sender_queue.rs index 8349a7f59..7ce33a4d9 100644 --- a/capnp-rpc/src/sender_queue.rs +++ b/capnp-rpc/src/sender_queue.rs @@ -136,7 +136,7 @@ where .. } = *self.inner.borrow_mut(); *next_id = 0; - let map = ::std::mem::replace(map, BTreeMap::new()); + let map = ::std::mem::take(map); Drain { iter: map.into_iter(), } From ab81cda321c9cd1a0e2c6ea8a56ea5f4944a17e6 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Sun, 20 Nov 2022 12:59:26 +0000 Subject: [PATCH 03/30] remove redundant clones --- capnp-rpc/examples/pubsub/server.rs | 2 +- capnp-rpc/src/rpc.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/capnp-rpc/examples/pubsub/server.rs b/capnp-rpc/examples/pubsub/server.rs index 9f65e4dc1..6bb0d93a2 100644 --- a/capnp-rpc/examples/pubsub/server.rs +++ b/capnp-rpc/examples/pubsub/server.rs @@ -83,7 +83,7 @@ impl PublisherImpl { next_id: 0, subscribers: subscribers.clone(), }, - subscribers.clone(), + subscribers, ) } } diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index 831bfb0b1..0d1c7f3fc 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -1505,7 +1505,6 @@ impl ConnectionState { .0 .upgrade() .expect("dangling ref to import client?") - .clone() } else { let import_client = ImportClient::new(&connection_state, import_id); v.import_client = Some(( @@ -1933,7 +1932,7 @@ impl RequestHook for Request { // The pipeline must get notified of resolution before the app does to maintain ordering. let pipeline = Pipeline::new( &connection_state, - question_ref.clone(), + question_ref, Some(Promise::from_future(forked_promise1)), ); @@ -2105,7 +2104,7 @@ impl Pipeline { })); state.borrow_mut().resolve_self_promise = resolve_self_promise; - state.borrow_mut().redirect_later = Some(RefCell::new(fork.clone())); + state.borrow_mut().redirect_later = Some(RefCell::new(fork)); } None => {} } @@ -3129,7 +3128,7 @@ impl ClientHook for Client { }); match maybe_request { - Err(e) => Promise::err(e.clone()), + Err(e) => Promise::err(e), Ok(request) => { let ::capnp::capability::RemotePromise { promise, .. } = request.send(); From 8a41773c8c009e684ca7a0540e2df09eff873bef Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Sun, 20 Nov 2022 13:03:36 +0000 Subject: [PATCH 04/30] remove redundant static lifetimes on consts --- capnpc/src/codegen.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/capnpc/src/codegen.rs b/capnpc/src/codegen.rs index 201630a36..495e91621 100644 --- a/capnpc/src/codegen.rs +++ b/capnpc/src/codegen.rs @@ -2457,14 +2457,12 @@ fn generate_node( } } - (type_::Text(()), value::Text(t)) => Line(format!( - "pub const {}: &'static str = {:?};", - styled_name, t? - )), - (type_::Data(()), value::Data(d)) => Line(format!( - "pub const {}: &'static [u8] = &{:?};", - styled_name, d? - )), + (type_::Text(()), value::Text(t)) => { + Line(format!("pub const {}: &str = {:?};", styled_name, t?)) + } + (type_::Data(()), value::Data(d)) => { + Line(format!("pub const {}: &[u8] = &{:?};", styled_name, d?)) + } (type_::List(_), value::List(v)) => { generate_pointer_constant(gen, &styled_name, typ, v)? From 010ee24824eba1ad9636d05418fc589f79792ad7 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 05:51:58 +0000 Subject: [PATCH 05/30] remove needless range loops --- capnp-futures/src/serialize.rs | 8 ++++---- capnp-futures/src/serialize_packed.rs | 4 ++-- capnp-rpc/examples/calculator/server.rs | 4 ++-- capnp-rpc/src/rpc.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/capnp-futures/src/serialize.rs b/capnp-futures/src/serialize.rs index 27e05e89c..c6953f86c 100644 --- a/capnp-futures/src/serialize.rs +++ b/capnp-futures/src/serialize.rs @@ -231,8 +231,8 @@ where .copy_from_slice(&((segments[idx].len() / 8) as u32).to_le_bytes()); } if segment_count == 2 { - for idx in 4..8 { - buf[idx] = 0 + for value in buf.iter_mut().skip(4).take(4) { + *value = 0; } } write.write_all(&buf).await?; @@ -258,8 +258,8 @@ async fn write_segments(mut write: W, segments: &[&[u8]]) -> Result<()> where W: AsyncWrite + Unpin, { - for i in 0..segments.len() { - write.write_all(segments[i]).await?; + for segment in segments { + write.write_all(segment).await?; } Ok(()) } diff --git a/capnp-futures/src/serialize_packed.rs b/capnp-futures/src/serialize_packed.rs index 94bb25193..2333be8df 100644 --- a/capnp-futures/src/serialize_packed.rs +++ b/capnp-futures/src/serialize_packed.rs @@ -122,8 +122,8 @@ where PackedReadStage::WritingZeroes => { let num_zeroes = std::cmp::min(outbuf.len(), *num_run_bytes_remaining); - for ii in 0..num_zeroes { - outbuf[ii] = 0; + for value in outbuf.iter_mut().take(num_zeroes) { + *value = 0; } if num_zeroes >= *num_run_bytes_remaining { *buf_pos = 0; diff --git a/capnp-rpc/examples/calculator/server.rs b/capnp-rpc/examples/calculator/server.rs index 9c23efd3b..0b981c0f5 100644 --- a/capnp-rpc/examples/calculator/server.rs +++ b/capnp-rpc/examples/calculator/server.rs @@ -80,8 +80,8 @@ fn evaluate_impl( let mut request = func.call_request(); { let mut params = request.get().init_params(param_values.len() as u32); - for ii in 0..param_values.len() { - params.set(ii as u32, param_values[ii]); + for (ii, value) in param_values.iter().enumerate() { + params.set(ii as u32, *value); } } Ok(request.send().promise.await?.get()?.get_value()) diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index 0d1c7f3fc..d704b8789 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -1464,8 +1464,8 @@ impl ConnectionState { ) -> Vec { let mut cap_table_builder = payload.init_cap_table(cap_table.len() as u32); let mut exports = Vec::new(); - for idx in 0..cap_table.len() { - match cap_table[idx] { + for (idx, value) in cap_table.iter().enumerate() { + match value { Some(ref cap) => { match ConnectionState::write_descriptor( state, From a30f2c87850a7203147f6a9c0f7fd516f7137597 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 05:55:00 +0000 Subject: [PATCH 06/30] use byte literals --- capnpc/test/test.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/capnpc/test/test.rs b/capnpc/test/test.rs index 1cb7d9223..caeb1d581 100644 --- a/capnpc/test/test.rs +++ b/capnpc/test/test.rs @@ -1329,9 +1329,7 @@ mod tests { // Data word. capnp::word(1, 0, 0, 0, 0x42, 0, 0, 0), // text pointer. offset zero. 1-byte elements. 8 total elements. - capnp::word( - 'h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8, '.' as u8, '\n' as u8, 0, - ), + capnp::word(b'h', b'e', b'l', b'l', b'o', b'.', b'\n', 0), ]; let segment_array = &[ From 3e1f8e46017bc75adeeff0c1dd81fc50c490fae1 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 06:00:12 +0000 Subject: [PATCH 07/30] use 'iter::next' --- capnpc/test/test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/capnpc/test/test.rs b/capnpc/test/test.rs index caeb1d581..15548ced9 100644 --- a/capnpc/test/test.rs +++ b/capnpc/test/test.rs @@ -1849,8 +1849,8 @@ mod tests { let mut iter = structs.iter(); assert_eq!(3, iter.nth(3).unwrap().get_u_int32_field()); - assert_eq!(4, iter.nth(0).unwrap().get_u_int32_field()); - assert_eq!(5, iter.nth(0).unwrap().get_u_int32_field()); + assert_eq!(4, iter.next().unwrap().get_u_int32_field()); + assert_eq!(5, iter.next().unwrap().get_u_int32_field()); let mut c = 2; for s in structs.iter().skip(2) { From 2418c5c924fbe1fc5b7d7dd8e43ddd51cc241a9f Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 06:03:16 +0000 Subject: [PATCH 08/30] avoid using 'ref' on let statements --- capnp-rpc/src/rpc.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index d704b8789..4c1a8a42a 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -984,7 +984,7 @@ impl ConnectionState { }; { - let ref mut slots = connection_state.answers.borrow_mut().slots; + let slots = &mut connection_state.answers.borrow_mut().slots; let answer = slots.entry(question_id).or_insert(answer); if answer.active { return Err(Error::failed("questionId is already in use".to_string())); @@ -1021,7 +1021,7 @@ impl ConnectionState { pipeline.drive(fork.clone()); { - let ref mut slots = connection_state.answers.borrow_mut().slots; + let slots = &mut connection_state.answers.borrow_mut().slots; match slots.get_mut(&question_id) { Some(ref mut answer) => { answer.pipeline = Some(Box::new(pipeline)); @@ -1146,7 +1146,7 @@ impl ConnectionState { }; // If the import is in the table, fulfill it. - let ref mut slots = connection_state.imports.borrow_mut().slots; + let slots = &mut connection_state.imports.borrow_mut().slots; if let Some(ref mut import) = slots.get_mut(&resolve.get_promise_id()) { match import.promise_client_to_resolve.take() { Some(weak_promise_client) => { @@ -2901,7 +2901,7 @@ impl Drop for PromiseClient { // contain a pointer back to it. Remove that pointer. Note that we have to verify that // the import still exists and the pointer still points back to this object because this // object may actually outlive the import. - let ref mut slots = self.connection_state.imports.borrow_mut().slots; + let slots = &mut self.connection_state.imports.borrow_mut().slots; if let Some(ref mut import) = slots.get_mut(&id) { let mut drop_it = false; if let Some(ref c) = import.app_client { From d0da77c4d99dcdffdb499921a22d18a7aa918e3c Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 06:10:58 +0000 Subject: [PATCH 09/30] remove redundant pattern matching --- capnp-rpc/src/attach.rs | 2 +- capnp-rpc/src/rpc.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/capnp-rpc/src/attach.rs b/capnp-rpc/src/attach.rs index d89144862..4a392e561 100644 --- a/capnp-rpc/src/attach.rs +++ b/capnp-rpc/src/attach.rs @@ -40,7 +40,7 @@ where fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll { let result = Pin::new(&mut self.original_future).poll(cx); - if let Poll::Ready(_) = result { + if result.is_ready() { self.value.take(); } result diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index 4c1a8a42a..7601c9f28 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -1676,7 +1676,7 @@ where DisconnectorState::Disconnecting } DisconnectorState::Disconnecting => { - if let Some(_) = *(self.connection_state.borrow()) { + if self.connection_state.borrow().is_some() { DisconnectorState::Disconnecting } else { DisconnectorState::Disconnected From 051e3f9485e7e24b0767d5e58ce063ba9690bc3c Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 06:13:17 +0000 Subject: [PATCH 10/30] add 'is_empty' method --- capnp-futures/src/write_queue.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/capnp-futures/src/write_queue.rs b/capnp-futures/src/write_queue.rs index ab5e919e0..dbea8cba9 100644 --- a/capnp-futures/src/write_queue.rs +++ b/capnp-futures/src/write_queue.rs @@ -101,6 +101,10 @@ where unimplemented!() } + pub fn is_empty(&mut self) -> bool { + self.len() == 0 + } + /// Commands the queue to stop writing messages once it is empty. After this method has been called, /// any new calls to `send()` will return a future that immediately resolves to an error. /// If the passed-in `result` is an error, then the `WriteQueue` will resolve to that error. From bcd387c6421b66a1be40fdd4c3cdd8531a0fc539 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 06:16:09 +0000 Subject: [PATCH 11/30] remove redundant field names --- benchmark/catrank.rs | 5 +---- capnp-rpc/examples/calculator/server.rs | 6 +++--- capnp-rpc/examples/pubsub/server.rs | 5 +---- capnp-rpc/test/impls.rs | 2 +- 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/benchmark/catrank.rs b/benchmark/catrank.rs index d7d81225d..3a1e7ab09 100644 --- a/benchmark/catrank.rs +++ b/benchmark/catrank.rs @@ -108,10 +108,7 @@ impl crate::TestCase for CatRank { if snippet.contains(" dog ") { score /= 10000.0; } - scored_results.push(ScoredResult { - score: score, - result: result, - }); + scored_results.push(ScoredResult { score, result }); } // sort in decreasing order diff --git a/capnp-rpc/examples/calculator/server.rs b/capnp-rpc/examples/calculator/server.rs index 0b981c0f5..1e2ff21d8 100644 --- a/capnp-rpc/examples/calculator/server.rs +++ b/capnp-rpc/examples/calculator/server.rs @@ -36,7 +36,7 @@ struct ValueImpl { impl ValueImpl { fn new(value: f64) -> ValueImpl { - ValueImpl { value: value } + ValueImpl { value } } } @@ -101,7 +101,7 @@ impl FunctionImpl { body: calculator::expression::Reader, ) -> ::capnp::Result { let mut result = FunctionImpl { - param_count: param_count, + param_count, body: ::capnp_rpc::ImbuedMessageBuilder::new(::capnp::message::HeapAllocator::new()), }; result.body.set_root(body)?; @@ -199,7 +199,7 @@ impl calculator::Server for CalculatorImpl { let op = pry!(pry!(params.get()).get_op()); results .get() - .set_func(capnp_rpc::new_client(OperatorImpl { op: op })); + .set_func(capnp_rpc::new_client(OperatorImpl { op })); Promise::ok(()) } } diff --git a/capnp-rpc/examples/pubsub/server.rs b/capnp-rpc/examples/pubsub/server.rs index 6bb0d93a2..fcd4e62e4 100644 --- a/capnp-rpc/examples/pubsub/server.rs +++ b/capnp-rpc/examples/pubsub/server.rs @@ -54,10 +54,7 @@ struct SubscriptionImpl { impl SubscriptionImpl { fn new(id: u64, subscribers: Rc>) -> SubscriptionImpl { - SubscriptionImpl { - id: id, - subscribers: subscribers, - } + SubscriptionImpl { id, subscribers } } } diff --git a/capnp-rpc/test/impls.rs b/capnp-rpc/test/impls.rs index 498532343..c5192aad1 100644 --- a/capnp-rpc/test/impls.rs +++ b/capnp-rpc/test/impls.rs @@ -544,7 +544,7 @@ impl Handle { fn new(count: &Rc>) -> Handle { let count = count.clone(); count.set(count.get() + 1); - Handle { count: count } + Handle { count } } } From 2967f9396f1f42ea37565a8d4045c40b058f39ef Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 06:30:00 +0000 Subject: [PATCH 12/30] flatten single match blocks --- capnp-rpc/src/rpc.rs | 184 ++++++++++++++-------------------- capnp-rpc/src/sender_queue.rs | 9 +- capnp-rpc/test/impls.rs | 7 +- 3 files changed, 82 insertions(+), 118 deletions(-) diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index 7601c9f28..f5d66c3dc 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -1007,12 +1007,11 @@ impl ConnectionState { }) }) .then(move |v| { - match redirected_results_done_fulfiller { - Some(f) => match v { + if let Some(f) = redirected_results_done_fulfiller { + match v { Ok(ref r) => drop(f.send(Ok(Response::redirected(r.clone())))), Err(ref e) => drop(f.send(Err(e.clone()))), - }, - None => (), + } } Promise::ok(()) }); @@ -1102,11 +1101,8 @@ impl ConnectionState { } } None => { - match ret.which()? { - return_::TakeFromOtherQuestion(_) => { - unimplemented!() - } - _ => {} + if let return_::TakeFromOtherQuestion(_) = ret.which()? { + unimplemented!() } // Looks like this question was canceled earlier, so `Finish` // was already sent, with `releaseResultCaps` set true so that @@ -1150,13 +1146,8 @@ impl ConnectionState { if let Some(ref mut import) = slots.get_mut(&resolve.get_promise_id()) { match import.promise_client_to_resolve.take() { Some(weak_promise_client) => { - match weak_promise_client.upgrade() { - Some(promise_client) => { - promise_client.borrow_mut().resolve(replacement_or_error); - } - None => { - // ? - } + if let Some(promise_client) = weak_promise_client.upgrade() { + promise_client.borrow_mut().resolve(replacement_or_error); } } None => { @@ -1467,17 +1458,14 @@ impl ConnectionState { for (idx, value) in cap_table.iter().enumerate() { match value { Some(ref cap) => { - match ConnectionState::write_descriptor( + if let Some(export_id) = ConnectionState::write_descriptor( state, cap, cap_table_builder.reborrow().get(idx as u32), ) .unwrap() { - Some(export_id) => { - exports.push(export_id); - } - None => {} + exports.push(export_id); } } None => { @@ -1590,19 +1578,13 @@ impl ConnectionState { cap_descriptor::ReceiverAnswer(receiver_answer) => { let promised_answer = receiver_answer?; let question_id = promised_answer.get_question_id(); - match state.answers.borrow().slots.get(&question_id) { - Some(answer) => { - if answer.active { - match answer.pipeline { - Some(ref pipeline) => { - let ops = to_pipeline_ops(promised_answer.get_transform()?)?; - return Ok(Some(pipeline.get_pipelined_cap(&ops))); - } - None => (), - } + if let Some(answer) = state.answers.borrow().slots.get(&question_id) { + if answer.active { + if let Some(ref pipeline) = answer.pipeline { + let ops = to_pipeline_ops(promised_answer.get_transform()?)?; + return Ok(Some(pipeline.get_pipelined_cap(&ops))); } } - None => (), } Ok(Some(broken::new_cap(Error::failed( "invalid 'receiver answer'".to_string(), @@ -2085,28 +2067,23 @@ impl Pipeline { promise_clients_to_resolve: RefCell::new(crate::sender_queue::SenderQueue::new()), resolution_waiters: crate::sender_queue::SenderQueue::new(), })); - match redirect_later { - Some(redirect_later_promise) => { - let fork = redirect_later_promise.shared(); - let this = Rc::downgrade(&state); - let resolve_self_promise = - connection_state.eagerly_evaluate(fork.clone().then(move |response| { - let state = match this.upgrade() { - Some(s) => s, - None => { - return Promise::err(Error::failed( - "dangling reference to this".into(), - )) - } - }; - PipelineState::resolve(&state, response); - Promise::ok(()) - })); + if let Some(redirect_later_promise) = redirect_later { + let fork = redirect_later_promise.shared(); + let this = Rc::downgrade(&state); + let resolve_self_promise = + connection_state.eagerly_evaluate(fork.clone().then(move |response| { + let state = match this.upgrade() { + Some(s) => s, + None => { + return Promise::err(Error::failed("dangling reference to this".into())) + } + }; + PipelineState::resolve(&state, response); + Promise::ok(()) + })); - state.borrow_mut().resolve_self_promise = resolve_self_promise; - state.borrow_mut().redirect_later = Some(RefCell::new(fork)); - } - None => {} + state.borrow_mut().resolve_self_promise = resolve_self_promise; + state.borrow_mut().redirect_later = Some(RefCell::new(fork)); } Pipeline { state } } @@ -2447,20 +2424,19 @@ impl ResultsDone { .complete(Box::new(local::Pipeline::new(hook.clone()))); // Send a Canceled return. - match connection_state.connection.borrow_mut().as_mut() { - Ok(ref mut connection) => { - let mut message = connection.new_outgoing_message(50); // XXX size hint - { - let root: message::Builder = - message.get_body()?.get_as()?; - let mut ret = root.init_return(); - ret.set_answer_id(answer_id); - ret.set_release_param_caps(false); - ret.set_canceled(()); - } - let _ = message.send(); + if let Ok(ref mut connection) = + connection_state.connection.borrow_mut().as_mut() + { + let mut message = connection.new_outgoing_message(50); // XXX size hint + { + let root: message::Builder = + message.get_body()?.get_as()?; + let mut ret = root.init_return(); + ret.set_answer_id(answer_id); + ret.set_release_param_caps(false); + ret.set_canceled(()); } - Err(_) => (), + let _ = message.send(); } connection_state.answer_has_sent_return(answer_id, Vec::new()); @@ -2498,21 +2474,20 @@ impl ResultsDone { } (false, Err(e)) => { // Send an error return. - match connection_state.connection.borrow_mut().as_mut() { - Ok(ref mut connection) => { - let mut message = connection.new_outgoing_message(50); // XXX size hint - { - let root: message::Builder = - message.get_body()?.get_as()?; - let mut ret = root.init_return(); - ret.set_answer_id(answer_id); - ret.set_release_param_caps(false); - let mut exc = ret.init_exception(); - from_error(&e, exc.reborrow()); - } - let _ = message.send(); + if let Ok(ref mut connection) = + connection_state.connection.borrow_mut().as_mut() + { + let mut message = connection.new_outgoing_message(50); // XXX size hint + { + let root: message::Builder = + message.get_body()?.get_as()?; + let mut ret = root.init_return(); + ret.set_answer_id(answer_id); + ret.set_release_param_caps(false); + let mut exc = ret.init_exception(); + from_error(&e, exc.reborrow()); } - Err(_) => (), + let _ = message.send(); } connection_state.answer_has_sent_return(answer_id, Vec::new()); @@ -2698,18 +2673,15 @@ impl Drop for ImportClient { // Send a message releasing our remote references. let mut tmp = connection_state.connection.borrow_mut(); - match (self.remote_ref_count > 0, tmp.as_mut()) { - (true, Ok(ref mut c)) => { - let mut message = c.new_outgoing_message(50); // XXX size hint - { - let root: message::Builder = message.get_body().unwrap().init_as(); - let mut release = root.init_release(); - release.set_id(self.import_id); - release.set_reference_count(self.remote_ref_count); - } - let _ = message.send(); + if let (true, Ok(ref mut c)) = (self.remote_ref_count > 0, tmp.as_mut()) { + let mut message = c.new_outgoing_message(50); // XXX size hint + { + let root: message::Builder = message.get_body().unwrap().init_as(); + let mut release = root.init_release(); + release.set_id(self.import_id); + release.set_reference_count(self.remote_ref_count); } - _ => (), + let _ = message.send(); } } } @@ -2995,14 +2967,13 @@ impl Client { let mut transform = builder.init_transform(pipeline_client.borrow().ops.len() as u32); for idx in 0..pipeline_client.borrow().ops.len() { - match pipeline_client.borrow().ops[idx] { - ::capnp::private::capability::PipelineOp::GetPointerField(ordinal) => { - transform - .reborrow() - .get(idx as u32) - .set_get_pointer_field(ordinal); - } - _ => {} + if let ::capnp::private::capability::PipelineOp::GetPointerField(ordinal) = + pipeline_client.borrow().ops[idx] + { + transform + .reborrow() + .get(idx as u32) + .set_get_pointer_field(ordinal); } } None @@ -3031,14 +3002,13 @@ impl Client { let mut transform = promised_answer.init_transform(pipeline_client.borrow().ops.len() as u32); for idx in 0..pipeline_client.borrow().ops.len() { - match pipeline_client.borrow().ops[idx] { - ::capnp::private::capability::PipelineOp::GetPointerField(ordinal) => { - transform - .reborrow() - .get(idx as u32) - .set_get_pointer_field(ordinal); - } - _ => {} + if let ::capnp::private::capability::PipelineOp::GetPointerField(ordinal) = + pipeline_client.borrow().ops[idx] + { + transform + .reborrow() + .get(idx as u32) + .set_get_pointer_field(ordinal); } } diff --git a/capnp-rpc/src/sender_queue.rs b/capnp-rpc/src/sender_queue.rs index 7ce33a4d9..432721afb 100644 --- a/capnp-rpc/src/sender_queue.rs +++ b/capnp-rpc/src/sender_queue.rs @@ -64,12 +64,9 @@ where Out: 'static, { fn drop(&mut self) { - match self.inner.upgrade() { - Some(inner) => { - let Inner { ref mut map, .. } = *inner.borrow_mut(); - map.remove(&self.id); - } - None => (), + if let Some(inner) = self.inner.upgrade() { + let Inner { ref mut map, .. } = *inner.borrow_mut(); + map.remove(&self.id); } } } diff --git a/capnp-rpc/test/impls.rs b/capnp-rpc/test/impls.rs index c5192aad1..aa5d64b6d 100644 --- a/capnp-rpc/test/impls.rs +++ b/capnp-rpc/test/impls.rs @@ -572,11 +572,8 @@ impl TestCapDestructor { impl Drop for TestCapDestructor { fn drop(&mut self) { - match self.fulfiller.take() { - Some(f) => { - let _ = f.send(()); - } - None => (), + if let Some(f) = self.fulfiller.take() { + let _ = f.send(()); } } } From 4adfc8f2c91dd9f22340c55095f493c9fbd4aa20 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:13:26 +0000 Subject: [PATCH 13/30] remove useless conversions --- benchmark/benchmark.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmark/benchmark.rs b/benchmark/benchmark.rs index ea94fd985..a07355802 100644 --- a/benchmark/benchmark.rs +++ b/benchmark/benchmark.rs @@ -106,7 +106,7 @@ impl Serialize for NoCompression { W: io::Write, A: message::Allocator, { - serialize::write_message(write, message).map_err(|e| e.into()) + serialize::write_message(write, message) } } @@ -133,7 +133,7 @@ impl Serialize for Packed { W: io::Write, A: message::Allocator, { - serialize_packed::write_message(write, message).map_err(|e| e.into()) + serialize_packed::write_message(write, message) } } From a99f036f95dcd24cc9e06bb842a90667bef7f817 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:16:42 +0000 Subject: [PATCH 14/30] suppress warnings for 'clippy::bool_assert_comparison' --- capnp-rpc/test/test.rs | 1 + capnpc/test/test.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/capnp-rpc/test/test.rs b/capnp-rpc/test/test.rs index 35d09150d..cf53ada38 100644 --- a/capnp-rpc/test/test.rs +++ b/capnp-rpc/test/test.rs @@ -20,6 +20,7 @@ // THE SOFTWARE. #![cfg(test)] +#![allow(clippy::bool_assert_comparison)] use capnp::capability::{FromClientHook, Promise}; use capnp::Error; diff --git a/capnpc/test/test.rs b/capnpc/test/test.rs index 15548ced9..c1369ae45 100644 --- a/capnpc/test/test.rs +++ b/capnpc/test/test.rs @@ -21,6 +21,7 @@ // Enable this lint to catch violations in the generated code. #![warn(elided_lifetimes_in_paths)] +#![allow(clippy::bool_assert_comparison)] pub mod test_capnp { include!(concat!(env!("OUT_DIR"), "/test_capnp.rs")); From 3497e2474b8a3225614a6618368e2eb84ee5513e Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:18:31 +0000 Subject: [PATCH 15/30] don't pass mut references if you don't need to --- benchmark/benchmark.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/benchmark/benchmark.rs b/benchmark/benchmark.rs index a07355802..7b9f0ec06 100644 --- a/benchmark/benchmark.rs +++ b/benchmark/benchmark.rs @@ -234,7 +234,7 @@ where { let mut writer: &mut [u8] = &mut request_bytes; - compression.write_message(&mut writer, &mut message_req)?; + compression.write_message(&mut writer, &message_req)?; } let mut request_bytes1: &[u8] = &request_bytes; @@ -247,7 +247,7 @@ where { let mut writer: &mut [u8] = &mut response_bytes; - compression.write_message(&mut writer, &mut message_res)?; + compression.write_message(&mut writer, &message_res)?; } let mut response_bytes1: &[u8] = &response_bytes; @@ -289,7 +289,7 @@ where testcase.handle_request(request_reader, response)?; } - compression.write_message(&mut out_buffered, &mut message_res)?; + compression.write_message(&mut out_buffered, &message_res)?; out_buffered.flush()?; } Ok(()) @@ -320,7 +320,7 @@ where let request = message_req.init_root(); testcase.setup_request(&mut rng, request) }; - compression.write_message(&mut out_buffered, &mut message_req)?; + compression.write_message(&mut out_buffered, &message_req)?; out_buffered.flush()?; let message_reader = compression.read_message(&mut in_buffered, Default::default())?; From 53ca6ab078cad59eb64112af3c8dcc6baff3f4f9 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:19:52 +0000 Subject: [PATCH 16/30] remove explicit deref --- benchmark/benchmark.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/benchmark/benchmark.rs b/benchmark/benchmark.rs index 7b9f0ec06..0df2aa892 100644 --- a/benchmark/benchmark.rs +++ b/benchmark/benchmark.rs @@ -475,11 +475,11 @@ fn try_main() -> ::capnp::Result<()> { } }; - let mode = Mode::parse(&*args[2])?; + let mode = Mode::parse(&args[2])?; match &*args[4] { - "none" => do_testcase2(&*args[1], mode, &*args[3], NoCompression, iters), - "packed" => do_testcase2(&*args[1], mode, &*args[3], Packed, iters), + "none" => do_testcase2(&args[1], mode, &args[3], NoCompression, iters), + "packed" => do_testcase2(&args[1], mode, &args[3], Packed, iters), s => Err(::capnp::Error::failed(format!( "unrecognized compression: {}", s From 8c83da0106f8e780f3a702fa46e260cdf889a64b Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:20:55 +0000 Subject: [PATCH 17/30] add 'Default' implementation --- benchmark/benchmark.rs | 12 +++++++++--- benchmark/common.rs | 12 +++++++++--- capnp-rpc/src/lib.rs | 13 +++++++++++-- capnp-rpc/test/impls.rs | 29 +++++++++++++---------------- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/benchmark/benchmark.rs b/benchmark/benchmark.rs index 0df2aa892..202ed83bb 100644 --- a/benchmark/benchmark.rs +++ b/benchmark/benchmark.rs @@ -161,15 +161,21 @@ pub struct UseScratch { buffer2: Vec, } -impl UseScratch { - pub fn new() -> UseScratch { - UseScratch { +impl Default for UseScratch { + fn default() -> Self { + Self { buffer1: capnp::Word::allocate_zeroed_vec(SCRATCH_SIZE), buffer2: capnp::Word::allocate_zeroed_vec(SCRATCH_SIZE), } } } +impl UseScratch { + pub fn new() -> Self { + Self::default() + } +} + impl<'a> Scratch<'a> for UseScratch { type Allocator = message::ScratchSpaceHeapAllocator<'a>; diff --git a/benchmark/common.rs b/benchmark/common.rs index 58ea8fb4d..9a165fa42 100644 --- a/benchmark/common.rs +++ b/benchmark/common.rs @@ -29,15 +29,21 @@ pub struct FastRand { w: u32, } -impl FastRand { - pub fn new() -> FastRand { - FastRand { +impl Default for FastRand { + fn default() -> Self { + Self { x: 0x1d2acd47, y: 0x58ca3e14, z: 0xf563f232, w: 0x0bc76199, } } +} + +impl FastRand { + pub fn new() -> FastRand { + Self::default() + } #[inline] pub fn next_u32(&mut self) -> u32 { diff --git a/capnp-rpc/src/lib.rs b/capnp-rpc/src/lib.rs index 9d5b56e4a..7f8318cc2 100644 --- a/capnp-rpc/src/lib.rs +++ b/capnp-rpc/src/lib.rs @@ -330,15 +330,24 @@ where caps: std::collections::HashMap>>, } -impl CapabilityServerSet +impl Default for CapabilityServerSet where C: capnp::capability::FromServer, { - pub fn new() -> Self { + fn default() -> Self { Self { caps: std::default::Default::default(), } } +} + +impl CapabilityServerSet +where + C: capnp::capability::FromServer, +{ + pub fn new() -> Self { + Self::default() + } /// Adds a new capability to the set and returns a client backed by it. pub fn new_client(&mut self, s: S) -> C { diff --git a/capnp-rpc/test/impls.rs b/capnp-rpc/test/impls.rs index aa5d64b6d..828b3c4bd 100644 --- a/capnp-rpc/test/impls.rs +++ b/capnp-rpc/test/impls.rs @@ -115,15 +115,14 @@ impl bootstrap::Server for Bootstrap { } } +#[derive(Default)] pub struct TestInterface { call_count: Rc>, } impl TestInterface { - pub fn new() -> TestInterface { - TestInterface { - call_count: Rc::new(Cell::new(0)), - } + pub fn new() -> Self { + Self::default() } pub fn get_call_count(&self) -> Rc> { self.call_count.clone() @@ -282,13 +281,14 @@ impl test_pipeline::Server for TestPipeline { } } +#[derive(Default)] pub struct TestCallOrder { count: u32, } impl TestCallOrder { - pub fn new() -> TestCallOrder { - TestCallOrder { count: 0 } + pub fn new() -> Self { + Self::default() } } @@ -304,6 +304,7 @@ impl test_call_order::Server for TestCallOrder { } } +#[derive(Default)] pub struct TestMoreStuff { call_count: u32, handle_count: Rc>, @@ -311,12 +312,8 @@ pub struct TestMoreStuff { } impl TestMoreStuff { - pub fn new() -> TestMoreStuff { - TestMoreStuff { - call_count: 0, - handle_count: Rc::new(Cell::new(0)), - client_to_hold: None, - } + pub fn new() -> Self { + Self::default() } /* pub fn get_call_count(&self) -> Rc> { @@ -604,16 +601,18 @@ impl test_interface::Server for TestCapDestructor { } } +#[derive(Default)] pub struct CssHandle {} impl CssHandle { pub fn new() -> Self { - Self {} + Self::default() } } impl test_capability_server_set::handle::Server for CssHandle {} +#[derive(Default)] pub struct TestCapabilityServerSet { set: Rc< RefCell< @@ -624,9 +623,7 @@ pub struct TestCapabilityServerSet { impl TestCapabilityServerSet { pub fn new() -> Self { - Self { - set: Rc::new(RefCell::new(capnp_rpc::CapabilityServerSet::new())), - } + Self::default() } } From b25b9f5bf0628c2a48db49f44cc93626340284f7 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:28:18 +0000 Subject: [PATCH 18/30] remove redundant static lifetimes --- benchmark/carsales.rs | 4 ++-- benchmark/catrank.rs | 2 +- benchmark/common.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/benchmark/carsales.rs b/benchmark/carsales.rs index fc89a4038..b04708755 100644 --- a/benchmark/carsales.rs +++ b/benchmark/carsales.rs @@ -78,8 +78,8 @@ macro_rules! car_value_impl( car_value_impl!(Reader); car_value_impl!(Builder); -const MAKES: [&'static str; 5] = ["Toyota", "GM", "Ford", "Honda", "Tesla"]; -const MODELS: [&'static str; 6] = ["Camry", "Prius", "Volt", "Accord", "Leaf", "Model S"]; +const MAKES: [&str; 5] = ["Toyota", "GM", "Ford", "Honda", "Tesla"]; +const MODELS: [&str; 6] = ["Camry", "Prius", "Volt", "Accord", "Leaf", "Model S"]; pub fn random_car(rng: &mut FastRand, mut car: car::Builder) { car.set_make(MAKES[rng.next_less_than(MAKES.len() as u32) as usize]); diff --git a/benchmark/catrank.rs b/benchmark/catrank.rs index 3a1e7ab09..bfe2965d1 100644 --- a/benchmark/catrank.rs +++ b/benchmark/catrank.rs @@ -28,7 +28,7 @@ pub struct ScoredResult<'a> { result: search_result::Reader<'a>, } -const URL_PREFIX: &'static str = "http://example.com"; +const URL_PREFIX: &str = "http://example.com"; pub struct CatRank; diff --git a/benchmark/common.rs b/benchmark/common.rs index 9a165fa42..f9166a2f7 100644 --- a/benchmark/common.rs +++ b/benchmark/common.rs @@ -94,7 +94,7 @@ pub fn modulus(a: i32, b: i32) -> i32 { return a % b; } -pub const WORDS: [&'static str; 13] = [ +pub const WORDS: [&str; 13] = [ "foo ", "bar ", "baz ", "qux ", "quux ", "corge ", "grault ", "garply ", "waldo ", "fred ", "plugh ", "xyzzy ", "thud ", ]; From 57fc748495210df00022c0b30615d666e55d3348 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:31:20 +0000 Subject: [PATCH 19/30] remove needless return statements --- benchmark/common.rs | 6 +++--- benchmark/eval.rs | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/benchmark/common.rs b/benchmark/common.rs index f9166a2f7..b4473a1f5 100644 --- a/benchmark/common.rs +++ b/benchmark/common.rs @@ -52,7 +52,7 @@ impl FastRand { self.y = self.z; self.z = self.w; self.w = self.w ^ (self.w >> 19) ^ tmp ^ (tmp >> 8); - return self.w; + self.w } #[inline] @@ -80,7 +80,7 @@ pub fn div(a: i32, b: i32) -> i32 { if a == i32::MIN && b == -1 { return i32::MAX; } - return a / b; + a / b } #[inline] @@ -91,7 +91,7 @@ pub fn modulus(a: i32, b: i32) -> i32 { if a == i32::MIN && b == -1 { return i32::MAX; } - return a % b; + a % b } pub const WORDS: [&str; 13] = [ diff --git a/benchmark/eval.rs b/benchmark/eval.rs index aa7d383ab..ee1d36308 100644 --- a/benchmark/eval.rs +++ b/benchmark/eval.rs @@ -42,11 +42,11 @@ fn make_expression(rng: &mut FastRand, mut exp: expression::Builder, depth: u32) }; match exp.get_op().unwrap() { - Operation::Add => return left + right, - Operation::Subtract => return left - right, - Operation::Multiply => return left * right, - Operation::Divide => return div(left, right), - Operation::Modulus => return modulus(left, right), + Operation::Add => left + right, + Operation::Subtract => left - right, + Operation::Multiply => left * right, + Operation::Divide => div(left, right), + Operation::Modulus => modulus(left, right), } } From fd264c6eae3c9747882d85394ff4046077119b79 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:33:34 +0000 Subject: [PATCH 20/30] remove needless borrows --- capnp-rpc/src/rpc.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index f5d66c3dc..eb15db492 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -642,7 +642,7 @@ impl ConnectionState { Err(_) => panic!(), } - let pipeline = Pipeline::new(&state, question_ref, Some(Promise::from_future(promise))); + let pipeline = Pipeline::new(state, question_ref, Some(Promise::from_future(promise))); pipeline.get_pipelined_cap_move(Vec::new()) } @@ -760,7 +760,7 @@ impl ConnectionState { } assert_eq!(cap_table.len(), 1); - ConnectionState::write_descriptors(&connection_state, &cap_table, payload) + ConnectionState::write_descriptors(connection_state, &cap_table, payload) }; let slots = &mut connection_state.answers.borrow_mut().slots; @@ -1561,10 +1561,10 @@ impl ConnectionState { match descriptor.which()? { cap_descriptor::None(()) => Ok(None), cap_descriptor::SenderHosted(sender_hosted) => { - Ok(Some(ConnectionState::import(&state, sender_hosted, false))) + Ok(Some(ConnectionState::import(state, sender_hosted, false))) } cap_descriptor::SenderPromise(sender_promise) => { - Ok(Some(ConnectionState::import(&state, sender_promise, true))) + Ok(Some(ConnectionState::import(state, sender_promise, true))) } cap_descriptor::ReceiverHosted(receiver_hosted) => { if let Some(ref mut exp) = state.exports.borrow_mut().find(receiver_hosted) { @@ -1602,7 +1602,7 @@ impl ConnectionState { ) -> ::capnp::Result>>> { let mut result = Vec::new(); for idx in 0..cap_table.len() { - result.push(ConnectionState::receive_cap(&state, cap_table.get(idx))?); + result.push(ConnectionState::receive_cap(state, cap_table.get(idx))?); } Ok(result) } @@ -1813,8 +1813,8 @@ where ) { // Build the cap table. let exports = ConnectionState::write_descriptors( - &connection_state, - &cap_table, + connection_state, + cap_table, get_call(&mut message).unwrap().get_params().unwrap(), ); From ba787d40744ef12f7bcb9203719aba2d86c2a800 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:35:21 +0000 Subject: [PATCH 21/30] use 'as_mut' --- capnp-rpc/src/rpc.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index eb15db492..e2582bd4a 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -127,10 +127,7 @@ impl ExportTable { pub fn find(&mut self, id: u32) -> Option<&mut T> { let idx = id as usize; if idx < self.slots.len() { - match self.slots[idx] { - Some(ref mut v) => Some(v), - None => None, - } + self.slots[idx].as_mut() } else { None } From bd182b603a3331f1a7443b1deb29f037707212a1 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:37:28 +0000 Subject: [PATCH 22/30] prefer 'if let' to 'Option::map' --- capnp-rpc/examples/pubsub/server.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/capnp-rpc/examples/pubsub/server.rs b/capnp-rpc/examples/pubsub/server.rs index fcd4e62e4..18a7407b2 100644 --- a/capnp-rpc/examples/pubsub/server.rs +++ b/capnp-rpc/examples/pubsub/server.rs @@ -172,11 +172,11 @@ pub async fn main() -> Result<(), Box> { tokio::task::spawn_local(request.send().promise.map( move |r| match r { Ok(_) => { - subscribers2.borrow_mut().subscribers.get_mut(&idx).map( - |ref mut s| { - s.requests_in_flight -= 1; - }, - ); + if let Some(ref mut s) = + subscribers2.borrow_mut().subscribers.get_mut(&idx) + { + s.requests_in_flight -= 1; + } } Err(e) => { println!("Got error: {:?}. Dropping subscriber.", e); From 8c6dfefdc3f7b54e46cb71ef408c9679b8766b1d Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:38:51 +0000 Subject: [PATCH 23/30] prefer 'map' to 'and_then' --- capnp-rpc/src/rpc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index e2582bd4a..402c1f707 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -3089,7 +3089,7 @@ impl ClientHook for Client { let maybe_request = params.get().and_then(|p| { let mut request = p .target_size() - .and_then(|s| Ok(self.new_call(interface_id, method_id, Some(s))))?; + .map(|s| self.new_call(interface_id, method_id, Some(s)))?; request.get().set_as(p)?; Ok(request) }); From e5d146206be6b416393b841991ada8bd074b2c1f Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:40:03 +0000 Subject: [PATCH 24/30] use built-in 'map' method --- capnp-rpc/src/sender_queue.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/capnp-rpc/src/sender_queue.rs b/capnp-rpc/src/sender_queue.rs index 432721afb..92e7d3cc3 100644 --- a/capnp-rpc/src/sender_queue.rs +++ b/capnp-rpc/src/sender_queue.rs @@ -155,9 +155,6 @@ where { type Item = (In, oneshot::Sender); fn next(&mut self) -> Option { - match self.iter.next() { - None => None, - Some((_k, v)) => Some(v), - } + self.iter.next().map(|(_k, v)| v) } } From 10f761a9a56637de54889daee58ff758669441a7 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 08:43:07 +0000 Subject: [PATCH 25/30] use 'is_empty' --- capnpc/src/codegen_types.rs | 2 +- capnpc/test/test.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/capnpc/src/codegen_types.rs b/capnpc/src/codegen_types.rs index 4e18af1bb..94e9110da 100644 --- a/capnpc/src/codegen_types.rs +++ b/capnpc/src/codegen_types.rs @@ -328,7 +328,7 @@ impl<'a> RustTypeInfo for type_::Reader<'a> { type_::Struct(st) => { let brand = st.get_brand()?; let scopes = brand.get_scopes()?; - Ok(scopes.len() > 0) + Ok(!scopes.is_empty()) } _ => Ok(false), } diff --git a/capnpc/test/test.rs b/capnpc/test/test.rs index c1369ae45..ba6277e43 100644 --- a/capnpc/test/test.rs +++ b/capnpc/test/test.rs @@ -1977,7 +1977,7 @@ mod tests { let reader = serialize::read_message(raw_code_gen_request.as_slice(), ReaderOptions::new()).unwrap(); let generator_context = capnpc::codegen::GeneratorContext::new(&reader).unwrap(); - assert!(generator_context.node_map.len() > 0); - assert!(generator_context.scope_map.len() > 0); + assert!(!generator_context.node_map.is_empty()); + assert!(!generator_context.scope_map.is_empty()); } } From c56b44e42c6bbc98007736ef4beb569a19bb7c4c Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 10:04:18 +0000 Subject: [PATCH 26/30] remove comparison to unit value --- capnpc/test/test.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/capnpc/test/test.rs b/capnpc/test/test.rs index ba6277e43..c246ab4f2 100644 --- a/capnpc/test/test.rs +++ b/capnpc/test/test.rs @@ -985,7 +985,6 @@ mod tests { #[test] fn test_constants() { use crate::test_capnp::{test_constants, TestEnum}; - assert_eq!(test_constants::VOID_CONST, ()); assert_eq!(test_constants::BOOL_CONST, true); assert_eq!(test_constants::INT8_CONST, -123); assert_eq!(test_constants::INT16_CONST, -12345); From c00d861dca4884a16e6a9acd8d63da26bab57235 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 10:06:34 +0000 Subject: [PATCH 27/30] remove redundant closures --- capnp-rpc/src/rpc.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/capnp-rpc/src/rpc.rs b/capnp-rpc/src/rpc.rs index 402c1f707..421eaf508 100644 --- a/capnp-rpc/src/rpc.rs +++ b/capnp-rpc/src/rpc.rs @@ -972,9 +972,7 @@ impl ConnectionState { let (redirected_results_done_promise, redirected_results_done_fulfiller) = if redirect_results { let (f, p) = oneshot::channel::, Error>>(); - let p = p - .map_err(crate::canceled_to_error) - .and_then(|x| future::ready(x)); + let p = p.map_err(crate::canceled_to_error).and_then(future::ready); (Some(Promise::from_future(p)), Some(f)) } else { (None, None) @@ -2805,7 +2803,7 @@ impl PromiseClient { let (fulfiller, promise) = oneshot::channel::>(); let promise = promise .map_err(crate::canceled_to_error) - .and_then(|v| future::ready(v)); + .and_then(future::ready); let embargo = Embargo::new(fulfiller); let embargo_id = connection_state.embargoes.borrow_mut().push(embargo); From 6c7e43bb7a6389a52f21a3888556cddb4e514777 Mon Sep 17 00:00:00 2001 From: "daniel.eades" Date: Mon, 21 Nov 2022 10:07:46 +0000 Subject: [PATCH 28/30] omit useless 'let' binding --- capnp-rpc/test/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/capnp-rpc/test/test.rs b/capnp-rpc/test/test.rs index cf53ada38..01059d18c 100644 --- a/capnp-rpc/test/test.rs +++ b/capnp-rpc/test/test.rs @@ -158,7 +158,7 @@ fn disconnector_disconnects() { spawn( &mut spawner, server_rpc_system.map(|x| { - let _ = tx.send(()).expect("sending on tx"); + tx.send(()).expect("sending on tx"); x }), ); From fc91ed8c56ccb9b69301cfbbda6333749f0377f7 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Mon, 21 Nov 2022 06:30:00 +0000 Subject: [PATCH 29/30] flatten single match blocks --- capnpc/src/codegen.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/capnpc/src/codegen.rs b/capnpc/src/codegen.rs index 495e91621..3ede5789f 100644 --- a/capnpc/src/codegen.rs +++ b/capnpc/src/codegen.rs @@ -617,7 +617,7 @@ pub fn getter_text( } else { Leaf::Builder("'a") }; - let member = camel_to_snake_case(&module_string.to_string()); + let member = camel_to_snake_case(module_string); fn primitive_case( typ: &str, @@ -986,7 +986,7 @@ fn generate_setter( "self.builder.set_data_field::({}, value as u16)", offset ))); - (Some(the_mod.to_string()), None) + (Some(the_mod), None) } type_::Struct(_) => { return_result = true; From 2cf9474ca4de908ba8f031a5e92dd70e1a210740 Mon Sep 17 00:00:00 2001 From: Daniel Eades Date: Sat, 19 Nov 2022 07:04:22 +0000 Subject: [PATCH 30/30] add clippy CI target --- .clippy.toml | 1 + .github/workflows/ci.yml | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 .clippy.toml diff --git a/.clippy.toml b/.clippy.toml new file mode 100644 index 000000000..e034672c7 --- /dev/null +++ b/.clippy.toml @@ -0,0 +1 @@ +msrv = "1.65.0" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f18c68bf..ad38a630e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,3 +103,23 @@ jobs: with: command: fmt args: --all -- --check + + lint: + name: lint + runs-on: ubuntu-latest + steps: + - name: Install Cap'n Proto + run: | + export DEBIAN_FRONTEND=noninteractive + sudo apt-get install -y capnproto libcapnp-dev + - uses: actions/checkout@v3 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly-2022-10-18 + override: true + components: clippy + - uses: giraffate/clippy-action@v1 + with: + clippy_flags: --all --all-targets + filter_mode: nofilter