[PATCH] D151437: [NFC][Driver] Change MultilibBuilder interface

[Michael Platings via Phabricator via cfe-commits]

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


================
Comment at: clang/include/clang/Driver/MultilibBuilder.h:80
+  /// \p Flag must be a flag accepted by the driver.
+  MultilibBuilder &flag(bool Required, StringRef Flag);
 
----------------
phosek wrote:
> I think it would be cleaner to swap the order of arguments and give the boolean argument a default value. When setting the optional argument, ideally we would always use put the argument in the comment which is a standard convention in LLVM.
> 
> I also think that `Required` is not the best name because it might suggest that `Required=false` means that the argument is optional. A better name might be something like `Negate` or `Disallow` which would also match the `!` notation.
> 
> Here's a concrete example of what I have in mind:
> 
> ```
>   Multilibs.push_back(MultilibBuilder("asan+noexcept", {}, {})
>                           .flag("-fsanitize=address")
>                           .flag("-fexceptions", /*Negate=*/false)
>                           .flag("-fno-exceptions")
>                           .makeMultilib());
> ```
OK, I've created D152353

`/*Negate=*/false` would mean the flag is not negated so I've changed that to `/*Negate=*/true`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151437