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