Descendc: CLI functionality for Descend #32
Conversation
…d new flags to main.rs, fixed typo in readme
| [dependencies.descend_derive] | ||
| path = "./descend_derive" | ||
|
|
||
| [dependencies.clap] |
There was a problem hiding this comment.
Could you give a brief overview over what these dependencies are used for, so that we can gauge how important they are?
There was a problem hiding this comment.
- clap: Declarative API for defining and parsing the command-line arguments.
- which: Used to check for the presence of system executables (
nvcc,clang-format). - predicates: Facilitates expressive assertions in the
cli_test.rs(verifying output contains expected text). - assert_cmd: Assists in testing command-line applications and enables CLI integration tests (as used in
cli_test.rs). - log and env_logger: Provides a consistent logging API and allows for configurable logging output. I used it to switch between general information and debug-level output in the CLI. Simple
println-style outputs would work as well. - anyhow: Simplifies error handling by performing type erasure for errors. It wraps concrete error types in a trait object, allowing you to propagate errors without having to specify their types explicitly. I used it as a neat way to provide error messages, but it may won't be applicable when we merge PR #31 and does not align with the use of custom error types. I will remove this dependency and change the error handling.
Cargo.toml
Outdated
| version = "2.0.16" | ||
|
|
||
| [workspace] | ||
| members = ["descend_derive"] No newline at end of file |
There was a problem hiding this comment.
we should make sure to always include an empty line at the end of every file:
src/main.rs
Outdated
| .output() | ||
| .with_context(|| "Failed to run nvcc command")?; | ||
| if !output.status.success() { | ||
| return Err(anyhow::anyhow!( |
There was a problem hiding this comment.
I think anyhow could be a custom error. When calling compile in l.84 we could simply bubble up the error.
How this should work will probably be more clear after PR #31 has been merged.
There was a problem hiding this comment.
I introduced two custom errors called NVCCError and ExecutableError in error.rs and removed the use of anyhow. I also added a std::fmt::Display implementation for ErrorReported. This should allow us to bubble up errors in a more controlled way.
src/lib.rs
Outdated
| pub mod ty_check; | ||
|
|
||
| pub fn compile(file_path: &str) -> Result<String, ErrorReported> { | ||
| pub fn compile(file_path: &str, output_path: Option<&str>) -> Result<String, ErrorReported> { |
There was a problem hiding this comment.
Before compile was the entry into the entire compiler. Now there is additional functionality on top. It now seems odd to me to mix the combination of the compilation stages with file access.
I suggest breaking this up:
| pub fn compile(file_path: &str, output_path: Option<&str>) -> Result<String, ErrorReported> { | |
| pub fn compile(source: &SourceCode) -> Result<String, ErrorReported> { | |
| // return the generated code or errors | |
| } |
I am ignoring that the error reporting should probably be changed according to PR #31.
This way the implementation is more modular. For now, the CLI could open the file and decide whether to write the output to a file or to the standard output stream.
There was a problem hiding this comment.
I refactored my implementation on that point. In the changes I just pushed, the compile function remains unchanged, and all file-handling logic has been moved to main.rs.
tests/example_infer_apps.rs
Outdated
| Ok(println!( | ||
| "{}", | ||
| descend::compile("examples/infer/transpose.desc")? | ||
| descend::compile("examples/infer/transpose.desc", None)? |
There was a problem hiding this comment.
By making compile only take the SourceCode we do not have to provide None everywhere.
| let mut nvcc_cmd = Command::new("nvcc"); | ||
| nvcc_cmd | ||
| .arg(cuda_file) | ||
| .arg("-o") |
There was a problem hiding this comment.
I think it makes sense to delete this file unless explicitly requested.
| env_logger::init(); | ||
| let cli = Cli::parse(); | ||
|
|
||
| if cli.debug { |
There was a problem hiding this comment.
Do the debug and verbose flags currently do anything?
There was a problem hiding this comment.
I initially integrated both flags to introduce separate logging levels, but I never properly implemented their use. I will push some changes that introduce different logging behavior via a debug flag. For now, maintaining different levels like verbose and debug is unnecessary.
2 participants
Emit,Build,Run) viaclapnvccandclang-formatanyhowfor streamlined error propagation and improved context, making debugging easierexamples/infer) usesunsafefor uninitialized holes, which results in empty assignments in the output. This design choice allows compiling GPU-side functions with static arguments without handling host-side IO.mainare translated to have avoidreturn type (reflecting Descend’s unit type), leading to conventional warnings in C++.mainfunction translation or integrating with existing host-side languages) to address these issues in future iterations is in the works.