From f76e6a175577e7f290668f27347af895edf35ef0 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Mon, 16 Oct 2023 02:20:19 +0200 Subject: [PATCH] nr: Add documentation and cleanup --- name_resolve/src/declarator.rs | 100 ++++++++++++++++----------------- name_resolve/src/lib.rs | 69 +++++++++++++---------- name_resolve/src/resolver.rs | 2 +- name_resolve/src/scoper.rs | 25 ++++----- 4 files changed, 98 insertions(+), 98 deletions(-) diff --git a/name_resolve/src/declarator.rs b/name_resolve/src/declarator.rs index 28499136..695f3324 100644 --- a/name_resolve/src/declarator.rs +++ b/name_resolve/src/declarator.rs @@ -1,80 +1,78 @@ -use fir::{Fallible, Fir, Node, RefIdx, Traversal}; +use fir::{Fallible, Fir, Node, OriginIdx, RefIdx, Traversal}; use flatten::FlattenData; use crate::{NameResolutionError, NameResolveCtx, UniqueError}; +enum DefinitionKind { + Function, + Type, + Binding, +} + pub(crate) struct Declarator<'ctx, 'enclosing>(pub(crate) &'ctx mut NameResolveCtx<'enclosing>); +impl<'ctx, 'enclosing> Declarator<'ctx, 'enclosing> { + fn define( + &mut self, + kind: DefinitionKind, + node: &Node, + ) -> Fallible { + let (map, kind) = match kind { + DefinitionKind::Function => (&mut self.0.mappings.functions, "function"), + DefinitionKind::Type => (&mut self.0.mappings.types, "type"), + DefinitionKind::Binding => (&mut self.0.mappings.bindings, "binding"), + }; + + map.insert( + node.data.ast.symbol().unwrap().clone(), + node.origin, + self.0.enclosing_scope[node.origin], + ) + .map_err(|existing| Declarator::unique_error(node, existing, kind)) + } + + fn unique_error( + node: &Node, + existing: OriginIdx, + kind: &'static str, + ) -> NameResolutionError { + NameResolutionError::non_unique(node.data.ast.location(), UniqueError(existing, kind)) + } +} + impl<'ast, 'ctx, 'enclosing> Traversal, NameResolutionError> for Declarator<'ctx, 'enclosing> { + // TODO: Can we factor these three functions? + fn traverse_function( &mut self, - _fir: &Fir, + _: &Fir, node: &Node, - _generics: &[RefIdx], - _args: &[RefIdx], - _return_ty: &Option, - _block: &Option, + _: &[RefIdx], + _: &[RefIdx], + _: &Option, + _: &Option, ) -> Fallible { - self.0 - .mappings - .functions - .insert( - node.data.ast.symbol().unwrap().clone(), - node.origin, - self.0.enclosing_scope[node.origin], - ) - .map_err(|existing| { - NameResolutionError::non_unique( - node.data.ast.location(), - UniqueError(existing, "function"), - ) - }) + self.define(DefinitionKind::Function, node) } fn traverse_type( &mut self, - _fir: &Fir, + _: &Fir, node: &Node, _: &[RefIdx], _: &[RefIdx], ) -> Fallible { - self.0 - .mappings - .types - .insert( - node.data.ast.symbol().unwrap().clone(), - node.origin, - self.0.enclosing_scope[node.origin], - ) - .map_err(|existing| { - NameResolutionError::non_unique( - node.data.ast.location(), - UniqueError(existing, "type"), - ) - }) + self.define(DefinitionKind::Type, node) } fn traverse_binding( &mut self, - _fir: &Fir, + _: &Fir, node: &Node, - _to: &RefIdx, + _: &RefIdx, ) -> Fallible { - self.0 - .mappings - .variables - .insert( - node.data.ast.symbol().unwrap().clone(), - node.origin, - self.0.enclosing_scope[node.origin], - ) - .map_err(|existing| { - NameResolutionError::non_unique( - node.data.ast.location(), - UniqueError(existing, "binding"), - ) - }) + self.define(DefinitionKind::Binding, node) } } diff --git a/name_resolve/src/lib.rs b/name_resolve/src/lib.rs index 3a31bbcf..876a4ac1 100644 --- a/name_resolve/src/lib.rs +++ b/name_resolve/src/lib.rs @@ -35,7 +35,7 @@ //! can be resolved to more than one definitions, we error out. The resolver does not take care //! of resolving complex usages, such as methods, generic function calls or specialization. -use std::{collections::HashMap, ops::Index}; +use std::{collections::HashMap, mem, ops::Index}; use error::{ErrKind, Error}; use fir::{Fallible, Fir, Incomplete, Kind, Mapper, OriginIdx, Pass, Traversal}; @@ -55,15 +55,29 @@ use scoper::Scoper; /// in the current scope. struct UniqueError(OriginIdx, &'static str); -/// Documentation: very useful data structure, useful for two things: -/// 1. knowing where nodes live -/// 2. knowing which scope is the parent scope of a given scope -/// Keeps a reference on a hashmap - cheap to copy and pass around +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] +pub(crate) struct Scope(pub(crate) OriginIdx); + +impl Scope { + pub fn replace(&mut self, new: OriginIdx) -> OriginIdx { + mem::replace(&mut self.0, new) + } + + pub fn origin(&self) -> OriginIdx { + self.0 + } +} + +/// This data structure maps each node from the [`Fir`] to the scope which contains it. This +/// makes finding the definition associated with a name very easy, as we can simply look at the +/// name's enclosing scope, and look for definitions. If no suitable definition, we look at the +/// parent scope of this scope, and repeat the process, until we find a definition or exhaust +/// valid parents. This struct keeps a reference on a map, making it cheap to copy and pass around. #[derive(Clone, Copy, Debug)] -struct EnclosingScope<'enclosing>(&'enclosing HashMap); +struct EnclosingScope<'enclosing>(&'enclosing HashMap); impl Index for EnclosingScope<'_> { - type Output = OriginIdx; + type Output = Scope; fn index(&self, index: OriginIdx) -> &Self::Output { &self.0[&index] @@ -72,27 +86,24 @@ impl Index for EnclosingScope<'_> { type Bindings = HashMap; -// FIXME: Documentation -// Each scope contains a set of bindings +/// Each scope in the [`scopes`] map contains the bindings associated with a given scope, +/// meaning that each scope contains a list of definitions. A definition can be thought of as the +/// mapping of a name ([`Symbol`]) to the node's index in the [`Fir`]. #[derive(Clone, Debug)] struct FlatScope<'enclosing> { - scopes: HashMap, + scopes: HashMap, enclosing_scope: EnclosingScope<'enclosing>, } /// Allow iterating on a [`FlatScope`] by going through the chain of enclosing scopes -struct FlatIterator<'scope, 'enclosing>(Option, &'scope FlatScope<'enclosing>); +struct FlatIterator<'scope, 'enclosing>(Option, &'scope FlatScope<'enclosing>); trait LookupIterator<'scope, 'enclosing> { - fn lookup_iterator(&'scope self, starting_scope: OriginIdx) - -> FlatIterator<'scope, 'enclosing>; + fn lookup_iterator(&'scope self, starting_scope: Scope) -> FlatIterator<'scope, 'enclosing>; } impl<'scope, 'enclosing> LookupIterator<'scope, 'enclosing> for FlatScope<'enclosing> { - fn lookup_iterator( - &'scope self, - starting_scope: OriginIdx, - ) -> FlatIterator<'scope, 'enclosing> { + fn lookup_iterator(&'scope self, starting_scope: Scope) -> FlatIterator<'scope, 'enclosing> { FlatIterator(Some(starting_scope), self) } } @@ -102,10 +113,11 @@ impl<'scope, 'enclosing> Iterator for FlatIterator<'scope, 'enclosing> { fn next(&mut self) -> Option { let cursor = self.0; - let bindings = cursor.and_then(|scope_idx| self.1.scopes.get(&scope_idx)); + let bindings = cursor.and_then(|scope| self.1.scopes.get(&scope)); self.0 = cursor - .and_then(|current| self.1.enclosing_scope.0.get(¤t)) + // TODO: Factor this in a method? + .and_then(|current| self.1.enclosing_scope.0.get(¤t.origin())) .copied(); bindings @@ -113,17 +125,12 @@ impl<'scope, 'enclosing> Iterator for FlatIterator<'scope, 'enclosing> { } impl<'enclosing> FlatScope<'enclosing> { - fn lookup(&self, name: &Symbol, starting_scope: OriginIdx) -> Option<&OriginIdx> { + fn lookup(&self, name: &Symbol, starting_scope: Scope) -> Option<&OriginIdx> { self.lookup_iterator(starting_scope) .find_map(|bindings| bindings.get(name)) } - fn insert( - &mut self, - name: Symbol, - idx: OriginIdx, - scope: OriginIdx, /* FIXME: Needs newtype idiom */ - ) -> Result<(), OriginIdx> { + fn insert(&mut self, name: Symbol, idx: OriginIdx, scope: Scope) -> Result<(), OriginIdx> { // we need to use the innermost scope here, not `lookup` if let Some(existing) = self .scopes @@ -144,7 +151,7 @@ impl<'enclosing> FlatScope<'enclosing> { /// level. #[derive(Clone, Debug)] struct ScopeMap<'enclosing> { - pub variables: FlatScope<'enclosing>, + pub bindings: FlatScope<'enclosing>, pub functions: FlatScope<'enclosing>, pub types: FlatScope<'enclosing>, } @@ -156,7 +163,7 @@ struct NameResolveCtx<'enclosing> { impl<'enclosing> NameResolveCtx<'enclosing> { fn new(enclosing_scope: EnclosingScope<'enclosing>) -> NameResolveCtx { - let empty_scope_map: HashMap = enclosing_scope + let empty_scope_map: HashMap = enclosing_scope .0 .values() .map(|scope_idx| (*scope_idx, Bindings::new())) @@ -165,7 +172,7 @@ impl<'enclosing> NameResolveCtx<'enclosing> { NameResolveCtx { enclosing_scope, mappings: ScopeMap { - variables: FlatScope { + bindings: FlatScope { scopes: empty_scope_map.clone(), enclosing_scope, }, @@ -306,11 +313,11 @@ impl NameResolutionError { } impl<'enclosing> NameResolveCtx<'enclosing> { - fn scope(fir: &Fir) -> HashMap { + fn scope(fir: &Fir) -> HashMap { let root = fir.nodes.last_key_value().unwrap(); let mut scoper = Scoper { - current_scope: scoper::Scope(*root.0), + current_scope: Scope(*root.0), enclosing_scope: HashMap::new(), }; diff --git a/name_resolve/src/resolver.rs b/name_resolve/src/resolver.rs index 5bb19759..df30e14a 100644 --- a/name_resolve/src/resolver.rs +++ b/name_resolve/src/resolver.rs @@ -44,7 +44,7 @@ impl<'ctx, 'enclosing> Resolver<'ctx, 'enclosing> { let origin = match kind { ResolveKind::Call => mappings.functions.lookup(symbol, scope), ResolveKind::Type => mappings.types.lookup(symbol, scope), - ResolveKind::Var => mappings.variables.lookup(symbol, scope), + ResolveKind::Var => mappings.bindings.lookup(symbol, scope), }; origin.map_or_else( diff --git a/name_resolve/src/scoper.rs b/name_resolve/src/scoper.rs index de708168..0a2b5e4c 100644 --- a/name_resolve/src/scoper.rs +++ b/name_resolve/src/scoper.rs @@ -1,30 +1,24 @@ -use std::{collections::HashMap, mem}; +use std::collections::HashMap; + +use crate::Scope; use fir::{Fallible, Fir, Kind, Node, OriginIdx, RefIdx, Traversal}; use flatten::FlattenData; -pub(crate) struct Scope(pub(crate) OriginIdx); - -impl Scope { - pub fn replace(&mut self, new: OriginIdx) -> OriginIdx { - mem::replace(&mut self.0, new) - } - - pub fn origin(&self) -> OriginIdx { - self.0 - } -} - pub(crate) struct Scoper { + /// The current scope we are visiting, which we will use when we assign a scope to each + /// node in [`Scoper::scope`] pub(crate) current_scope: Scope, - pub(crate) enclosing_scope: HashMap, + /// Map of each node to the scope it is contained in. This will be built progressively as + /// we visit each node + pub(crate) enclosing_scope: HashMap, } impl Scoper { /// Set the enclosing scope of `to_scope` to the current scope fn scope(&mut self, to_scope: &Node) { self.enclosing_scope - .insert(to_scope.origin, self.current_scope.origin()); + .insert(to_scope.origin, self.current_scope); } /// Enter a new scope, replacing the context's current scope. This returns the old scope, @@ -33,6 +27,7 @@ impl Scoper { self.current_scope.replace(new_scope) } + // TODO: Move this function in `Traversal`? fn maybe_visit_child(&mut self, fir: &Fir>, ref_idx: &RefIdx) -> Fallible<()> { match ref_idx { RefIdx::Resolved(origin) => self.traverse_node(fir, &fir[origin]),