[clang][cli] Port DependencyOutput option flags to new option parsing system

[jansvoboda11]

Summary

Depends on D91861.

[jansvoboda11]

Taking over this revision as Daniel is no longer involved.

[jansvoboda11]

Rebase, undo the move of options, introduce makeFlagToValueNormalizer

[jansvoboda11]

clang/include/clang/Driver/Options.td

473

The original patch used the following normalizer:
(normalizeFlagToValue<DependencyOutputFormat, DependencyOutputFormat::NMake>).

I wanted to remove the parenthesis and redundant type argument, hence makeFlagToValueNormalizer. This could be useful later on when normalizing flags to non-literal types.

[dexonsmith]

clang/include/clang/Driver/Options.td

473

This seems a lot cleaner, thanks!

clang/lib/Frontend/CompilerInvocation.cpp

139–145

On the switch from T to unsigned, it'd be nice to avoid a separate instantiation for each enumeration, since the code is identical... it'll just bloat compile time for no reason. Maybe:

/// The tblgen-erated code passes in a fifth parameter of an arbitrary type, but
/// denormalizeSimpleFlags never looks at it. Avoid bloating compile-time with
/// unnecessary template instantiations and just ignore it with a variadic
/// argument.
static void denormalizeSimpleFlag(
    SmallVectorImpl<const char *> &Args, const char *Spelling,
    CompilerInvocation::StringAllocator, unsigned, /*T*/...) {
  Args.push_back(Spelling);
}

Note that it'd also be nice to make this static and drop unused parameter names, as I've done here.

158–185

Please put this in an anonymous namespace.

161–162

Please drop the parameter names for TableIndex and Diags, since they aren't used.

169–172

Please declare this static.

[dexonsmith]

clang/lib/Frontend/CompilerInvocation.cpp

169–172

BTW, I suspect we can save some compile time (once there are lots of these) with:

template <
  class T,
  std::enable_if_t<
      sizeof(T) <= sizeof(uint64_t) &&
          std::is_trivially_convertible<T, uint64_t>::value &&
          std::is_trivially_convertible<uint64_t, T>::value,
      bool> = false>
FlagToValueNormalizer<uint64_t> makeFlagToValueNormalizer(T Value) {
  return FlagToValueNormalizer<uint64_t>{std::move(Value)};
}

template <
  class T,
  std::enable_if_t<
      !(sizeof(T) <= sizeof(uint64_t) &&
            std::is_trivially_convertible<T, uint64_t>::value &&
            std::is_trivially_convertible<uint64_t, T>::value),
      bool> = false>
FlagToValueNormalizer<T> makeFlagToValueNormalizer(T Value) {
  return FlagToValueNormalizer<T>{std::move(Value)};
}

It'd be worth trying if/once the compile-time begins to climb to catch the enums (that aren't strongly typed) in a single instantiation.

[jansvoboda11]

Reduce template instantiations, remove names of unused parameters

[jansvoboda11]

Addressed review feedback.

[jansvoboda11]

Rebase

[dexonsmith]

This LGTM, with one more nit to consider.

clang/lib/Frontend/CompilerInvocation.cpp

173

I don't remember seeing std::uint*_t elsewhere in LLVM code; I think we tend to just say uint64_t (getting it from the C namespace). Unless you have a specific reason for adding it I suggest dropping the qualifier.

[jansvoboda11]

Remove unnecessary std namespace

clang/include/clang/Driver/Options.td

def E : Flag<["-"], "E">, Flags<[NoXarchOption,CC1Option, FlangOption, FC1Option]>, Group<Action_Group>,
     HelpText<"Only run the preprocessor">;
 def F : JoinedOrSeparate<["-"], "F">, Flags<[RenderJoined,CC1Option]>,
     HelpText<"Add directory to framework include search path">;
 def G : JoinedOrSeparate<["-"], "G">, Flags<[NoXarchOption]>, Group<m_Group>,
     MetaVarName<"<size>">, HelpText<"Put objects of at most <size> bytes "
     "into small data section (MIPS / Hexagon)">;
 def G_EQ : Joined<["-"], "G=">, Flags<[NoXarchOption]>, Group<m_Group>, Alias<G>;
 def H : Flag<["-"], "H">, Flags<[CC1Option]>, Group<Preprocessor_Group>,
