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

Mon Mar 13 10:49:51 PDT 2023

michaelplatings marked an inline comment as done.
michaelplatings added a comment.

Thanks @peter.smith. I've opted to leave the comment as-is. If we can expect a tag_set to actually contain flags then I've continued to use that terminology.

================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:202
+                     Multilib::tag_set &Flags);

 void addX86AlignBranchArgs(const Driver &D, const llvm::opt::ArgList &Args,
----------------
peter.smith wrote:
> 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.
For the way this function is used I would expect `Flags` to be command line flags prefixed with `+` or `-`, the same as for the `Flag` parameter. Therefore I think it best to leave it as-is.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145567