Refactor GccrsArgs struct

[CohenArthur]

As pointed out by @philberty here, we need to refactor the GccrsArg structure defined here and clean it up

[philberty]

I have a few ideas here but it would be best to do one last review when you get #11 merged as this introduces the ar command.

[philberty]

I think the starting point for refactoring this work should be here

cargo-gccrs/src/gccrs/args.rs

Line 171 in 4c9a222

pub fn from_rustc_args(rustc_args: &[String]) -> Result<Vec<GccrsArgs>> {

This signifies that we are converting rustc arguments into a list of GccArgs which doesn't seem right. I think the name GccArgument seems more appropriate if we are returning a list of arguments. I assume something like gccrs -g -O2 etc.

[philberty]

I also don't know why this is part of the GccArgs impl block it seems like a separate function outside of the impl block makes more sense.

cargo-gccrs/src/gccrs/args.rs

Line 97 in 4c9a222

fn generate_parser() -> Options {

[bjorn3]

Maybe the parsing and the translation should be split? So the parsing would create a struct like https://github.com/rust-lang/rust/blob/456a03227e3c81a51631f87ec80cac301e5fa6d7/compiler/rustc_session/src/options.rs#L66-L71 (maybe even just copy this file verbatim? that way the parsing is guaranteed to be correct.) and then translation would create the appropriate arguments for gccrs.

[CohenArthur]

I think the starting point for refactoring this work should be here

cargo-gccrs/src/gccrs/args.rs

Line 171 in 4c9a222

pub fn from_rustc_args(rustc_args: &[String]) -> Result<Vec<GccrsArgs>> {

This signifies that we are converting rustc arguments into a list of GccArgs which doesn't seem right. I think the name GccArgument seems more appropriate if we are returning a list of arguments. I assume something like gccrs -g -O2 etc.

So the reason why we are returning a vector of arguments is that we actually need multiple gccrs commands for one rustc command.

As pointed out by @bjorn3, rustc can be invoked with the following set of arguments:

rustc --crate-name=example --crate-type=staticlib --crate-type=dylib --crate-type=bin ...

and should generate a binary, as well as a static library and a shared object. Since this is currently not handled by gcc, we have to split it into three sets of arguments (wrong arguments, but you get the idea):

gccrs -o example
gccrs -shared -fPIC -o example
gccrs -c example && ar rcs libexample.a

Since this is the second time that it's being pointed out, I think it's pretty clear that the names aren't great and need changing :D Maybe this could return a structure containing a vector of GccrsArgs, or something clearer.

Maybe the parsing and the translation should be split? So the parsing would create a struct like https://github.com/rust-lang/rust/blob/456a03227e3c81a51631f87ec80cac301e5fa6d7/compiler/rustc_session/src/options.rs#L66-L71 (maybe even just copy this file verbatim? that way the parsing is guaranteed to be correct.) and then translation would create the appropriate arguments for gccrs.

This is an interesting idea. I wonder if we'd rather implement these options as we go and need, or if we should rather have all of them and implement the equivalent gccrs arguments as we go.

[philberty]

Thanks @CohenArthur sorry I forgot gcc forces you to make more calls than rustc. I see what you mean.

Yeah, I think the goal for this issue is that we split the parsing of options, translation and invocation. The struct from rustic seems like an interesting idea for the options we need to support. Maybe an unhandled panic macro could be used for translation of unsupported options which can be updated as we go is a good path forward.

[bjorn3]

One thing: Rustc uses the same object files for all crate types to emit. At most it omits like the metadata object file for crate types other than dylib and the allocator shim for crate types other than dylib, cdylib and bin. So maybe cargo-gccrs could invoke gccrs once to compile to an object file (and metadata file) and then call into gcc/ar several times for linking?

[CohenArthur]

One thing: Rustc uses the same object files for all crate types to emit. At most it omits like the metadata object file for crate types other than dylib and the allocator shim for crate types other than dylib, cdylib and bin. So maybe cargo-gccrs could invoke gccrs once to compile to an object file (and metadata file) and then call into gcc/ar several times for linking?

I think that's a good idea as well :D However we'll run into issues if we ever want to compile this object file differently, for example using fPIC when creating a shared library.

[bjorn3]

Rustc uses PIC for everything by default on most targets. Just grep for relocation_model in compiler/rustc_target. That is necessary to create position independent executables. Note also that when --emit obj is used (eg rust-for-linux), you don't have a choice but to emit a single object file for all crate types combined suitable for all crate types.