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

Petr Hosek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 6 22:25:41 PDT 2023


phosek 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);
 
----------------
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());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151437



More information about the cfe-commits mailing list