Imports Support#95
Conversation
LesterEvSe
force-pushed
the
feature/imports-v2
branch
from
667340e to
4084ed0
Compare
LesterEvSe
force-pushed
the
feature/imports-v2
branch
from
4084ed0 to
3cab2bd
Compare
| simf_dir: &CanonPath, | ||
| context: &CanonPath, | ||
| ) -> Result<(), BuildError> { | ||
| for (drp_name, dep) in &deps_config.inner { |
There was a problem hiding this comment.
| for (drp_name, dep) in &deps_config.inner { | |
| for (dep_name, dep) in &deps_config.inner { |
| struct DepCollector<'a> { | ||
| builder: DependencyMapBuilder, | ||
| visited: HashSet<CanonPath>, | ||
| config_filename: &'a str, |
There was a problem hiding this comment.
Let's just use the constant everywhere instead of saving it here.
There was a problem hiding this comment.
Since we have an architecture where the cli crate loads the build crate, I cannot use the CONFIG_FILENAME constant inside the build. So I think a better solution is to just pass it to the function directly
There was a problem hiding this comment.
Makes sense, although I don't like that. Can we at least store String to avoid the lifetime code noise?
| self.builder | ||
| .add_dependency(simf_dir.clone(), drp_name.clone(), loaded_simf_dir.clone()); | ||
|
|
||
| if !self.visited.insert(loaded_context.clone()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Is it intended to add_dependency before the visited check?
There was a problem hiding this comment.
Yep, I will add the docstring for that
| } | ||
| } | ||
|
|
||
| pub fn build_dependency_map( |
There was a problem hiding this comment.
Let's not leave such functions hanging. Please add it either to the Resolver or Generator object.
| /// Intermediate Representation | ||
| const IR_SIMF_DIR: &str = "simf"; |
There was a problem hiding this comment.
Should this be taken from the build config as well?
| /// Intermediate Representation | |
| const IR_SIMF_DIR: &str = "simf"; | |
| /// Flattened Representation | |
| const FLAT_SIMF_DIR: &str = "simf"; |
There was a problem hiding this comment.
Good idea, I will change it to DEFAULT_SRC_DIR_NAME
There was a problem hiding this comment.
I mean to use the src_dir variable.
LesterEvSe
force-pushed
the
feature/imports-v2
branch
from
3cab2bd to
e509d10
Compare
2 participants
Replaces #69. Now that SimplicityHL natively supports flattening (PR #337) functionality, it made more sense to rewrite this implementation from scratch in a clean PR rather than trying to salvage and rebase the old one