[clang][cli] Let denormalizer decide how to render the option based on the option class

[jansvoboda11]

Summary

Before this patch, you needed to use AutoNormalizeEnumJoined whenever you wanted to denormalize joined enum.
Besides the naming confusion, this means the fact the option is joined is specified in two places: in the normalization multiclass and in the Joined<["-"], ...> multiclass.
This patch makes this work automatically, taking into account the OptionClass of options.

Also, the enum denormalizer now just looks up the spelling of the present enum case in a table and forwards it to the string denormalizer.

I also added more tests that exercise this.

[jansvoboda11]

Taking over this patch, as Daniel is no longer involved.

[jansvoboda11]

Rebase, add tests

[dexonsmith]

LGTM.

clang/include/clang/Driver/Options.td

def ast_dump_lookups : Flag<["-"], "ast-dump-lookups">,
   Group<Action_Group>,
   HelpText<"Build ASTs and then debug dump their name lookup tables">,
   MarshallingInfoFlag<"FrontendOpts.ASTDumpLookups", "false">;
 def arcmt_action_EQ : Joined<["-"], "arcmt-action=">, Flags<[CC1Option, NoDriverOption]>,
   HelpText<"The ARC migration action to take">, Values<"check,modify,migrate">,
   NormalizedValuesScope<"FrontendOptions">,
   NormalizedValues<["ARCMT_Check", "ARCMT_Modify", "ARCMT_Migrate"]>,
   MarshallingInfoString<"FrontendOpts.ARCMTAction", "ARCMT_None">,
-  AutoNormalizeEnumJoined;
+  AutoNormalizeEnum;
 def print_stats : Flag<["-"], "print-stats">,
   HelpText<"Print performance metrics and statistics">,
   MarshallingInfoFlag<"FrontendOpts.ShowStats", "false">;
 def fix_what_you_can : Flag<["-"], "fix-what-you-can">,
   HelpText<"Apply fix-it advice even in the presence of unfixable errors">,
   MarshallingInfoFlag<"FrontendOpts.FixWhatYouCan", "false">;
 def fix_only_warnings : Flag<["-"], "fix-only-warnings">,
   HelpText<"Apply fix-it advice only for warnings, not errors">,

clang/lib/Frontend/CompilerInvocation.cpp

normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned TableIndex,
                             const ArgList &Args, DiagnosticsEngine &Diags) {
   return !Args.hasArg(Opt);
 }
 
 template <typename T>
 void denormalizeSimpleFlag(SmallVectorImpl<const char *> &Args,
                            const char *Spelling,
                            CompilerInvocation::StringAllocator SA,
-                           unsigned TableIndex, T Value) {
+                           Option::OptionClass OptClass, unsigned TableIndex,
+                           T Value) {
   Args.push_back(Spelling);
 }
 
 template <typename T, T Value>
 static llvm::Optional<T>
 normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList &Args,
                      DiagnosticsEngine &Diags) {
   if (Args.hasArg(Opt))
static void denormalizeBooleanFlag(SmallVectorImpl<const char *> &Args,
                                    CompilerInvocation::StringAllocator SA,
                                    unsigned TableIndex, unsigned Value) {
   if (Value)
     Args.push_back(Spelling);
   else
     Args.push_back(NegSpelling);
 }
 
+static Optional<std::string> normalizeString(OptSpecifier Opt, int TableIndex,
+                                             const ArgList &Args,
+                                             DiagnosticsEngine &Diags) {
+  auto *Arg = Args.getLastArg(Opt);
+  if (!Arg)
+    return None;
+  return std::string(Arg->getValue());
+}
+
+template <typename T>
+static void denormalizeString(SmallVectorImpl<const char *> &Args,
+                              const char *Spelling,
+                              CompilerInvocation::StringAllocator SA,
+                              Option::OptionClass OptClass, unsigned TableIndex,
+                              T &&Value) {
+  static_assert(std::is_constructible<Twine, T>::value,
+                "Cannot convert this value to Twine");
+  switch (OptClass) {
+  case Option::SeparateClass:
+  case Option::JoinedOrSeparateClass:
+    Args.push_back(Spelling);
+    Args.push_back(SA(Twine(std::forward<T>(Value))));
+    break;
+  case Option::JoinedClass:
+    Args.push_back(SA(Twine(Spelling) + Twine(std::forward<T>(Value))));
+    break;
+  default:
+    llvm_unreachable("Cannot denormalize an option with option class "
+                     "incompatible with string denormalization.");
+  }
+}
+
 static Optional<SimpleEnumValue>
 findValueTableByName(const SimpleEnumValueTable &Table, StringRef Name) {
   for (int I = 0, E = Table.Size; I != E; ++I)
     if (Name == Table.Table[I].Name)
       return Table.Table[I];
 
   return None;
 }
static llvm::Optional<unsigned> normalizeSimpleEnum(OptSpecifier Opt,
   Diags.Report(diag::err_drv_invalid_value)
       << Arg->getAsString(Args) << ArgValue;
   return None;
 }
 
 static void denormalizeSimpleEnum(SmallVectorImpl<const char *> &Args,
                                   const char *Spelling,
                                   CompilerInvocation::StringAllocator SA,
+                                  Option::OptionClass OptClass,
                                   unsigned TableIndex, unsigned Value) {
   assert(TableIndex < SimpleEnumValueTablesSize);
   const SimpleEnumValueTable &Table = SimpleEnumValueTables[TableIndex];
   if (auto MaybeEnumVal = findValueTableByValue(Table, Value)) {
-    Args.push_back(Spelling);
-    Args.push_back(MaybeEnumVal->Name);
+    denormalizeString(Args, Spelling, SA, OptClass, TableIndex,
+                      MaybeEnumVal->Name);
   } else {
     llvm_unreachable("The simple enum value was not correctly defined in "
                      "the tablegen option description");
   }
 }
 
-static void denormalizeSimpleEnumJoined(SmallVectorImpl<const char *> &Args,
-                                        const char *Spelling,
-                                        CompilerInvocation::StringAllocator SA,
-                                        unsigned TableIndex, unsigned Value) {
-  assert(TableIndex < SimpleEnumValueTablesSize);
-  const SimpleEnumValueTable &Table = SimpleEnumValueTables[TableIndex];
-  if (auto MaybeEnumVal = findValueTableByValue(Table, Value))
-    Args.push_back(SA(Twine(Spelling) + MaybeEnumVal->Name));
-  else
-    llvm_unreachable("The simple enum value was not correctly defined in "
-                     "the tablegen option description");
-}
-
-static Optional<std::string> normalizeString(OptSpecifier Opt, int TableIndex,
-                                             const ArgList &Args,
-                                             DiagnosticsEngine &Diags) {
-  auto *Arg = Args.getLastArg(Opt);
-  if (!Arg)
-    return None;
-  return std::string(Arg->getValue());
-}
-
-template <typename T>
-static void denormalizeString(SmallVectorImpl<const char *> &Args,
-                              const char *Spelling,
-                              CompilerInvocation::StringAllocator SA,
-                              unsigned TableIndex, T &&Value) {
-  static_assert(std::is_constructible<Twine, T>::value,
-                "Cannot convert this value to Twine");
-  Args.push_back(Spelling);
-  Args.push_back(SA(Twine(std::forward<T>(Value))));
-}
-
 template <typename IntTy>
 static Optional<IntTy> normalizeStringIntegral(OptSpecifier Opt, int TableIndex,
                                                const ArgList &Args,
                                                DiagnosticsEngine &Diags) {
   auto *Arg = Args.getLastArg(Opt);
   if (!Arg)
     return None;
   IntTy Res;

 #define OPTION_WITH_MARSHALLING(                                               \
     PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
     HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  \
     NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)                  \
   if ((FLAGS)&options::CC1Option) {                                            \
     const auto &Extracted = EXTRACTOR(this->KEYPATH);                          \
     if (ALWAYS_EMIT ||                                                         \
         static_cast<decltype(DEFAULT_VALUE)>(Extracted) != DEFAULT_VALUE)      \
-      DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);                \
+      DENORMALIZER(Args, SPELLING, SA, Option::KIND##Class, TABLE_INDEX,       \
+                   Extracted);                                                 \
   }
 
 #define OPTION_WITH_MARSHALLING_BOOLEAN(                                       \
     PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
     HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  \
     NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX, NEG_ID,          \
     NEG_SPELLING)                                                              \
   if ((FLAGS)&options::CC1Option) {                                            \

llvm/include/llvm/Option/OptParser.td

 class Normalizer<code normalizer> { code Normalizer = normalizer; }
 class Denormalizer<code denormalizer> { code Denormalizer = denormalizer; }
 class NormalizedValuesScope<code scope> { code NormalizedValuesScope = scope; }
 class NormalizedValues<list<code> definitions> { list<code> NormalizedValues = definitions; } 
 class AutoNormalizeEnum {
   code Normalizer = "normalizeSimpleEnum";
   code Denormalizer = "denormalizeSimpleEnum";
 }
-class AutoNormalizeEnumJoined : AutoNormalizeEnum {
-  code Denormalizer = "denormalizeSimpleEnumJoined";
-}
 class ValueMerger<code merger> { code ValueMerger = merger; }
 class ValueExtractor<code extractor> { code ValueExtractor = extractor; }
 
 // Predefined options.
 
 // FIXME: Have generator validate that these appear in correct position (and
 // aren't duplicated).
 def INPUT : Option<[], "<input>", KIND_INPUT>;
 def UNKNOWN : Option<[], "<unknown>", KIND_UNKNOWN>;