Hmm, I see. Then I think /Zc:dllexportInlines- and /fallback should not be used at the same time. https://reviews.llvm.org/D54426
Or will you remove /fallback? 2018年11月12日(月) 19:59 Nico Weber <tha...@chromium.org>: > > On Mon, Nov 12, 2018 at 4:58 AM Hans Wennborg <h...@chromium.org> wrote: >> >> Hmm, maybe I misunderstood your initial request for this. >> >> The current implementation does what the warnings says: If the >> compiler falls back to cl.exe, /Zc:dllexportInlines- will be ignored. >> This suggests to the user that it's a bad idea, but they can go ahead >> if they want to. >> >> It sounds like you're suggesting we should always ignore >> /Zc:dllexportInlines- when used together with the /fallback flag? > > > I thought we'd always emit an error if both flags are present, actually, > something like "/dllexportInlines- is ABI-changing and not compatible with > /fallback". If you still need /fallback, you can't really use > /dllexportInlines-. > >> >> I'm >> not sure that's necessarily better.. it depends on why they're using >> /fallback I guess. If it's because the user has one file that clang-cl >> can't handle for some reason, but they still want >> /Zc:dllexportInlines- for the rest, then ignoring it would break that. >> >> In general, I'm not sure how widely used /fallback is anymore. I >> wonder if it would be possible to remove it... >> >> On Fri, Nov 9, 2018 at 9:52 PM, Nico Weber <tha...@chromium.org> wrote: >> > 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
