[PATCH] D146757: [Driver] Enable defining multilib print options independently of flags

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added inline comments.


================
Comment at: clang/include/clang/Driver/Multilib.h:58
+           StringRef IncludeSuffix = {}, const flags_list &Flags = flags_list(),
+           PrintOptionsType PrintOptions = PrintOptionsType::PlusFlags,
+           const flags_list &PrintOptionsList = flags_list());
----------------
peter.smith wrote:
> How would someone using `makeMultilib()` from `MultilibBuilder` use these parameters? If they can't do they need to? If so we may need something like a makeMultilib overload to supply the extra parameters.
MultilibBuilder is constructed around the concept of using '+' and '-' to indicate which command line options are compatible and incompatible, so in that case you'd only want to use PlusFlags.


================
Comment at: clang/lib/Driver/Multilib.cpp:44
+  assert(PrintOptions == PrintOptionsType::List || PrintOptionsList.empty());
+}
+
----------------
peter.smith wrote:
> If there are any restrictions on the strings in PrintOptionsList could be worth assertions. Please ignore if there are no restrictions.
There are restrictions - PrintOptionsList should only contain valid Clang options. However if you violate this then the only consequence is that the -print-multi-lib output will be equally invalid. Since there would be a lot of complexity involved in enforcing the restriction I think it's best to leave it unenforced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146757