Allow multiple library paths #2436
Conversation
|
I've changed it so that it emits an error if it finds an import in multiple library paths. |
|
This pull request has not seen activity in 14 days and is being marked as stale. If you're continuing work on this, please reply and state how to get this PR in a mergeable state or what issues it is blocked on. If the PR is not ready for review, please mark it as a draft. The PR will be closed in 7 days if there is no further activity. |
|
@rachitnigam if the changes addressed your concerns then I'll go ahead and merge this? |
| @@ -19,11 +19,11 @@ fn cell_share_bench(c: &mut Criterion) { | |||
| let name = | |||
| format!("benches/component-sharing/{}.futil", name); | |||
| let bench = Path::new(&name); | |||
| let lib = Path::new("."); | |||
| let lib = [Path::new(".").to_path_buf()]; | |||
There was a problem hiding this comment.
Silly nit that you can feel free to ignore. But might be better to just use PathBuf::from instead
| let library_imports: Vec<_> = lib_paths | ||
| .iter() | ||
| .filter_map(|lib_path| { | ||
| let library_import = lib_path.join(import.clone()); |
There was a problem hiding this comment.
nit: clone() can be replaced with as_ref
| if library_imports.len() > 1 { | ||
| return Err(Error::misc(format!( | ||
| "Import path `{}` found in multiple library paths ({})", | ||
| import.as_ref().to_string_lossy(), | ||
| library_imports | ||
| .iter() | ||
| .map(|p| p.to_string_lossy()) | ||
| .format(", ") | ||
| )) | ||
| .with_pos(&import)); |
There was a problem hiding this comment.
Not super important, but I think these blocks could probably be arms of a match
on the len of library imports rather than a bunch of if with returns. Might be a
bit easier to parse?
This makes it possible to specify multiple library paths, which is useful for frontends that wish to provide their own primitive libraries in addition to Calyx's standard library.