From e24a16bd31d207342fe751cbc47799584e60f4fe Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Sun, 23 Jul 2023 16:26:00 -0700
Subject: [PATCH] function: make inherit_vars a boxed slice instead of a hash
 map

Empty hash maps muck around with TLS. Per code review, use a boxed slice
of a tuple instead. This has the nice benefit of printing inherited vars
in sorted order.
---
 fish-rust/src/builtins/function.rs | 24 +++++++++++++++---------
 fish-rust/src/function.rs          |  8 ++++----
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/fish-rust/src/builtins/function.rs b/fish-rust/src/builtins/function.rs
index 5d3ff4f51..58aca6b04 100644
--- a/fish-rust/src/builtins/function.rs
+++ b/fish-rust/src/builtins/function.rs
@@ -315,12 +315,25 @@ pub fn function(
     let definition_file = unsafe { parser.pin().libdata().get_current_filename().as_ref() }
         .map(|s| Arc::new(s.from_ffi()));
 
+    // Ensure inherit_vars is unique and then populate it.
+    opts.inherit_vars.sort_unstable();
+    opts.inherit_vars.dedup();
+
+    let inherit_vars: Vec<(WString, Vec<WString>)> = opts
+        .inherit_vars
+        .into_iter()
+        .filter_map(|name| {
+            let vals = parser.get_vars().get(&name)?.as_list().to_vec();
+            Some((name, vals))
+        })
+        .collect();
+
     // We have what we need to actually define the function.
-    let mut props = function::FunctionProperties {
+    let props = function::FunctionProperties {
         func_node,
         named_arguments: opts.named_arguments,
         description: opts.description,
-        inherit_vars: Default::default(),
+        inherit_vars: inherit_vars.into_boxed_slice(),
         shadow_scope: opts.shadow_scope,
         is_autoload: RelaxedAtomicBool::new(false),
         definition_file,
@@ -329,13 +342,6 @@ pub fn function(
         copy_definition_lineno: 0,
     };
 
-    // Populate inherit_vars.
-    for name in opts.inherit_vars.into_iter() {
-        if let Some(var) = parser.get_vars().get(&name) {
-            props.inherit_vars.insert(name, var.as_list().to_vec());
-        }
-    }
-
     // Add the function itself.
     function::add(function_name.clone(), Arc::new(props));
 
diff --git a/fish-rust/src/function.rs b/fish-rust/src/function.rs
index 77688c731..4d261520a 100644
--- a/fish-rust/src/function.rs
+++ b/fish-rust/src/function.rs
@@ -33,8 +33,8 @@ pub struct FunctionProperties {
     pub description: WString,
 
     /// Mapping of all variables that were inherited from the function definition scope to their
-    /// values.
-    pub inherit_vars: HashMap<WString, Vec<WString>>,
+    /// values, as (key, values) pairs.
+    pub inherit_vars: Box<[(WString, Vec<WString>)]>,
 
     /// Set to true if invoking this function shadows the variables of the underlying function.
     pub shadow_scope: bool,
@@ -374,7 +374,7 @@ impl FunctionProperties {
     }
 
     /// Return a reference to the vars that this function has inherited from its definition scope.
-    pub fn inherit_vars(&self) -> &HashMap<WString, Vec<WString>> {
+    pub fn inherit_vars(&self) -> &[(WString, Vec<WString>)] {
         &self.inherit_vars
     }
 
@@ -484,7 +484,7 @@ impl FunctionProperties {
         }
 
         // Output any inherited variables as `set -l` lines.
-        for (name, values) in &self.inherit_vars {
+        for (name, values) in self.inherit_vars.iter() {
             // We don't know what indentation style the function uses,
             // so we do what fish_indent would.
             sprintf!(=> &mut out, "\n    set -l %ls", name);