Skip to content

Commit

Permalink
merge_tools: box external tool variant to minimize stack size
Browse files Browse the repository at this point in the history
The ExternalMergeTool struct has four 24-byte fields plus one bool. It could
be shrunk by dropping Vec/String capacity, but the resulting type would still
be bigger compared to the default Builtin variant.
  • Loading branch information
yuja committed Mar 3, 2024
1 parent 78edb61 commit 0f42c56
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions cli/src/merge_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@ pub enum MergeToolConfigError {
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum MergeTool {
Builtin,
External(ExternalMergeTool),
// Boxed because ExternalMergeTool is big compared to the Builtin variant.
External(Box<ExternalMergeTool>),
}

impl MergeTool {
fn external(tool: ExternalMergeTool) -> Self {
MergeTool::External(Box::new(tool))
}
}

/// Finds the appropriate tool for diff editing or merges
Expand Down Expand Up @@ -130,7 +137,7 @@ fn get_tool_config(settings: &UserSettings, name: &str) -> Result<Option<MergeTo
if name == BUILTIN_EDITOR_NAME {
Ok(Some(MergeTool::Builtin))
} else {
Ok(get_external_tool_config(settings, name)?.map(MergeTool::External))
Ok(get_external_tool_config(settings, name)?.map(MergeTool::external))
}
}

Expand Down Expand Up @@ -178,7 +185,7 @@ impl DiffEditor {
} else {
None
}
.unwrap_or_else(|| MergeTool::External(ExternalMergeTool::with_edit_args(&args)));
.unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_edit_args(&args)));
Ok(DiffEditor {
tool,
base_ignores,
Expand Down Expand Up @@ -228,7 +235,7 @@ impl MergeEditor {
} else {
None
}
.unwrap_or_else(|| MergeTool::External(ExternalMergeTool::with_merge_args(&args)));
.unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_merge_args(&args)));
if matches!(&tool, MergeTool::External(mergetool) if mergetool.merge_args.is_empty()) {
return Err(MergeToolConfigError::MergeArgsNotConfigured {
tool_name: args.to_string(),
Expand Down

0 comments on commit 0f42c56

Please sign in to comment.