[PATCH] D40127: [Driver][ARM] For assembler files recognize -Xassembler or -Wa, -mthumb

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

Would be nice to rename the variable prior to commit.



================
Comment at: lib/Driver/ToolChain.cpp:549-556
+    bool IsIntegratedAssemblerThumb = false;
+    for (const Arg *A :
+         Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) {
+      for (StringRef Value : A->getValues()) {
+        if (Value == "-mthumb")
+          IsIntegratedAssemblerThumb = true;
+      }
----------------
peter.smith wrote:
> compnerd wrote:
> > Why not write this as:
> > 
> >     const auto &Values = Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler);
> >     bool IsIntegratedAssemblerThumb =
> >         std::any_of(std::begin(A->getValues()),
> >                     std::end(A->getValues()),
> >                     [](StringRef Arg) { return Arg == "-mthumb"});
> I gave it a try and I don't think it can be done simply as there are two loops and not one. There could be two Args, one for Wa_COMMA and one for X_Assembler. Unless there is a way of combining the iterator ranges into one I don't think this can be done without nested loops. I checked the other uses of Args.filtered and I couldn't find any of use outside of the range for. Please do let me know if I'm missing something though as I'm happy to change it if there is something better. 
I should've hoisted the `->getValues()`, but I suspect that would work.  But that is a slight tweak, not anything to hold this up on.


================
Comment at: lib/Driver/ToolChain.cpp:547
+    // passed to the assember via -Wa or -Xassembler.
+    bool IsMThumb = false;
+    if (InputType != types::TY_PP_Asm)
----------------
I think that this should be `IsThumb`.  The `-m` is just an indicator that this is a machine flag.


https://reviews.llvm.org/D40127