[PATCH] D145567: [Driver] Rename multilib flags to tags

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I've set approved from the Arm side. Please leave some time for people in the US time zone to leave any final comments or ask for extensions. 
>From looking at the source code alone - assuming that most people would not track down the commit message/review for extra context - I found it difficult to work out the convention for when Flags is used and when Tags is used. I've made a suggestion for some comments. This can be applied prior to commit if you want to take it up.



================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:202
+                     Multilib::tag_set &Flags);
 
 void addX86AlignBranchArgs(const Driver &D, const llvm::opt::ArgList &Args,
----------------
I can see the reason to keep the name `addMultilibFlag`. At this point is the tag_set expected to be simplified tags or full command line flags. If it is the former I think it would be good to change Flags to Tags here.

May also be useul to add a \p for Flags (or Tags) if there are any requirements, or just useful information on what it is expected to be.

Parameter name also applies to CommonArgs.cpp below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145567