dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.
This is close, just a few nits. Also, since this has the first uses of
`IsNegative`, it'd be great to have a couple of tests for one of the flags it's
used on.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:137
+static llvm::Optional<bool>
+normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned TableIndex,
----------------
Nit: clang/Basic/LLVM.h pulls Optional in `namespace clang` so you don't need
the `llvm::` qualifier.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:138-139
+static llvm::Optional<bool>
+normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned TableIndex,
+ const ArgList &Args, DiagnosticsEngine &Diags) {
+ if (Args.hasArg(Opt))
----------------
Please drop parameter names for `TableIndex` and `Diags`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83697/new/
https://reviews.llvm.org/D83697
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
