Accept multiple --error-format flags

[Xanewok]

This makes the compiler accept multiple --error-format flags with the last one taking precedence.
Fixes the error: Option 'error-format' given more than once error when RLS is used together with pipelined build feature enabled in cargo; see #60988 (comment) for more context and rationale.

r? @alexcrichton

Closes rust-lang/rls#1471
Closes rust-lang/rls#1484

[alexcrichton]

This looks good to me implementation-wise but I don't think I can personally sign off on this becoming insta-stable, could I perhaps request that a @rust-lang/compiler folk proposes stabilization for this via FCP?

[oli-obk]

Couldn't we also hide this behind an unstable flag that drivers can inject? Something like -Zlenient-flag-duplication-processing or even something that can be passed just by drivers as an argument

Other tools have similar requirements. E.g. clippy and miri use --sysroot, but only if it isn't already set: https://github.com/rust-lang/rust-clippy/blob/e3cb40e4f7e566ffe260b2b9d606485c7c22d642/src/driver.rs#L270

[oli-obk]

I feel like this is abusing the multi argument config. While getopts doesn't support this kind of "pick last" feature, maybe it would make more sense to teach it about it than work around it here by suggesting that giving an argument multiple times makes sense.

[eddyb]

We could also stop using getopts, it's not that great.

[Xanewok]

I think we'd either have to

• duplicate the logic and create a separate multi*_last which only differs by returning optionally a single element (picked last) or
• introduce some kind of strategy to the multi* but different strategies might have different requirements and return different values (single or multiple)

which I think is more trouble than it's worth in this case?

[Xanewok]

Other tools have similar requirements

FWIW RLS also has the same problem with sysroot and we also merge our custom config rustflags by sticking them at the end and deduping the entire thing, which I think is a good another motivating use case.

For some prior art to this kind of change, e.g. GCC accepts last -O value https://stackoverflow.com/questions/5557261/gcc-multiple-optimization-flags, so does ls with --color etc.

[Xanewok]

Should I add -Zlenient-flag-duplication-processing? If so, which flags should it affect?

[alexcrichton]

FWIW I personally consider it sort of a misfeature how lenient gcc/clang are about option parsing. It's definitely convenient and useful some of the time, but more often than not I've found that the leniency leads to bugs or surpsing results as to who's taking precedence (vs who wants to take precedence)

Is it the case that both the RLS and Cargo want JSON output? If that's the case rustc could accept multiple and just assert they're all the same, only producing an error if they're different.

[Dylan-DPC-zz]

ping from triage @alexcrichton any updates on this? should it mark as blocked, closed or is it waiting for anything?

[alexcrichton]

This is blocked on figuring out the design for this AFAIK, but I am not going to be able to lead such design.

[JohnCSimon]

Ping from triage @alexcrichton @rust-lang/compiler this has sat idle for a week and a half.
Can someone else be assigned to this?

[JohnTitor]

Ping from triage: @alexcrichton @rust-lang/compiler can someone figure out and lead the design for this?

[Xanewok]

I don’t think it’s required to pursuit this specifically since the underlying error (RLS not working with pipelined cargo) is fixed and I can agree that „last option takes precedence” might not be a silver bullet per Alex’s comment.

I’ll close this for now and come back if I have a better design in mind.

[jyn514]

This also is an issue if I want to control the error format, because cargo also passes --error-format json.

$ cargo rustc --lib -- --error-format short --verbose
   Compiling parser v0.1.0 (/home/joshua/src/rust/rcc/parser)
error: Option 'error-format' given more than once

error: could not compile `parser`.

[jyn514]

Oops, looks like this is --message-format short in Cargo. It's confusing that it's named differently but I guess that's a separate issue.