Fix CFG node uniqueness. (FuelLabs#3232)

The CFG now keeps track of previously processed declarations and makes
sure to re-use the previously added graph node, instead of creating
duplicate nodes for the same declarations.

We do this by introducing a new IdentUnique type, which is pretty much
the same as an Ident, but hashes with the full span, as opposed to an
Ident which just uses the name as the hash, hence not fully ideal to use
as the key for the CFG namespace, since it won't work correctly in
multi-file cases.

Closes FuelLabs#3093.

<del>
This does multiple fixes:

- The declaration engine now tries to keep sure we do not create new
declaration ids for previously added types.

Right now this is only implemented for enums, but should be extended in
the future for other declaration types. This is kinda like the
monormophization cache we previously discussed, but working for
non-generic types.

- The CFG/DCA graph now also keeps track of previously processed
declaration ids and makes sure to re-use the previously added graph
node, instead of creating duplicate ones for the same declaration nodes.

This also needs to be extended for other declaration types.

Closes FuelLabs#3093.
</del>

sway-core/src/control_flow_analysis/dead_code_analysis.rs

@@ -265,7 +265,12 @@ fn connect_node(
         ty::TyAstNodeContent::SideEffect => (leaves.to_vec(), exit_node),
         ty::TyAstNodeContent::Declaration(decl) => {
             // all leaves connect to this node, then this node is the singular leaf
-            let decl_node = graph.add_node(node.into());
+            let cfg_node: ControlFlowGraphNode = node.into();
+            // check if node for this decl already exists
+            let decl_node = match graph.get_node_from_decl(&cfg_node) {
+                Some(node) => node,
+                None => graph.add_node(cfg_node),
+            };
             for leaf in leaves {
                 graph.add_edge(*leaf, decl_node, "".into());
             }

sway-core/src/control_flow_analysis/flow_graph/mod.rs

@@ -1,9 +1,14 @@
 //! This is the flow graph, a graph which contains edges that represent possible steps of program
 //! execution.
 
-use crate::{language::ty, Ident};
+use std::collections::HashMap;
 
-use sway_types::span::Span;
+use crate::{
+    language::ty::{self, GetDeclIdent},
+    Ident,
+};
+
+use sway_types::{span::Span, IdentUnique};
 
 use petgraph::{graph::EdgeIndex, prelude::NodeIndex};
 
@@ -22,6 +27,7 @@ pub struct ControlFlowGraph {
     pub(crate) graph: Graph,
     pub(crate) entry_points: Vec<NodeIndex>,
     pub(crate) namespace: ControlFlowNamespace,
+    pub(crate) decls: HashMap<IdentUnique, NodeIndex>,
 }
 
 pub type Graph = petgraph::Graph<ControlFlowGraphNode, ControlFlowGraphEdge>;
@@ -64,6 +70,23 @@ pub enum ControlFlowGraphNode {
     },
 }
 
+impl GetDeclIdent for ControlFlowGraphNode {
+    fn get_decl_ident(&self) -> Option<Ident> {
+        match self {
+            ControlFlowGraphNode::OrganizationalDominator(_) => None,
+            ControlFlowGraphNode::ProgramNode(node) => node.get_decl_ident(),
+            ControlFlowGraphNode::EnumVariant { variant_name, .. } => Some(variant_name.clone()),
+            ControlFlowGraphNode::MethodDeclaration { method_name, .. } => {
+                Some(method_name.clone())
+            }
+            ControlFlowGraphNode::StructField {
+                struct_field_name, ..
+            } => Some(struct_field_name.clone()),
+            ControlFlowGraphNode::StorageField { field_name, .. } => Some(field_name.clone()),
+        }
+    }
+}
+
 impl std::fmt::Debug for ControlFlowGraphNode {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let text = match self {
@@ -128,7 +151,12 @@ impl ControlFlowGraph {
         }
     }
     pub(crate) fn add_node(&mut self, node: ControlFlowGraphNode) -> NodeIndex {
-        self.graph.add_node(node)
+        let ident_opt = node.get_decl_ident();
+        let node_index = self.graph.add_node(node);
+        if let Some(ident) = ident_opt {
+            self.decls.insert(ident.into(), node_index);
+        }
+        node_index
     }
     pub(crate) fn add_edge(
         &mut self,
@@ -139,11 +167,22 @@ impl ControlFlowGraph {
         self.graph.add_edge(from, to, edge)
     }
 
+    pub(crate) fn get_node_from_decl(&self, cfg_node: &ControlFlowGraphNode) -> Option<NodeIndex> {
+        if let Some(ident) = cfg_node.get_decl_ident() {
+            self.decls.get(&ident.into()).cloned()
+        } else {
+            None
+        }
+    }
+
     #[allow(dead_code)]
     /// Prints out graphviz for this graph
     pub(crate) fn visualize(&self) {
-        use petgraph::dot::Dot;
-        tracing::info!("{:?}", Dot::with_config(&self.graph, &[]));
+        use petgraph::dot::{Config, Dot};
+        tracing::info!(
+            "{:?}",
+            Dot::with_config(&self.graph, &[Config::EdgeNoLabel])
+        );
     }
 }
 

sway-core/src/control_flow_analysis/flow_graph/namespace.rs

@@ -6,6 +6,7 @@ use crate::{
 };
 use petgraph::prelude::NodeIndex;
 use std::collections::HashMap;
+use sway_types::IdentUnique;
 
 #[derive(Default, Clone)]
 /// Represents a single entry in the [ControlFlowNamespace]'s function namespace. Contains various
@@ -32,7 +33,7 @@ pub(crate) struct StructNamespaceEntry {
 /// process.
 pub struct ControlFlowNamespace {
     pub(crate) function_namespace: HashMap<Ident, FunctionNamespaceEntry>,
-    pub(crate) enum_namespace: HashMap<Ident, (NodeIndex, HashMap<Ident, NodeIndex>)>,
+    pub(crate) enum_namespace: HashMap<IdentUnique, (NodeIndex, HashMap<Ident, NodeIndex>)>,
     pub(crate) trait_namespace: HashMap<CallPath, NodeIndex>,
     /// This is a mapping from trait name to method names and their node indexes
     pub(crate) trait_method_namespace: HashMap<CallPath, HashMap<Ident, NodeIndex>>,
@@ -58,7 +59,7 @@ impl ControlFlowNamespace {
     }
     pub(crate) fn insert_enum(&mut self, enum_name: Ident, enum_decl_index: NodeIndex) {
         self.enum_namespace
-            .insert(enum_name, (enum_decl_index, HashMap::new()));
+            .insert(enum_name.into(), (enum_decl_index, HashMap::new()));
     }
     pub(crate) fn insert_enum_variant(
         &mut self,
@@ -67,7 +68,7 @@ impl ControlFlowNamespace {
         variant_name: Ident,
         variant_index: NodeIndex,
     ) {
-        match self.enum_namespace.get_mut(&enum_name) {
+        match self.enum_namespace.get_mut(&enum_name.clone().into()) {
             Some((_ix, variants)) => {
                 variants.insert(variant_name, variant_index);
             }
@@ -78,19 +79,19 @@ impl ControlFlowNamespace {
                     map
                 };
                 self.enum_namespace
-                    .insert(enum_name, (enum_decl_index, variant_space));
+                    .insert(enum_name.into(), (enum_decl_index, variant_space));
             }
         }
     }
     pub(crate) fn find_enum(&self, enum_name: &Ident) -> Option<&NodeIndex> {
-        self.enum_namespace.get(enum_name).map(|f| &f.0)
+        self.enum_namespace.get(&enum_name.into()).map(|f| &f.0)
     }
     pub(crate) fn find_enum_variant_index(
         &self,
         enum_name: &Ident,
         variant_name: &Ident,
     ) -> Option<(NodeIndex, NodeIndex)> {
-        let (enum_ix, enum_decl) = self.enum_namespace.get(enum_name)?;
+        let (enum_ix, enum_decl) = self.enum_namespace.get(&enum_name.into())?;
         Some((*enum_ix, *enum_decl.get(variant_name)?))
     }
 

sway-core/src/language/ty/ast_node.rs

@@ -1,7 +1,7 @@
 use std::fmt;
 
 use derivative::Derivative;
-use sway_types::Span;
+use sway_types::{Ident, Span};
 
 use crate::{
     declaration_engine::{de_get_function, DeclMapping, ReplaceDecls},
@@ -11,6 +11,10 @@ use crate::{
     types::DeterministicallyAborts,
 };
 
+pub trait GetDeclIdent {
+    fn get_decl_ident(&self) -> Option<Ident>;
+}
+
 #[derive(Clone, Debug, Eq, Derivative)]
 #[derivative(PartialEq)]
 pub struct TyAstNode {
@@ -89,6 +93,12 @@ impl DeterministicallyAborts for TyAstNode {
     }
 }
 
+impl GetDeclIdent for TyAstNode {
+    fn get_decl_ident(&self) -> Option<Ident> {
+        self.content.get_decl_ident()
+    }
+}
+
 impl TyAstNode {
     /// recurse into `self` and get any return statements -- used to validate that all returns
     /// do indeed return the correct type
@@ -193,3 +203,14 @@ impl CollectTypesMetadata for TyAstNodeContent {
         }
     }
 }
+
+impl GetDeclIdent for TyAstNodeContent {
+    fn get_decl_ident(&self) -> Option<Ident> {
+        match self {
+            TyAstNodeContent::Declaration(decl) => decl.get_decl_ident(),
+            TyAstNodeContent::Expression(_expr) => None, //expr.get_decl_ident(),
+            TyAstNodeContent::ImplicitReturnExpression(_expr) => None, //expr.get_decl_ident(),
+            TyAstNodeContent::SideEffect => None,
+        }
+    }
+}

sway-core/src/language/ty/declaration/declaration.rs

@@ -246,6 +246,41 @@ impl CollectTypesMetadata for TyDeclaration {
     }
 }
 
+impl GetDeclIdent for TyDeclaration {
+    fn get_decl_ident(&self) -> Option<Ident> {
+        match self {
+            TyDeclaration::VariableDeclaration(decl) => Some(decl.name.clone()),
+            TyDeclaration::ConstantDeclaration(decl) => {
+                Some(de_get_constant(decl.clone(), &decl.span()).unwrap().name)
+            }
+            TyDeclaration::FunctionDeclaration(decl) => {
+                Some(de_get_function(decl.clone(), &decl.span()).unwrap().name)
+            }
+            TyDeclaration::TraitDeclaration(decl) => {
+                Some(de_get_trait(decl.clone(), &decl.span()).unwrap().name)
+            }
+            TyDeclaration::StructDeclaration(decl) => {
+                Some(de_get_struct(decl.clone(), &decl.span()).unwrap().name)
+            }
+            TyDeclaration::EnumDeclaration(decl) => {
+                Some(de_get_enum(decl.clone(), &decl.span()).unwrap().name)
+            }
+            TyDeclaration::ImplTrait(decl) => Some(
+                de_get_impl_trait(decl.clone(), &decl.span())
+                    .unwrap()
+                    .trait_name
+                    .suffix,
+            ),
+            TyDeclaration::AbiDeclaration(decl) => {
+                Some(de_get_abi(decl.clone(), &decl.span()).unwrap().name)
+            }
+            TyDeclaration::GenericTypeForFunctionScope { name, .. } => Some(name.clone()),
+            TyDeclaration::ErrorRecovery(_) => None,
+            TyDeclaration::StorageDeclaration(_decl) => None,
+        }
+    }
+}
+
 impl TyDeclaration {
     /// Retrieves the declaration as an enum declaration.
     ///