Ah cool. But the diagnostic is "option /dllexportInlines- is ignored when /fallback happens" – shouldn't it be ignored regardless of if fallback happens? Does that happen and the warning text is wrong?
On Fri, Nov 9, 2018 at 11:20 AM Hans Wennborg <h...@chromium.org> wrote: > On Fri, Nov 9, 2018 at 4:53 PM, Nico Weber <tha...@chromium.org> wrote: > > This only prints the warning when /fallback actually happens, right? > > No, it prints it when the fallback job is created, not when (or if) it > runs. I.e. it prints if the /fallback flag is used, regardless of > whether it actually falls back or not. This is reflected by the test > which uses -###, i.e. nothing is getting run. > > So I think we're good :-) > > > I don't > > think that's good enough: If we build a few TUs with /dllexportInlines- > and > > don't fall back, those .obj files are not abi compatible with the > cl-built > > ones. (Consider all dllexport TUs of a header being built with clang-cl > but > > all dllimport versions of the same header being built by cl – I think > this > > will cause link errors). SO I think we should error out if > > /dllexportIlnlines- /fallback is on the same line, even if the fallback > > doesn't actually happen. > > > > On Fri, Nov 9, 2018 at 8:28 AM Takuto Ikuta via cfe-commits > > <cfe-commits@lists.llvm.org> wrote: > >> > >> Author: tikuta > >> Date: Fri Nov 9 05:25:45 2018 > >> New Revision: 346491 > >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=346491&view=rev > >> Log: > >> [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used > >> with /fallback > >> > >> Summary: > >> This is followup of > >> https://reviews.llvm.org/D51340 > >> > >> Reviewers: hans, thakis > >> > >> Reviewed By: hans > >> > >> Subscribers: cfe-commits, llvm-commits > >> > >> Differential Revision: https://reviews.llvm.org/D54298 > >> > >> Modified: > >> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td > >> cfe/trunk/lib/Driver/ToolChains/MSVC.cpp > >> cfe/trunk/test/Driver/cl-options.c > >> > >> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td > >> URL: > >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=346491&r1=346490&r2=346491&view=diff > >> > >> > ============================================================================== > >> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original) > >> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Nov 9 > >> 05:25:45 2018 > >> @@ -161,6 +161,10 @@ def warn_drv_yc_multiple_inputs_clang_cl > >> "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_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< > >> > >> Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp > >> URL: > >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=346491&r1=346490&r2=346491&view=diff > >> > >> > ============================================================================== > >> --- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original) > >> +++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Fri Nov 9 05:25:45 2018 > >> @@ -669,6 +669,12 @@ std::unique_ptr<Command> visualstudio::C > >> // 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]; > >> > >> Modified: cfe/trunk/test/Driver/cl-options.c > >> URL: > >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=346491&r1=346490&r2=346491&view=diff > >> > >> > ============================================================================== > >> --- cfe/trunk/test/Driver/cl-options.c (original) > >> +++ cfe/trunk/test/Driver/cl-options.c Fri Nov 9 05:25:45 2018 > >> @@ -494,6 +494,8 @@ > >> // 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] > >> > >> // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi > %s > >> // Zi: "-gcodeview" > >> > >> > >> _______________________________________________ > >> cfe-commits mailing list > >> cfe-commits@lists.llvm.org > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits