Ordering between `flag` and `flag_if_supported` is not maintained

[nagisa]

trafficstars

C and C++ compilers consider the ordering of arguments important and semantic. However, if you construct a builder like this:

cc::Build::new().flag_if_supported("-xc").flag("-xc++").flag("-std=c++11")

the constructed command line will look like this:

$CC -xc++ -std=c++11 -xc ...

which will then error (or warn...), even though the otherwise nonsensical, but valid invocation would be

$CC -xc -xc++ -std=c++11

Some valid use-cases of this ordering generally involve overriding previous arguments.

[nagisa]

As far as fixing this is concerned, I imagine we would maintain some sort of a per-Builder sequence index that gets associated with each flag pushed to one of the many vectors that are used to maintain these flags.

So instead of flags: Vec<Arc<str>> or flags_supported: Vec<Arc<str>> we'd have flags: Vec<(usize, Arc<str>)>, flags_supported: Vec<(usize, Arc<str>)> and so on for all the flag vectors. Then, in the try_get_compiler function we’d carefully draw from all vectors with flags in the order of usize sequence numbers. This may end up being slower, but with how many flags there are generally, it shouldn’t pose a problem.

We could also replace the many vectors with a vector of enum variants each representing different kind of flag.

Worth noting that changing this would be a breaking change, so it'd be worthwhile to see if we want to or are willing change the behaviour here at all.

[thomcc]

You mean it would be breaking to fix the ordering because someone might be relying on us using the current order? I suppose that's true, but I'm not worried about it. We never wrote down that this is the behavior, and this isn't a crate where we'd get anything done if we preserved Hyrum-esque stability promises.

I think I'd take a PR for this if it was reasonably implemented. I don't have a strong opinion between enum vs (usize, Arc<str>), either is fine (the enum approach seems simpler, though). Obviously if the enum approach is used, the type should not be part of the public API.

[nagisa]

You mean it would be breaking to fix the ordering because someone might be relying on us using the current order?

That too, but especially that now

Build().flag("-bar").flag_if_supported("-foo").flag("-baz")

will test whether appending -foo is valid for $CC -bar -baz rather than for $CC -bar. For some arguments I suspect that might make a difference :shrug: It just isn’t particularly clear to me if people generally just write their flag and flag_if_supporteds out in a randomish order, or if they anticipate that the constructed CLI will roughly match the order of these methods.