[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