[PATCH] D102812: [clang] Don't pass multiple backend options if mixing -mimplicit-it and -Wa,-mimplicit-it

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D102812#2770821 <https://reviews.llvm.org/D102812#2770821>, @DavidSpickett wrote:

> Given that using both options currently crashes clang (therefore no one is relying on this yet) and GCC doesn't have `-mimplicit-it` I think what you have is actually the best way to go. If the two options agree, fine, if they don't, error. Let's not make it more complicated than it has to be.

Well it's complicated in the sense that there is an error diagnostic that we need to provide, and all that. Just picking the last set value should be fine too and simplifies the code in that sense.

I'll update the patch with the code in that form, and let you say what you think of that form. If you think we should keep the error though, I'll add it back (and see if I need to add a new diagnostic id for that case, to get a proper error mesasge). It's a bit inconsistent in the sense that one can pass multiple conflicting options via each of the options (where the last one of each wins); gas does handle it in that way too, but two different kinds of options error out instead.



================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2369
 
-static bool AddARMImplicitITArgs(const ArgList &Args, ArgStringList &CmdArgs,
+static bool CheckARMImplicitITArg(StringRef Value) {
+  return Value == "always" || Value == "never" || Value == "arm" ||
----------------
MaskRay wrote:
> Use `functionName` for new functions.
Hmm, the whole rest of the file uses function names starting with a capital, so they do look a bit out of place if adding one new that differs...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102812/new/

https://reviews.llvm.org/D102812