-    HelpText<"Show header includes and nesting depth">;
+    HelpText<"Show header includes and nesting depth">,
+    MarshallingInfoFlag<"DependencyOutputOpts.ShowHeaderIncludes">;
 def I_ : Flag<["-"], "I-">, Group<I_Group>,
     HelpText<"Restrict all prior -I flags to double-quoted inclusion and "
              "remove current directory from include path">;
 def I : JoinedOrSeparate<["-"], "I">, Group<I_Group>,
     Flags<[CC1Option,CC1AsOption]>, MetaVarName<"<dir>">,
     HelpText<"Add directory to include search path. If there are multiple -I "
              "options, these directories are searched in the order they are "
              "given before the standard system directories are searched. "

 def M : Flag<["-"], "M">, Group<M_Group>,
     HelpText<"Like -MD, but also implies -E and writes to stdout by default">;
 def MM : Flag<["-"], "MM">, Group<M_Group>,
     HelpText<"Like -MMD, but also implies -E and writes to stdout by default">;
 def MF : JoinedOrSeparate<["-"], "MF">, Group<M_Group>,
     HelpText<"Write depfile output from -MMD, -MD, -MM, or -M to <file>">,
     MetaVarName<"<file>">;
 def MG : Flag<["-"], "MG">, Group<M_Group>, Flags<[CC1Option]>,
-    HelpText<"Add missing headers to depfile">;
+    HelpText<"Add missing headers to depfile">,
+    MarshallingInfoFlag<"DependencyOutputOpts.AddMissingHeaderDeps">;
 def MJ : JoinedOrSeparate<["-"], "MJ">, Group<M_Group>,
     HelpText<"Write a compilation database entry per input">;
 def MP : Flag<["-"], "MP">, Group<M_Group>, Flags<[CC1Option]>,
-    HelpText<"Create phony target for each dependency (other than main file)">;
+    HelpText<"Create phony target for each dependency (other than main file)">,
+    MarshallingInfoFlag<"DependencyOutputOpts.UsePhonyTargets">;
 def MQ : JoinedOrSeparate<["-"], "MQ">, Group<M_Group>, Flags<[CC1Option]>,
     HelpText<"Specify name of main file output to quote in depfile">;
 def MT : JoinedOrSeparate<["-"], "MT">, Group<M_Group>, Flags<[CC1Option]>,
     HelpText<"Specify name of main file output in depfile">;
 def MV : Flag<["-"], "MV">, Group<M_Group>, Flags<[CC1Option]>,
-    HelpText<"Use NMake/Jom format for the depfile">;
+    HelpText<"Use NMake/Jom format for the depfile">,
+    MarshallingInfoFlag<"DependencyOutputOpts.OutputFormat", "DependencyOutputFormat::Make">,
+    Normalizer<"makeFlagToValueNormalizer(DependencyOutputFormat::NMake)">;
 def Mach : Flag<["-"], "Mach">, Group<Link_Group>;
 def O0 : Flag<["-"], "O0">, Group<O_Group>, Flags<[CC1Option, HelpHidden]>;
 def O4 : Flag<["-"], "O4">, Group<O_Group>, Flags<[CC1Option, HelpHidden]>;
 def ObjCXX : Flag<["-"], "ObjC++">, Flags<[NoXarchOption]>,
   HelpText<"Treat source input files as Objective-C++ inputs">;
 def ObjC : Flag<["-"], "ObjC">, Flags<[NoXarchOption]>,
   HelpText<"Treat source input files as Objective-C inputs">;
 def O : Joined<["-"], "O">, Group<O_Group>, Flags<[CC1Option]>;

clang/lib/Frontend/CompilerInvocation.cpp

static llvm::Optional<bool> normalizeSimpleFlag(OptSpecifier Opt,
                                                 unsigned TableIndex,
                                                 const ArgList &Args,
                                                 DiagnosticsEngine &Diags) {
   if (Args.hasArg(Opt))
     return true;
   return None;
 }
 
 static Optional<bool> normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned,
                                                   const ArgList &Args,
                                                   DiagnosticsEngine &) {
   if (Args.hasArg(Opt))
     return false;
   return None;
 }
 
