chore: pull most logic from get_all_contracts up out of the CrateDefMap

compiler/noirc_driver/src/lib.rs

@@ -18,12 +18,13 @@ use noirc_evaluator::ssa::{SsaLogging, SsaProgramArtifact};
 use noirc_frontend::debug::build_debug_crate_file;
 use noirc_frontend::elaborator::{FrontendOptions, UnstableFeature};
 use noirc_frontend::hir::Context;
-use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
+use noirc_frontend::hir::def_map::{CrateDefMap, ModuleDefId, ModuleId};
 use noirc_frontend::monomorphization::{
     errors::MonomorphizationError, monomorphize, monomorphize_debug,
 };
-use noirc_frontend::node_interner::FuncId;
+use noirc_frontend::node_interner::{FuncId, GlobalId, TypeId};
 use noirc_frontend::token::SecondaryAttribute;
+use std::collections::HashMap;
 use std::path::Path;
 use tracing::info;
 
@@ -428,39 +429,42 @@ pub fn compile_contract(
 ) -> CompilationResult<CompiledContract> {
     let (_, warnings) = check_crate(context, crate_id, options)?;
 
-    // TODO: We probably want to error if contracts is empty
-    let contracts = context.get_all_contracts(&crate_id);
+    let def_map = context.def_map(&crate_id).expect("The local crate should be analyzed already");
+    let mut contracts = def_map.get_all_contracts();
 
-    let mut compiled_contracts = vec![];
-    let mut errors = warnings;
-
-    if contracts.len() > 1 {
+    let Some((module_id, name)) = contracts.next() else {
         let err = CustomDiagnostic::from_message(
-            "Packages are limited to a single contract",
+            "cannot compile crate into a contract as it does not contain any contracts",
             FileId::default(),
         );
         return Err(vec![err]);
-    } else if contracts.is_empty() {
+    };
+
+    if contracts.next().is_some() {
         let err = CustomDiagnostic::from_message(
-            "cannot compile crate into a contract as it does not contain any contracts",
+            "Packages are limited to a single contract",
             FileId::default(),
         );
         return Err(vec![err]);
-    };
+    }
+    drop(contracts);
 
-    for contract in contracts {
-        match compile_contract_inner(context, contract, options) {
-            Ok(contract) => compiled_contracts.push(contract),
-            Err(mut more_errors) => errors.append(&mut more_errors),
+    let module_id = ModuleId { krate: crate_id, local_id: module_id };
+    let contract = read_contract(context, module_id, name);
+
+    let mut errors = warnings;
+
+    let compiled_contract = match compile_contract_inner(context, contract, options) {
+        Ok(contract) => contract,
+        Err(mut more_errors) => {
+            errors.append(&mut more_errors);
+            return Err(errors);
         }
-    }
+    };
 
     if has_errors(&errors, options.deny_warnings) {
         Err(errors)
     } else {
-        assert_eq!(compiled_contracts.len(), 1);
-        let compiled_contract = compiled_contracts.remove(0);
-
         if options.print_acir {
             for contract_function in &compiled_contract.functions {
                 if let Some(ref name) = options.show_contract_fn {
@@ -480,6 +484,52 @@ pub fn compile_contract(
     }
 }
 
+/// Return a Vec of all `contract` declarations in the source code and the functions they contain
+fn read_contract(context: &Context, module_id: ModuleId, name: String) -> Contract {
+    let module = context.module(module_id);
+
+    let functions = module
+        .value_definitions()
+        .filter_map(|id| {
+            id.as_function().map(|function_id| {
+                let attrs = context.def_interner.function_attributes(&function_id);
+                let is_entry_point = attrs.is_contract_entry_point();
+                ContractFunctionMeta { function_id, is_entry_point }
+            })
+        })
+        .collect();
+
+    let mut outputs = ContractOutputs { structs: HashMap::new(), globals: HashMap::new() };
+
+    context.def_interner.get_all_globals().iter().for_each(|global_info| {
+        context.def_interner.global_attributes(&global_info.id).iter().for_each(|attr| {
+            if let SecondaryAttribute::Abi(tag) = attr {
+                if let Some(tagged) = outputs.globals.get_mut(tag) {
+                    tagged.push(global_info.id);
+                } else {
+                    outputs.globals.insert(tag.to_string(), vec![global_info.id]);
+                }
+            }
+        });
+    });
+
+    module.type_definitions().for_each(|id| {
+        if let ModuleDefId::TypeId(struct_id) = id {
+            context.def_interner.type_attributes(&struct_id).iter().for_each(|attr| {
+                if let SecondaryAttribute::Abi(tag) = attr {
+                    if let Some(tagged) = outputs.structs.get_mut(tag) {
+                        tagged.push(struct_id);
+                    } else {
+                        outputs.structs.insert(tag.to_string(), vec![struct_id]);
+                    }
+                }
+            });
+        }
+    });
+
+    Contract { name, functions, outputs }
+}
+
 /// True if there are (non-warning) errors present and we should halt compilation
 fn has_errors(errors: &[CustomDiagnostic], deny_warnings: bool) -> bool {
     if deny_warnings { !errors.is_empty() } else { errors.iter().any(|error| error.is_error()) }
@@ -721,3 +771,29 @@ pub fn compile_no_check(
         brillig_names,
     })
 }
+
+/// Specifies a contract function and extra metadata that
+/// one can use when processing a contract function.
+///
+/// One of these is whether the contract function is an entry point.
+/// The caller should only type-check these functions and not attempt
+/// to create a circuit for them.
+struct ContractFunctionMeta {
+    function_id: FuncId,
+    /// Indicates whether the function is an entry point
+    is_entry_point: bool,
+}
+
+struct ContractOutputs {
+    structs: HashMap<String, Vec<TypeId>>,
+    globals: HashMap<String, Vec<GlobalId>>,
+}
+
+/// A 'contract' in Noir source code with a given name, functions and events.
+/// This is not an AST node, it is just a convenient form to return for CrateDefMap::get_all_contracts.
+struct Contract {
+    /// To keep `name` semi-unique, it is prefixed with the names of parent modules via CrateDefMap::get_module_path
+    name: String,
+    functions: Vec<ContractFunctionMeta>,
+    outputs: ContractOutputs,
+}

compiler/noirc_frontend/Cargo.toml

@@ -39,6 +39,5 @@ proptest.workspace = true
 proptest-derive.workspace = true
 
 [features]
-experimental_parser = []
 bn254 = []
 bls12_381 = []

compiler/noirc_frontend/src/hir/def_map/mod.rs

@@ -2,10 +2,10 @@ use crate::elaborator::FrontendOptions;
 use crate::graph::{CrateGraph, CrateId};
 use crate::hir::Context;
 use crate::hir::def_collector::dc_crate::{CompilationError, DefCollector};
-use crate::node_interner::{FuncId, GlobalId, NodeInterner, TypeId};
+use crate::node_interner::{FuncId, NodeInterner};
 use crate::parse_program;
 use crate::parser::{ParsedModule, ParserError};
-use crate::token::{FunctionAttribute, SecondaryAttribute, TestScope};
+use crate::token::{FunctionAttribute, TestScope};
 use fm::{FileId, FileManager};
 use noirc_arena::{Arena, Index};
 use noirc_errors::Location;
@@ -234,61 +234,16 @@ impl CrateDefMap {
         })
     }
 
-    /// Go through all modules in this crate, find all `contract ... { ... }` declarations,
-    /// and collect them all into a Vec.
-    pub fn get_all_contracts(&self, interner: &NodeInterner) -> Vec<Contract> {
-        self.modules
-            .iter()
-            .filter_map(|(id, module)| {
-                if module.is_contract {
-                    let functions = module
-                        .value_definitions()
-                        .filter_map(|id| {
-                            id.as_function().map(|function_id| {
-                                let is_entry_point = interner
-                                    .function_attributes(&function_id)
-                                    .is_contract_entry_point();
-                                ContractFunctionMeta { function_id, is_entry_point }
-                            })
-                        })
-                        .collect();
-
-                    let mut outputs =
-                        ContractOutputs { structs: HashMap::new(), globals: HashMap::new() };
-
-                    interner.get_all_globals().iter().for_each(|global_info| {
-                        interner.global_attributes(&global_info.id).iter().for_each(|attr| {
-                            if let SecondaryAttribute::Abi(tag) = attr {
-                                if let Some(tagged) = outputs.globals.get_mut(tag) {
-                                    tagged.push(global_info.id);
-                                } else {
-                                    outputs.globals.insert(tag.to_string(), vec![global_info.id]);
-                                }
-                            }
-                        });
-                    });
-
-                    module.type_definitions().for_each(|id| {
-                        if let ModuleDefId::TypeId(struct_id) = id {
-                            interner.type_attributes(&struct_id).iter().for_each(|attr| {
-                                if let SecondaryAttribute::Abi(tag) = attr {
-                                    if let Some(tagged) = outputs.structs.get_mut(tag) {
-                                        tagged.push(struct_id);
-                                    } else {
-                                        outputs.structs.insert(tag.to_string(), vec![struct_id]);
-                                    }
-                                }
-                            });
-                        }
-                    });
-
-                    let name = self.get_module_path(LocalModuleId::new(id), module.parent);
-                    Some(Contract { name, location: module.location, functions, outputs })
-                } else {
-                    None
-                }
-            })
-            .collect()
+    /// Returns an iterator over all contract modules within the crate.
+    pub fn get_all_contracts(&self) -> impl Iterator<Item = (LocalModuleId, String)> {
+        self.modules.iter().filter_map(|(id, module)| {
+            if module.is_contract {
+                let name = self.get_module_path(LocalModuleId::new(id), module.parent);
+                Some((LocalModuleId(id), name))
+            } else {
+                None
+            }
+        })
     }
 
     /// Find a child module's name by inspecting its parent.
@@ -386,33 +341,6 @@ pub fn fully_qualified_module_path(
     }
 }
 
-/// Specifies a contract function and extra metadata that
-/// one can use when processing a contract function.
-///
-/// One of these is whether the contract function is an entry point.
-/// The caller should only type-check these functions and not attempt
-/// to create a circuit for them.
-pub struct ContractFunctionMeta {
-    pub function_id: FuncId,
-    /// Indicates whether the function is an entry point
-    pub is_entry_point: bool,
-}
-
-pub struct ContractOutputs {
-    pub structs: HashMap<String, Vec<TypeId>>,
-    pub globals: HashMap<String, Vec<GlobalId>>,
-}
-
-/// A 'contract' in Noir source code with a given name, functions and events.
-/// This is not an AST node, it is just a convenient form to return for CrateDefMap::get_all_contracts.
-pub struct Contract {
-    /// To keep `name` semi-unique, it is prefixed with the names of parent modules via CrateDefMap::get_module_path
-    pub name: String,
-    pub location: Location,
-    pub functions: Vec<ContractFunctionMeta>,
-    pub outputs: ContractOutputs,
-}
-
 /// Given a FileId, fetch the File, from the FileManager and parse it's content
 pub fn parse_file(fm: &FileManager, file_id: FileId) -> (ParsedModule, Vec<ParserError>) {
     let file_source = fm.fetch_file(file_id).expect("File does not exist");

compiler/noirc_frontend/src/hir/mod.rs

@@ -14,7 +14,7 @@ use crate::parser::ParserError;
 use crate::usage_tracker::UsageTracker;
 use crate::{Generics, Kind, ParsedModule, ResolvedGeneric, TypeVariable};
 use def_collector::dc_crate::CompilationError;
-use def_map::{Contract, CrateDefMap, fully_qualified_module_path};
+use def_map::{CrateDefMap, fully_qualified_module_path};
 use fm::{FileId, FileManager};
 use iter_extended::vecmap;
 use noirc_errors::Location;
@@ -209,13 +209,6 @@ impl Context<'_, '_> {
             .collect()
     }
 
-    /// Return a Vec of all `contract` declarations in the source code and the functions they contain
-    pub fn get_all_contracts(&self, crate_id: &CrateId) -> Vec<Contract> {
-        self.def_map(crate_id)
-            .expect("The local crate should be analyzed already")
-            .get_all_contracts(&self.def_interner)
-    }
-
     pub fn module(&self, module_id: def_map::ModuleId) -> &def_map::ModuleData {
         module_id.module(&self.def_maps)
     }

tooling/lsp/src/requests/code_lens_request.rs

@@ -4,7 +4,7 @@ use async_lsp::{ErrorCode, ResponseError};
 
 use nargo::{package::Package, workspace::Workspace};
 use noirc_driver::check_crate;
-use noirc_frontend::hir::FunctionNameMatch;
+use noirc_frontend::hir::{FunctionNameMatch, def_map::ModuleId};
 
 use crate::{
     LspState, byte_span_to_range, prepare_source, resolve_workspace_for_source_path,
@@ -175,7 +175,13 @@ pub(crate) fn collect_lenses_for_package(
 
     if package.is_contract() {
         // Currently not looking to deduplicate this since we don't have a clear decision on if the Contract stuff is staying
-        for contract in context.get_all_contracts(&crate_id) {
+        let def_map =
+            context.def_map(&crate_id).expect("The local crate should be analyzed already");
+
+        for contract in def_map
+            .get_all_contracts()
+            .map(|(local_id, _)| context.module(ModuleId { krate: crate_id, local_id }))
+        {
             let location = contract.location;
             let file_id = location.file;