[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/include/clang/Driver/CC1Options.td:216-221
 def mrelocation_model : Separate<["-"], "mrelocation-model">,
-  HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">;
+  HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">,
+  MarshallingInfoString<"CodeGenOpts.RelocationModel", "llvm::Reloc::PIC_">,
+  Normalizer<"normalizeRelocationModel">, Denormalizer<"denormalizeRelocationModel">,
+  ValuesAssociatedDefinitions<["llvm::Reloc::Static", "llvm::Reloc::PIC_", "llvm::Reloc::ROPI",
+  "llvm::Reloc::RWPI", "llvm::Reloc::ROPI_RWPI", "llvm::Reloc::DynamicNoPIC"]>;
----------------
Maybe you can avoid boilerplate with something like this:
```
ValuesAssociatedDefinitionsScope<"llvm::Reloc">,
ValuesAssociatedDefinitions<["Static", "PIC_", "RPOI", "RWPI", "ROPI_RWPI", "DynamicNoPIC"]>,
MarshallingInfoString<"CodeGenOpts.RelocationModel", "PIC_">,
```
Note that I shortened the second argument to `MarshallingInfoString` as well.

@Bigcheese, WDYT?

Also wondering if `ValuesAssociatedDefinitions` could be shorter. Maybe `NormalizedValues` (and then `NormalizedValuesScope`)?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:125-127
+static std::string normalizeTriple(const Arg *Arg, const ArgList &ArgList,
+                                   DiagnosticsEngine &Diags,
+                                   StringRef DefaultTriple) {
----------------
Should we drop parameter names for `ArgList`, `Diags`, and `DefaultTriple`?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:150-158
+static const char *denormalizeRelocationModel(llvm::Reloc::Model RM) {
+  switch (RM) {
+#define HANDLE_MRELOCATION_MODEL_VALUES(V, D)                                  \
+  case D:                                                                      \
+    return V;
+#include "clang/Driver/Options.inc"
+#undef HANDLE_MRELOCATION_MODEL_VALUES
----------------
This looks like boilerplate that's going to be repeated a lot. I'm curious how many of the enum options will end up like this.

If more than a few, maybe we can add a `.inc` file that includes these function definitions? Triggered by adding:
```
AutoNormalizeEnum<"llvm::Reloc::Model", "RelocationModel">,
```
or something (instead of `Normalizer<>` and `Denormalizer<>`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79796/new/

https://reviews.llvm.org/D79796