clang-cl Add support for two MSVC conversion warnings

[rnk]

It doesn't look like there's a really clean mapping for these warnings from MSVC to clang. I don't want to have overly broad mappings from cl warnings to clang warnings because hopefully clang's on-by-default warnings have lower false positive rates.

If you're actually investigating cleaning up clang warnings, I recommend using the gcc style -W / -Wno flags that we expose through clang-cl. These are much finer grained.

include/clang/Driver/CLCompatOptions.td

131

Doesn't this also cover integer conversion? The MSDN article makes it appear that way:
http://msdn.microsoft.com/en-us/library/th7a07tz.aspx

We could make this -Wno-conversion, but that's a pretty big hammer.

test/Driver/cl-options.c

167–175

I'd combine this to speed up the test a bit.

[• ehsan]

In D6041#4, @rnk wrote:

It doesn't look like there's a really clean mapping for these warnings from MSVC to clang. I don't want to have overly broad mappings from cl warnings to clang warnings because hopefully clang's on-by-default warnings have lower false positive rates.

If you're actually investigating cleaning up clang warnings, I recommend using the gcc style -W / -Wno flags that we expose through clang-cl. These are much finer grained.

These are two MSVC warnings that we disable for Firefox. That's why I'm implementing them in clang-cl.

include/clang/Driver/CLCompatOptions.td

131

Originally I mapped this to Wno-int-conversion too, but it seems like the support for passing more than one string to ArgList is broken (in that I was getting -Wno-float-conversion no-int-conversion passed to cc1). After applying this local LLVM patch, things worked correctly, but I'm not sure if this patch is correct or not (it doesn't break any tests, it seems.)

diff --git a/lib/Option/Arg.cpp b/lib/Option/Arg.cpp
index 4c8da58..c6120d8 100644
--- a/lib/Option/Arg.cpp
+++ b/lib/Option/Arg.cpp
@@ -107,10 +107,9 @@ void Arg::render(const ArgList &Args, ArgStringList &Output) const {
   }

  case Option::RenderJoinedStyle:
-    Output.push_back(Args.GetOrMakeJoinedArgString(
-                       getIndex(), getSpelling(), getValue(0)));
     for (unsigned i = 1, e = getNumValues(); i != e; ++i)
-      Output.push_back(getValue(i));
+      Output.push_back(Args.GetOrMakeJoinedArgString(
+                         getIndex(), getSpelling(), getValue(i)));
     break;

   case Option::RenderSeparateStyle:

[hans]

Do we really want to try supporting the individual MSVC warning flags?

It will never be a clean mapping anyway, and the diagnostics won't be very nice. For example, Clang won't print the MSVC style warning names when issuing a warning.

Can't you just use the same list of enabled/disabled warning that you use with Clang on other platforms?

[• ehsan]

In D6041#7, @hans wrote:

Do we really want to try supporting the individual MSVC warning flags?

It will never be a clean mapping anyway, and the diagnostics won't be very nice. For example, Clang won't print the MSVC style warning names when issuing a warning.

Can't you just use the same list of enabled/disabled warning that you use with Clang on other platforms?

Yeah, I suppose we could do that... Should we can this patch, then?

[hans]

In D6041#9, @ehsan wrote:

In D6041#7, @hans wrote:

Do we really want to try supporting the individual MSVC warning flags?

It will never be a clean mapping anyway, and the diagnostics won't be very nice. For example, Clang won't print the MSVC style warning names when issuing a warning.

Can't you just use the same list of enabled/disabled warning that you use with Clang on other platforms?

Yeah, I suppose we could do that... Should we can this patch, then?

I'd be in favor of that.

I think we should certainly support the broad flags, like turning all warnings off, treating them as errors, etc. But for controlling individual Clang warnings, I think it makes sense to use Clang flags.

[• ehsan]

OK, makes sense.

include/clang/Driver/CLCompatOptions.td

 def _SLASH_Wall : CLFlag<"Wall">, HelpText<"Enable -Wall">, Alias<Wall>;
 def _SLASH_WX : CLFlag<"WX">, HelpText<"Treat warnings as errors">,
   Alias<W_Joined>, AliasArgs<["error"]>;
 def _SLASH_WX_ : CLFlag<"WX-">, HelpText<"Do not treat warnings as errors">,
   Alias<W_Joined>, AliasArgs<["no-error"]>;
 def _SLASH_w_flag : CLFlag<"w">, HelpText<"Disable all warnings">, Alias<w>;
 def _SLASH_wd4005 : CLFlag<"wd4005">, Alias<W_Joined>,
   AliasArgs<["no-macro-redefined"]>;
+def _SLASH_wd4244 : CLFlag<"wd4244">, Alias<W_Joined>,
+  AliasArgs<["no-float-conversion"]>,
+  HelpText<"Disable narrowing conversion warnings">;
+def _SLASH_wd4267 : CLFlag<"wd4267">, Alias<W_Joined>,
+  AliasArgs<["no-shorten-64-to-32"]>,
+  HelpText<"Disable 64-bit to 32-bit conversion warnings">;
 def _SLASH_vd : CLJoined<"vd">, HelpText<"Control vtordisp placement">,
   Alias<vtordisp_mode_EQ>;
 def _SLASH_Z7 : CLFlag<"Z7">, Alias<gline_tables_only>;
 def _SLASH_Zi : CLFlag<"Zi">, HelpText<"Enable debug information">,
   Alias<gline_tables_only>;
 def _SLASH_Zp : CLJoined<"Zp">,
   HelpText<"Specify the default maximum struct packing alignment">,
   Alias<fpack_struct_EQ>;

test/Driver/cl-options.c

 // RUN: %clang_cl -c -### -- %s 2>&1 | FileCheck -check-prefix=DEFAULTSTRICT %s
 // DEFAULTSTRICT: "-relaxed-aliasing"
 // RUN: %clang_cl -c -fstrict-aliasing -### -- %s 2>&1 | FileCheck -check-prefix=STRICT %s
 // STRICT-NOT: "-relaxed-aliasing"
 // RUN: %clang_cl -c -fno-strict-aliasing -### -- %s 2>&1 | FileCheck -check-prefix=NOSTRICT %s
 // NOSTRICT: "-relaxed-aliasing"
 
 // For some warning ids, we can map from MSVC warning to Clang warning.
 // RUN: %clang_cl -wd4005 -### -- %s 2>&1 | FileCheck -check-prefix=wd4005 %s
 // wd4005: "-cc1"
 // wd4005: "-Wno-macro-redefined"
+// RUN: %clang_cl -wd4244 -### -- %s 2>&1 | FileCheck -check-prefix=wd4244 %s
+// wd4244: "-cc1"
+// wd4244: "-Wno-float-conversion"
+// RUN: %clang_cl -wd4267 -### -- %s 2>&1 | FileCheck -check-prefix=wd4267 %s
+// wd4267: "-cc1"
+// wd4267: "-Wno-shorten-64-to-32"
 
 // Ignored options. Check that we don't get "unused during compilation" errors.
 // (/Zs is for syntax-only)
 // RUN: %clang_cl /Zs \
 // RUN:    /analyze- \
 // RUN:    /errorReport:foo \
 // RUN:    /FS \
 // RUN:    /GF \