[clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

[takuto.ikuta]

Summary

/Zc:dllexportInlines with /fallback may cause unexpected linker error. It is better to disallow compile rather than warn for this combination.

[hans]

clang/include/clang/Basic/DiagnosticDriverKinds.td

164 ↗

(On Diff #173667)

Let's make the name shorter. I think "err_drv_dllexport_inlines_and_fallback" or something is descriptive enough.

clang/lib/Driver/ToolChains/Clang.cpp

5529 ↗

(On Diff #173667)

We know there must be a OPT__SLASH_Zc_dllexportInlines_ flag at this point (because it's checked in the if above), so I think think you need "&& dllexportInlines" below.

In the previous version, you output the getAsString() of the arg to the diagnostic. Don't you want to do this still?

[takuto.ikuta]

short diag

[takuto.ikuta]

Thank you for review!

clang/lib/Driver/ToolChains/Clang.cpp

5529 ↗

(On Diff #173667)

Ah, I don't need to call getAsString() anymore.

[hans]

lgtm

[takuto.ikuta]

rebase

[thakis]

Thanks!

include/clang/Basic/DiagnosticDriverKinds.td

def warn_drv_ycyu_different_arg_clang_cl : Warning<
   InGroup<ClangClPch>;
 def warn_drv_ycyu_no_fi_arg_clang_cl : Warning<
   "support for '%0' without a corresponding /FI flag not implemented yet; flag ignored">,
   InGroup<ClangClPch>;
 def warn_drv_yc_multiple_inputs_clang_cl : Warning<
   "support for '/Yc' with more than one source file not implemented yet; flag ignored">,
   InGroup<ClangClPch>;
 
-def warn_drv_non_fallback_argument_clang_cl : Warning<
-  "option '%0' is ignored when /fallback happens">,
-  InGroup<OptionIgnored>;
+def err_drv_dllexport_inlines_and_fallback : Error<
+  "option '/Zc:dllexportInlines-' is ABI-changing and not compatible with '/fallback'">;
 
 def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
 def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">;
 def err_drv_invalid_remap_file : Error<
     "invalid option '%0' not of the form <from-file>;<to-file>">;
 def err_drv_invalid_gcc_output_type : Error<
     "invalid output type '%0' for use with gcc tool">;
 def err_drv_cc_print_options_failure : Error<

lib/Driver/ToolChains/Clang.cpp

void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType,
   if (Arg *A = Args.getLastArg(options::OPT__SLASH_volatile_Group))
     VolatileOptionID = A->getOption().getID();
 
   if (VolatileOptionID == options::OPT__SLASH_volatile_ms)
     CmdArgs.push_back("-fms-volatile");
 
  if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
                   options::OPT__SLASH_Zc_dllexportInlines,
-                  false))
+                  false)) {
+   if (Args.hasArg(options::OPT__SLASH_fallback)) {
+     D.Diag(clang::diag::err_drv_dllexport_inlines_and_fallback);
+   } else {
     CmdArgs.push_back("-fno-dllexport-inlines");
+   }
+ }
 
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
   if (MostGeneralArg && BestCaseArg)
     D.Diag(clang::diag::err_drv_argument_not_allowed_with)
         << MostGeneralArg->getAsString(Args) << BestCaseArg->getAsString(Args);
 
   if (MostGeneralArg) {

lib/Driver/ToolChains/MSVC.cpp

CmdArgs.push_back(A->getOption().getID() == options::OPT_fthreadsafe_statics
                           ? "/Zc:threadSafeInit"
                           : "/Zc:threadSafeInit-");
   }
 
   // Pass through all unknown arguments so that the fallback command can see
   // them too.
   Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
 
-  // Warning for ignored flag.
-  if (const Arg *dllexportInlines =
-      Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-    C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
-      << dllexportInlines->getAsString(Args);
-
   // Input filename.
   assert(Inputs.size() == 1);
   const InputInfo &II = Inputs[0];
   assert(II.getType() == types::TY_C || II.getType() == types::TY_CXX);
   CmdArgs.push_back(II.getType() == types::TY_C ? "/Tc" : "/Tp");
   if (II.isFilename())
     CmdArgs.push_back(II.getFilename());
   else

test/Driver/cl-options.c

 // RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
 // ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
 
 // RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
 // NoDllExportInlines: "-fno-dllexport-inlines"
 // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
 // DllExportInlines-NOT: "-fno-dllexport-inlines"
 // RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is ABI-changing and not compatible with '/fallback'
 
 // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
 // Zi: "-gcodeview"
 // Zi: "-debug-info-kind=limited"
 
 // RUN: %clang_cl /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7 %s
 // Z7: "-gcodeview"
 // Z7: "-debug-info-kind=limited"