[clang][cli] Generate and round-trip preprocessor options

[jansvoboda11]

Summary

This patch implements generation of remaining preprocessor options and tests it by performing parse-generate-parse round trip.

[jansvoboda11]

Rebase

[dexonsmith]

LGTM.

clang/lib/Frontend/CompilerInvocation.cpp

3189

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.

3325–3326

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.

[jansvoboda11]

clang/lib/Frontend/CompilerInvocation.cpp

3189

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.

3325–3326

In this case, where one of the lambdas contains a long comment, I suggest keeping it separate. It's easier to read that way.

[jansvoboda11]

Rename variable

[jansvoboda11]

Rebase, rename parse function

clang/include/clang/Driver/Options.td

class PreprocessorOutputOpts<string base>
   : KeyPathAndMacro<"PreprocessorOutputOpts.", base> {}
 class DependencyOutputOpts<string base>
   : KeyPathAndMacro<"DependencyOutputOpts.", base> {}
 class CodeGenOpts<string base>
   : KeyPathAndMacro<"CodeGenOpts.", base, "CODEGEN_"> {}
 class HeaderSearchOpts<string base>
   : KeyPathAndMacro<"HeaderSearchOpts->", base, "HEADER_SEARCH_"> {}
 class PreprocessorOpts<string base>
-  : KeyPathAndMacro<"PreprocessorOpts->", base> {}
+  : KeyPathAndMacro<"PreprocessorOpts->", base, "PREPROCESSOR_"> {}
 class FileSystemOpts<string base>
   : KeyPathAndMacro<"FileSystemOpts.", base> {}
 class AnalyzerOpts<string base>
   : KeyPathAndMacro<"AnalyzerOpts->", base, "ANALYZER_"> {}
 class MigratorOpts<string base>
   : KeyPathAndMacro<"MigratorOpts.", base> {}
 
 // A boolean option which is opt-in in CC1. The positive option exists in CC1 and

clang/lib/Frontend/CompilerInvocation.cpp

static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
   case frontend::RewriteMacros:
   case frontend::RunPreprocessorOnly:
   case frontend::PrintDependencyDirectivesSourceMinimizerOutput:
     return true;
   }
   llvm_unreachable("invalid frontend action");
 }
 
-static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
+static void GeneratePreprocessorArgs(PreprocessorOptions &Opts,
+                                     SmallVectorImpl<const char *> &Args,
+                                     CompilerInvocation::StringAllocator SA,
+                                     const LangOptions &LangOpts,
+                                     const FrontendOptions &FrontendOpts,
+                                     const CodeGenOptions &CodeGenOpts) {
+  PreprocessorOptions *PreprocessorOpts = &Opts;
+
+#define PREPROCESSOR_OPTION_WITH_MARSHALLING(                                  \
+    PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
+    HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+    DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER,     \
+    MERGER, EXTRACTOR, TABLE_INDEX)                                            \
+  GENERATE_OPTION_WITH_MARSHALLING(                                            \
+      Args, SA, KIND, FLAGS, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,    \
+      IMPLIED_CHECK, IMPLIED_VALUE, DENORMALIZER, EXTRACTOR, TABLE_INDEX)
+#include "clang/Driver/Options.inc"
+#undef PREPROCESSOR_OPTION_WITH_MARSHALLING
+
+  if (Opts.PCHWithHdrStop && !Opts.PCHWithHdrStopCreate)
+    GenerateArg(Args, OPT_pch_through_hdrstop_use, SA);
+
+  for (const auto &D : Opts.DeserializedPCHDeclsToErrorOn)
+    GenerateArg(Args, OPT_error_on_deserialized_pch_decl, D, SA);
+
+  for (const auto &MP : Opts.MacroPrefixMap)
+    GenerateArg(Args, OPT_fmacro_prefix_map_EQ, MP.first + "=" + MP.second, SA);
+
+  if (Opts.PrecompiledPreambleBytes != std::make_pair(0u, false))
+    GenerateArg(Args, OPT_preamble_bytes_EQ,
+                Twine(Opts.PrecompiledPreambleBytes.first) + "," +
+                    (Opts.PrecompiledPreambleBytes.second ? "1" : "0"),
+                SA);
+
+  for (const auto &M : Opts.Macros) {
+    // Don't generate __CET__ macro definitions. They are implied by the
+    // -fcf-protection option that is generated elsewhere.
+    if (M.first == "__CET__=1" && !M.second &&
+        !CodeGenOpts.CFProtectionReturn && CodeGenOpts.CFProtectionBranch)
+      continue;
+    if (M.first == "__CET__=2" && !M.second && CodeGenOpts.CFProtectionReturn &&
+        !CodeGenOpts.CFProtectionBranch)
+      continue;
+    if (M.first == "__CET__=3" && !M.second && CodeGenOpts.CFProtectionReturn &&
+        CodeGenOpts.CFProtectionBranch)
+      continue;
+
+    GenerateArg(Args, M.second ? OPT_U : OPT_D, M.first, SA);
+  }
+
+  for (const auto &I : Opts.Includes) {
+    // Don't generate OpenCL includes. They are implied by other flags that are
+    // generated elsewhere.
+    if (LangOpts.OpenCL && LangOpts.IncludeDefaultHeader &&
+        ((LangOpts.DeclareOpenCLBuiltins && I == "opencl-c-base.h") ||
+         I == "opencl-c.h"))
+      continue;
+
+    GenerateArg(Args, OPT_include, I, SA);
+  }
+
+  for (const auto &CI : Opts.ChainedIncludes)
+    GenerateArg(Args, OPT_chain_include, CI, SA);
+
+  for (const auto &RF : Opts.RemappedFiles)
+    GenerateArg(Args, OPT_remap_file, RF.first + ";" + RF.second, SA);
+
+  // Don't handle LexEditorPlaceholders. It is implied by the action that is
+  // generated elsewhere.
+}
+
+static bool ParsePreprocessorArgsImpl(PreprocessorOptions &Opts, ArgList &Args,
                                       DiagnosticsEngine &Diags,
-                                  frontend::ActionKind Action) {
+                                      frontend::ActionKind Action,
+                                      const FrontendOptions &FrontendOpts) {
+  PreprocessorOptions *PreprocessorOpts = &Opts;
+  bool Success = true;
+
+#define PREPROCESSOR_OPTION_WITH_MARSHALLING(                                  \
+    PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
+    HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH,   \
+    DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER,     \
+    MERGER, EXTRACTOR, TABLE_INDEX)                                            \
+  PARSE_OPTION_WITH_MARSHALLING(Args, Diags, Success, ID, FLAGS, PARAM,        \
+                                SHOULD_PARSE, KEYPATH, DEFAULT_VALUE,          \
+                                IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER,      \
+                                MERGER, TABLE_INDEX)
+#include "clang/Driver/Options.inc"
+#undef PREPROCESSOR_OPTION_WITH_MARSHALLING
+
   Opts.PCHWithHdrStop = Args.hasArg(OPT_pch_through_hdrstop_create) ||
                         Args.hasArg(OPT_pch_through_hdrstop_use);
 
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
     Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
   for (const auto &A : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ)) {
     auto Split = StringRef(A).split('=');
for (const auto *A : Args.filtered(OPT_remap_file)) {
     Opts.addRemappedFile(Split.first, Split.second);
   }
 
   // Always avoid lexing editor placeholders when we're just running the
   // preprocessor as we never want to emit the
   // "editor placeholder in source file" error in PP only mode.
   if (isStrictlyPreprocessorAction(Action))
     Opts.LexEditorPlaceholders = false;
+
+  return Success;
+}
+
+static bool ParsePreprocessorArgs(CompilerInvocation &Res,
+                                  PreprocessorOptions &Opts, ArgList &Args,
+                                  DiagnosticsEngine &Diags,
+                                  frontend::ActionKind Action,
+                                  FrontendOptions &FrontendOpts) {
+  auto DummyOpts = std::make_shared<PreprocessorOptions>();
+
+  auto Parse = [Action](CompilerInvocation &Res, ArgList &Args,
+                        DiagnosticsEngine &Diags) {
+    return ParsePreprocessorArgsImpl(Res.getPreprocessorOpts(), Args, Diags,
+                                     Action, Res.getFrontendOpts());
+  };
+
+  auto Generate = [&Args](CompilerInvocation &Res,
+                     SmallVectorImpl<const char *> &GeneratedArgs,
+                     CompilerInvocation::StringAllocator SA) {
+    GeneratePreprocessorArgs(Res.getPreprocessorOpts(), GeneratedArgs, SA,
+                             *Res.getLangOpts(), Res.getFrontendOpts(),
+                             Res.getCodeGenOpts());
+    // The ParsePreprocessorArgs function queries the -fcf-protection option,
+    // which means that it won't be directly copied during argument generation.
+    // The GeneratePreprocessorArgs function isn't responsible for generating it
+    // either. This would cause -fcf-protection to get forgotten during
+    // round-trip and the __CET__ macros wouldn't get deduced during second call
+    // to ParsePreprocessorArgs. Let's fix this by generating -fcf-protection
+    // here.
+    // TODO: Remove this once we're doing one big round-trip instead of many
+    // small ones.
+    if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ))
+      GenerateArg(GeneratedArgs, OPT_fcf_protection_EQ, A->getValue(), SA);
+  };
+
+  auto Swap = [&DummyOpts](CompilerInvocation &Res) {
+    std::swap(Res.PreprocessorOpts, DummyOpts);
+  };
+
+  return RoundTrip(Parse, Generate, Swap, Res, Args, Diags,
+                   "PreprocessorOptions");
 }
 
 static void ParsePreprocessorOutputArgs(PreprocessorOutputOptions &Opts,
                                         ArgList &Args,
                                         frontend::ActionKind Action) {
   if (isStrictlyPreprocessorAction(Action))
     Opts.ShowCPP = !Args.hasArg(OPT_dM);
   else
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
   // backend passes depend on the name of the alloca in order to print out
   // names.
   Res.getCodeGenOpts().DiscardValueNames &=
       !LangOpts.Sanitize.has(SanitizerKind::Address) &&
       !LangOpts.Sanitize.has(SanitizerKind::KernelAddress) &&
       !LangOpts.Sanitize.has(SanitizerKind::Memory) &&
       !LangOpts.Sanitize.has(SanitizerKind::KernelMemory);
 
-  ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, Diags,
-                        Res.getFrontendOpts().ProgramAction);
+  ParsePreprocessorArgs(Res, Res.getPreprocessorOpts(), Args, Diags,
+                        Res.getFrontendOpts().ProgramAction,
+                        Res.getFrontendOpts());
   ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
                               Res.getFrontendOpts().ProgramAction);
 
   // Turn on -Wspir-compat for SPIR target.
   if (T.isSPIR())
     Res.getDiagnosticOpts().Warnings.push_back("spir-compat");
 
   // If sanitizer is enabled, disable OPT_ffine_grained_bitfield_accesses.

 
 #include "clang/Driver/Options.inc"
 
 #undef CODEGEN_OPTION_WITH_MARSHALLING
 #undef LANG_OPTION_WITH_MARSHALLING
 #undef DIAG_OPTION_WITH_MARSHALLING
 #undef OPTION_WITH_MARSHALLING
 
+  GeneratePreprocessorArgs(*PreprocessorOpts, Args, SA, *LangOpts,
+                           FrontendOpts, CodeGenOpts);
   GenerateAnalyzerArgs(*AnalyzerOpts, Args, SA);
   GenerateHeaderSearchArgs(*HeaderSearchOpts, Args, SA);
   GenerateLangArgs(*LangOpts, Args, SA);
 }
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 clang::createVFSFromCompilerInvocation(const CompilerInvocation &CI,
                                        DiagnosticsEngine &Diags) {