Merge #363: Move file_id into Span

7b6db6c feat: move `file_id` into `Span` (LesterEvSe)

Pull request description:

  This PR moves `file_id` into `Span` as a structural component of the source location. It also removes the unnecessary `file_id` and `set_file_id` methods from the `parse.rs` file.
  Refactored the driver code a bit

ACKs for top commit:
  apoelstra:
    ACK 7b6db6c; successfully ran local tests
  KyrylR:
    ACK 7b6db6c; successfully ran local tests

Tree-SHA512: e5859cc7329444e5a06d0cdf5fefaca1e61b1cd8c634419a92da82d357687c016cb4d2c22ff0e54a6b251b60126421df20abc901eb6d6f81d424abea1e980302

Author: LesterEvSe

src/driver/mod.rs

@@ -85,11 +85,15 @@ impl SourceMap {
         }
     }
 
-    fn insert(&mut self, path: CanonPath) -> usize {
+    fn insert(&mut self, path: CanonPath) {
         let id = self.paths.len();
         self.ids.insert(path.clone(), id);
         self.paths.push(path);
-        id
+        debug_assert_eq!(self.ids.len(), self.paths.len());
+    }
+
+    fn len(&self) -> usize {
+        self.paths.len()
     }
 
     pub fn get_id(&self, path: &CanonPath) -> Option<usize> {
@@ -137,10 +141,10 @@ pub(crate) struct DependencyGraph {
     /// Fast bidirectional lookup between `CanonPath` and Module IDs.
     source_map: SourceMap,
 
-    /// Memoized results of [`crate::resolution::DependencyMap::resolve_path`] to avoid
-    /// resolving the same [`parse::UseDecl`] twice — once during the driver phase
-    /// and once during the building phase after linearization.
-    use_cache: HashMap<parse::UseDecl, ResolvedUse>,
+    /// Memoizes [`crate::resolution::DependencyMap::resolve_path_internal`] results,
+    /// keyed by the source [`Span`] of each `use` declaration, so the resolver runs
+    /// once per occurrence and the linearization phase can look the result up directly.
+    use_cache: HashMap<Span, ResolvedUse>,
 
     // TODO: Consider to optimising this with `Vec` instead of `HashMap`
     /// The Adjacency List: Defines the Directed acyclic Graph (DAG) of imports.
@@ -241,45 +245,8 @@ impl DependencyGraph {
         Ok((!handler.has_errors()).then_some(graph))
     }
 
-    /// This helper cleanly encapsulates the process of loading source text, parsing it
-    /// into an `parse::Program`, and combining them so the compiler can easily work with the file.
-    /// If the file is missing or contains syntax errors, it logs the diagnostic to the
-    /// `ErrorCollector` and safely returns `None`.
-    fn parse_and_get_source_module(
-        path: &CanonPath,
-        importer_source: &CanonSourceFile,
-        span: Span,
-        handler: &mut ErrorCollector,
-        unstable_features: &UnstableFeatures,
-    ) -> Option<SourceModule> {
-        let Ok(content) = std::fs::read_to_string(path.as_path()) else {
-            let err = RichError::new(
-                Error::FileNotFound {
-                    filename: PathBuf::from(path.as_path()),
-                },
-                span,
-            )
-            .with_source(importer_source.clone());
-
-            handler.push(err);
-            return None;
-        };
-
-        let mut error_handler = ErrorCollector::new();
-        let source = CanonSourceFile::new(path.clone(), Arc::from(content));
-
-        let ast = parse::Program::parse_from_str_with_errors(
-            source.clone(),
-            unstable_features,
-            &mut error_handler,
-        );
-
-        if error_handler.has_errors() {
-            handler.extend_with_handler(source, &error_handler);
-            None
-        } else {
-            ast.map(|program| SourceModule { source, program })
-        }
+    pub fn source_map(&self) -> &SourceMap {
+        &self.source_map
     }
 
     /// PHASE 1 OF GRAPH CONSTRUCTION: Resolves all `use` declarations within a single
@@ -292,7 +259,7 @@ impl DependencyGraph {
         current_program: &parse::Program,
         current_module: &CurrentModule,
         dependency_map: &DependencyMap,
-        use_cache: &mut HashMap<parse::UseDecl, ResolvedUse>,
+        use_cache: &mut HashMap<Span, ResolvedUse>,
         handler: &mut ErrorCollector,
     ) -> Vec<(CanonPath, Span)> {
         let mut ctx = ImportContext {
@@ -330,7 +297,10 @@ impl DependencyGraph {
                 continue;
             }
 
+            let new_id = self.source_map.len();
+
             let Some(module) = Self::parse_and_get_source_module(
+                new_id,
                 &path,
                 &current.source,
                 import_span,
@@ -342,7 +312,7 @@ impl DependencyGraph {
                 continue;
             };
 
-            let new_id = self.source_map.insert(path.clone());
+            self.source_map.insert(path.clone());
             self.modules.push(module);
 
             self.dependencies
@@ -353,8 +323,46 @@ impl DependencyGraph {
         }
     }
 
-    pub fn source_map(&self) -> &SourceMap {
-        &self.source_map
+    /// This helper cleanly encapsulates the process of loading source text, parsing it
+    /// into an `parse::Program`, and combining them so the compiler can easily work with the file.
+    /// If the file is missing or contains syntax errors, it logs the diagnostic to the
+    /// `ErrorCollector` and safely returns `None`.
+    fn parse_and_get_source_module(
+        new_id: usize,
+        path: &CanonPath,
+        importer_source: &CanonSourceFile,
+        span: Span,
+        handler: &mut ErrorCollector,
+        unstable_features: &UnstableFeatures,
+    ) -> Option<SourceModule> {
+        let Ok(content) = std::fs::read_to_string(path.as_path()) else {
+            let err = RichError::new(
+                Error::FileNotFound {
+                    filename: PathBuf::from(path.as_path()),
+                },
+                span,
+            )
+            .with_source(importer_source.clone());
+
+            handler.push(err);
+            return None;
+        };
+
+        let mut error_handler = ErrorCollector::new();
+        let source = CanonSourceFile::new(path.clone(), Arc::from(content));
+        let ast = parse::Program::parse_from_str_with_errors(
+            new_id,
+            source.clone(),
+            unstable_features,
+            &mut error_handler,
+        );
+
+        if error_handler.has_errors() {
+            handler.extend_with_handler(source, &error_handler);
+            return None;
+        }
+
+        ast.map(|program| SourceModule { source, program })
     }
 }
 
@@ -363,11 +371,27 @@ impl DependencyGraph {
 struct ImportContext<'a> {
     current: CurrentModule,
     dependency_map: &'a DependencyMap,
-    use_cache: &'a mut HashMap<parse::UseDecl, ResolvedUse>,
+    use_cache: &'a mut HashMap<Span, ResolvedUse>,
     handler: &'a mut ErrorCollector,
 }
 
 impl<'a> ImportContext<'a> {
+    /// Recursively walks an item, collecting resolved imports.
+    /// Recurses into inline `mod` blocks.
+    fn process_item(&mut self, item: &parse::Item, valid_imports: &mut Vec<(CanonPath, Span)>) {
+        match item {
+            parse::Item::Use(use_decl) => valid_imports.extend(self.resolve_single(use_decl)),
+            parse::Item::Module(module) => {
+                for item in module.items() {
+                    self.process_item(item, valid_imports);
+                }
+            }
+
+            // These items carry no import information at this stage and can be safely skipped.
+            parse::Item::TypeAlias(_) | parse::Item::Function(_) | parse::Item::Ignored => {}
+        }
+    }
+
     /// Resolves a single `use` declaration, caches the result for reuse during
     /// later graph construction phases, and returns the resolved path and span.
     /// Returns `None` and reports to `handler` if resolution fails.
@@ -384,18 +408,12 @@ impl<'a> ImportContext<'a> {
             }
         };
 
-        // We assign a file ID to prevent collisions between identical `use` statements across different files.
-        // For instance, `use crate::A::foo;` in the local workspace and in an external dependency
-        // might resolve to completely different implementations of the `foo` function.
-        let mut use_decl = use_decl.clone();
         let span = *use_decl.span();
-        use_decl.set_file_id(self.current.id);
-
         let result: (CanonPath, Span) = (resolved.path.clone(), span);
 
         // Since we found an error, when we can reevalute the result, we do not want to break it again
         // So, add error to prevent similar cases in the future
-        if let Some(old_value) = self.use_cache.insert(use_decl, resolved) {
+        if let Some(old_value) = self.use_cache.insert(span, resolved) {
             let msg = format!(
                 "Reevaluated an existing use_decl. Old value was: {:?}",
                 old_value
@@ -406,26 +424,6 @@ impl<'a> ImportContext<'a> {
         }
         Some(result)
     }
-
-    /// Recursively walks an item, collecting resolved imports.
-    /// Recurses into inline `mod` blocks.
-    fn process_item(&mut self, item: &parse::Item, valid_imports: &mut Vec<(CanonPath, Span)>) {
-        match item {
-            parse::Item::Use(use_decl) => {
-                if let Some(import) = self.resolve_single(use_decl) {
-                    valid_imports.push(import);
-                }
-            }
-            parse::Item::Module(module) => {
-                for item in module.items() {
-                    self.process_item(item, valid_imports);
-                }
-            }
-
-            // These items carry no import information at this stage and can be safely skipped.
-            parse::Item::TypeAlias(_) | parse::Item::Function(_) | parse::Item::Ignored => {}
-        }
-    }
 }
 
 /// Shared mutable state threaded through dependency loading.
@@ -508,6 +506,7 @@ pub(crate) mod tests {
         let main_canon_source = CanonSourceFile::new(root_p, Arc::from(root_content));
 
         let main_program_option = parse::Program::parse_from_str_with_errors(
+            MAIN_MODULE,
             main_canon_source.clone(),
             &UnstableFeatures::all(),
             &mut handler,

src/driver/resolve_order.rs

@@ -16,10 +16,6 @@ impl DependencyGraph {
         }
     }
 
-    fn get_module_name(source_id: usize) -> Identifier {
-        Identifier::from_str_unchecked(format!("unit_{}", source_id).as_str())
-    }
-
     /// Constructs the unified array of items for the entire multi-program.
     fn build_program(
         &self,
@@ -35,7 +31,7 @@ impl DependencyGraph {
                 .program
                 .items()
                 .iter()
-                .filter_map(|item| self.rewrite_item(source_id, item))
+                .filter_map(|item| self.rewrite_item(item))
                 .collect();
 
             if source_id == MAIN_MODULE {
@@ -64,63 +60,58 @@ impl DependencyGraph {
     }
 
     /// Rewrites a single item for the flattened single-file representation.
-    fn rewrite_item(&self, source_id: usize, item: &parse::Item) -> Option<parse::Item> {
+    fn rewrite_item(&self, item: &parse::Item) -> Option<parse::Item> {
         match item {
-            parse::Item::TypeAlias(alias) => {
-                let mut alias = alias.clone();
-                alias.set_file_id(source_id);
-                Some(parse::Item::TypeAlias(alias))
-            }
-            parse::Item::Function(function) => {
-                let mut function = function.clone();
-                function.set_file_id(source_id);
-                Some(parse::Item::Function(function))
-            }
-            parse::Item::Use(use_decl) => Some(self.rewrite_use(source_id, use_decl)),
+            parse::Item::Use(use_decl) => Some(self.rewrite_use(use_decl)),
             parse::Item::Module(module) => {
                 let items: Vec<parse::Item> = module
                     .items()
                     .iter()
-                    .filter_map(|inner_item| self.rewrite_item(source_id, inner_item))
+                    .filter_map(|inner_item| self.rewrite_item(inner_item))
                     .collect();
 
                 Some(parse::Item::Module(parse::Module::new(
-                    source_id,
+                    module.span().file_id,
                     module.visibility().clone(),
                     module.name().clone(),
                     &items,
                 )))
             }
+            parse::Item::TypeAlias(_) | parse::Item::Function(_) => Some(item.clone()),
             parse::Item::Ignored => None,
         }
     }
 
-    /// Rewrites a `use` declaration by replacing the drp alias with the canonical
-    /// `file_N` module name, prepending it to the remaining `mod_path` from the cache.
-    /// If the target is the `MAIN_MODULE`, the `file_N` segment is safely omitted.
+    /// Rewrites a `use` declaration to its canonical `crate`-rooted form.
     ///
-    /// ## Example
+    /// The resolved path becomes `crate::<module>::<mod_path...>`, where
+    /// `<module>` is `file_N` for dependency files and is omitted when the
+    /// target is `MAIN_MODULE` (via `get_module_name`).
     ///
-    /// `use base_math::simple_op::hash` into `use file_2::hash`
-    /// `use crate::inline_mod::item` into `use crate::inline_mod::item`
-    fn rewrite_use(&self, source_id: usize, use_decl: &parse::UseDecl) -> parse::Item {
-        let mut use_decl = use_decl.clone();
-        use_decl.set_file_id(source_id);
-        let resolved = &self.use_cache[&use_decl];
+    /// ## Examples
+    ///
+    /// - `use base_math::simple_op::hash` → `use crate::file_2::hash`
+    /// - `use some_dep::item` (target = `MAIN_MODULE`) → `use crate::item`
+    fn rewrite_use(&self, use_decl: &parse::UseDecl) -> parse::Item {
+        let resolved = &self.use_cache[use_decl.span()];
         let target_id = self
             .source_map
             .get_id(&resolved.path)
             .expect("resolved path must be registered");
 
         let mut new_path = Vec::with_capacity(resolved.mod_path.len() + 2);
         new_path.push(Identifier::from_str_unchecked(CRATE_STR));
-
         new_path.push(Self::get_module_name(target_id));
         new_path.extend(resolved.mod_path.iter().cloned());
 
+        let mut use_decl = use_decl.clone();
         use_decl.set_path(&new_path);
         parse::Item::Use(use_decl)
     }
+
+    fn get_module_name(source_id: usize) -> Identifier {
+        Identifier::from_str_unchecked(format!("unit_{}", source_id).as_str())
+    }
 }
 
 #[cfg(test)]