Skip to content

Commit cae4216

Browse files
authored
chore: pull most logic from get_all_contracts up out of the CrateDefMap (#7715)
1 parent d587fcc commit cae4216

File tree

5 files changed

+117
-115
lines changed

5 files changed

+117
-115
lines changed

compiler/noirc_driver/src/lib.rs

+96-20
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ use noirc_evaluator::ssa::{SsaLogging, SsaProgramArtifact};
1818
use noirc_frontend::debug::build_debug_crate_file;
1919
use noirc_frontend::elaborator::{FrontendOptions, UnstableFeature};
2020
use noirc_frontend::hir::Context;
21-
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
21+
use noirc_frontend::hir::def_map::{CrateDefMap, ModuleDefId, ModuleId};
2222
use noirc_frontend::monomorphization::{
2323
errors::MonomorphizationError, monomorphize, monomorphize_debug,
2424
};
25-
use noirc_frontend::node_interner::FuncId;
25+
use noirc_frontend::node_interner::{FuncId, GlobalId, TypeId};
2626
use noirc_frontend::token::SecondaryAttribute;
27+
use std::collections::HashMap;
2728
use std::path::Path;
2829
use tracing::info;
2930

@@ -428,39 +429,42 @@ pub fn compile_contract(
428429
) -> CompilationResult<CompiledContract> {
429430
let (_, warnings) = check_crate(context, crate_id, options)?;
430431

431-
// TODO: We probably want to error if contracts is empty
432-
let contracts = context.get_all_contracts(&crate_id);
432+
let def_map = context.def_map(&crate_id).expect("The local crate should be analyzed already");
433+
let mut contracts = def_map.get_all_contracts();
433434

434-
let mut compiled_contracts = vec![];
435-
let mut errors = warnings;
436-
437-
if contracts.len() > 1 {
435+
let Some((module_id, name)) = contracts.next() else {
438436
let err = CustomDiagnostic::from_message(
439-
"Packages are limited to a single contract",
437+
"cannot compile crate into a contract as it does not contain any contracts",
440438
FileId::default(),
441439
);
442440
return Err(vec![err]);
443-
} else if contracts.is_empty() {
441+
};
442+
443+
if contracts.next().is_some() {
444444
let err = CustomDiagnostic::from_message(
445-
"cannot compile crate into a contract as it does not contain any contracts",
445+
"Packages are limited to a single contract",
446446
FileId::default(),
447447
);
448448
return Err(vec![err]);
449-
};
449+
}
450+
drop(contracts);
450451

451-
for contract in contracts {
452-
match compile_contract_inner(context, contract, options) {
453-
Ok(contract) => compiled_contracts.push(contract),
454-
Err(mut more_errors) => errors.append(&mut more_errors),
452+
let module_id = ModuleId { krate: crate_id, local_id: module_id };
453+
let contract = read_contract(context, module_id, name);
454+
455+
let mut errors = warnings;
456+
457+
let compiled_contract = match compile_contract_inner(context, contract, options) {
458+
Ok(contract) => contract,
459+
Err(mut more_errors) => {
460+
errors.append(&mut more_errors);
461+
return Err(errors);
455462
}
456-
}
463+
};
457464

458465
if has_errors(&errors, options.deny_warnings) {
459466
Err(errors)
460467
} else {
461-
assert_eq!(compiled_contracts.len(), 1);
462-
let compiled_contract = compiled_contracts.remove(0);
463-
464468
if options.print_acir {
465469
for contract_function in &compiled_contract.functions {
466470
if let Some(ref name) = options.show_contract_fn {
@@ -480,6 +484,52 @@ pub fn compile_contract(
480484
}
481485
}
482486

487+
/// Return a Vec of all `contract` declarations in the source code and the functions they contain
488+
fn read_contract(context: &Context, module_id: ModuleId, name: String) -> Contract {
489+
let module = context.module(module_id);
490+
491+
let functions = module
492+
.value_definitions()
493+
.filter_map(|id| {
494+
id.as_function().map(|function_id| {
495+
let attrs = context.def_interner.function_attributes(&function_id);
496+
let is_entry_point = attrs.is_contract_entry_point();
497+
ContractFunctionMeta { function_id, is_entry_point }
498+
})
499+
})
500+
.collect();
501+
502+
let mut outputs = ContractOutputs { structs: HashMap::new(), globals: HashMap::new() };
503+
504+
context.def_interner.get_all_globals().iter().for_each(|global_info| {
505+
context.def_interner.global_attributes(&global_info.id).iter().for_each(|attr| {
506+
if let SecondaryAttribute::Abi(tag) = attr {
507+
if let Some(tagged) = outputs.globals.get_mut(tag) {
508+
tagged.push(global_info.id);
509+
} else {
510+
outputs.globals.insert(tag.to_string(), vec![global_info.id]);
511+
}
512+
}
513+
});
514+
});
515+
516+
module.type_definitions().for_each(|id| {
517+
if let ModuleDefId::TypeId(struct_id) = id {
518+
context.def_interner.type_attributes(&struct_id).iter().for_each(|attr| {
519+
if let SecondaryAttribute::Abi(tag) = attr {
520+
if let Some(tagged) = outputs.structs.get_mut(tag) {
521+
tagged.push(struct_id);
522+
} else {
523+
outputs.structs.insert(tag.to_string(), vec![struct_id]);
524+
}
525+
}
526+
});
527+
}
528+
});
529+
530+
Contract { name, functions, outputs }
531+
}
532+
483533
/// True if there are (non-warning) errors present and we should halt compilation
484534
fn has_errors(errors: &[CustomDiagnostic], deny_warnings: bool) -> bool {
485535
if deny_warnings { !errors.is_empty() } else { errors.iter().any(|error| error.is_error()) }
@@ -721,3 +771,29 @@ pub fn compile_no_check(
721771
brillig_names,
722772
})
723773
}
774+
775+
/// Specifies a contract function and extra metadata that
776+
/// one can use when processing a contract function.
777+
///
778+
/// One of these is whether the contract function is an entry point.
779+
/// The caller should only type-check these functions and not attempt
780+
/// to create a circuit for them.
781+
struct ContractFunctionMeta {
782+
function_id: FuncId,
783+
/// Indicates whether the function is an entry point
784+
is_entry_point: bool,
785+
}
786+
787+
struct ContractOutputs {
788+
structs: HashMap<String, Vec<TypeId>>,
789+
globals: HashMap<String, Vec<GlobalId>>,
790+
}
791+
792+
/// A 'contract' in Noir source code with a given name, functions and events.
793+
/// This is not an AST node, it is just a convenient form to return for CrateDefMap::get_all_contracts.
794+
struct Contract {
795+
/// To keep `name` semi-unique, it is prefixed with the names of parent modules via CrateDefMap::get_module_path
796+
name: String,
797+
functions: Vec<ContractFunctionMeta>,
798+
outputs: ContractOutputs,
799+
}

compiler/noirc_frontend/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,5 @@ proptest.workspace = true
3939
proptest-derive.workspace = true
4040

4141
[features]
42-
experimental_parser = []
4342
bn254 = []
4443
bls12_381 = []

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

+12-84
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ use crate::elaborator::FrontendOptions;
22
use crate::graph::{CrateGraph, CrateId};
33
use crate::hir::Context;
44
use crate::hir::def_collector::dc_crate::{CompilationError, DefCollector};
5-
use crate::node_interner::{FuncId, GlobalId, NodeInterner, TypeId};
5+
use crate::node_interner::{FuncId, NodeInterner};
66
use crate::parse_program;
77
use crate::parser::{ParsedModule, ParserError};
8-
use crate::token::{FunctionAttribute, SecondaryAttribute, TestScope};
8+
use crate::token::{FunctionAttribute, TestScope};
99
use fm::{FileId, FileManager};
1010
use noirc_arena::{Arena, Index};
1111
use noirc_errors::Location;
@@ -234,61 +234,16 @@ impl CrateDefMap {
234234
})
235235
}
236236

237-
/// Go through all modules in this crate, find all `contract ... { ... }` declarations,
238-
/// and collect them all into a Vec.
239-
pub fn get_all_contracts(&self, interner: &NodeInterner) -> Vec<Contract> {
240-
self.modules
241-
.iter()
242-
.filter_map(|(id, module)| {
243-
if module.is_contract {
244-
let functions = module
245-
.value_definitions()
246-
.filter_map(|id| {
247-
id.as_function().map(|function_id| {
248-
let is_entry_point = interner
249-
.function_attributes(&function_id)
250-
.is_contract_entry_point();
251-
ContractFunctionMeta { function_id, is_entry_point }
252-
})
253-
})
254-
.collect();
255-
256-
let mut outputs =
257-
ContractOutputs { structs: HashMap::new(), globals: HashMap::new() };
258-
259-
interner.get_all_globals().iter().for_each(|global_info| {
260-
interner.global_attributes(&global_info.id).iter().for_each(|attr| {
261-
if let SecondaryAttribute::Abi(tag) = attr {
262-
if let Some(tagged) = outputs.globals.get_mut(tag) {
263-
tagged.push(global_info.id);
264-
} else {
265-
outputs.globals.insert(tag.to_string(), vec![global_info.id]);
266-
}
267-
}
268-
});
269-
});
270-
271-
module.type_definitions().for_each(|id| {
272-
if let ModuleDefId::TypeId(struct_id) = id {
273-
interner.type_attributes(&struct_id).iter().for_each(|attr| {
274-
if let SecondaryAttribute::Abi(tag) = attr {
275-
if let Some(tagged) = outputs.structs.get_mut(tag) {
276-
tagged.push(struct_id);
277-
} else {
278-
outputs.structs.insert(tag.to_string(), vec![struct_id]);
279-
}
280-
}
281-
});
282-
}
283-
});
284-
285-
let name = self.get_module_path(LocalModuleId::new(id), module.parent);
286-
Some(Contract { name, location: module.location, functions, outputs })
287-
} else {
288-
None
289-
}
290-
})
291-
.collect()
237+
/// Returns an iterator over all contract modules within the crate.
238+
pub fn get_all_contracts(&self) -> impl Iterator<Item = (LocalModuleId, String)> {
239+
self.modules.iter().filter_map(|(id, module)| {
240+
if module.is_contract {
241+
let name = self.get_module_path(LocalModuleId::new(id), module.parent);
242+
Some((LocalModuleId(id), name))
243+
} else {
244+
None
245+
}
246+
})
292247
}
293248

294249
/// Find a child module's name by inspecting its parent.
@@ -386,33 +341,6 @@ pub fn fully_qualified_module_path(
386341
}
387342
}
388343

389-
/// Specifies a contract function and extra metadata that
390-
/// one can use when processing a contract function.
391-
///
392-
/// One of these is whether the contract function is an entry point.
393-
/// The caller should only type-check these functions and not attempt
394-
/// to create a circuit for them.
395-
pub struct ContractFunctionMeta {
396-
pub function_id: FuncId,
397-
/// Indicates whether the function is an entry point
398-
pub is_entry_point: bool,
399-
}
400-
401-
pub struct ContractOutputs {
402-
pub structs: HashMap<String, Vec<TypeId>>,
403-
pub globals: HashMap<String, Vec<GlobalId>>,
404-
}
405-
406-
/// A 'contract' in Noir source code with a given name, functions and events.
407-
/// This is not an AST node, it is just a convenient form to return for CrateDefMap::get_all_contracts.
408-
pub struct Contract {
409-
/// To keep `name` semi-unique, it is prefixed with the names of parent modules via CrateDefMap::get_module_path
410-
pub name: String,
411-
pub location: Location,
412-
pub functions: Vec<ContractFunctionMeta>,
413-
pub outputs: ContractOutputs,
414-
}
415-
416344
/// Given a FileId, fetch the File, from the FileManager and parse it's content
417345
pub fn parse_file(fm: &FileManager, file_id: FileId) -> (ParsedModule, Vec<ParserError>) {
418346
let file_source = fm.fetch_file(file_id).expect("File does not exist");

compiler/noirc_frontend/src/hir/mod.rs

+1-8
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::parser::ParserError;
1414
use crate::usage_tracker::UsageTracker;
1515
use crate::{Generics, Kind, ParsedModule, ResolvedGeneric, TypeVariable};
1616
use def_collector::dc_crate::CompilationError;
17-
use def_map::{Contract, CrateDefMap, fully_qualified_module_path};
17+
use def_map::{CrateDefMap, fully_qualified_module_path};
1818
use fm::{FileId, FileManager};
1919
use iter_extended::vecmap;
2020
use noirc_errors::Location;
@@ -209,13 +209,6 @@ impl Context<'_, '_> {
209209
.collect()
210210
}
211211

212-
/// Return a Vec of all `contract` declarations in the source code and the functions they contain
213-
pub fn get_all_contracts(&self, crate_id: &CrateId) -> Vec<Contract> {
214-
self.def_map(crate_id)
215-
.expect("The local crate should be analyzed already")
216-
.get_all_contracts(&self.def_interner)
217-
}
218-
219212
pub fn module(&self, module_id: def_map::ModuleId) -> &def_map::ModuleData {
220213
module_id.module(&self.def_maps)
221214
}

tooling/lsp/src/requests/code_lens_request.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use async_lsp::{ErrorCode, ResponseError};
44

55
use nargo::{package::Package, workspace::Workspace};
66
use noirc_driver::check_crate;
7-
use noirc_frontend::hir::FunctionNameMatch;
7+
use noirc_frontend::hir::{FunctionNameMatch, def_map::ModuleId};
88

99
use crate::{
1010
LspState, byte_span_to_range, prepare_source, resolve_workspace_for_source_path,
@@ -175,7 +175,13 @@ pub(crate) fn collect_lenses_for_package(
175175

176176
if package.is_contract() {
177177
// Currently not looking to deduplicate this since we don't have a clear decision on if the Contract stuff is staying
178-
for contract in context.get_all_contracts(&crate_id) {
178+
let def_map =
179+
context.def_map(&crate_id).expect("The local crate should be analyzed already");
180+
181+
for contract in def_map
182+
.get_all_contracts()
183+
.map(|(local_id, _)| context.module(ModuleId { krate: crate_id, local_id }))
184+
{
179185
let location = contract.location;
180186
let file_id = location.file;
181187

0 commit comments

Comments
 (0)