support c2rust <path/to/*.c> in lieu of compile_commands.json

[aneksteind]

• Fixes Lack of support for simple projects or the cc crate #1033.

This adds support for transpiling C sources without needing to generate a compile_commands.json with something such as bear or cmake. The way it works is that a temporary temp_compile_commands file is created. One alternative to this approach would have been to alter the AST exporter to use a FixedCompilationDatabase but it seemed simpler to support this feature from the front end.

[fw-immunant]

Looks mostly sane. Commented a few cleanups/improvements inline; the only user-significant one is to use a temporary directory for the generated compile_commands.json.

Error messages could be more helpful, e.g. including paths and mentioning what the JSON file is for.

[fw-immunant]

This probably should have some corresponding doc/diagnostics changes, e.g. the "Could not find compile_commands.json file at path: {}" error in c2rust-transpile.rs and the docs/name of Args.compile_commands in that file.

[kkysen]

Why can't we put this new logic in fn gen_compile_commands and not need to write a temporary compile_commands.json to disk?

c2rust/c2rust-transpile/src/compile_cmds/mod.rs

Lines 171 to 176 in c603b25

	/// Read `compile_commands` file, optionally ignore any entries not matching
	/// `filter`, and filter out any .S files since they're likely assembly files.
	pub fn get_compile_commands(
	compile_commands: &Path,
	filter: &Option<Regex>,
	) -> Result<Vec<LinkCmd>, Error> {

[thedataking]

Once the CLI is agreed upon, don't forget to update the readme.

[LegNeato]

I too think it would be cleaner to make transpile take CompileCmd. That way tests as well as library users can generate one with whatever content they want and pass it in rather than hitting the fs first. All this file and json stuff is not required to actually transpile, so it feels weird to tie them together.

As an aside, there appears to be an existing project that could be leveraged though I am not sure if it makes sense:

https://github.com/rizsotto/json_compilation_db

[kkysen]

I too think it would be cleaner to make transpile take CompileCmd. That way tests as well as library users can generate one with whatever content they want and pass it in rather than hitting the fs first. All this file and json stuff is not required to actually transpile, so it feels weird to tie them together.

☝️

As an aside, there appears to be an existing project that could be leveraged though I am not sure if it makes sense:

https://github.com/rizsotto/json_compilation_db

I think the existing

c2rust/c2rust-transpile/src/compile_cmds/mod.rs

Lines 12 to 33 in c603b25

	pub struct CompileCmd {
	/// The working directory of the compilation. All paths specified in the command
	/// or file fields must be either absolute or relative to this directory.
	directory: PathBuf,
	/// The main translation unit source processed by this compilation step. This is
	/// used by tools as the key into the compilation database. There can be multiple
	/// command objects for the same file, for example if the same source file is compiled
	/// with different configurations.
	pub file: PathBuf,
	/// The compile command executed. After JSON unescaping, this must be a valid command
	/// to rerun the exact compilation step for the translation unit in the environment
	/// the build system uses. Parameters use shell quoting and shell escaping of quotes,
	/// with ‘"’ and ‘\’ being the only special characters. Shell expansion is not supported.
	#[serde(skip_deserializing)]
	_command: Option<String>,
	/// The compile command executed as list of strings. Either arguments or command is required.
	#[serde(default, skip_deserializing)]
	_arguments: Vec<String>,
	/// The name of the output created by this compilation step. This field is optional. It can
	/// be used to distinguish different processing modes of the same input file.
	output: Option<String>,
	}

we have already does what json_compilation_db does.

[kkysen]

Another suggestion I thought of: most clang tooling that I've seen that accepts compilation databases (e.x. clang-tidy) also accepts a list of files as normal arguments, and then after --, a list of cflags that applies to all of those files and would go after the clang <file> in the command/arguments. Following that I think would be best for compatibility and familiarity, and is also a bit more flexible since it allows for other arguments (albeit the same ones for every file).

[aneksteind]

@thedataking @kkysen thoughts on 6c497c7? The new interface would either be providing --compile-commands or --source, e.g. c2rust transpile --source add/*.h

[kkysen]

I want to look at the interface with clang we're using. I'm still not sure why we have to create a temporary compile commands. If we do, though, I think we should use the tempfile crate to do so, as that deletes the file on destruction, so it will get deleted if we panic or return early in between, which very well may happen. Also, the clap CLI seems okay, but I think it'd be better if we can match clang tooling's API for specifying either a compile commands or a list of sources and a list of flags/other arguments, which is more flexible and consistent with existing tooling.

[thedataking]

Also, the clap CLI seems okay, but I think it'd be better if we can match clang tooling's API for specifying either a compile commands or a list of sources and a list of flags/other arguments, which is more flexible and consistent with existing tooling.

@kkysen this is a reasonable line of thought but it would expand the scope of the task beyond what was planned. This PR to narrowly resolve the feature request in #1033 as it already turned out to be more work than expected.

Edited to acknowledge the merit of the idea being proposed.

[thedataking]

The new interface would either be providing --compile-commands or --source, e.g. c2rust transpile --source add/*.h

PR #1033 asked if we could accept source files instead of a .json file, why do we need --compile-commands or --source? Can't we just assume the user is providing source files if the (base of) the file name isn't exactly compile_commans.json? That's how I read the request in the PR.

This PR has received more work and review than I expected. I'd like it to be wrapped up in the simplest possible way that satisfied the original goals and doesn't break or extend the existing interface (unless we must).

[thedataking]

LGTM. Should we update the docs?

[kkysen]

LGTM after that path canonicalization comment. I still think there are ways to improve how we do this, but this is a good and simple start to it that should be good for now.