r241952 - Disable C++ EH by default for clang-cl and MSVC environments

Nico Weber thakis at chromium.org
Mon Jul 13 13:19:00 PDT 2015


On Fri, Jul 10, 2015 at 3:25 PM, Reid Kleckner <reid at kleckner.net> wrote:

> Author: rnk
> Date: Fri Jul 10 17:25:44 2015
> New Revision: 241952
>
> URL: http://llvm.org/viewvc/llvm-project?rev=241952&view=rev
> Log:
> Disable C++ EH by default for clang-cl and MSVC environments
>
> We don't need any more bug reports from users telling us that MSVC-style
> C++ exceptions are broken. Developers and adventurous users can still
> test the existing functionality by passing along -fexceptions to either
> clang or clang-cl.
>

Cool. Is it really worth it to make -fexceptions a core option though? One
day, exceptions will work and then /EHsc does the right thing. Until then,
regular users shouldn't use -fexceptions, and irregular users can use
-Xclang -fexceptions (which makes this look unsupported, just like it is).


>
> Modified:
>     cfe/trunk/include/clang/Driver/Options.td
>     cfe/trunk/lib/Driver/Tools.cpp
>     cfe/trunk/test/Driver/cl-eh.cpp
>
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=241952&r1=241951&r2=241952&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Fri Jul 10 17:25:44 2015
> @@ -506,7 +506,7 @@ def femit_all_decls : Flag<["-"], "femit
>    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>,
> Flags<[CoreOption]>;
> -def fexceptions : Flag<["-"], "fexceptions">, Group<f_Group>,
> Flags<[CC1Option]>,
> +def fexceptions : Flag<["-"], "fexceptions">, Group<f_Group>,
> Flags<[CC1Option, CoreOption]>,
>    HelpText<"Enable support for exception handling">;
>  def fexcess_precision_EQ : Joined<["-"], "fexcess-precision=">,
>      Group<clang_ignored_gcc_optimization_f_Group>;
>
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=241952&r1=241951&r2=241952&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Jul 10 17:25:44 2015
> @@ -2041,17 +2041,6 @@ shouldUseExceptionTablesForObjCException
>             Triple.getArch() == llvm::Triple::arm));
>  }
>
> -// exceptionSettings() exists to share the logic between -cc1 and linker
> -// invocations.
> -static bool exceptionSettings(const ArgList &Args, const llvm::Triple
> &Triple) {
> -  if (Arg *A = Args.getLastArg(options::OPT_fexceptions,
> -                               options::OPT_fno_exceptions))
> -    if (A->getOption().matches(options::OPT_fexceptions))
> -      return true;
> -
> -  return false;
> -}
> -
>  /// Adds exception related arguments to the driver command arguments.
> There's a
>  /// master flag, -fexceptions and also language specific flags to
> enable/disable
>  /// C++ and Objective-C exceptions. This makes it possible to for example
> @@ -2075,8 +2064,9 @@ static void addExceptionArgs(const ArgLi
>      return;
>    }
>
> -  // Gather the exception settings from the command line arguments.
> -  bool EH = exceptionSettings(Args, Triple);
> +  // See if the user explicitly enabled exceptions.
> +  bool EH = Args.hasFlag(options::OPT_fexceptions,
> options::OPT_fno_exceptions,
> +                         false);
>
>    // Obj-C exceptions are enabled by default, regardless of -fexceptions.
> This
>    // is not necessarily sensible, but follows GCC.
> @@ -2089,8 +2079,11 @@ static void addExceptionArgs(const ArgLi
>    }
>
>    if (types::isCXX(InputType)) {
> -    bool CXXExceptionsEnabled =
> -        Triple.getArch() != llvm::Triple::xcore && !Triple.isPS4CPU();
> +    // Disable C++ EH by default on XCore, PS4, and MSVC.
> +    // FIXME: Remove MSVC from this list once things work.
> +    bool CXXExceptionsEnabled = Triple.getArch() != llvm::Triple::xcore &&
> +                                !Triple.isPS4CPU() &&
> +                                !Triple.isWindowsMSVCEnvironment();
>      Arg *ExceptionArg = Args.getLastArg(
>          options::OPT_fcxx_exceptions, options::OPT_fno_cxx_exceptions,
>          options::OPT_fexceptions, options::OPT_fno_exceptions);
> @@ -5040,6 +5033,7 @@ struct EHFlags {
>  /// The default is /EHs-c-, meaning cleanups are disabled.
>  static EHFlags parseClangCLEHFlags(const Driver &D, const ArgList &Args) {
>    EHFlags EH;
> +
>    std::vector<std::string> EHArgs =
>        Args.getAllArgValues(options::OPT__SLASH_EH);
>    for (auto EHVal : EHArgs) {
> @@ -5061,6 +5055,15 @@ static EHFlags parseClangCLEHFlags(const
>        break;
>      }
>    }
> +
> +  // Only enable C++ exceptions if the user opts into it by passing
> +  // -fexceptions. Lots of build systems implicitly pass /EHsc when users
> don't
> +  // actually need it.
> +  // FIXME: Remove this when they work out of the box.
> +  if (!Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions,
> +                    /*default=*/false))
> +    EH = EHFlags();
> +
>    return EH;
>  }
>
> @@ -9102,7 +9105,9 @@ void XCore::Linker::ConstructJob(Compila
>    if (Args.hasArg(options::OPT_v))
>      CmdArgs.push_back("-v");
>
> -  if (exceptionSettings(Args, getToolChain().getTriple()))
> +  // Pass -fexceptions through to the linker if it was present.
> +  if (Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions,
> +                   false))
>      CmdArgs.push_back("-fexceptions");
>
>    AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs);
>
> Modified: cfe/trunk/test/Driver/cl-eh.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-eh.cpp?rev=241952&r1=241951&r2=241952&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Driver/cl-eh.cpp (original)
> +++ cfe/trunk/test/Driver/cl-eh.cpp Fri Jul 10 17:25:44 2015
> @@ -4,9 +4,13 @@
>  // Note: %s must be preceded by --, otherwise it may be interpreted as a
>  // command-line option, e.g. on Mac where %s is commonly under /Users.
>
> +// RUN: %clang_cl /c /EHsc -fexceptions -### -- %s 2>&1 | FileCheck
> -check-prefix=EHscfex %s
> +// EHscfex: "-fcxx-exceptions"
> +// EHscfex: "-fexceptions"
> +
>  // RUN: %clang_cl /c /EHsc -### -- %s 2>&1 | FileCheck -check-prefix=EHsc
> %s
> -// EHsc: "-fcxx-exceptions"
> -// EHsc: "-fexceptions"
> +// EHsc-NOT: "-fcxx-exceptions"
> +// EHsc-NOT: "-fexceptions"
>
>  // RUN: %clang_cl /c /EHs-c- -### -- %s 2>&1 | FileCheck
> -check-prefix=EHs_c_ %s
>  // EHs_c_-NOT: "-fcxx-exceptions"
> @@ -16,14 +20,14 @@
>  // EHs_EHc_-NOT: "-fcxx-exceptions"
>  // EHs_EHc_-NOT: "-fexceptions"
>
> -// RUN: %clang_cl /c /EHs- /EHs -### -- %s 2>&1 | FileCheck
> -check-prefix=EHs_EHs %s
> +// RUN: %clang_cl /c /EHs- /EHs -fexceptions -### -- %s 2>&1 | FileCheck
> -check-prefix=EHs_EHs %s
>  // EHs_EHs: "-fcxx-exceptions"
>  // EHs_EHs: "-fexceptions"
>
> -// RUN: %clang_cl /c /EHs- /EHsa -### -- %s 2>&1 | FileCheck
> -check-prefix=EHs_EHa %s
> +// RUN: %clang_cl /c /EHs- /EHsa -fexceptions -### -- %s 2>&1 | FileCheck
> -check-prefix=EHs_EHa %s
>  // EHs_EHa: "-fcxx-exceptions"
>  // EHs_EHa: "-fexceptions"
>
> -// RUN: %clang_cl /c /EHinvalid -### -- %s 2>&1 | FileCheck
> -check-prefix=EHinvalid %s
> +// RUN: %clang_cl /c /EHinvalid -fexceptions -### -- %s 2>&1 | FileCheck
> -check-prefix=EHinvalid %s
>  // EHinvalid: error: invalid value 'invalid' in '/EH'
>  // EHinvalid-NOT: error:
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150713/63d77a8e/attachment.html>


More information about the cfe-commits mailing list