[OptTable] Make explicitly included options override excluded ones

When we have both explicitly included and excluded option sets, we
were excluding anything from the latter set regardless of what was in
the former. This doesn't compose well and led to an overly complicated
design around DXC options where a third flag was introduced to handle
options that overlapped between DXC and CL.

With this change we check the included options before excluding
anything from the exclude list, which allows for options that are in
multiple categories to be handled in a sensible way. This allows us to
remove CLDXCOption but should otherwise be NFC.

Differential Revision: https://reviews.llvm.org/D155729

Author: bogner

clang-tools-extra/clangd/CompileCommands.cpp

@@ -238,13 +238,10 @@ void CommandMangler::operator()(tooling::CompileCommand &Command,
   ArgList = OptTable.ParseArgs(
       llvm::ArrayRef(OriginalArgs).drop_front(), IgnoredCount, IgnoredCount,
       /*FlagsToInclude=*/
-      IsCLMode ? (driver::options::CLOption | driver::options::CoreOption |
-                  driver::options::CLDXCOption)
+      IsCLMode ? (driver::options::CLOption | driver::options::CoreOption)
                : /*everything*/ 0,
       /*FlagsToExclude=*/driver::options::NoDriverOption |
-          (IsCLMode
-               ? 0
-               : (driver::options::CLOption | driver::options::CLDXCOption)));
+          (IsCLMode ? 0 : driver::options::CLOption));
 
   llvm::SmallVector<unsigned, 1> IndicesToDrop;
   // Having multiple architecture options (e.g. when building fat binaries)
