Generic refactor of DeclEngine (FuelLabs#4181)

## Description

Refactor the `DeclEngine` to use generic `DeclId`s that carry the type
of the declaration they point to with them. This allows us to avoid a
*lot* of unnecessary error handling and passing around of spans.

`DeclId` types mostly to not mix except in the case of Functions and
TraitFns, in this case we use a FunctionalDeclId enum to mix them in a
way they can still be discriminated by the user.


## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

Co-authored-by: Joshua Batty <[email protected]>

forc-pkg/src/pkg.rs

@@ -30,7 +30,7 @@ use std::{
 use sway_core::{
     abi_generation::{evm_json_abi, fuel_json_abi},
     asm_generation::ProgramABI,
-    decl_engine::{DeclEngine, DeclRef},
+    decl_engine::{DeclEngine, DeclRefFunction},
     fuel_prelude::{
         fuel_crypto,
         fuel_tx::{self, Contract, ContractId, StorageSlot},
@@ -1850,9 +1850,9 @@ impl PkgEntryKind {
 }
 
 impl PkgTestEntry {
-    fn from_decl(decl_ref: DeclRef, decl_engine: &DeclEngine) -> Result<Self> {
+    fn from_decl(decl_ref: DeclRefFunction, decl_engine: &DeclEngine) -> Result<Self> {
         let span = decl_ref.span();
-        let test_function_decl = decl_engine.get_function(&decl_ref, &span)?;
+        let test_function_decl = decl_engine.get_function(&decl_ref);
 
         let test_args: HashSet<String> = test_function_decl
             .attributes

forc-plugins/forc-doc/src/descriptor.rs

@@ -10,17 +10,13 @@ use sway_core::{
 };
 
 trait RequiredMethods {
-    fn to_methods(&self, decl_engine: &DeclEngine) -> Result<Vec<TyTraitFn>>;
+    fn to_methods(&self, decl_engine: &DeclEngine) -> Vec<TyTraitFn>;
 }
-impl RequiredMethods for Vec<sway_core::decl_engine::DeclRef> {
-    fn to_methods(&self, decl_engine: &DeclEngine) -> Result<Vec<TyTraitFn>> {
+impl RequiredMethods for Vec<DeclRefTraitFn> {
+    fn to_methods(&self, decl_engine: &DeclEngine) -> Vec<TyTraitFn> {
         self.iter()
-            .map(|DeclRef { id, decl_span, .. }| {
-                decl_engine
-                    .get_trait_fn(id, decl_span)
-                    .map_err(|e| anyhow::anyhow!("{}", e))
-            })
-            .collect::<anyhow::Result<_>>()
+            .map(|DeclRef { id, .. }| decl_engine.get_trait_fn(id))
+            .collect()
     }
 }
 
@@ -43,10 +39,8 @@ impl Descriptor {
         use swayfmt::parse;
         use TyDeclaration::*;
         match ty_decl {
-            StructDeclaration {
-                decl_id, decl_span, ..
-            } => {
-                let struct_decl = decl_engine.get_struct(decl_id, decl_span)?;
+            StructDeclaration { decl_id, .. } => {
+                let struct_decl = decl_engine.get_struct(decl_id);
                 if !document_private_items && struct_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
@@ -77,10 +71,8 @@ impl Descriptor {
                     }))
                 }
             }
-            EnumDeclaration {
-                decl_id, decl_span, ..
-            } => {
-                let enum_decl = decl_engine.get_enum(decl_id, decl_span)?;
+            EnumDeclaration { decl_id, .. } => {
+                let enum_decl = decl_engine.get_enum(decl_id);
                 if !document_private_items && enum_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
@@ -111,10 +103,8 @@ impl Descriptor {
                     }))
                 }
             }
-            TraitDeclaration {
-                decl_id, decl_span, ..
-            } => {
-                let trait_decl = decl_engine.get_trait(decl_id, decl_span)?;
+            TraitDeclaration { decl_id, .. } => {
+                let trait_decl = decl_engine.get_trait(decl_id);
                 if !document_private_items && trait_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
@@ -130,7 +120,7 @@ impl Descriptor {
                                     TyTraitInterfaceItem::TraitFn(fn_decl) => Some(fn_decl),
                                 })
                                 .collect::<Vec<_>>()
-                                .to_methods(decl_engine)?,
+                                .to_methods(decl_engine),
                         ),
                     );
 
@@ -155,10 +145,8 @@ impl Descriptor {
                     }))
                 }
             }
-            AbiDeclaration {
-                decl_id, decl_span, ..
-            } => {
-                let abi_decl = decl_engine.get_abi(decl_id, decl_span)?;
+            AbiDeclaration { decl_id, .. } => {
+                let abi_decl = decl_engine.get_abi(decl_id);
                 let item_name = abi_decl.name;
                 let attrs_opt =
                     (!abi_decl.attributes.is_empty()).then(|| abi_decl.attributes.to_html_string());
@@ -171,7 +159,7 @@ impl Descriptor {
                                 TyTraitInterfaceItem::TraitFn(fn_decl) => Some(fn_decl),
                             })
                             .collect::<Vec<_>>()
-                            .to_methods(decl_engine)?,
+                            .to_methods(decl_engine),
                     ),
                 );
 
@@ -193,10 +181,8 @@ impl Descriptor {
                     raw_attributes: attrs_opt,
                 }))
             }
