jansvoboda11 added inline comments.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3093
+
+static bool ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
DiagnosticsEngine &Diags,
----------------
dexonsmith wrote:
> 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.
I thought about it and decided to keep it the same. We'd be renaming it back to
`ParsePreprocessorArgs` in a few weeks, when we round-trip the whole
CompilerInvocation at once and the need for the fine-grained interposed
functions disappears.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3215-3216
+
+ return RoundTrip(Parse, Generate, Swap, Res, Args, Diags,
+ "PreprocessorOptions");
}
----------------
dexonsmith wrote:
> 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.
In this case, where one of the lambdas contains a long comment, I suggest
keeping it separate. It's easier to read that way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95366/new/
https://reviews.llvm.org/D95366
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
