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.
This commit is contained in:
ridiculousfish 2023-07-23 16:26:00 -07:00 committed by Peter Ammon
parent a672edc0d5
commit e24a16bd31
2 changed files with 19 additions and 13 deletions

View file

@ -315,12 +315,25 @@ pub fn function(
let definition_file = unsafe { parser.pin().libdata().get_current_filename().as_ref() } let definition_file = unsafe { parser.pin().libdata().get_current_filename().as_ref() }
.map(|s| Arc::new(s.from_ffi())); .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. // We have what we need to actually define the function.
let mut props = function::FunctionProperties { let props = function::FunctionProperties {
func_node, func_node,
named_arguments: opts.named_arguments, named_arguments: opts.named_arguments,
description: opts.description, description: opts.description,
inherit_vars: Default::default(), inherit_vars: inherit_vars.into_boxed_slice(),
shadow_scope: opts.shadow_scope, shadow_scope: opts.shadow_scope,
is_autoload: RelaxedAtomicBool::new(false), is_autoload: RelaxedAtomicBool::new(false),
definition_file, definition_file,
@ -329,13 +342,6 @@ pub fn function(
copy_definition_lineno: 0, 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. // Add the function itself.
function::add(function_name.clone(), Arc::new(props)); function::add(function_name.clone(), Arc::new(props));

View file

@ -33,8 +33,8 @@ pub struct FunctionProperties {
pub description: WString, pub description: WString,
/// Mapping of all variables that were inherited from the function definition scope to their /// Mapping of all variables that were inherited from the function definition scope to their
/// values. /// values, as (key, values) pairs.
pub inherit_vars: HashMap<WString, Vec<WString>>, pub inherit_vars: Box<[(WString, Vec<WString>)]>,
/// Set to true if invoking this function shadows the variables of the underlying function. /// Set to true if invoking this function shadows the variables of the underlying function.
pub shadow_scope: bool, 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. /// 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 &self.inherit_vars
} }
@ -484,7 +484,7 @@ impl FunctionProperties {
} }
// Output any inherited variables as `set -l` lines. // 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, // We don't know what indentation style the function uses,
// so we do what fish_indent would. // so we do what fish_indent would.
sprintf!(=> &mut out, "\n set -l %ls", name); sprintf!(=> &mut out, "\n set -l %ls", name);