-            StorageDeclaration {
-                decl_id, decl_span, ..
-            } => {
-                let storage_decl = decl_engine.get_storage(decl_id, decl_span)?;
+            StorageDeclaration { decl_id, .. } => {
+                let storage_decl = decl_engine.get_storage(decl_id);
                 let item_name = sway_types::BaseIdent::new_no_trim(
                     sway_types::span::Span::from_string(CONTRACT_STORAGE.to_string()),
                 );
@@ -253,10 +239,8 @@ impl Descriptor {
             //         raw_attributes: None,
             //     }))
             // }
-            FunctionDeclaration {
-                decl_id, decl_span, ..
-            } => {
-                let fn_decl = decl_engine.get_function(decl_id, decl_span)?;
+            FunctionDeclaration { decl_id, .. } => {
+                let fn_decl = decl_engine.get_function(decl_id);
                 if !document_private_items && fn_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {
@@ -285,10 +269,8 @@ impl Descriptor {
                     }))
                 }
             }
-            ConstantDeclaration {
-                decl_id, decl_span, ..
-            } => {
-                let const_decl = decl_engine.get_constant(decl_id, decl_span)?;
+            ConstantDeclaration { decl_id, .. } => {
+                let const_decl = decl_engine.get_constant(decl_id);
                 if !document_private_items && const_decl.visibility.is_private() {
                     Ok(Descriptor::NonDocumentable)
                 } else {

sway-core/src/asm_generation/finalized_asm.rs

@@ -5,7 +5,7 @@ use super::{
     ProgramABI, ProgramKind,
 };
 use crate::asm_lang::allocated_ops::{AllocatedOp, AllocatedOpcode};
-use crate::decl_engine::DeclRef;
+use crate::decl_engine::DeclRefFunction;
 use crate::error::*;
 use crate::source_map::SourceMap;
 
@@ -37,7 +37,7 @@ pub struct FinalizedEntry {
     pub selector: Option<[u8; 4]>,
     /// If this entry is constructed from a test function contains the declaration id for that
     /// function, otherwise contains `None`.
-    pub test_decl_ref: Option<DeclRef>,
+    pub test_decl_ref: Option<DeclRefFunction>,
 }
 
 /// The bytecode for a sway program as well as the byte offsets of configuration-time constants in

sway-core/src/asm_generation/fuel/fuel_asm_builder.rs

@@ -13,7 +13,7 @@ use crate::{
         ProgramKind,
     },
     asm_lang::{virtual_register::*, Label, Op, VirtualImmediate12, VirtualImmediate18, VirtualOp},
-    decl_engine::DeclRef,
+    decl_engine::DeclRefFunction,
     error::*,
     fuel_prelude::fuel_crypto::Hasher,
     metadata::MetadataManager,
@@ -63,7 +63,7 @@ pub struct FuelAsmBuilder<'ir> {
 
     // Final resulting VM bytecode ops; entry functions with their function and label, and regular
     // non-entry functions.
-    pub(super) entries: Vec<(Function, Label, Vec<Op>, Option<DeclRef>)>,
+    pub(super) entries: Vec<(Function, Label, Vec<Op>, Option<DeclRefFunction>)>,
     pub(super) non_entries: Vec<Vec<Op>>,
 
     // In progress VM bytecode ops.
@@ -73,7 +73,12 @@ pub struct FuelAsmBuilder<'ir> {
 pub type FuelAsmBuilderResult = (
     DataSection,
     RegisterSequencer,
-    Vec<(Function, Label, AbstractInstructionSet, Option<DeclRef>)>,
+    Vec<(
+        Function,
+        Label,
+        AbstractInstructionSet,
+        Option<DeclRefFunction>,
+    )>,
     Vec<AbstractInstructionSet>,
 );
 

sway-core/src/asm_generation/programs.rs

@@ -10,7 +10,7 @@ use super::fuel::{
 
 use crate::{
     asm_lang::{allocated_ops::AllocatedOp, Label},
-    decl_engine::DeclRef,
+    decl_engine::DeclRefFunction,
 };
 
 type SelectorOpt = Option<[u8; 4]>;
@@ -44,7 +44,7 @@ pub(super) struct AbstractEntry {
     pub(super) label: Label,
     pub(super) ops: AbstractInstructionSet,
     pub(super) name: FnName,
-    pub(super) test_decl_ref: Option<DeclRef>,
+    pub(super) test_decl_ref: Option<DeclRefFunction>,
 }
 
 /// An AllocatedProgram represents code which has allocated registers but still has abstract
@@ -54,7 +54,7 @@ pub(super) struct AllocatedProgram {
     data_section: DataSection,
     prologue: AllocatedAbstractInstructionSet,
     functions: Vec<AllocatedAbstractInstructionSet>,
-    entries: Vec<(SelectorOpt, Label, FnName, Option<DeclRef>)>,
+    entries: Vec<(SelectorOpt, Label, FnName, Option<DeclRefFunction>)>,
 }
 
 /// A FinalProgram represents code which may be serialized to VM bytecode.
@@ -63,7 +63,7 @@ pub(super) enum FinalProgram {
         kind: ProgramKind,
         data_section: DataSection,
         ops: Vec<AllocatedOp>,
-        entries: Vec<(SelectorOpt, ImmOffset, FnName, Option<DeclRef>)>,
+        entries: Vec<(SelectorOpt, ImmOffset, FnName, Option<DeclRefFunction>)>,
     },
     Evm {
         ops: Vec<etk_asm::ops::AbstractOp>,

sway-core/src/concurrent_slab.rs

@@ -1,5 +1,7 @@
 use std::{fmt, sync::RwLock};
 
+use sway_types::{Named, Spanned};
+
 use crate::{decl_engine::*, engine_threading::*, type_system::TypeId, TypeInfo};
 
 #[derive(Debug)]
@@ -19,10 +21,7 @@ where
     }
 }
 
-impl<T> Default for ConcurrentSlab<T>
-where
-    T: Default,
-{
+impl<T> Default for ConcurrentSlab<T> {
     fn default() -> Self {
         Self {
             inner: Default::default(),
@@ -106,10 +105,14 @@ impl ConcurrentSlab<TypeInfo> {
     }
 }
 
-impl ConcurrentSlab<DeclWrapper> {
-    pub fn replace(&self, index: DeclId, new_value: DeclWrapper) -> Option<DeclWrapper> {
+impl<T> ConcurrentSlab<T>
+where
+    DeclEngine: DeclEngineIndex<T>,
+    T: Named + Spanned,
+{
+    pub fn replace(&self, index: DeclId<T>, new_value: T) -> Option<T> {
         let mut inner = self.inner.write().unwrap();
-        inner[*index] = new_value;
+        inner[index.inner()] = new_value;
         None
     }
 }

sway-core/src/control_flow_analysis/analyze_return_paths.rs

@@ -12,7 +12,7 @@ use crate::{
 };
 use petgraph::prelude::NodeIndex;
 use sway_error::error::CompileError;
-use sway_types::{ident::Ident, span::Span, IdentUnique, Spanned};
+use sway_types::{ident::Ident, span::Span, IdentUnique};
 
 impl<'cfg> ControlFlowGraph<'cfg> {
     pub(crate) fn construct_return_path_graph<'eng: 'cfg>(
@@ -131,7 +131,6 @@ fn connect_node<'eng: 'cfg, 'cfg>(
     graph: &mut ControlFlowGraph<'cfg>,
     leaves: &[NodeIndex],
 ) -> Result<NodeConnection, CompileError> {
-    let span = node.span.clone();
     match &node.content {
         ty::TyAstNodeContent::Expression(ty::TyExpression {
             expression: ty::TyExpressionVariant::Return(..),
@@ -168,7 +167,7 @@ fn connect_node<'eng: 'cfg, 'cfg>(
         }
         ty::TyAstNodeContent::SideEffect(_) => Ok(NodeConnection::NextStep(leaves.to_vec())),
         ty::TyAstNodeContent::Declaration(decl) => Ok(NodeConnection::NextStep(
-            connect_declaration(engines, node, decl, graph, span, leaves)?,
+            connect_declaration(engines, node, decl, graph, leaves)?,
         )),
     }
 }
@@ -178,7 +177,6 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
     node: &ty::TyAstNode,
     decl: &ty::TyDeclaration,
     graph: &mut ControlFlowGraph<'cfg>,
-    span: Span,
     leaves: &[NodeIndex],
 ) -> Result<Vec<NodeIndex>, CompileError> {
     use ty::TyDeclaration::*;
@@ -198,18 +196,18 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
             Ok(vec![entry_node])
         }
         FunctionDeclaration { decl_id, .. } => {
-            let fn_decl = decl_engine.get_function(decl_id, &decl.span())?;
+            let fn_decl = decl_engine.get_function(decl_id);
             let entry_node = graph.add_node(node.into());
             for leaf in leaves {
                 graph.add_edge(*leaf, entry_node, "".into());
             }
-            connect_typed_fn_decl(engines, &fn_decl, graph, entry_node, span)?;
+            connect_typed_fn_decl(engines, &fn_decl, graph, entry_node)?;
             Ok(leaves.to_vec())
         }
         ImplTrait { decl_id, .. } => {
             let ty::TyImplTrait {
                 trait_name, items, ..
-            } = decl_engine.get_impl_trait(decl_id, &span)?;
+            } = decl_engine.get_impl_trait(decl_id);
             let entry_node = graph.add_node(node.into());
             for leaf in leaves {
                 graph.add_edge(*leaf, entry_node, "".into());
@@ -240,7 +238,7 @@ fn connect_impl_trait<'eng: 'cfg, 'cfg>(
     for item in items {
         match item {
             TyImplItem::Fn(method_decl_ref) => {
-                let fn_decl = decl_engine.get_function(method_decl_ref, &trait_name.span())?;
+                let fn_decl = decl_engine.get_function(method_decl_ref);
                 let fn_decl_entry_node = graph.add_node(ControlFlowGraphNode::MethodDeclaration {
                     span: fn_decl.span.clone(),
                     method_name: fn_decl.name.clone(),
@@ -250,13 +248,7 @@ fn connect_impl_trait<'eng: 'cfg, 'cfg>(
                 graph.add_edge(entry_node, fn_decl_entry_node, "".into());
                 // connect the impl declaration node to the functions themselves, as all trait functions are
                 // public if the trait is in scope
-                connect_typed_fn_decl(
-                    engines,
-                    &fn_decl,
-                    graph,
-                    fn_decl_entry_node,
-                    fn_decl.span.clone(),
-                )?;
+                connect_typed_fn_decl(engines, &fn_decl, graph, fn_decl_entry_node)?;
                 methods_and_indexes.push((fn_decl.name.clone(), fn_decl_entry_node));
             }
         }
@@ -286,7 +278,6 @@ fn connect_typed_fn_decl<'eng: 'cfg, 'cfg>(
     fn_decl: &ty::TyFunctionDeclaration,
     graph: &mut ControlFlowGraph<'cfg>,
     entry_node: NodeIndex,
-    _span: Span,
 ) -> Result<(), CompileError> {
     let type_engine = engines.te();
     let fn_exit_node = graph.add_node(format!("\"{}\" fn exit", fn_decl.name.as_str()).into());