From 747c3a3b284806277a90629de404c32ccc5df318 Mon Sep 17 00:00:00 2001 From: Pedro Tacla Yamada Date: Tue, 18 Jun 2024 14:58:28 +1000 Subject: [PATCH] Avoid boxing RequestTracker parameters (#9806) We only need to box these types when they are consumed. This is related to https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-smart and makes the APIs more flexible most of the time. We can change back if these APIs will take ownership of the requests. Co-authored-by: pyamada (Remote Dev Environment) --- crates/parcel/src/request_tracker/request.rs | 3 +-- .../src/request_tracker/request_tracker.rs | 4 ++-- crates/parcel/src/request_tracker/test.rs | 16 ++++++++-------- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/parcel/src/request_tracker/request.rs b/crates/parcel/src/request_tracker/request.rs index 1bbf7912b7a..45d6557a875 100644 --- a/crates/parcel/src/request_tracker/request.rs +++ b/crates/parcel/src/request_tracker/request.rs @@ -29,8 +29,7 @@ impl<'a, T: Clone> RunRequestContext<'a, T> { // TODO } - // TODO: Why is this boxed? - pub fn run_request(&mut self, request: Box<&dyn Request>) -> anyhow::Result { + pub fn run_request(&mut self, request: &impl Request) -> anyhow::Result { self .request_tracker .run_child_request(request, self.parent_request_hash) diff --git a/crates/parcel/src/request_tracker/request_tracker.rs b/crates/parcel/src/request_tracker/request_tracker.rs index 443a2cd5958..c0e351d3962 100644 --- a/crates/parcel/src/request_tracker/request_tracker.rs +++ b/crates/parcel/src/request_tracker/request_tracker.rs @@ -27,13 +27,13 @@ impl RequestTracker { } } - pub fn run_request(&mut self, request: Box<&dyn Request>) -> anyhow::Result { + pub fn run_request(&mut self, request: &impl Request) -> anyhow::Result { self.run_child_request(request, None) } pub fn run_child_request( &mut self, - request: Box<&dyn Request>, + request: &impl Request, parent_request_hash: Option, ) -> anyhow::Result { let request_id = request.id(); diff --git a/crates/parcel/src/request_tracker/test.rs b/crates/parcel/src/request_tracker/test.rs index 3025d170a29..f0fe731f327 100644 --- a/crates/parcel/src/request_tracker/test.rs +++ b/crates/parcel/src/request_tracker/test.rs @@ -13,7 +13,7 @@ fn should_run_request() { let request_b = TestRequest::new("B", &[&request_c]); let request_a = TestRequest::new("A", &[&request_b]); - let result = rt.run_request(Box::new(&request_a)).unwrap(); + let result = rt.run_request(&request_a).unwrap(); assert_eq!(result[0], "A"); assert_eq!(result[1], "B"); @@ -28,13 +28,13 @@ fn should_reuse_previously_run_request() { let request_b = TestRequest::new("B", &[&request_c]); let request_a = TestRequest::new("A", &[&request_b]); - let result = rt.run_request(Box::new(&request_a)).unwrap(); + let result = rt.run_request(&request_a).unwrap(); assert_eq!(result[0], "A"); assert_eq!(result[1], "B"); assert_eq!(result[2], "C"); - let result = rt.run_request(Box::new(&request_a)).unwrap(); + let result = rt.run_request(&request_a).unwrap(); assert_eq!(result[0], "A"); assert_eq!(result[1], "B"); assert_eq!(result[2], "C"); @@ -46,12 +46,12 @@ fn should_run_request_once() { let request_a = TestRequest::new("A", &[]); - let result = rt.run_request(Box::new(&request_a)).unwrap(); + let result = rt.run_request(&request_a).unwrap(); assert_eq!(result[0], "A"); assert_eq!(request_a.run_count(), 1); - let result = rt.run_request(Box::new(&request_a)).unwrap(); + let result = rt.run_request(&request_a).unwrap(); assert_eq!(result[0], "A"); assert_eq!(request_a.run_count(), 1); } @@ -63,14 +63,14 @@ fn should_run_request_once_2() { let request_b = TestRequest::new("B", &[]); let request_a = TestRequest::new("A", &[&request_b]); - let result = rt.run_request(Box::new(&request_a)).unwrap(); + let result = rt.run_request(&request_a).unwrap(); assert_eq!(result[0], "A"); assert_eq!(result[1], "B"); assert_eq!(request_a.run_count(), 1); assert_eq!(request_b.run_count(), 1); - let result = rt.run_request(Box::new(&request_a)).unwrap(); + let result = rt.run_request(&request_a).unwrap(); assert_eq!(result[0], "A"); assert_eq!(result[1], "B"); assert_eq!(request_a.run_count(), 1); @@ -127,7 +127,7 @@ impl<'a> Request> for TestRequest<'a> { while let Some(subrequest) = subrequets.pop() { let req = subrequest.clone(); - let subrequest_result = context.run_request(Box::new(&req))?; + let subrequest_result = context.run_request(&req)?; result.extend(subrequest_result); }