diff --git a/crates/nargo_cli/tests/execution_success/type_aliases/src/main.nr b/crates/nargo_cli/tests/execution_success/type_aliases/src/main.nr index 7197c23f664..573a501367f 100644 --- a/crates/nargo_cli/tests/execution_success/type_aliases/src/main.nr +++ b/crates/nargo_cli/tests/execution_success/type_aliases/src/main.nr @@ -2,9 +2,9 @@ type Foo = [T; 2]; type Bar = Field; -type Three = Two; -type Two = One; type One = (A, B); +type Two = One; +type Three = Two; struct MyStruct { foo: Bar, diff --git a/crates/noirc_frontend/src/ast/mod.rs b/crates/noirc_frontend/src/ast/mod.rs index e92d333fd69..aa79929fdd7 100644 --- a/crates/noirc_frontend/src/ast/mod.rs +++ b/crates/noirc_frontend/src/ast/mod.rs @@ -124,17 +124,17 @@ impl std::fmt::Display for UnresolvedTypeData { }, FormatString(len, elements) => write!(f, "fmt<{len}, {elements}"), Function(args, ret, env) => { - let args = vecmap(args, ToString::to_string); + let args = vecmap(args, ToString::to_string).join(", "); match &env.as_ref().typ { UnresolvedTypeData::Unit => { - write!(f, "fn({}) -> {ret}", args.join(", ")) + write!(f, "fn({args}) -> {ret}") } UnresolvedTypeData::Tuple(env_types) => { - let env_types = vecmap(env_types, |arg| ToString::to_string(&arg.typ)); - write!(f, "fn[{}]({}) -> {ret}", env_types.join(", "), args.join(", ")) + let env_types = vecmap(env_types, |arg| arg.typ.to_string()).join(", "); + write!(f, "fn[{env_types}]({args}) -> {ret}") } - _ => unreachable!(), + other => write!(f, "fn[{other}]({args}) -> {ret}"), } } MutableReference(element) => write!(f, "&mut {element}"), diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs index af79219d5de..d8c539038df 100644 --- a/crates/noirc_frontend/src/graph/mod.rs +++ b/crates/noirc_frontend/src/graph/mod.rs @@ -10,7 +10,7 @@ use fm::FileId; use rustc_hash::{FxHashMap, FxHashSet}; use smol_str::SmolStr; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum CrateId { Root(usize), Crate(usize), diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index 389e6038775..57e5fb8197c 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -21,7 +21,7 @@ use fm::FileId; use iter_extended::vecmap; use noirc_errors::Span; use noirc_errors::{CustomDiagnostic, FileDiagnostic}; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::rc::Rc; use std::vec; @@ -69,9 +69,9 @@ pub struct DefCollector { pub(crate) def_map: CrateDefMap, pub(crate) collected_imports: Vec, pub(crate) collected_functions: Vec, - pub(crate) collected_types: HashMap, - pub(crate) collected_type_aliases: HashMap, - pub(crate) collected_traits: HashMap, + pub(crate) collected_types: BTreeMap, + pub(crate) collected_type_aliases: BTreeMap, + pub(crate) collected_traits: BTreeMap, pub(crate) collected_globals: Vec, pub(crate) collected_impls: ImplMap, pub(crate) collected_traits_impls: ImplMap, @@ -80,6 +80,10 @@ pub struct DefCollector { /// Maps the type and the module id in which the impl is defined to the functions contained in that /// impl along with the generics declared on the impl itself. This also contains the Span /// of the object_type of the impl, used to issue an error if the object type fails to resolve. +/// +/// Note that because these are keyed by unresolved types, the impl map is one of the few instances +/// of HashMap rather than BTreeMap. For this reason, we should be careful not to iterate over it +/// since it would be non-deterministic. type ImplMap = HashMap<(UnresolvedType, LocalModuleId), Vec<(UnresolvedGenerics, Span, UnresolvedFunctions)>>; @@ -89,9 +93,9 @@ impl DefCollector { def_map, collected_imports: vec![], collected_functions: vec![], - collected_types: HashMap::new(), - collected_type_aliases: HashMap::new(), - collected_traits: HashMap::new(), + collected_types: BTreeMap::new(), + collected_type_aliases: BTreeMap::new(), + collected_traits: BTreeMap::new(), collected_impls: HashMap::new(), collected_traits_impls: HashMap::new(), collected_globals: vec![], @@ -375,7 +379,7 @@ fn type_check_globals( /// so that expressions can access the fields of structs fn resolve_structs( context: &mut Context, - structs: HashMap, + structs: BTreeMap, crate_id: CrateId, errors: &mut Vec, ) { @@ -481,7 +485,7 @@ fn take_errors_filter_self_not_resolved(resolver: Resolver<'_>) -> Vec, + traits: BTreeMap, crate_id: CrateId, errors: &mut Vec, ) { @@ -524,7 +528,7 @@ fn resolve_struct_fields( fn resolve_type_aliases( context: &mut Context, - type_aliases: HashMap, + type_aliases: BTreeMap, crate_id: CrateId, all_errors: &mut Vec, ) { @@ -546,7 +550,7 @@ fn resolve_type_aliases( fn resolve_impls( interner: &mut NodeInterner, crate_id: CrateId, - def_maps: &HashMap, + def_maps: &BTreeMap, collected_impls: ImplMap, errors: &mut Vec, ) -> Vec<(FileId, FuncId)> { @@ -600,7 +604,7 @@ fn resolve_impls( fn resolve_free_functions( interner: &mut NodeInterner, crate_id: CrateId, - def_maps: &HashMap, + def_maps: &BTreeMap, collected_functions: Vec, self_type: Option, errors: &mut Vec, @@ -625,7 +629,7 @@ fn resolve_free_functions( fn resolve_function_set( interner: &mut NodeInterner, crate_id: CrateId, - def_maps: &HashMap, + def_maps: &BTreeMap, unresolved_functions: UnresolvedFunctions, self_type: Option, impl_generics: Vec<(Rc, Shared, Span)>, diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index c7cc02b8dac..f37ebeee06e 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -7,7 +7,7 @@ use crate::token::{Attribute, TestScope}; use arena::{Arena, Index}; use fm::{FileId, FileManager}; use noirc_errors::{FileDiagnostic, Location}; -use std::collections::HashMap; +use std::collections::BTreeMap; mod module_def; pub use module_def::*; @@ -27,7 +27,7 @@ pub const MAIN_FUNCTION: &str = "main"; // XXX: Ultimately, we want to constrain an index to be of a certain type just like in RA /// Lets first check if this is offered by any external crate /// XXX: RA has made this a crate on crates.io -#[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] +#[derive(Debug, PartialEq, Eq, Copy, Clone, Hash, PartialOrd, Ord)] pub struct LocalModuleId(pub Index); impl LocalModuleId { @@ -36,7 +36,7 @@ impl LocalModuleId { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct ModuleId { pub krate: CrateId, pub local_id: LocalModuleId, @@ -49,7 +49,7 @@ impl ModuleId { } impl ModuleId { - pub fn module(self, def_maps: &HashMap) -> &ModuleData { + pub fn module(self, def_maps: &BTreeMap) -> &ModuleData { &def_maps[&self.krate].modules()[self.local_id.0] } } @@ -65,7 +65,7 @@ pub struct CrateDefMap { pub(crate) krate: CrateId, - pub(crate) extern_prelude: HashMap, + pub(crate) extern_prelude: BTreeMap, } impl CrateDefMap { @@ -100,7 +100,7 @@ impl CrateDefMap { root: LocalModuleId(root), modules, krate: crate_id, - extern_prelude: HashMap::new(), + extern_prelude: BTreeMap::new(), }; // Now we want to populate the CrateDefMap using the DefCollector diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index 5868872fa1b..1bdd3a62b72 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -9,7 +9,7 @@ use crate::hir_def::function::FuncMeta; use crate::node_interner::{FuncId, NodeInterner, StructId}; use def_map::{Contract, CrateDefMap}; use fm::FileManager; -use std::collections::HashMap; +use std::collections::BTreeMap; use self::def_map::TestFunction; @@ -19,12 +19,12 @@ use self::def_map::TestFunction; pub struct Context { pub def_interner: NodeInterner, pub crate_graph: CrateGraph, - pub(crate) def_maps: HashMap, + pub(crate) def_maps: BTreeMap, pub file_manager: FileManager, /// Maps a given (contract) module id to the next available storage slot /// for that contract. - pub storage_slots: HashMap, + pub storage_slots: BTreeMap, } #[derive(Debug, Copy, Clone)] @@ -40,10 +40,10 @@ impl Context { pub fn new(file_manager: FileManager, crate_graph: CrateGraph) -> Context { Context { def_interner: NodeInterner::default(), - def_maps: HashMap::new(), + def_maps: BTreeMap::new(), crate_graph, file_manager, - storage_slots: HashMap::new(), + storage_slots: BTreeMap::new(), } } diff --git a/crates/noirc_frontend/src/hir/resolution/import.rs b/crates/noirc_frontend/src/hir/resolution/import.rs index 8949c766881..6f3140a65d4 100644 --- a/crates/noirc_frontend/src/hir/resolution/import.rs +++ b/crates/noirc_frontend/src/hir/resolution/import.rs @@ -2,7 +2,7 @@ use iter_extended::partition_results; use noirc_errors::CustomDiagnostic; use crate::graph::CrateId; -use std::collections::HashMap; +use std::collections::BTreeMap; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId, PerNs}; use crate::{Ident, Path, PathKind}; @@ -52,7 +52,7 @@ impl From for CustomDiagnostic { pub fn resolve_imports( crate_id: CrateId, imports_to_resolve: Vec, - def_maps: &HashMap, + def_maps: &BTreeMap, ) -> (Vec, Vec<(PathResolutionError, LocalModuleId)>) { let def_map = &def_maps[&crate_id]; @@ -71,7 +71,7 @@ pub fn resolve_imports( } pub(super) fn allow_referencing_contracts( - def_maps: &HashMap, + def_maps: &BTreeMap, krate: CrateId, local_id: LocalModuleId, ) -> bool { @@ -81,7 +81,7 @@ pub(super) fn allow_referencing_contracts( pub fn resolve_path_to_ns( import_directive: &ImportDirective, def_map: &CrateDefMap, - def_maps: &HashMap, + def_maps: &BTreeMap, allow_contracts: bool, ) -> PathResolution { let import_path = &import_directive.path.segments; @@ -111,7 +111,7 @@ pub fn resolve_path_to_ns( fn resolve_path_from_crate_root( def_map: &CrateDefMap, import_path: &[Ident], - def_maps: &HashMap, + def_maps: &BTreeMap, allow_contracts: bool, ) -> PathResolution { resolve_name_in_module(def_map, import_path, def_map.root, def_maps, allow_contracts) @@ -121,7 +121,7 @@ fn resolve_name_in_module( def_map: &CrateDefMap, import_path: &[Ident], starting_mod: LocalModuleId, - def_maps: &HashMap, + def_maps: &BTreeMap, allow_contracts: bool, ) -> PathResolution { let mut current_mod = &def_map.modules[starting_mod.0]; @@ -185,7 +185,7 @@ fn resolve_path_name(import_directive: &ImportDirective) -> Ident { fn resolve_external_dep( current_def_map: &CrateDefMap, directive: &ImportDirective, - def_maps: &HashMap, + def_maps: &BTreeMap, allow_contracts: bool, ) -> PathResolution { // Use extern_prelude to get the dep diff --git a/crates/noirc_frontend/src/hir/resolution/path_resolver.rs b/crates/noirc_frontend/src/hir/resolution/path_resolver.rs index eb0483fbf54..4c16edd56f1 100644 --- a/crates/noirc_frontend/src/hir/resolution/path_resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/path_resolver.rs @@ -2,7 +2,7 @@ use super::import::{ allow_referencing_contracts, resolve_path_to_ns, ImportDirective, PathResolutionError, }; use crate::Path; -use std::collections::HashMap; +use std::collections::BTreeMap; use crate::graph::CrateId; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleDefId, ModuleId}; @@ -11,7 +11,7 @@ pub trait PathResolver { /// Resolve the given path returning the resolved ModuleDefId. fn resolve( &self, - def_maps: &HashMap, + def_maps: &BTreeMap, path: Path, ) -> Result; @@ -34,7 +34,7 @@ impl StandardPathResolver { impl PathResolver for StandardPathResolver { fn resolve( &self, - def_maps: &HashMap, + def_maps: &BTreeMap, path: Path, ) -> Result { resolve_path(def_maps, self.module_id, path) @@ -52,7 +52,7 @@ impl PathResolver for StandardPathResolver { /// Resolve the given path to a function or a type. /// In the case of a conflict, functions are given priority pub fn resolve_path( - def_maps: &HashMap, + def_maps: &BTreeMap, module_id: ModuleId, path: Path, ) -> Result { diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index cf0f8f80751..1ab4118099d 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -19,7 +19,7 @@ use crate::hir_def::expr::{ }; use crate::token::Attribute; use regex::Regex; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashSet}; use std::rc::Rc; use crate::graph::CrateId; @@ -75,7 +75,7 @@ pub struct LambdaContext { pub struct Resolver<'a> { scopes: ScopeForest, path_resolver: &'a dyn PathResolver, - def_maps: &'a HashMap, + def_maps: &'a BTreeMap, interner: &'a mut NodeInterner, errors: Vec, file: FileId, @@ -107,7 +107,7 @@ impl<'a> Resolver<'a> { pub fn new( interner: &'a mut NodeInterner, path_resolver: &'a dyn PathResolver, - def_maps: &'a HashMap, + def_maps: &'a BTreeMap, file: FileId, ) -> Resolver<'a> { Self { @@ -438,7 +438,16 @@ impl<'a> Resolver<'a> { type_alias_string }); - return self.interner.get_type_alias(id).get_type(&args); + let result = self.interner.get_type_alias(id).get_type(&args); + + // Because there is no ordering to when type aliases (and other globals) are resolved, + // it is possible for one to refer to an Error type and issue no error if it is set + // equal to another type alias. Fixing this fully requires an analysis to create a DFG + // of definition ordering, but for now we have an explicit check here so that we at + // least issue an error that the type was not found instead of silently passing. + if result != Type::Error { + return result; + } } match self.lookup_struct_or_error(path) { @@ -828,7 +837,7 @@ impl<'a> Resolver<'a> { parameters: &[Type], return_type: &Type, ) -> Vec<(String, TypeVariable)> { - let mut found = HashMap::new(); + let mut found = BTreeMap::new(); for parameter in parameters { Self::find_numeric_generics_in_type(parameter, &mut found); } @@ -836,7 +845,10 @@ impl<'a> Resolver<'a> { found.into_iter().collect() } - fn find_numeric_generics_in_type(typ: &Type, found: &mut HashMap>) { + fn find_numeric_generics_in_type( + typ: &Type, + found: &mut BTreeMap>, + ) { match typ { Type::FieldElement | Type::Integer(_, _) @@ -1508,7 +1520,7 @@ pub fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result< mod test { use core::panic; - use std::collections::HashMap; + use std::collections::BTreeMap; use fm::FileId; use iter_extended::vecmap; @@ -1536,7 +1548,8 @@ mod test { // and functions can be forward declared fn init_src_code_resolution( src: &str, - ) -> (ParsedModule, NodeInterner, HashMap, FileId, TestPathResolver) { + ) -> (ParsedModule, NodeInterner, BTreeMap, FileId, TestPathResolver) + { let (program, errors) = parse_program(src); if errors.iter().any(|e| e.is_error()) { panic!("Unexpected parse errors in test code: {:?}", errors); @@ -1544,14 +1557,14 @@ mod test { let interner: NodeInterner = NodeInterner::default(); - let mut def_maps: HashMap = HashMap::new(); + let mut def_maps: BTreeMap = BTreeMap::new(); let file = FileId::default(); let mut modules = arena::Arena::new(); let location = Location::new(Default::default(), file); modules.insert(ModuleData::new(None, location, false)); - let path_resolver = TestPathResolver(HashMap::new()); + let path_resolver = TestPathResolver(BTreeMap::new()); def_maps.insert( CrateId::dummy_id(), @@ -1559,7 +1572,7 @@ mod test { root: path_resolver.local_module_id(), modules, krate: CrateId::dummy_id(), - extern_prelude: HashMap::new(), + extern_prelude: BTreeMap::new(), }, ); @@ -1992,12 +2005,12 @@ mod test { } } - struct TestPathResolver(HashMap); + struct TestPathResolver(BTreeMap); impl PathResolver for TestPathResolver { fn resolve( &self, - _def_maps: &HashMap, + _def_maps: &BTreeMap, path: Path, ) -> Result { // Not here that foo::bar and hello::foo::bar would fetch the same thing diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs index 8596a9cc28c..d17fbdc17de 100644 --- a/crates/noirc_frontend/src/hir/type_check/mod.rs +++ b/crates/noirc_frontend/src/hir/type_check/mod.rs @@ -167,7 +167,7 @@ impl<'interner> TypeChecker<'interner> { /// We can either build a test apparatus or pass raw code through the resolver #[cfg(test)] mod test { - use std::collections::HashMap; + use std::collections::{BTreeMap, HashMap}; use std::vec; use fm::FileId; @@ -365,7 +365,7 @@ mod test { impl PathResolver for TestPathResolver { fn resolve( &self, - _def_maps: &HashMap, + _def_maps: &BTreeMap, path: Path, ) -> Result { // Not here that foo::bar and hello::foo::bar would fetch the same thing @@ -415,7 +415,7 @@ mod test { path_resolver.insert_func(name.to_owned(), id); } - let mut def_maps: HashMap = HashMap::new(); + let mut def_maps = BTreeMap::new(); let file = FileId::default(); let mut modules = arena::Arena::new(); @@ -428,7 +428,7 @@ mod test { root: path_resolver.local_module_id(), modules, krate: CrateId::dummy_id(), - extern_prelude: HashMap::new(), + extern_prelude: BTreeMap::new(), }, ); diff --git a/crates/noirc_frontend/src/monomorphization/mod.rs b/crates/noirc_frontend/src/monomorphization/mod.rs index 7a25c5e6b77..58dc619952b 100644 --- a/crates/noirc_frontend/src/monomorphization/mod.rs +++ b/crates/noirc_frontend/src/monomorphization/mod.rs @@ -1327,7 +1327,7 @@ fn undo_instantiation_bindings(bindings: TypeBindings) { #[cfg(test)] mod tests { - use std::collections::HashMap; + use std::collections::{BTreeMap, HashMap}; use fm::FileId; use iter_extended::vecmap; @@ -1372,7 +1372,7 @@ mod tests { path_resolver.insert_func(name.to_owned(), id); } - let mut def_maps: HashMap = HashMap::new(); + let mut def_maps = BTreeMap::new(); let file = FileId::default(); let mut modules = arena::Arena::new(); @@ -1385,7 +1385,7 @@ mod tests { root: path_resolver.local_module_id(), modules, krate: CrateId::dummy_id(), - extern_prelude: HashMap::new(), + extern_prelude: BTreeMap::new(), }, ); @@ -1424,7 +1424,7 @@ mod tests { impl PathResolver for TestPathResolver { fn resolve( &self, - _def_maps: &HashMap, + _def_maps: &BTreeMap, path: crate::Path, ) -> Result { // Not here that foo::bar and hello::foo::bar would fetch the same thing diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index d72b8d9d949..24015d76a07 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -139,7 +139,7 @@ impl FuncId { } } -#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] +#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)] pub struct StructId(ModuleId); impl StructId { @@ -163,7 +163,7 @@ impl StructId { } } -#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)] +#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)] pub struct TypeAliasId(pub usize); impl TypeAliasId { @@ -172,7 +172,7 @@ impl TypeAliasId { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct TraitId(pub ModuleId); impl TraitId {