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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
