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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D95366: [c... Duncan P. N. Exon Smith via Phabricator via cfe-commits

Reply via email to