Sometimes it’s possible to reason with the open source community, then they 
change their code and make modifications which accommodate the various 
compilers. E.g. we could ask chromium team if they could use the operator 
symbol instead of ‘and’.

From: Nico Weber [mailto:tha...@google.com]
Sent: Thursday, May 25, 2017 12:56 PM
To: Keane, Erich <erich.ke...@intel.com>
Cc: Blower, Melanie <melanie.blo...@intel.com>; rnk <r...@chromium.org>; 
cfe-commits <cfe-commits@lists.llvm.org>; Hans Wennborg <h...@chromium.org>
Subject: Re: r303798 - For Microsoft compatibility, set fno_operator_names

On Thu, May 25, 2017 at 12:18 PM, Keane, Erich 
<erich.ke...@intel.com<mailto:erich.ke...@intel.com>> wrote:
How does chromium compiler in CL?  It seems that it would have a similar 
problem here…

That's a good question! It looks like iso646.h is included, and in MSVC that 
contains something like `#define and &&`. But clang's lib/Headers/iso646.h 
assumes that the compiler provides this and doesn't define `and`. So for the 
reland, that header would have to check if the operator name is disabled, and 
if so, define it. (Or, better, maybe we can come up with something more 
targeted for the system header, similar to e.g. 
http://llvm.org/viewvc/llvm-project?view=revision&revision=212238)

Thanks for the revert!


From: tha...@google.com<mailto:tha...@google.com> 
[mailto:tha...@google.com<mailto:tha...@google.com>] On Behalf Of Nico Weber
Sent: Thursday, May 25, 2017 9:16 AM
To: Blower, Melanie <melanie.blo...@intel.com<mailto:melanie.blo...@intel.com>>
Cc: rnk <r...@chromium.org<mailto:r...@chromium.org>>; Keane, Erich 
<erich.ke...@intel.com<mailto:erich.ke...@intel.com>>; cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>>; Hans Wennborg 
<h...@chromium.org<mailto:h...@chromium.org>>

Subject: RE: r303798 - For Microsoft compatibility, set fno_operator_names

In addition to this making clang-cl silently accept invalid code, it also 
breaks existing valid code, building chromium now fails. Let's revert and come 
up with something better asynchronously.

FAILED: obj/third_party/WebKit/Source/core/dom/dom/CustomElementRegistry.obj
E:\b\c\goma_client/gomacc.exe 
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo 
/showIncludes /FC 
@obj/third_party/WebKit/Source/core/dom/dom/CustomElementRegistry.obj.rsp /c 
../../third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp 
/Foobj/third_party/WebKit/Source/core/dom/dom/CustomElementRegistry.obj 
/Fd"obj/third_party/WebKit/Source/core/dom/dom_cc.pdb"
E:\b\c\b\win_clang\src\third_party\WebKit\Source\core\dom\custom\CustomElementRegistry.cpp(229,7):
  error: unknown type name 'definition'
  if (definition and definition->Descriptor().LocalName() == desc.LocalName()) {
      ^
E:\b\c\b\win_clang\src\third_party\WebKit\Source\core\dom\custom\CustomElementRegistry.cpp(229,18):
  error: variable declaration in condition must have an initializer
  if (definition and definition->Descriptor().LocalName() == desc.LocalName()) {
                 ^
2 errors generated.


On May 24, 2017 4:01 PM, "Blower, Melanie" 
<melanie.blo...@intel.com<mailto:melanie.blo...@intel.com>> wrote:
Thanks for the feedback, working on it…

From: Keane, Erich
Sent: Wednesday, May 24, 2017 3:47 PM
To: Nico Weber <tha...@chromium.org<mailto:tha...@chromium.org>>; Blower, 
Melanie <melanie.blo...@intel.com<mailto:melanie.blo...@intel.com>>

Cc: cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>>; rnk 
<r...@chromium.org<mailto:r...@chromium.org>>
Subject: RE: r303798 - For Microsoft compatibility, set fno_operator_names

Adding Melanie, the author of the patch.

From: tha...@google.com<mailto:tha...@google.com> [mailto:tha...@google.com] On 
Behalf Of Nico Weber
Sent: Wednesday, May 24, 2017 12:43 PM
To: Keane, Erich <erich.ke...@intel.com<mailto:erich.ke...@intel.com>>
Cc: cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>>; rnk 
<r...@chromium.org<mailto:r...@chromium.org>>
Subject: Re: r303798 - For Microsoft compatibility, set fno_operator_names

Reviewed here: https://reviews.llvm.org/D33505

Still, please make this warn.

On Wed, May 24, 2017 at 3:42 PM, Nico Weber 
<tha...@google.com<mailto:tha...@google.com>> wrote:
Was this reviewed somewhere?

Please make it so that this emits a warning. We want clang-cl to warn on 
invalid code (and in system headers warnings are suppressed).

On Wed, May 24, 2017 at 3:31 PM, Erich Keane via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Wed May 24 14:31:19 2017
New Revision: 303798

URL: http://llvm.org/viewvc/llvm-project?rev=303798&view=rev
Log:
For Microsoft compatibility, set fno_operator_names

There's a Microsoft header in the Windows SDK which won't
compile with clang because it uses an operator name (and)
as a field name. This patch allows that file to compile by
setting the option which disables operator names.
The header which doesn't compile <Query.h> C:/Program Files (x86)/
Windows Kits/10/include/10.0.14393.0/um\Query.h:259:40:
error: expected member name or ';' after declaration specifiers

  /* [case()] */ NODERESTRICTION or;
                   ~~~~~~~~~~~~~~~ ^

                   1 error generated.

Contributed for Melanie Blower

Differential Revision:https://reviews.llvm.org/D33505

Modified:
    cfe/trunk/lib/Frontend/CompilerInvocation.cpp
    cfe/trunk/test/Parser/MicrosoftExtensions.cpp
    cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=303798&r1=303797&r2=303798&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed May 24 14:31:19 2017
@@ -1882,7 +1882,7 @@ static void ParseLangArgs(LangOptions &O
   Opts.GNUKeywords = Args.hasFlag(OPT_fgnu_keywords, OPT_fno_gnu_keywords,
                                   Opts.GNUKeywords);

-  if (Args.hasArg(OPT_fno_operator_names))
+  if (Args.hasArg(OPT_fno_operator_names) || 
Args.hasArg(OPT_fms_compatibility))
     Opts.CXXOperatorNames = 0;

   if (Args.hasArg(OPT_fcuda_is_device))

Modified: cfe/trunk/test/Parser/MicrosoftExtensions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=303798&r1=303797&r2=303798&view=diff
==============================================================================
--- cfe/trunk/test/Parser/MicrosoftExtensions.cpp (original)
+++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp Wed May 24 14:31:19 2017
@@ -261,9 +261,8 @@ int __identifier(else} = __identifier(fo
 #define identifier_weird(x) __identifier(x
 int k = identifier_weird(if)); // expected-error {{use of undeclared 
identifier 'if'}}

-// This is a bit weird, but the alternative tokens aren't keywords, and this
-// behavior matches MSVC. FIXME: Consider supporting this anyway.
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token 
to an identifier}}
+// 'and' is not an operator name with Microsoft compatibility.
+extern int __identifier(and) r; // expected-error {{expected ';' after top 
level declarator}}

 void f() {
   __identifier(() // expected-error {{cannot convert '(' token to an 
identifier}}
@@ -355,7 +354,6 @@ void TestProperty() {
   ++sp.V11;
 }

-//expected-warning@+1 {{C++ operator 'and' (aka '&&') used as a macro name}}
 #define and foo

 struct __declspec(uuid("00000000-0000-0000-C000-000000000046")) 
__declspec(novtable) IUnknown {};

Modified: cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp?rev=303798&r1=303797&r2=303798&view=diff
==============================================================================
--- cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp (original)
+++ cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp Wed May 24 14:31:19 2017
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -E -verify -DOPERATOR_NAMES
 // RUN: %clang_cc1 %s -E -verify -fno-operator-names
+// RUN: %clang_cc1 %s    -verify -DTESTWIN -fms-compatibility

 #ifndef OPERATOR_NAMES
 //expected-error@+3 {{token is not a valid binary operator in a preprocessor 
subexpression}}
@@ -29,3 +30,14 @@
 #ifdef and
 #warning and is defined
 #endif
+
+#ifdef TESTWIN
+// For cl compatibility, fno-operator-names is enabled by default.
+int and;
+int bitand;
+int bitor;
+int compl;
+int not;
+int or;
+int xor;
+#endif


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org<mailto: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

Reply via email to