-void denormalizeSimpleFlag(SmallVectorImpl<const char *> &Args,
+/// The tblgen-erated code passes in a fifth parameter of an arbitrary type, but
+/// denormalizeSimpleFlags never looks at it. Avoid bloating compile-time with
+/// unnecessary template instantiations and just ignore it with a variadic
+/// argument.
+static void denormalizeSimpleFlag(SmallVectorImpl<const char *> &Args,
                                   const char *Spelling,
-                           CompilerInvocation::StringAllocator SA,
-                           unsigned TableIndex, unsigned Value) {
+                                  CompilerInvocation::StringAllocator, unsigned,
+                                  /*T*/...) {
   Args.push_back(Spelling);
 }
 
-template <typename T, T Value>
-static llvm::Optional<T>
-normalizeFlagToValue(OptSpecifier Opt, unsigned TableIndex, const ArgList &Args,
-                     DiagnosticsEngine &Diags) {
+namespace {
+template <typename T> struct FlagToValueNormalizer {
+  T Value;
+
+  Optional<T> operator()(OptSpecifier Opt, unsigned, const ArgList &Args,
+                         DiagnosticsEngine &) {
     if (Args.hasArg(Opt))
       return Value;
     return None;
   }
+};
+} // namespace
+
+template <typename T> static constexpr bool is_int_convertible() {
+  return sizeof(T) <= sizeof(uint64_t) &&
+         std::is_trivially_constructible<T, uint64_t>::value &&
+         std::is_trivially_constructible<uint64_t, T>::value;
+}
+
+template <typename T, std::enable_if_t<is_int_convertible<T>(), bool> = false>
+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) {
   if (const Arg *A = Args.getLastArg(PosOpt, NegOpt))
     return A->getOption().matches(PosOpt);
static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
 
   return Success;
 }
 
 static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
                                       ArgList &Args) {
   Opts.OutputFile = std::string(Args.getLastArgValue(OPT_dependency_file));
   Opts.Targets = Args.getAllArgValues(OPT_MT);
-  Opts.IncludeSystemHeaders = Args.hasArg(OPT_sys_header_deps);
-  Opts.IncludeModuleFiles = Args.hasArg(OPT_module_file_deps);
-  Opts.UsePhonyTargets = Args.hasArg(OPT_MP);
-  Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.HeaderIncludeOutputFile =
       std::string(Args.getLastArgValue(OPT_header_include_file));
-  Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
   if (Args.hasArg(OPT_show_includes)) {
     // Writing both /showIncludes and preprocessor output to stdout
     // would produce interleaved output, so use stderr for /showIncludes.
     // This behaves the same as cl.exe, when /E, /EP or /P are passed.
     if (Args.hasArg(options::OPT_E) || Args.hasArg(options::OPT_P))
       Opts.ShowIncludesDest = ShowIncludesDestination::Stderr;
     else
       Opts.ShowIncludesDest = ShowIncludesDestination::Stdout;
   } else {
     Opts.ShowIncludesDest = ShowIncludesDestination::None;
   }
   Opts.DOTOutputFile = std::string(Args.getLastArgValue(OPT_dependency_dot));
   Opts.ModuleDependencyOutputDir =
       std::string(Args.getLastArgValue(OPT_module_dependency_dir));
-  if (Args.hasArg(OPT_MV))
-    Opts.OutputFormat = DependencyOutputFormat::NMake;
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
   if (!Args.hasArg(OPT_fno_sanitize_blacklist)) {
     for (const auto *A : Args.filtered(OPT_fsanitize_blacklist)) {
       StringRef Val = A->getValue();
       if (Val.find('=') == StringRef::npos)
         Opts.ExtraDeps.push_back(std::string(Val));

llvm/include/llvm/Option/OptParser.td

 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 = "(normalizeFlagToValue<unsigned, "#value#">)";
+  code Normalizer = "makeFlagToValueNormalizer("#value#")";
   code ValueMerger = "mergeMaskValue";
   code ValueExtractor = "(extractMaskValue<unsigned, decltype("#value#"), "#value#">)";
 }
 
 class MarshallingInfoBooleanFlag<code keypath, code defaultvalue, Option negopt>
   : MarshallingInfoFlag<keypath, defaultvalue> {
   bit ShouldAlwaysEmit = 1;
   string MarshallingInfoKind = "BooleanFlag";