[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

It looks like this Wall -> Weverything alias is from my 2017 change:
https://reviews.llvm.org/rGf9b08a382cc1e0669805991849ad69efbd4703e8

The commit message doesn't link to any bugs or any other motivating material. 
All I said is that this is being done for MSVC compatibility. I remember having 
some motivating reason, but it looks like I won't be able to find it. I see 
Hans thanked me for the behavior change, maybe he recalls.

Anyway, I would approve a patch to remove the alias, but I'd want to confirm 
with Hans that he is also on board with it, since I suspect he was the one who 
talked me into adding the alias. I don't have more time to follow up on this at 
the moment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D114651#3158807 , @rnk wrote:

> I will add that multiple users have run into this problem, and I think it 
> might be more practical to consider unaliasing Wall altogether. Clang doesn't 
> emit the same set of warnings as MSVC. Anyone seriously using `clang-cl 
> /Wall` is going to receive a pile of `-WcxxNN-compat` warnings that are 
> self-contradictory, and they are going to need to curate a set of 
> clang-specific warning flags anyway.

One possible downside with that, is that you'd easily end up making 
cl-compatible build files that works nicely with clang-cl but spew warnings 
with cl.exe. (Although maybe cl.exe's -Wall produce a much safer/saner set of 
warnings?) But I don't feel strongly about it..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I will add that multiple users have run into this problem, and I think it might 
be more practical to consider unaliasing Wall altogether. Clang doesn't emit 
the same set of warnings as MSVC. Anyone seriously using `clang-cl /Wall` is 
going to receive a pile of `-WcxxNN-compat` warnings that are 
self-contradictory, and they are going to need to curate a set of 
clang-specific warning flags anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Sylvain Audi via Phabricator via cfe-commits]

saudi abandoned this revision.
saudi added a comment.

I understand, making `clang-cl -Wall` behave differently than `/Wall' is indeed 
confusing.
We'll take different steps for setting up the warnings in our Build System.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D114651#3156374 , @zero9178 wrote:

> Is the deviation from MSVC behaviour here intentional? MSVC flags allow both 
> using a `/` as well as `-` as prefix. That means Both `-Wall` and `/Wall` are 
> accepted by MSVC as well as clang-cl and in both compilers currently lead to 
> ALL warnings being emitted. So this patch would deviate from that behaviour 
> as well as add confusion as every other option behaves the same in MSVC and 
> clang-cl, regardless of whether `-` or `/` is used as prefix.

+1, while it's annoying with `-Wall` not doing the expected thing when using 
(clang-)cl, that's to be expected - cl and gcc style drivers have entirely 
different options overall, so one generally have to take care and use the right 
kind of options for them (and it's just a bit inconvenient that some options 
overlap but differ). So specialcasing an exception for this particular option 
doesn't seem worth it. Even in the context of cl-only options, I tend to always 
use the slash form, because there's less risk of mixups.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

Is the deviation from MSVC behaviour here intentional? MSVC flags allow both 
using a `/` as well as `-` as prefix. That means Both `-Wall` and `/Wall` are 
accepted by MSVC as well as clang-cl and in both compilers currently lead to 
ALL warnings being emitted. So this patch would deviate from that behaviour as 
well as add confusion as every other option behaves the same in MSVC and 
clang-cl, regardless of whether `-` or `/` is used as prefix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114651/new/

https://reviews.llvm.org/D114651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114651: [clang-cl] Expose -Wall to clang-cl by unaliasing -Wall, keeping /Wall as alias to -Weverything

[Sylvain Audi via Phabricator via cfe-commits]

saudi created this revision.
saudi added reviewers: rnk, hans.
saudi added a project: clang.
Herald added subscribers: jeroen.dobbelaere, dang.
saudi requested review of this revision.
Herald added a subscriber: cfe-commits.

Currently, for clang-cl, `-Wall` is treated identically as `/Wall` and is 
aliased to `-Weverything`.

This patch allows to expose `-Wall` to clang-cl while keeping the order of 
warning arguments, which the `-Xclang` alternative doesn't allow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114651

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -266,9 +266,11 @@
 // RUN: %clang_cl /W3 -### -- %s 2>&1 | FileCheck -check-prefix=W1 %s
 // RUN: %clang_cl /W4 -### -- %s 2>&1 | FileCheck -check-prefix=W4 %s
 // RUN: %clang_cl /Wall -### -- %s 2>&1 | FileCheck -check-prefix=Weverything 
%s
+// RUN: %clang_cl -Wall -### -- %s 2>&1 | FileCheck -check-prefix=Wall %s
 // W1: -Wall
 // W4: -WCL4
 // Weverything: -Weverything
+// Wall: -Wall
 
 // RUN: %clang_cl /WX -### -- %s 2>&1 | FileCheck -check-prefix=WX %s
 // WX: -Werror
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6152,7 +6152,8 @@
 def _SLASH_W2 : CLFlag<"W2">, HelpText<"Enable -Wall">, Alias;
 def _SLASH_W3 : CLFlag<"W3">, HelpText<"Enable -Wall">, Alias;
 def _SLASH_W4 : CLFlag<"W4">, HelpText<"Enable -Wall and -Wextra">, 
Alias;
-def _SLASH_Wall : CLFlag<"Wall">, HelpText<"Enable -Weverything">,
+def _SLASH_Wall : Option<["/"], "Wall", KIND_FLAG>, Group,
+  Flags<[CLOption, NoXarchOption]>, HelpText<"Enable -Weverything">,
   Alias, AliasArgs<["everything"]>;
 def _SLASH_WX : CLFlag<"WX">, HelpText<"Treat warnings as errors">,
   Alias, AliasArgs<["error"]>;


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -266,9 +266,11 @@
 // RUN: %clang_cl /W3 -### -- %s 2>&1 | FileCheck -check-prefix=W1 %s
 // RUN: %clang_cl /W4 -### -- %s 2>&1 | FileCheck -check-prefix=W4 %s
 // RUN: %clang_cl /Wall -### -- %s 2>&1 | FileCheck -check-prefix=Weverything %s
+// RUN: %clang_cl -Wall -### -- %s 2>&1 | FileCheck -check-prefix=Wall %s
 // W1: -Wall
 // W4: -WCL4
 // Weverything: -Weverything
+// Wall: -Wall
 
 // RUN: %clang_cl /WX -### -- %s 2>&1 | FileCheck -check-prefix=WX %s
 // WX: -Werror
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6152,7 +6152,8 @@
 def _SLASH_W2 : CLFlag<"W2">, HelpText<"Enable -Wall">, Alias;
 def _SLASH_W3 : CLFlag<"W3">, HelpText<"Enable -Wall">, Alias;
 def _SLASH_W4 : CLFlag<"W4">, HelpText<"Enable -Wall and -Wextra">, Alias;
-def _SLASH_Wall : CLFlag<"Wall">, HelpText<"Enable -Weverything">,
+def _SLASH_Wall : Option<["/"], "Wall", KIND_FLAG>, Group,
+  Flags<[CLOption, NoXarchOption]>, HelpText<"Enable -Weverything">,
   Alias, AliasArgs<["everything"]>;
 def _SLASH_WX : CLFlag<"WX">, HelpText<"Treat warnings as errors">,
   Alias, AliasArgs<["error"]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits