Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions c2rust-transpile/src/build_files/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ use serde_json::json;

use super::compile_cmds::LinkCmd;
use super::TranspilerConfig;
use crate::get_module_name;
use crate::CrateSet;
use crate::ExternCrateDetails;
use crate::PragmaSet;
use crate::{get_module_name, rustfmt};

#[derive(Debug, Copy, Clone)]
pub enum BuildDirectoryContents {
Expand Down Expand Up @@ -225,7 +225,10 @@ fn emit_build_rs(
});
let output = reg.render("build.rs", &json).unwrap();
let output_path = build_dir.join("build.rs");
maybe_write_to_file(&output_path, output, tcfg.overwrite_existing)
let path = maybe_write_to_file(&output_path, output, tcfg.overwrite_existing)?;
rustfmt(&output_path, build_dir);

Some(path)
}

/// Emit lib.rs (main.rs) for a library (binary). Returns `Some(path)`
Expand All @@ -252,8 +255,10 @@ fn emit_lib_rs(

let output_path = build_dir.join(file_name);
let output = reg.render("lib.rs", &json).unwrap();
let path = maybe_write_to_file(&output_path, output, tcfg.overwrite_existing)?;
rustfmt(&output_path, build_dir);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make this optional (with a command line argument)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we not do it, though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user might want a custom formatting configuration. They could add that to rustfmt.toml, but that creates a race between adding that file and running the transpiler.

Another reason is performance: what if the user wants to run the transpiler, a bunch of refactoring steps, then run the formatter?

And what if the user just doesn't want to run it at all?


maybe_write_to_file(&output_path, output, tcfg.overwrite_existing)
Some(path)
}

/// If we translate variadic functions, the output will only compile
Expand Down
16 changes: 16 additions & 0 deletions c2rust-transpile/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ fn transpile_single(
),
};

rustfmt(&output_path, build_dir);

Ok((output_path, pragmas, crates))
}

Expand Down Expand Up @@ -667,3 +669,17 @@ fn get_output_path(
input_path
}
}

fn rustfmt(output_path: &Path, build_dir: &Path) {
let edition = "2021";

let status = Command::new("rustfmt")
.args(["--edition", edition])
.arg(output_path)
.current_dir(build_dir)
.status();

if !status.map_or(false, |status| status.success()) {
warn!("rustfmt failed, code may not be well-formatted");
}
Comment on lines +682 to +684
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we handle these errors differently? The command will fail if rustc isn't found or something, and the status will fail if rustc exits with an error. That said, they shouldn't really fail, so I think we can just panic here, maybe with an .expect.

Also, if we're calling rustfmt here, we can remove the rustfmt run in snapshots.rs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a panic is appropriate, as this is just an extra convenience for the user and not a hard requirement of the transpilation process. The transpilation should still succeed if rustfmt is not installed or doesn't complete for whatever reason. Though unlikely, rustfmt may not be installed even if cargo and rustc are.

}
6 changes: 0 additions & 6 deletions c2rust-transpile/tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,6 @@ fn transpile(platform: Option<&str>, c_path: &Path) {

let edition = "2021";

let status = Command::new("rustfmt")
.args(["--edition", edition])
.arg(&rs_path)
.status();
assert!(status.unwrap().success());

let rs = fs::read_to_string(&rs_path).unwrap();
let debug_expr = format!("cat {}", rs_path.display());

Expand Down
Loading