Hi rnk, chandlerc,
Chandler requested that I take this patch to Phabricator to make it easier to
look at, so here it goes.
(Please bear with me, this is my first attempt at uploading to this tool.)
Original email thread is here:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130722/084370.html
On Tue, Jul 23, 2013 at 3:46 PM, Reid Kleckner <[email protected]> wrote:
> LGTM, but you should get an OK from Chandler since this was basically his
> idea.
>
> On Tue, Jul 23, 2013 at 5:58 PM, Hans Wennborg <[email protected]> wrote:
>>
>> On Tue, Jul 23, 2013 at 10:56 AM, Reid Kleckner <[email protected]> wrote:
>> > I think we want to have a whitelist of "clang options" that are distinct
>> > from gcc options, rather than blacklisting a troublesome few (-MD) as
>> > GCCOptions. That seemed to be the main conclusion of the discussion on
>> > cfe-dev.
>>
>> I thought for a while that blacklisting "a troublesome few" would make
>> for a much shorter list than whitelisting "clang options", but now
>> that I look at the file again, I see that there's a very large amount
>> of options that don't apply to clang-cl, and lots of conflicts too.
>
>
>> I've attached a new patch that does the whitelist approach, and starts
>> with a pretty short whitelist. I figure we can add more stuff too it
>> as needed.
>
>
> I agree, I think the risk of exposing too much is greater than the risk of
> exposing too little, in which case we just add it and the user can work
> around it with -Xclang.
http://llvm-reviews.chandlerc.com/D1215
Files:
include/clang/Driver/Options.h
include/clang/Driver/Options.td
lib/Driver/Driver.cpp
test/Driver/cl.c
Index: include/clang/Driver/Options.h
===================================================================
--- include/clang/Driver/Options.h
+++ include/clang/Driver/Options.h
@@ -29,7 +29,9 @@
NoForward = (1 << 7),
Unsupported = (1 << 8),
CC1Option = (1 << 9),
- NoDriverOption = (1 << 10)
+ NoDriverOption = (1 << 10),
+ ClangOption = (1 << 11),
+ CLOption = (1 << 12)
};
enum ID {
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -42,6 +42,13 @@
// NoDriverOption - This option should not be accepted by the driver.
def NoDriverOption : OptionFlag;
+// ClangOption - This is a core clang option, available in both gcc and
+// cl.exe compatibility modes.
+def ClangOption : OptionFlag;
+
+// CLOption - This is a cl.exe compatibility option.
+def CLOption : OptionFlag;
+
/////////
// Groups
@@ -111,7 +118,7 @@
def ccc_debug_Group : OptionGroup<"<clang debug/development internal options>">,
Group<ccc_Group>, HelpText<"DEBUG/DEVELOPMENT OPTIONS">;
-class CCCDriverOpt : Group<ccc_driver_Group>, Flags<[DriverOption, HelpHidden]>;
+class CCCDriverOpt : Group<ccc_driver_Group>, Flags<[DriverOption, HelpHidden, ClangOption]>;
def driver_mode : Joined<["--"], "driver-mode=">, CCCDriverOpt,
HelpText<"Set the driver mode to either 'gcc', 'g++' or 'cpp'">;
def ccc_gcc_name : Separate<["-"], "ccc-gcc-name">, CCCDriverOpt,
@@ -125,7 +132,7 @@
def ccc_pch_is_pth : Flag<["-"], "ccc-pch-is-pth">, CCCDriverOpt,
HelpText<"Use pretokenized headers for precompiled headers">;
-class CCCDebugOpt : Group<ccc_debug_Group>, Flags<[DriverOption, HelpHidden]>;
+class CCCDebugOpt : Group<ccc_debug_Group>, Flags<[DriverOption, HelpHidden, ClangOption]>;
def ccc_install_dir : Separate<["-"], "ccc-install-dir">, CCCDebugOpt,
HelpText<"Simulate installation in the given directory">;
def ccc_print_options : Flag<["-"], "ccc-print-options">, CCCDebugOpt,
@@ -165,7 +172,7 @@
// Standard Options
-def _HASH_HASH_HASH : Flag<["-"], "###">, Flags<[DriverOption]>,
+def _HASH_HASH_HASH : Flag<["-"], "###">, Flags<[DriverOption, ClangOption]>,
HelpText<"Print the commands to run for this compilation">;
// The '--' option is here for the sake of compatibility with gcc, but is
// being ignored by the driver.
@@ -252,7 +259,7 @@
HelpText<"Pass <arg> to the assembler">, MetaVarName<"<arg>">;
def Xclang : Separate<["-"], "Xclang">,
HelpText<"Pass <arg> to the clang compiler">, MetaVarName<"<arg>">,
- Flags<[NoForward]>;
+ Flags<[NoForward, ClangOption]>;
def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
HelpText<"Pass <arg> to the linker">, MetaVarName<"<arg>">;
def Xpreprocessor : Separate<["-"], "Xpreprocessor">,
@@ -305,9 +312,9 @@
def dynamic : Flag<["-"], "dynamic">, Flags<[NoArgumentUnused]>;
def d_Flag : Flag<["-"], "d">, Group<d_Group>;
def d_Joined : Joined<["-"], "d">, Group<d_Group>;
-def emit_ast : Flag<["-"], "emit-ast">,
+def emit_ast : Flag<["-"], "emit-ast">, Flags<[ClangOption]>,
HelpText<"Emit Clang AST files for source inputs">;
-def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, Group<Action_Group>,
+def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, ClangOption]>, Group<Action_Group>,
HelpText<"Use the LLVM representation for assembler and object files">;
def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
def e : JoinedOrSeparate<["-"], "e">;
@@ -355,11 +362,11 @@
def fbuiltin_strcat : Flag<["-"], "fbuiltin-strcat">, Group<f_Group>;
def fbuiltin_strcpy : Flag<["-"], "fbuiltin-strcpy">, Group<f_Group>;
def fbuiltin : Flag<["-"], "fbuiltin">, Group<f_Group>;
-def fcaret_diagnostics : Flag<["-"], "fcaret-diagnostics">, Group<f_Group>;
+def fcaret_diagnostics : Flag<["-"], "fcaret-diagnostics">, Flags<[ClangOption]>, Group<f_Group>;
def fcatch_undefined_behavior : Flag<["-"], "fcatch-undefined-behavior">, Group<f_Group>;
def fclasspath_EQ : Joined<["-"], "fclasspath=">, Group<f_Group>;
-def fcolor_diagnostics : Flag<["-"], "fcolor-diagnostics">, Group<f_Group>, Flags<[CC1Option]>,
- HelpText<"Use colors in diagnostics">;
+def fcolor_diagnostics : Flag<["-"], "fcolor-diagnostics">, Group<f_Group>,
+ Flags<[CC1Option, ClangOption]>, HelpText<"Use colors in diagnostics">;
def fdiagnostics_color : Flag<["-"], "fdiagnostics-color">, Group<f_Group>;
def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, Group<f_Group>;
def fcomment_block_commands : CommaJoined<["-"], "fcomment-block-commands=">, Group<f_clang_Group>, Flags<[CC1Option]>,
@@ -374,29 +381,29 @@
def fconstexpr_steps_EQ : Joined<["-"], "fconstexpr-steps=">, Group<f_Group>;
def fconstexpr_backtrace_limit_EQ : Joined<["-"], "fconstexpr-backtrace-limit=">,
Group<f_Group>;
-def fno_crash_diagnostics : Flag<["-"], "fno-crash-diagnostics">, Group<f_clang_Group>, Flags<[NoArgumentUnused]>;
+def fno_crash_diagnostics : Flag<["-"], "fno-crash-diagnostics">, Group<f_clang_Group>, Flags<[NoArgumentUnused, ClangOption]>;
def fcreate_profile : Flag<["-"], "fcreate-profile">, Group<f_Group>;
def fcxx_exceptions: Flag<["-"], "fcxx-exceptions">, Group<f_Group>,
HelpText<"Enable C++ exceptions">, Flags<[CC1Option]>;
def fcxx_modules : Flag <["-"], "fcxx-modules">, Group<f_Group>, Flags<[NoForward]>;
def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, Group<f_Group>;
def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, Group<f_Group>;
-def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, Group<f_clang_Group>;
+def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, Flags<[ClangOption]>, Group<f_clang_Group>;
def fdiagnostics_parseable_fixits : Flag<["-"], "fdiagnostics-parseable-fixits">, Group<f_clang_Group>,
- Flags<[CC1Option]>, HelpText<"Print fix-its in machine parseable form">;
+ Flags<[CC1Option, ClangOption]>, HelpText<"Print fix-its in machine parseable form">;
def fdiagnostics_print_source_range_info : Flag<["-"], "fdiagnostics-print-source-range-info">,
- Group<f_clang_Group>, Flags<[CC1Option]>,
+ Group<f_clang_Group>, Flags<[CC1Option, ClangOption]>,
HelpText<"Print source range spans in numeric form">;
def fdiagnostics_show_option : Flag<["-"], "fdiagnostics-show-option">, Group<f_Group>,
- Flags<[CC1Option]>, HelpText<"Print option name with mappable diagnostics">;
+ Flags<[CC1Option, ClangOption]>, HelpText<"Print option name with mappable diagnostics">;
def fdiagnostics_show_name : Flag<["-"], "fdiagnostics-show-name">, Group<f_Group>,
- Flags<[CC1Option]>, HelpText<"Print diagnostic name">;
+ Flags<[CC1Option, ClangOption]>, HelpText<"Print diagnostic name">;
def fdiagnostics_show_note_include_stack : Flag<["-"], "fdiagnostics-show-note-include-stack">,
- Group<f_Group>, Flags<[CC1Option]>, HelpText<"Display include stacks for diagnostic notes">;
-def fdiagnostics_format_EQ : Joined<["-"], "fdiagnostics-format=">, Group<f_clang_Group>;
-def fdiagnostics_show_category_EQ : Joined<["-"], "fdiagnostics-show-category=">, Group<f_clang_Group>;
+ Group<f_Group>, Flags<[CC1Option, ClangOption]>, HelpText<"Display include stacks for diagnostic notes">;
+def fdiagnostics_format_EQ : Joined<["-"], "fdiagnostics-format=">, Flags<[ClangOption]>, Group<f_clang_Group>;
+def fdiagnostics_show_category_EQ : Joined<["-"], "fdiagnostics-show-category=">, Flags<[ClangOption]>, Group<f_clang_Group>;
def fdiagnostics_show_template_tree : Flag<["-"], "fdiagnostics-show-template-tree">,
- Group<f_Group>, Flags<[CC1Option]>,
+ Group<f_Group>, Flags<[CC1Option, ClangOption]>,
HelpText<"Print a template comparison tree for differing templates">;
def fdollars_in_identifiers : Flag<["-"], "fdollars-in-identifiers">, Group<f_Group>,
HelpText<"Allow '$' in identifiers">, Flags<[CC1Option]>;
@@ -406,13 +413,13 @@
def fno_dwarf_directory_asm : Flag<["-"], "fno-dwarf-directory-asm">, Group<f_Group>, Flags<[CC1Option]>;
def felide_constructors : Flag<["-"], "felide-constructors">, Group<f_Group>;
def fno_elide_type : Flag<["-"], "fno-elide-type">, Group<f_Group>,
- Flags<[CC1Option]>,
+ Flags<[CC1Option, ClangOption]>,
HelpText<"Do not elide types when printing diagnostics">;
def feliminate_unused_debug_symbols : Flag<["-"], "feliminate-unused-debug-symbols">, Group<f_Group>;
def femit_all_decls : Flag<["-"], "femit-all-decls">, Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Emit all declarations, even if unused">;
def fencoding_EQ : Joined<["-"], "fencoding=">, Group<f_Group>;
-def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group<f_Group>;
+def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group<f_Group>, Flags<[ClangOption]>;
def fexceptions : Flag<["-"], "fexceptions">, Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Enable support for exception handling">;
def fextdirs_EQ : Joined<["-"], "fextdirs=">, Group<f_Group>;
@@ -570,7 +577,7 @@
HelpText<"Disable implicit builtin knowledge of math functions">;
def fno_caret_diagnostics : Flag<["-"], "fno-caret-diagnostics">, Group<f_Group>,
Flags<[CC1Option]>;
-def fno_color_diagnostics : Flag<["-"], "fno-color-diagnostics">, Group<f_Group>;
+def fno_color_diagnostics : Flag<["-"], "fno-color-diagnostics">, Flags<[ClangOption]>, Group<f_Group>;
def fno_diagnostics_color : Flag<["-"], "fno-diagnostics-color">, Group<f_Group>;
def fno_common : Flag<["-"], "fno-common">, Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Compile common globals like normal definitions">;
@@ -580,11 +587,11 @@
def fno_cxx_exceptions: Flag<["-"], "fno-cxx-exceptions">, Group<f_Group>;
def fno_cxx_modules : Flag <["-"], "fno-cxx-modules">, Group<f_Group>, Flags<[NoForward]>;
def fno_diagnostics_fixit_info : Flag<["-"], "fno-diagnostics-fixit-info">, Group<f_Group>,
- Flags<[CC1Option]>, HelpText<"Do not include fixit information in diagnostics">;
-def fno_diagnostics_show_name : Flag<["-"], "fno-diagnostics-show-name">, Group<f_Group>;
-def fno_diagnostics_show_option : Flag<["-"], "fno-diagnostics-show-option">, Group<f_Group>;
+ Flags<[CC1Option, ClangOption]>, HelpText<"Do not include fixit information in diagnostics">;
+def fno_diagnostics_show_name : Flag<["-"], "fno-diagnostics-show-name">, Flags<[ClangOption]>, Group<f_Group>;
+def fno_diagnostics_show_option : Flag<["-"], "fno-diagnostics-show-option">, Flags<[ClangOption]>, Group<f_Group>;
def fno_diagnostics_show_note_include_stack : Flag<["-"], "fno-diagnostics-show-note-include-stack">,
- Flags<[CC1Option]>, Group<f_Group>;
+ Flags<[CC1Option, ClangOption]>, Group<f_Group>;
def fno_dollars_in_identifiers : Flag<["-"], "fno-dollars-in-identifiers">, Group<f_Group>,
HelpText<"Disallow '$' in identifiers">, Flags<[CC1Option]>;
def fno_elide_constructors : Flag<["-"], "fno-elide-constructors">, Group<f_Group>,
@@ -620,7 +627,7 @@
def fno_show_source_location : Flag<["-"], "fno-show-source-location">, Group<f_Group>,
Flags<[CC1Option]>, HelpText<"Do not include source location information with diagnostics">;
def fno_spell_checking : Flag<["-"], "fno-spell-checking">, Group<f_Group>,
- Flags<[CC1Option]>, HelpText<"Disable spell-checking">;
+ Flags<[CC1Option, ClangOption]>, HelpText<"Disable spell-checking">;
def fno_stack_protector : Flag<["-"], "fno-stack-protector">, Group<f_Group>;
def fno_strict_aliasing : Flag<["-"], "fno-strict-aliasing">, Group<f_Group>;
def fstruct_path_tbaa : Flag<["-"], "fstruct-path-tbaa">, Group<f_Group>;
@@ -713,7 +720,7 @@
"best|all; defaults to all">;
def fshow_column : Flag<["-"], "fshow-column">, Group<f_Group>, Flags<[CC1Option]>;
def fshow_source_location : Flag<["-"], "fshow-source-location">, Group<f_Group>;
-def fspell_checking : Flag<["-"], "fspell-checking">, Group<f_Group>;
+def fspell_checking : Flag<["-"], "fspell-checking">, Flags<[ClangOption]>, Group<f_Group>;
def fsigned_bitfields : Flag<["-"], "fsigned-bitfields">, Group<f_Group>;
def fsigned_char : Flag<["-"], "fsigned-char">, Group<f_Group>;
def fsplit_stack : Flag<["-"], "fsplit-stack">, Group<f_Group>;
@@ -758,7 +765,7 @@
def fterminated_vtables : Flag<["-"], "fterminated-vtables">, Alias<fapple_kext>;
def fthreadsafe_statics : Flag<["-"], "fthreadsafe-statics">, Group<f_Group>;
-def ftime_report : Flag<["-"], "ftime-report">, Group<f_Group>, Flags<[CC1Option]>;
+def ftime_report : Flag<["-"], "ftime-report">, Group<f_Group>, Flags<[CC1Option, ClangOption]>;
def ftlsmodel_EQ : Joined<["-"], "ftls-model=">, Group<f_Group>, Flags<[CC1Option]>;
def ftrapv : Flag<["-"], "ftrapv">, Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Trap on integer overflow">;
@@ -849,7 +856,7 @@
HelpText<"Include precompiled header file">, MetaVarName<"<file>">;
def init : Separate<["-"], "init">;
def install__name : Separate<["-"], "install_name">;
-def integrated_as : Flag<["-"], "integrated-as">, Flags<[DriverOption]>;
+def integrated_as : Flag<["-"], "integrated-as">, Flags<[DriverOption, ClangOption]>;
def iprefix : JoinedOrSeparate<["-"], "iprefix">, Group<clang_i_Group>, Flags<[CC1Option]>,
HelpText<"Set the -iwithprefix/-iwithprefixbefore prefix">, MetaVarName<"<dir>">;
def iquote : JoinedOrSeparate<["-"], "iquote">, Group<clang_i_Group>, Flags<[CC1Option]>,
@@ -907,7 +914,7 @@
def mios_simulator_version_min_EQ : Joined<["-"], "mios-simulator-version-min=">, Group<m_Group>;
def mkernel : Flag<["-"], "mkernel">, Group<m_Group>;
def mlinker_version_EQ : Joined<["-"], "mlinker-version=">, Flags<[NoForward]>;
-def mllvm : Separate<["-"], "mllvm">, Flags<[CC1Option]>,
+def mllvm : Separate<["-"], "mllvm">, Flags<[CC1Option, ClangOption]>,
HelpText<"Additional arguments to forward to LLVM's option processing">;
def mmacosx_version_min_EQ : Joined<["-"], "mmacosx-version-min=">, Group<m_Group>;
def mms_bitfields : Flag<["-"], "mms-bitfields">, Group<m_Group>, Flags<[CC1Option]>,
@@ -1038,7 +1045,7 @@
def no_canonical_prefixes : Flag<["-"], "no-canonical-prefixes">, Flags<[HelpHidden]>,
HelpText<"Use relative instead of canonical paths">;
def no_cpp_precomp : Flag<["-"], "no-cpp-precomp">, Group<clang_ignored_f_Group>;
-def no_integrated_as : Flag<["-"], "no-integrated-as">, Flags<[DriverOption]>;
+def no_integrated_as : Flag<["-"], "no-integrated-as">, Flags<[DriverOption, ClangOption]>;
def no_integrated_cpp : Flag<["-", "--"], "no-integrated-cpp">, Flags<[DriverOption]>;
def no_pedantic : Flag<["-", "--"], "no-pedantic">, Group<pedantic_Group>;
def no__dead__strip__inits__and__terms : Flag<["-"], "no_dead_strip_inits_and_terms">;
@@ -1309,4 +1316,16 @@
def Z_reserved_lib_cckext : Flag<["-"], "Z-reserved-lib-cckext">,
Flags<[LinkerInput, NoArgumentUnused, Unsupported]>, Group<reserved_lib_Group>;
+
+// clang-cl options.
+def cl_Group : OptionGroup<"<clang-cl options>">,
+ HelpText<"CL.EXE COMPATIBILITY OPTIONS">;
+
+class CLFlag<string name> : Option<["/", "-"], name, KIND_FLAG>,
+ Group<cl_Group>, Flags<[CLOption]>;
+
+def _QUESTION : CLFlag<"?">, Alias<help>, HelpText<"Display available options">;
+def cl_help : CLFlag<"help">, Alias<help>, HelpText<"Display available options">;
+
+
include "CC1Options.td"
Index: lib/Driver/Driver.cpp
===================================================================
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -108,9 +108,21 @@
InputArgList *Driver::ParseArgStrings(ArrayRef<const char *> ArgList) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
+
+ unsigned FlagsToInclude = 0;
+ unsigned FlagsToExclude = 0;
+
+ if (Mode == CLMode) {
+ // In CL mode, only allow core Clang options and CL options.
+ FlagsToInclude = options::ClangOption | options::CLOption;
+ } else {
+ FlagsToExclude |= options::CLOption;
+ }
+
unsigned MissingArgIndex, MissingArgCount;
InputArgList *Args = getOpts().ParseArgs(ArgList.begin(), ArgList.end(),
- MissingArgIndex, MissingArgCount);
+ MissingArgIndex, MissingArgCount,
+ FlagsToInclude, FlagsToExclude);
// Check for missing argument error.
if (MissingArgCount)
@@ -607,9 +619,21 @@
}
void Driver::PrintHelp(bool ShowHidden) const {
- getOpts().PrintHelp(
- llvm::outs(), Name.c_str(), DriverTitle.c_str(), /*Include*/ 0,
- /*Exclude*/ options::NoDriverOption | (ShowHidden ? 0 : HelpHidden));
+ unsigned FlagsToInclude = 0;
+ unsigned FlagsToExclude = options::NoDriverOption;
+
+ if (!ShowHidden)
+ FlagsToExclude |= HelpHidden;
+
+ if (Mode == CLMode) {
+ // In CL mode, only allow core Clang options and CL options.
+ FlagsToInclude = options::ClangOption | options::CLOption;
+ } else {
+ FlagsToExclude |= options::CLOption;
+ }
+
+ getOpts().PrintHelp(llvm::outs(), Name.c_str(), DriverTitle.c_str(),
+ FlagsToInclude, FlagsToExclude);
}
void Driver::PrintVersion(const Compilation &C, raw_ostream &OS) const {
Index: test/Driver/cl.c
===================================================================
--- test/Driver/cl.c
+++ test/Driver/cl.c
@@ -1,3 +1,23 @@
-// RUN: %clang_cl -fsyntax-only -c %s
+// Check that clang-cl options are not available by default.
+// RUN: %clang -help | FileCheck %s -check-prefix=DEFAULT
+// DEFAULT-NOT: CL.EXE COMPATIBILITY OPTIONS
+// DEFAULT-NOT: {{/[?]}}
+// DEFAULT-NOT: /help
+// RUN: not %clang /?
+// RUN: not %clang -?
+// RUN: not %clang /help
-void f();
+
+// Check that /? and /help are available as clang-cl options.
+// RUN: %clang_cl /? | FileCheck %s -check-prefix=CL
+// RUN: %clang_cl /help | FileCheck %s -check-prefix=CL
+// RUN: %clang_cl -help | FileCheck %s -check-prefix=CL
+// CL: CL.EXE COMPATIBILITY OPTIONS
+// CL: {{/[?]}}
+// CL: /help
+
+
+// Options available in clang that are not whitelisted are not
+// available in clang-cl.
+// DEFAULT: -fapple-kext
+// CL-NOT: -fapple-kext
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