@@ -449,8 +446,6 @@ unsigned char getModes(const llvm::opt::Option &Opt) {
   if (!Opt.hasFlag(driver::options::NoDriverOption)) {
     if (Opt.hasFlag(driver::options::CLOption)) {
       Result |= DM_CL;
-    } else if (Opt.hasFlag(driver::options::CLDXCOption)) {
-      Result |= DM_CL;
     } else {
       Result |= DM_GCC;
       if (Opt.hasFlag(driver::options::CoreOption)) {

clang/include/clang/Driver/Options.h

@@ -36,9 +36,8 @@ enum ClangFlags {
   FC1Option = (1 << 15),
   FlangOnlyOption = (1 << 16),
   DXCOption = (1 << 17),
-  CLDXCOption = (1 << 18),
-  Ignored = (1 << 19),
-  TargetSpecific = (1 << 20),
+  Ignored = (1 << 18),
+  TargetSpecific = (1 << 19),
 };
 
 enum ID {

clang/include/clang/Driver/Options.td

@@ -53,10 +53,6 @@ def CC1AsOption : OptionFlag;
 // are made available when the driver is running in DXC compatibility mode.
 def DXCOption : OptionFlag;
 
-// CLDXCOption - This is a cl.exe/dxc.exe compatibility option. Options with this flag
-// are made available when the driver is running in CL/DXC compatibility mode.
-def CLDXCOption : OptionFlag;
-
 // NoDriverOption - This option should not be accepted by the driver.
 def NoDriverOption : OptionFlag;
 
@@ -6856,7 +6852,7 @@ def defsym : Separate<["-"], "defsym">,
 // clang-cl Options
 //===----------------------------------------------------------------------===//
 
-def cl_Group : OptionGroup<"<clang-cl options>">, Flags<[CLDXCOption]>,
+def cl_Group : OptionGroup<"<clang-cl options>">,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;
 
 def cl_compile_Group : OptionGroup<"<clang-cl compile-only options>">,
@@ -6865,42 +6861,45 @@ def cl_compile_Group : OptionGroup<"<clang-cl compile-only options>">,
 def cl_ignored_Group : OptionGroup<"<clang-cl ignored options>">,
   Group<cl_Group>;
 
-class CLFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
-  Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
-
-class CLDXCFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
-  Group<cl_Group>, Flags<[CLDXCOption, NoXarchOption]>;
-
-class CLCompileFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
-  Group<cl_compile_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLFlagOpts<list<OptionFlag> flags>
+  : Flags<!listconcat(flags, [NoXarchOption])>;
 
-class CLIgnoredFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
-  Group<cl_ignored_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLFlag<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_FLAG>,
+    Group<cl_Group>, CLFlagOpts<flags>;
 
-class CLJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
-  Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLCompileFlag<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_FLAG>,
+    Group<cl_compile_Group>, CLFlagOpts<flags>;
 
-class CLDXCJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
-  Group<cl_Group>, Flags<[CLDXCOption, NoXarchOption]>;
+class CLIgnoredFlag<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_FLAG>,
+    Group<cl_ignored_Group>, CLFlagOpts<flags>;
 
-class CLCompileJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
-  Group<cl_compile_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLJoined<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_JOINED>,
+    Group<cl_Group>, CLFlagOpts<flags>;
 
-class CLIgnoredJoined<string name> : Option<["/", "-"], name, KIND_JOINED>,
-  Group<cl_ignored_Group>, Flags<[CLOption, NoXarchOption, HelpHidden]>;
+class CLCompileJoined<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_JOINED>,
+    Group<cl_compile_Group>, CLFlagOpts<flags>;
 
-class CLJoinedOrSeparate<string name> : Option<["/", "-"], name,
-  KIND_JOINED_OR_SEPARATE>, Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLIgnoredJoined<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_JOINED>,
+    Group<cl_ignored_Group>, CLFlagOpts<!listconcat(flags, [HelpHidden])>;
 
-class CLDXCJoinedOrSeparate<string name> : Option<["/", "-"], name,
-  KIND_JOINED_OR_SEPARATE>, Group<cl_Group>, Flags<[CLDXCOption, NoXarchOption]>;
+class CLJoinedOrSeparate<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_JOINED_OR_SEPARATE>,
+    Group<cl_Group>, CLFlagOpts<flags>;
 
-class CLCompileJoinedOrSeparate<string name> : Option<["/", "-"], name,
-  KIND_JOINED_OR_SEPARATE>, Group<cl_compile_Group>,
-  Flags<[CLOption, NoXarchOption]>;
+class CLCompileJoinedOrSeparate<string name,
+                                list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_JOINED_OR_SEPARATE>,
+    Group<cl_compile_Group>, CLFlagOpts<flags>;
 
-class CLRemainingArgsJoined<string name> : Option<["/", "-"], name,
-  KIND_REMAINING_ARGS_JOINED>, Group<cl_Group>, Flags<[CLOption, NoXarchOption]>;
+class CLRemainingArgsJoined<string name, list<OptionFlag> flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_REMAINING_ARGS_JOINED>,
+    Group<cl_Group>, CLFlagOpts<flags>;
 
 // Aliases:
 // (We don't put any of these in cl_compile_Group as the options they alias are
@@ -6971,15 +6970,15 @@ def _SLASH_help : CLFlag<"help">, Alias<help>,
 def _SLASH_HELP : CLFlag<"HELP">, Alias<help>;
 def _SLASH_hotpatch : CLFlag<"hotpatch">, Alias<fms_hotpatch>,
   HelpText<"Create hotpatchable image">;
-def _SLASH_I : CLDXCJoinedOrSeparate<"I">,
+def _SLASH_I : CLJoinedOrSeparate<"I", [CLOption, DXCOption]>,
   HelpText<"Add directory to include search path">, MetaVarName<"<dir>">,
   Alias<I>;
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
   Alias<funsigned_char>;
 
 // The _SLASH_O option handles all the /O flags, but we also provide separate
 // aliased options to provide separate help messages.
-def _SLASH_O : CLDXCJoined<"O">,
+def _SLASH_O : CLJoined<"O", [CLOption, DXCOption]>,
   HelpText<"Set multiple /O flags at once; e.g. '/O2y-' for '/O2 /Oy-'">,
   MetaVarName<"<flags>">;
 def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>,
@@ -6992,7 +6991,7 @@ def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>,
   HelpText<"Only inline functions explicitly or implicitly marked inline">;
 def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>,
   HelpText<"Inline functions as deemed beneficial by the compiler">;
-def : CLDXCFlag<"Od">, Alias<_SLASH_O>, AliasArgs<["d"]>,
+def : CLFlag<"Od", [CLOption, DXCOption]>, Alias<_SLASH_O>, AliasArgs<["d"]>,
   HelpText<"Disable optimization">;
 def : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>,
   HelpText<"No effect">;

clang/lib/Driver/Driver.cpp

@@ -6496,21 +6496,17 @@ Driver::getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const {
   if (IsClCompatMode) {
     // Include CL and Core options.
     IncludedFlagsBitmask |= options::CLOption;
-    IncludedFlagsBitmask |= options::CLDXCOption;
     IncludedFlagsBitmask |= options::CoreOption;
   } else {
     ExcludedFlagsBitmask |= options::CLOption;
   }
   if (IsDXCMode()) {
     // Include DXC and Core options.
     IncludedFlagsBitmask |= options::DXCOption;
-    IncludedFlagsBitmask |= options::CLDXCOption;
     IncludedFlagsBitmask |= options::CoreOption;
   } else {
     ExcludedFlagsBitmask |= options::DXCOption;
   }
-  if (!IsClCompatMode && !IsDXCMode())
-    ExcludedFlagsBitmask |= options::CLDXCOption;
 
   return std::make_pair(IncludedFlagsBitmask, ExcludedFlagsBitmask);
 }

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

@@ -164,8 +164,8 @@ struct TransferableCommand {
       const unsigned OldPos = Pos;
       std::unique_ptr<llvm::opt::Arg> Arg(OptTable.ParseOneArg(
           ArgList, Pos,
-          /* Include */ ClangCLMode ? CoreOption | CLOption | CLDXCOption : 0,
-          /* Exclude */ ClangCLMode ? 0 : CLOption | CLDXCOption));
+          /* Include */ ClangCLMode ? CoreOption | CLOption : 0,
+          /* Exclude */ ClangCLMode ? 0 : CLOption));
 
       if (!Arg)
         continue;

llvm/lib/Option/OptTable.cpp

@@ -421,7 +421,8 @@ std::unique_ptr<Arg> OptTable::ParseOneArg(const ArgList &Args, unsigned &Index,
     if (FlagsToInclude && !Opt.hasFlag(FlagsToInclude))
       continue;
     if (Opt.hasFlag(FlagsToExclude))
-      continue;
+      if (!FlagsToInclude || !Opt.hasFlag(FlagsToInclude))
+        continue;
 
     // See if this option matches.
     if (std::unique_ptr<Arg> A =
@@ -650,7 +651,8 @@ void OptTable::printHelp(raw_ostream &OS, const char *Usage, const char *Title,
     if (FlagsToInclude && !(Flags & FlagsToInclude))
       continue;
     if (Flags & FlagsToExclude)
-      continue;
+      if (!FlagsToInclude || !(Flags & FlagsToInclude))
+        continue;
 
     // If an alias doesn't have a help text, show a help text for the aliased
     // option instead.