[clang][cli] Unify boolean marshalling

[jansvoboda11]

Summary

Use lambdas with captures to replace the redundant infrastructure for marshalling of two boolean flags that control the same keypath.

[dexonsmith]

LGTM! This is a nice cleanup.

clang/include/clang/Driver/Options.td

def fno_#NAME : Flag<["-"], "fno-"#name>, Flags<!listconcat([CC1Option], flags)>,
                Group<f_Group>, HelpText<!strconcat(neg_prefix, help)>,
                MarshallingInfoFlag<keypath, "false">,
                ImpliedByAnyOf<disablers, "true">;
 }
 
 multiclass BooleanMarshalledFFlag<string name, code keypath, code default_value, string pos_help = "", string neg_help=""> {
   def fno_#NAME : Flag<["-"], "fno-"#name>, HelpText<neg_help>;
   def f#NAME : Flag<["-"], "f"#name>, HelpText<pos_help>,
-    MarshallingInfoBooleanFlag<keypath, default_value, !cast<Option>("fno_"#NAME)>;
+    MarshallingInfoBooleanFlag<keypath, default_value, "fno_"#NAME, "-fno-"#name>;
 }
 
 /////////
 // Options
 
 // The internal option ID must be a valid C++ identifier and results in a
 // clang::driver::options::OPT_XX enum constant for XX.
 //

clang/lib/Frontend/CompilerInvocation.cpp

 //===- CompilerInvocation.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
static FlagToValueNormalizer<uint64_t> makeFlagToValueNormalizer(T Value) {
   return FlagToValueNormalizer<uint64_t>{Value};
 }
 
 template <typename T, std::enable_if_t<!is_int_convertible<T>(), bool> = false>
 static FlagToValueNormalizer<T> makeFlagToValueNormalizer(T Value) {
   return FlagToValueNormalizer<T>{std::move(Value)};
 }
 
-static Optional<bool> normalizeBooleanFlag(OptSpecifier PosOpt,
-                                           OptSpecifier NegOpt,
-                                           unsigned TableIndex,
-                                           const ArgList &Args,
-                                           DiagnosticsEngine &Diags) {
+static auto makeBooleanFlagNormalizer(OptSpecifier NegOpt) {
+  return [NegOpt](OptSpecifier PosOpt, unsigned, const ArgList &Args,
+                  DiagnosticsEngine &) -> Optional<bool> {
     if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
       return A->getOption().matches(PosOpt);
     return None;
+  };
 }
 
-static void denormalizeBooleanFlag(SmallVectorImpl<const char *> &Args,
-                                   const char *Spelling,
-                                   const char *NegSpelling,
-                                   CompilerInvocation::StringAllocator SA,
-                                   unsigned TableIndex, unsigned Value) {
-  if (Value)
-    Args.push_back(Spelling);
-  else
-    Args.push_back(NegSpelling);
+static auto makeBooleanFlagDenormalizer(const char *NegSpelling) {
+  return [NegSpelling](
+             SmallVectorImpl<const char *> &Args, const char *PosSpelling,
+             CompilerInvocation::StringAllocator, unsigned, unsigned Value) {
+    Args.push_back(Value ? PosSpelling : NegSpelling);
+  };
 }
 
 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];
 
#define OPTION_WITH_MARSHALLING(                                               \
     this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);                      \
     if (IMPLIED_CHECK)                                                         \
       this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE);                    \
     if (auto MaybeValue = NORMALIZER(OPT_##ID, TABLE_INDEX, Args, Diags))      \
       this->KEYPATH = MERGER(                                                  \
           this->KEYPATH, static_cast<decltype(this->KEYPATH)>(*MaybeValue));   \
   }
 
-#define OPTION_WITH_MARSHALLING_BOOLEAN(                                       \
-    PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
-    HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  \
-    IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
-    TABLE_INDEX, NEG_ID, NEG_SPELLING)                                         \
-  {                                                                            \
-    this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);                      \
-    if (IMPLIED_CHECK)                                                         \
-      this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE);                    \
-    if (auto MaybeValue =                                                      \
-            NORMALIZER(OPT_##ID, OPT_##NEG_ID, TABLE_INDEX, Args, Diags))      \
-      this->KEYPATH = MERGER(                                                  \
-          this->KEYPATH, static_cast<decltype(this->KEYPATH)>(*MaybeValue));   \
-  }
-
 #include "clang/Driver/Options.inc"
-#undef OPTION_WITH_MARSHALLING_BOOLEAN
 #undef OPTION_WITH_MARSHALLING
   return true;
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
                                         ArrayRef<const char *> CommandLineArgs,
                                         DiagnosticsEngine &Diags,
                                         const char *Argv0) {
#define OPTION_WITH_MARSHALLING(                                               \
   if ((FLAGS)&options::CC1Option) {                                            \
     [&](const auto &Extracted) {                                               \
       if (ALWAYS_EMIT || (Extracted != ((IMPLIED_CHECK) ? (IMPLIED_VALUE)      \
                                                         : (DEFAULT_VALUE))))   \
         DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);              \
     }(EXTRACTOR(this->KEYPATH));                                               \
   }
 
