Implement DependencyGraph and foundational driver architecture#271
Implement DependencyGraph and foundational driver architecture#271LesterEvSe wants to merge 1 commit intoBlockstreamResearch:dev/importsfrom
DependencyGraph and foundational driver architecture#271Conversation
src/driver.rs
Outdated
| pub struct ProjectGraph { | ||
| /// Arena Pattern: the data itself lives here. | ||
| /// A flat vector guarantees that module data is stored contiguously in memory. | ||
| #[expect(dead_code)] |
There was a problem hiding this comment.
| #[expect(dead_code)] | |
| #[allow(dead_code)] |
LesterEvSe
force-pushed
the
feature/simple-driver
branch
from
874e4a5 to
77e449a
Compare
LesterEvSe
changed the title
ProjectGraph and foundational driver architectureDependencyGraph and foundational driver architecture
| @@ -0,0 +1,327 @@ | |||
| use std::collections::{HashMap, VecDeque}; | |||
There was a problem hiding this comment.
Could we move it to driver/mod.rs?
The reasoning is that we gonna introduce a few independent algorithms here, so we could have them under same module, as they are just related to the driver itself
| /// | ||
| /// This function will return an `Err(String)` only for critical internal compiler errors | ||
| /// (e.g., if a provided `SourceFile` is unexpectedly missing its underlying file path). | ||
| pub fn new( |
There was a problem hiding this comment.
Let's have pub functions first then private
src/driver.rs
Outdated
|
|
||
| while let Some(curr_id) = queue.pop_front() { | ||
| // We need this to report errors inside THIS file. | ||
| let importer_source = graph.modules[curr_id].source.clone(); |
There was a problem hiding this comment.
Let's handle modules[curr_id], via let statement to avoid panic
LesterEvSe
force-pushed
the
feature/simple-driver
branch
from
77e449a to
006099a
Compare
| ); | ||
| } | ||
|
|
||
| Ok((!handler.has_errors()).then_some(graph)) |
There was a problem hiding this comment.
Is it expected that ParseFromStrWithErrors now returns None whenever the shared ErrorCollector already contains any earlier error, and DependencyGraph::new reuses one collector across the entire BFS?
There was a problem hiding this comment.
This was not expected. I could add a local ErrorCollector inside the parse_and_get_program function to parse more files and collect more errors. I think this would be better than the current approach
There was a problem hiding this comment.
Yes, then you could return Result<Self, ErrorCollector>
| if let Some(&existing_id) = self.lookup.get(&path) { | ||
| let deps = self.dependencies.entry(curr_id).or_default(); | ||
| if !deps.contains(&existing_id) { | ||
| deps.push(existing_id); | ||
| } |
There was a problem hiding this comment.
Does it actually detect import cycles? As it is written it treats any already-seen path as a normal resolved dependency and continues. For A -> B -> A, B’s import of A hits the existing_id branch, and in Ok((!handler.has_errors()).then_some(graph)) still returns Some(graph) with no diagnostic
There was a problem hiding this comment.
No, it will be the responsibility of C3 and the other algorithms in the following PR
…parse trait to the `SourceFile`
LesterEvSe
force-pushed
the
feature/simple-driver
branch
from
006099a to
571453c
Compare
| let Some(module) = | ||
| Self::parse_and_get_program(&path, importer_source.clone(), import_span, handler) | ||
| else { | ||
| continue; | ||
| }; |
There was a problem hiding this comment.
If parse_and_get_program fails here, the path is never recorded in lookup, so another parent importing the same file will try to parse it again and emit the same lex or syntax errors again
We could try to fix it here, or have a GitHub issue for it
3 participants
Project Graph: Added
driver.rsfeaturing theProjectGraphimplementation to build a Directed Acyclic Graph of project dependencies.Trait Refactoring: Updated the
ParseFromStrWithErrorstrait to accept a fullSourceFile(containing both metadata and source text) instead of just the raw string content.