r346491 - [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback

[Nico Weber via cfe-commits]

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 <hans at chromium.org> wrote:

> On Fri, Nov 9, 2018 at 4:53 PM, Nico Weber <thakis at 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 at 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 at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181109/8eb6d9e8/attachment.html>