-#define OPTION_WITH_MARSHALLING_BOOLEAN(                                       \
-    PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
-    HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  \
-    IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
-    TABLE_INDEX, NEG_ID, NEG_SPELLING)                                         \
-  if ((FLAGS)&options::CC1Option) {                                            \
-    bool Extracted = EXTRACTOR(this->KEYPATH);                                 \
-    if (ALWAYS_EMIT ||                                                         \
-        (Extracted != ((IMPLIED_CHECK) ? (IMPLIED_VALUE) : (DEFAULT_VALUE))))  \
-      DENORMALIZER(Args, SPELLING, NEG_SPELLING, SA, TABLE_INDEX, Extracted);  \
-  }
-
 #include "clang/Driver/Options.inc"
-#undef OPTION_WITH_MARSHALLING_BOOLEAN
 #undef OPTION_WITH_MARSHALLING
 }
 
 IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 clang::createVFSFromCompilerInvocation(const CompilerInvocation &CI,
                                        DiagnosticsEngine &Diags) {
   return createVFSFromCompilerInvocation(CI, Diags,
                                          llvm::vfs::getRealFileSystem());

llvm/include/llvm/Option/OptParser.td

class Option<list<string> prefixes, string name, OptionKind kind> {
   string HelpText = ?;
   string MetaVarName = ?;
   string Values = ?;
   code ValuesCode = ?;
   list<OptionFlag> Flags = [];
   OptionGroup Group = ?;
   Option Alias = ?;
   list<string> AliasArgs = [];
-  string MarshallingInfoKind = ?;
   code KeyPath = ?;
   code DefaultValue = ?;
   code ImpliedValue = ?;
   code ImpliedCheck = "false";
   bit ShouldAlwaysEmit = false;
   code NormalizerRetTy = ?;
   code NormalizedValuesScope = "";
   code Normalizer = "";
   code Denormalizer = "";
   code ValueMerger = "mergeForwardValue";
   code ValueExtractor = "extractForwardValue";
   list<code> NormalizedValues = ?;
-  // Used for BooleanFlagKind
-  Option NegOption = ?;
 }
 
 // Helpers for defining options.
 
 class Flag<list<string> prefixes, string name>
   : Option<prefixes, name, KIND_FLAG>;
 class Joined<list<string> prefixes, string name>
   : Option<prefixes, name, KIND_JOINED>;

 
 class ImpliedByAnyOf<list<Option> options, code value = "true"> {
   code ImpliedCheck = !foldl("false", options, accumulator, option,
                              !strconcat(accumulator, " || ", !cast<string>(option.KeyPath)));
   code ImpliedValue = value;
 }
 
 class MarshallingInfo<code keypath, code defaultvalue> {
-  string MarshallingInfoKind = "Default";
   code KeyPath = keypath;
   code DefaultValue = defaultvalue;
 }
 
 class MarshallingInfoString<code keypath, code defaultvalue>
   : MarshallingInfo<keypath, defaultvalue> {}
 
 class MarshallingInfoFlag<code keypath, code defaultvalue = "false">
   : MarshallingInfo<keypath, defaultvalue> {
   code Normalizer = "normalizeSimpleFlag";
   code Denormalizer = "denormalizeSimpleFlag";
 }
 
 class MarshallingInfoBitfieldFlag<code keypath, code value>
   : MarshallingInfoFlag<keypath, "0u"> {
   code Normalizer = "makeFlagToValueNormalizer("#value#")";
   code ValueMerger = "mergeMaskValue";
   code ValueExtractor = "(extractMaskValue<unsigned, decltype("#value#"), "#value#">)";
 }
 
-class MarshallingInfoBooleanFlag<code keypath, code defaultvalue, Option negopt>
+class MarshallingInfoBooleanFlag<code keypath, code defaultvalue, code neg_name, string neg_spelling>
   : MarshallingInfoFlag<keypath, defaultvalue> {
   bit ShouldAlwaysEmit = 1;
-  string MarshallingInfoKind = "BooleanFlag";
-  code Normalizer = "normalizeBooleanFlag";
-  code Denormalizer = "denormalizeBooleanFlag";
-  Option NegOption = negopt;
+  code Normalizer = "makeBooleanFlagNormalizer(OPT_"#neg_name#")";
+  code Denormalizer = "makeBooleanFlagDenormalizer(\""#neg_spelling#"\")";
 }
 
 // Mixins for additional marshalling attributes.
 
 class IsNegative {
   code Normalizer = "normalizeSimpleNegativeFlag";
 }
 class AlwaysEmit { bit ShouldAlwaysEmit = true; }

llvm/utils/TableGen/OptParserEmitter.cpp

 //===- OptParserEmitter.cpp - Table Driven Command Line Parsing -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
static void emitNameUsingSpelling(raw_ostream &OS, const Record &R) {
   size_t PrefixLength;
   OS << "&";
   write_cstring(OS, StringRef(getOptionSpelling(R, PrefixLength)));
   OS << "[" << PrefixLength << "]";
 }
 
 class MarshallingInfo {
 public:
-  using Ptr = std::unique_ptr<MarshallingInfo>;
-
-  const char *MacroName;
+  static constexpr const char *MacroName = "OPTION_WITH_MARSHALLING";
   const Record &R;
   bool ShouldAlwaysEmit;
   StringRef KeyPath;
   StringRef DefaultValue;
   StringRef NormalizedValuesScope;
   StringRef ImpliedCheck;
   StringRef ImpliedValue;
   StringRef Normalizer;
struct SimpleEnumValueTable {
   const SimpleEnumValue *Table;
   unsigned Size;
 };
 )";
 
   static constexpr const char *ValueTablesDecl =
       "static const SimpleEnumValueTable SimpleEnumValueTables[] = ";
 
-  MarshallingInfo(const Record &R)
-      : MacroName("OPTION_WITH_MARSHALLING"), R(R) {}
-  MarshallingInfo(const char *MacroName, const Record &R)
-      : MacroName(MacroName), R(R){};
-
-  virtual ~MarshallingInfo() = default;
+  MarshallingInfo(const Record &R) : R(R) {}
 
-  virtual void emit(raw_ostream &OS) const {
+  void emit(raw_ostream &OS) const {
     write_cstring(OS, StringRef(getOptionSpelling(R)));
     OS << ", ";
     OS << ShouldAlwaysEmit;
     OS << ", ";
     OS << KeyPath;
     OS << ", ";
     emitScopedNormalizedValue(OS, DefaultValue);
     OS << ", ";
void emitScopedNormalizedValue(raw_ostream &OS,
     if (!NormalizedValuesScope.empty())
       OS << NormalizedValuesScope << "::";
     OS << NormalizedValue;
   }
 };
 
 size_t MarshallingInfo::NextTableIndex = 0;
 
-class MarshallingInfoBooleanFlag : public MarshallingInfo {
-public:
-  const Record &NegOption;
-
-  MarshallingInfoBooleanFlag(const Record &Option, const Record &NegOption)
-      : MarshallingInfo("OPTION_WITH_MARSHALLING_BOOLEAN", Option),
-        NegOption(NegOption) {}
-
-  void emit(raw_ostream &OS) const override {
-    MarshallingInfo::emit(OS);
-    OS << ", ";
-    OS << getOptionName(NegOption);
-    OS << ", ";
-    write_cstring(OS, getOptionSpelling(NegOption));
-  }
-};
-
-static MarshallingInfo::Ptr createMarshallingInfo(const Record &R) {
+static MarshallingInfo createMarshallingInfo(const Record &R) {
   assert(!isa<UnsetInit>(R.getValueInit("KeyPath")) &&
          !isa<UnsetInit>(R.getValueInit("DefaultValue")) &&
          !isa<UnsetInit>(R.getValueInit("ValueMerger")) &&
          "MarshallingInfo must have a provide a keypath, default value and a "
          "value merger");
 
-  MarshallingInfo::Ptr Ret;
-  StringRef KindString = R.getValueAsString("MarshallingInfoKind");
-  if (KindString == "Default") {
-    Ret = std::make_unique<MarshallingInfo>(R);
-  } else if (KindString == "BooleanFlag") {
-    Ret = std::make_unique<MarshallingInfoBooleanFlag>(
-        R, *R.getValueAsDef("NegOption"));
-  }
-
-  Ret->ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
-  Ret->KeyPath = R.getValueAsString("KeyPath");
-  Ret->DefaultValue = R.getValueAsString("DefaultValue");
-  Ret->NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
-  Ret->ImpliedCheck = R.getValueAsString("ImpliedCheck");
-  Ret->ImpliedValue =
-      R.getValueAsOptionalString("ImpliedValue").getValueOr(Ret->DefaultValue);
-
-  Ret->Normalizer = R.getValueAsString("Normalizer");
-  Ret->Denormalizer = R.getValueAsString("Denormalizer");
-  Ret->ValueMerger = R.getValueAsString("ValueMerger");
-  Ret->ValueExtractor = R.getValueAsString("ValueExtractor");
+  MarshallingInfo Ret(R);
+
+  Ret.ShouldAlwaysEmit = R.getValueAsBit("ShouldAlwaysEmit");
+  Ret.KeyPath = R.getValueAsString("KeyPath");
+  Ret.DefaultValue = R.getValueAsString("DefaultValue");
+  Ret.NormalizedValuesScope = R.getValueAsString("NormalizedValuesScope");
+  Ret.ImpliedCheck = R.getValueAsString("ImpliedCheck");
+  Ret.ImpliedValue =
+      R.getValueAsOptionalString("ImpliedValue").getValueOr(Ret.DefaultValue);
+
+  Ret.Normalizer = R.getValueAsString("Normalizer");
+  Ret.Denormalizer = R.getValueAsString("Denormalizer");
+  Ret.ValueMerger = R.getValueAsString("ValueMerger");
+  Ret.ValueExtractor = R.getValueAsString("ValueExtractor");
 
   if (!isa<UnsetInit>(R.getValueInit("NormalizedValues"))) {
     assert(!isa<UnsetInit>(R.getValueInit("Values")) &&
            "Cannot provide normalized values for value-less options");
-    Ret->TableIndex = MarshallingInfo::NextTableIndex++;
-    Ret->NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
-    Ret->Values.reserve(Ret->NormalizedValues.size());
-    Ret->ValueTableName = getOptionName(R) + "ValueTable";
+    Ret.TableIndex = MarshallingInfo::NextTableIndex++;
+    Ret.NormalizedValues = R.getValueAsListOfStrings("NormalizedValues");
+    Ret.Values.reserve(Ret.NormalizedValues.size());
+    Ret.ValueTableName = getOptionName(R) + "ValueTable";
 
     StringRef ValuesStr = R.getValueAsString("Values");
     for (;;) {
       size_t Idx = ValuesStr.find(',');
       if (Idx == StringRef::npos)
         break;
       if (Idx > 0)
-        Ret->Values.push_back(ValuesStr.slice(0, Idx));
+        Ret.Values.push_back(ValuesStr.slice(0, Idx));
       ValuesStr = ValuesStr.slice(Idx + 1, StringRef::npos);
     }
     if (!ValuesStr.empty())
-      Ret->Values.push_back(ValuesStr);
+      Ret.Values.push_back(ValuesStr);
 
-    assert(Ret->Values.size() == Ret->NormalizedValues.size() &&
+    assert(Ret.Values.size() == Ret.NormalizedValues.size() &&
            "The number of normalized values doesn't match the number of "
            "values");
   }
 
-  // FIXME: This is a workaround for a bug in older versions of clang (< 3.9)
-  //   The constructor that is supposed to allow for Derived to Base
-  //   conversion does not work. Remove this if we drop support for such
-  //   configurations.
-  return MarshallingInfo::Ptr(Ret.release());
+  return Ret;
 }
 
 /// OptParserEmitter - This tablegen backend takes an input .td file
 /// describing a list of options and emits a data structure for parsing and
 /// working with those options when given an input command line.
 namespace llvm {
 void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
   // Get the option groups and options.
void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
   };
   // The RecordKeeper stores records (options) in lexicographical order, and we
   // have reordered the options again when generating prefix groups. We need to
   // restore the original definition order of options with marshalling to honor
   // the topology of the dependency graph implied by `DefaultAnyOf`.
   array_pod_sort(OptsWithMarshalling.begin(), OptsWithMarshalling.end(),
                  CmpMarshallingOpts);
 
-  std::vector<MarshallingInfo::Ptr> MarshallingInfos;
+  std::vector<MarshallingInfo> MarshallingInfos;
   for (const auto *R : OptsWithMarshalling)
     MarshallingInfos.push_back(createMarshallingInfo(*R));
 
   for (const auto &MI : MarshallingInfos) {
-    OS << "#ifdef " << MI->MacroName << "\n";
-    OS << MI->MacroName << "(";
-    WriteOptRecordFields(OS, MI->R);
+    OS << "#ifdef " << MarshallingInfo::MacroName << "\n";
+    OS << MarshallingInfo::MacroName << "(";
+    WriteOptRecordFields(OS, MI.R);
     OS << ", ";
-    MI->emit(OS);
+    MI.emit(OS);
     OS << ")\n";
-    OS << "#endif // " << MI->MacroName << "\n";
+    OS << "#endif // " << MarshallingInfo::MacroName << "\n";
   }
 
   OS << "\n";
   OS << "#ifdef SIMPLE_ENUM_VALUE_TABLE";
   OS << "\n";
   OS << MarshallingInfo::ValueTablePreamble;
   std::vector<StringRef> ValueTableNames;
   for (const auto &MI : MarshallingInfos)
-    if (auto MaybeValueTableName = MI->emitValueTable(OS))
+    if (auto MaybeValueTableName = MI.emitValueTable(OS))
       ValueTableNames.push_back(*MaybeValueTableName);
 
   OS << MarshallingInfo::ValueTablesDecl << "{";
   for (auto ValueTableName : ValueTableNames)
     OS << "{" << ValueTableName << ", sizeof(" << ValueTableName
        << ") / sizeof(SimpleEnumValue)"
        << "},\n";
   OS << "};\n";