From 3fddf4aebaa113f95b533ec3250747b7a214ffd4 Mon Sep 17 00:00:00 2001 From: Joao Matos Date: Tue, 24 Jan 2023 19:01:07 +0000 Subject: [PATCH] Fix DCA for generic functions. Closes https://github.com/FuelLabs/sway/issues/3671. --- .../analyze_return_paths.rs | 6 +- .../dead_code_analysis.rs | 60 +++++++++++++++---- .../control_flow_analysis/flow_graph/mod.rs | 14 +---- .../flow_graph/namespace.rs | 31 +++++++--- .../src/language/ty/declaration/function.rs | 19 ++++++ sway-core/src/lib.rs | 6 +- 6 files changed, 94 insertions(+), 42 deletions(-) diff --git a/sway-core/src/control_flow_analysis/analyze_return_paths.rs b/sway-core/src/control_flow_analysis/analyze_return_paths.rs index 3ea6f9b5cc9..c383c00e286 100644 --- a/sway-core/src/control_flow_analysis/analyze_return_paths.rs +++ b/sway-core/src/control_flow_analysis/analyze_return_paths.rs @@ -38,7 +38,7 @@ impl<'cfg> ControlFlowGraph<'cfg> { pub(crate) fn analyze_return_paths(&self, engines: Engines<'_>) -> Vec { let mut errors = vec![]; for ( - name, + (name, _sig), FunctionNamespaceEntry { entry_point, exit_point, @@ -305,9 +305,7 @@ fn connect_typed_fn_decl<'eng: 'cfg, 'cfg>( .to_typeinfo(fn_decl.return_type, &fn_decl.return_type_span) .unwrap_or_else(|_| TypeInfo::Tuple(Vec::new())), }; - graph - .namespace - .insert_function(fn_decl.name.clone(), namespace_entry); + graph.namespace.insert_function(fn_decl, namespace_entry); Ok(()) } diff --git a/sway-core/src/control_flow_analysis/dead_code_analysis.rs b/sway-core/src/control_flow_analysis/dead_code_analysis.rs index 592e5eb9d31..13453cf7cee 100644 --- a/sway-core/src/control_flow_analysis/dead_code_analysis.rs +++ b/sway-core/src/control_flow_analysis/dead_code_analysis.rs @@ -152,9 +152,7 @@ impl<'cfg> ControlFlowGraph<'cfg> { &leaves, exit_node, tree_type, - NodeConnectionOptions { - force_struct_fields_connection: false, - }, + NodeConnectionOptions::default(), )?; leaves = l_leaves; @@ -188,7 +186,7 @@ fn collect_entry_points( /// This struct is used to pass node connection further down the tree as /// we are processing AST nodes. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Default)] struct NodeConnectionOptions { /// When this is enabled, connect struct fields to the struct itself, /// thus making all struct fields considered as being used in the graph. @@ -444,7 +442,6 @@ fn connect_impl_trait<'eng: 'cfg, 'cfg>( graph.add_edge(entry_node, node_ix, "".into()); } Some(trait_decl_node) => { - graph.add_edge_from_entry(entry_node, "".into()); graph.add_edge(entry_node, trait_decl_node, "".into()); } }; @@ -687,9 +684,7 @@ fn connect_typed_fn_decl<'eng: 'cfg, 'cfg>( return_type: ty, }; - graph - .namespace - .insert_function(fn_decl.name.clone(), namespace_entry); + graph.namespace.insert_function(fn_decl, namespace_entry); connect_fn_params_struct_enums(engines, fn_decl, graph, entry_node)?; Ok(()) @@ -824,11 +819,50 @@ fn connect_expression<'eng: 'cfg, 'cfg>( } => { let fn_decl = decl_engine.get_function(function_decl_id.clone(), &expression_span)?; let mut is_external = false; + // println!( + // "connect_expression fn_decl {:?} {:?}", + // fn_decl.name, + // function_decl_id.clone() + // ); + + // in the case of monomorphized functions, first check if we already have a node for + // it in the namespace. if not then we need to check to see if the namespace contains + // the decl id parents (the original generic non monomorphized decl id). + let mut exists = false; + let parents = decl_engine.find_all_parents(engines, function_decl_id.clone()); + for parent in parents { + if let Ok(parent) = decl_engine.get_function(parent.clone(), &expression_span) { + exists |= graph.namespace.get_function(&parent).is_some(); + } + } + // find the function in the namespace - let (fn_entrypoint, fn_exit_point) = graph - .namespace - .get_function(&fn_decl.name) - .cloned() + let fn_namespace_entry = graph.namespace.get_function(&fn_decl).cloned(); + + let mut leaves = leaves.to_vec(); + + // if the parent node exists in this module, then add the monomorphized version + // to the graph. + if fn_namespace_entry.is_none() && exists { + let (l_leaves, _new_exit_node) = connect_node( + engines, + &ty::TyAstNode { + content: ty::TyAstNodeContent::Declaration( + ty::TyDeclaration::FunctionDeclaration(function_decl_id.clone()), + ), + span: expression_span.clone(), + }, + graph, + &leaves, + exit_node, + tree_type, + NodeConnectionOptions::default(), + )?; + + leaves = l_leaves; + } + + let (fn_entrypoint, fn_exit_point) = fn_namespace_entry .map( |FunctionNamespaceEntry { entry_point, @@ -860,7 +894,7 @@ fn connect_expression<'eng: 'cfg, 'cfg>( } for leaf in leaves { - graph.add_edge(*leaf, fn_entrypoint, label.into()); + graph.add_edge(leaf, fn_entrypoint, label.into()); } // save the existing options value to restore after handling the arguments diff --git a/sway-core/src/control_flow_analysis/flow_graph/mod.rs b/sway-core/src/control_flow_analysis/flow_graph/mod.rs index f2e7f59fcb3..794b63116ce 100644 --- a/sway-core/src/control_flow_analysis/flow_graph/mod.rs +++ b/sway-core/src/control_flow_analysis/flow_graph/mod.rs @@ -27,7 +27,6 @@ pub type ExitPoint = NodeIndex; pub struct ControlFlowGraph<'cfg> { pub(crate) graph: Graph<'cfg>, pub(crate) entry_points: Vec, - pub(crate) pending_entry_points_edges: Vec<(NodeIndex, ControlFlowGraphEdge)>, pub(crate) namespace: ControlFlowNamespace, pub(crate) decls: HashMap, } @@ -167,9 +166,6 @@ impl<'cfg> std::fmt::Debug for ControlFlowGraphNode<'cfg> { } impl<'cfg> ControlFlowGraph<'cfg> { - pub(crate) fn add_edge_from_entry(&mut self, to: NodeIndex, label: ControlFlowGraphEdge) { - self.pending_entry_points_edges.push((to, label)); - } pub(crate) fn add_node<'eng: 'cfg>( &mut self, engines: Engines<'eng>, @@ -179,6 +175,7 @@ impl<'cfg> ControlFlowGraph<'cfg> { let ident_opt = node.get_decl_ident(decl_engine); let node_index = self.graph.add_node(node); if let Some(ident) = ident_opt { + //println!("CFG add_node: {:?}", ident); self.decls.insert(ident.into(), node_index); } node_index @@ -192,15 +189,6 @@ impl<'cfg> ControlFlowGraph<'cfg> { self.graph.add_edge(from, to, edge) } - pub(crate) fn connect_pending_entry_edges(&mut self) { - for entry in &self.entry_points { - for (to, label) in &self.pending_entry_points_edges { - self.graph.add_edge(*entry, *to, label.clone()); - } - } - self.pending_entry_points_edges.clear(); - } - pub(crate) fn get_node_from_decl( &self, engines: Engines<'_>, diff --git a/sway-core/src/control_flow_analysis/flow_graph/namespace.rs b/sway-core/src/control_flow_analysis/flow_graph/namespace.rs index 8b71586c9c7..0b5a462b495 100644 --- a/sway-core/src/control_flow_analysis/flow_graph/namespace.rs +++ b/sway-core/src/control_flow_analysis/flow_graph/namespace.rs @@ -1,6 +1,9 @@ use super::{EntryPoint, ExitPoint}; use crate::{ - language::{ty, CallPath}, + language::{ + ty::{self, TyFunctionDeclaration, TyFunctionSig}, + CallPath, + }, type_system::TypeInfo, Ident, }; @@ -32,7 +35,7 @@ pub(crate) struct StructNamespaceEntry { /// of scope at this point, as that would have been caught earlier and aborted the compilation /// process. pub struct ControlFlowNamespace { - pub(crate) function_namespace: HashMap, + pub(crate) function_namespace: HashMap<(IdentUnique, TyFunctionSig), FunctionNamespaceEntry>, pub(crate) enum_namespace: HashMap)>, pub(crate) trait_namespace: HashMap, /// This is a mapping from trait name to method names and their node indexes @@ -45,13 +48,25 @@ pub struct ControlFlowNamespace { } impl ControlFlowNamespace { - pub(crate) fn get_function(&self, ident: &Ident) -> Option<&FunctionNamespaceEntry> { - let ident: IdentUnique = ident.into(); - self.function_namespace.get(&ident) - } - pub(crate) fn insert_function(&mut self, ident: Ident, entry: FunctionNamespaceEntry) { + pub(crate) fn get_function( + &self, + fn_decl: &TyFunctionDeclaration, + ) -> Option<&FunctionNamespaceEntry> { + //println!("get_function ident {:#?}", ident.span); + let ident: IdentUnique = fn_decl.name.clone().into(); + self.function_namespace + .get(&(ident, TyFunctionSig::from_fn_decl(fn_decl))) + } + pub(crate) fn insert_function( + &mut self, + fn_decl: &ty::TyFunctionDeclaration, + entry: FunctionNamespaceEntry, + ) { + let ident = &fn_decl.name; + //println!("insert function ident {:#?}", ident.span); let ident: IdentUnique = ident.into(); - self.function_namespace.insert(ident, entry); + self.function_namespace + .insert((ident, TyFunctionSig::from_fn_decl(fn_decl)), entry); } pub(crate) fn get_constant(&self, ident: &Ident) -> Option<&NodeIndex> { self.const_namespace.get(ident) diff --git a/sway-core/src/language/ty/declaration/function.rs b/sway-core/src/language/ty/declaration/function.rs index c1831376051..2c7cc9e224f 100644 --- a/sway-core/src/language/ty/declaration/function.rs +++ b/sway-core/src/language/ty/declaration/function.rs @@ -372,3 +372,22 @@ impl TyFunctionParameter { self.name.as_str() == "self" } } + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct TyFunctionSig { + pub return_type: TypeId, + pub parameters: Vec, +} + +impl TyFunctionSig { + pub fn from_fn_decl(fn_decl: &TyFunctionDeclaration) -> Self { + Self { + return_type: fn_decl.return_type, + parameters: fn_decl + .parameters + .iter() + .map(|p| p.type_id) + .collect::>(), + } + } +} diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs index 193e879493c..dc6ce042fdb 100644 --- a/sway-core/src/lib.rs +++ b/sway-core/src/lib.rs @@ -786,7 +786,7 @@ fn module_dead_code_analysis<'eng: 'cfg, 'cfg>( module_dead_code_analysis(engines, &submodule.module, &tree_type, graph) }) }); - let res = submodules_res.flat_map(|()| { + submodules_res.flat_map(|()| { ControlFlowGraph::append_module_to_dead_code_graph( engines, &module.all_nodes, @@ -795,9 +795,7 @@ fn module_dead_code_analysis<'eng: 'cfg, 'cfg>( ) .map(|_| ok((), vec![], vec![])) .unwrap_or_else(|error| err(vec![], vec![error])) - }); - graph.connect_pending_entry_edges(); - res + }) } fn return_path_analysis(engines: Engines<'_>, program: &ty::TyProgram) -> Vec {