From 801cfae279cb384ecef137caab0b77d899a26b44 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Wed, 10 Jul 2024 19:14:05 -0700 Subject: [PATCH] Avoid clone in `Signature::get_positional()` (#13338) # Description `Signature::get_positional()` was returning an owned `PositionalArg`, which contains a bunch of strings. `ClosureEval` uses this in `try_add_arg`, making all of that unnecessary cloning a little bit hot. # User-Facing Changes Slightly better performance --- crates/nu-protocol/src/signature.rs | 7 +++---- crates/nu-protocol/tests/test_signature.rs | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/nu-protocol/src/signature.rs b/crates/nu-protocol/src/signature.rs index 5928ce0c0c..3241b0df22 100644 --- a/crates/nu-protocol/src/signature.rs +++ b/crates/nu-protocol/src/signature.rs @@ -485,15 +485,14 @@ impl Signature { (name, s) } - pub fn get_positional(&self, position: usize) -> Option { + pub fn get_positional(&self, position: usize) -> Option<&PositionalArg> { if position < self.required_positional.len() { - self.required_positional.get(position).cloned() + self.required_positional.get(position) } else if position < (self.required_positional.len() + self.optional_positional.len()) { self.optional_positional .get(position - self.required_positional.len()) - .cloned() } else { - self.rest_positional.clone() + self.rest_positional.as_ref() } } diff --git a/crates/nu-protocol/tests/test_signature.rs b/crates/nu-protocol/tests/test_signature.rs index e22029bab9..8faf772c38 100644 --- a/crates/nu-protocol/tests/test_signature.rs +++ b/crates/nu-protocol/tests/test_signature.rs @@ -39,7 +39,7 @@ fn test_signature_chained() { assert_eq!( signature.get_positional(0), - Some(PositionalArg { + Some(&PositionalArg { name: "required".to_string(), desc: "required description".to_string(), shape: SyntaxShape::String, @@ -49,7 +49,7 @@ fn test_signature_chained() { ); assert_eq!( signature.get_positional(1), - Some(PositionalArg { + Some(&PositionalArg { name: "optional".to_string(), desc: "optional description".to_string(), shape: SyntaxShape::String, @@ -59,7 +59,7 @@ fn test_signature_chained() { ); assert_eq!( signature.get_positional(2), - Some(PositionalArg { + Some(&PositionalArg { name: "rest".to_string(), desc: "rest description".to_string(), shape: SyntaxShape::String,