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