bogner created this revision.
bogner added reviewers: beanz, kadircet, python3kgae, hans.
Herald added subscribers: arphaman, hiraditya, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, MaskRay.
Herald added projects: clang, LLVM, clang-tools-extra.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D155729
Files:
clang-tools-extra/clangd/CompileCommands.cpp
clang/include/clang/Driver/Options.h
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
llvm/lib/Option/OptTable.cpp
Index: llvm/lib/Option/OptTable.cpp
===================================================================
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -421,7 +421,8 @@
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 @@
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.
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -164,8 +164,8 @@
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;
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -6482,7 +6482,6 @@
if (IsClCompatMode) {
// Include CL and Core options.
IncludedFlagsBitmask |= options::CLOption;
- IncludedFlagsBitmask |= options::CLDXCOption;
IncludedFlagsBitmask |= options::CoreOption;
} else {
ExcludedFlagsBitmask |= options::CLOption;
@@ -6490,13 +6489,10 @@
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);
}
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -53,10 +53,6 @@
// 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;
@@ -6844,7 +6840,7 @@
// 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>">,
@@ -6853,42 +6849,45 @@
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
@@ -6959,7 +6958,7 @@
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">,
@@ -6967,7 +6966,7 @@
// 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"]>,
@@ -6980,7 +6979,7 @@
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">;
Index: clang/include/clang/Driver/Options.h
===================================================================
--- clang/include/clang/Driver/Options.h
+++ clang/include/clang/Driver/Options.h
@@ -36,9 +36,8 @@
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 {
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -238,13 +238,10 @@
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 @@
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)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
