[PATCH] D95366: [clang][cli] Generate and round-trip preprocessor options
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 28 12:06:34 PST 2021
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.
LGTM.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3093
+
+static bool ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
DiagnosticsEngine &Diags,
----------------
Can we name this differently, so it's obvious which is being called without looking at the argument list? I suggest `ParsePreprocessorArgsImpl` for this one, since it's doing the actual parsing.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3215-3216
+
+ return RoundTrip(Parse, Generate, Swap, Res, Args, Diags,
+ "PreprocessorOptions");
}
----------------
Have you considered just defining the lambdas inline in the call to `RoundTrip`? I'm fine either way, but the way clang-format tends to clean this up seems pretty readable to me, and the names don't really add much value since they match the functions being called. Up to you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95366/new/
https://reviews.llvm.org/D95366
More information about the cfe-commits
mailing list