<div dir="ltr">Among the goals of the clang-cl project are a) being able to parse MS system headers b) helper users to write standards-compliant C++ (in particular, if your code builds with clang-cl without warnings, it'd be good if it also built with regular clang then).<div><br></div><div>The regular change plus header change plus warning would be one way to achieve this, yes.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 25, 2017 at 1:01 PM, Keane, Erich <span dir="ltr"><<a href="mailto:erich.keane@intel.com" target="_blank">erich.keane@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">





<div lang="EN-US" link="blue" vlink="purple">
<div class="m_-4040376343460842455WordSection1">
<p class="MsoNormal"><a name="m_-4040376343460842455__MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">No problem, I definitely think it was the right choice.<u></u><u></u></span></a></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">A change that contains all of what Melanie’s original patch did, plus the Header changes (plus the clang-tidy fixes that came out of this) would be acceptable?<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Also, I believe she’s working on the warning as well.  We were discussing it, and I was thinking that if we track down where the operator-identification is done
 (hints as to where that is is appreciated), that we could simply warn if the user is using one of the Operators with operators off.  The warning itself could then be suppressed if necessary.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Thoughts?<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Nico Weber [mailto:<a href="mailto:thakis@google.com" target="_blank">thakis@google.com</a>]
<br>
<b>Sent:</b> Thursday, May 25, 2017 9:56 AM<br>
<b>To:</b> Keane, Erich <<a href="mailto:erich.keane@intel.com" target="_blank">erich.keane@intel.com</a>><br>
<b>Cc:</b> Blower, Melanie <<a href="mailto:melanie.blower@intel.com" target="_blank">melanie.blower@intel.com</a>>; rnk <<a href="mailto:rnk@chromium.org" target="_blank">rnk@chromium.org</a>>; cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>>; Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span></p><div><div class="h5"><br>
<b>Subject:</b> Re: r303798 - For Microsoft compatibility, set fno_operator_names<u></u><u></u></div></div><p></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<div>
<p class="MsoNormal">On Thu, May 25, 2017 at 12:18 PM, Keane, Erich <<a href="mailto:erich.keane@intel.com" target="_blank">erich.keane@intel.com</a>> wrote:<u></u><u></u></p>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">How does chromium compiler in CL?  It seems that it would have a similar problem here…</span><u></u><u></u></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">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.
<a href="http://llvm.org/viewvc/llvm-project?view=revision&revision=212238" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?view=revision&<wbr>revision=212238</a>)<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks for the revert!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">
<a href="mailto:thakis@google.com" target="_blank">thakis@google.com</a> [mailto:<a href="mailto:thakis@google.com" target="_blank">thakis@google.com</a>]
<b>On Behalf Of </b>Nico Weber<br>
<b>Sent:</b> Thursday, May 25, 2017 9:16 AM<br>
<b>To:</b> Blower, Melanie <<a href="mailto:melanie.blower@intel.com" target="_blank">melanie.blower@intel.com</a>><br>
<b>Cc:</b> rnk <<a href="mailto:rnk@chromium.org" target="_blank">rnk@chromium.org</a>>; Keane, Erich <<a href="mailto:erich.keane@intel.com" target="_blank">erich.keane@intel.com</a>>; cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>>;
 Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<b>Subject:</b> RE: r303798 - For Microsoft compatibility, set fno_operator_names<u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">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.<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">FAILED: obj/third_party/WebKit/Source/<wbr>core/dom/dom/<wbr>CustomElementRegistry.obj
</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">E:\b\c\goma_client/gomacc.exe ../../third_party/llvm-build/<wbr>Release+Asserts/bin/clang-cl.<wbr>exe /nologo /showIncludes /FC @obj/third_party/WebKit/<wbr>Source/core/dom/dom/<wbr>CustomElementRegistry.obj.rsp
 /c ../../third_party/WebKit/<wbr>Source/core/dom/custom/<wbr>CustomElementRegistry.cpp /Foobj/third_party/WebKit/<wbr>Source/core/dom/dom/<wbr>CustomElementRegistry.obj /Fd"obj/third_party/WebKit/<wbr>Source/core/dom/dom_cc.pdb"</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">E:\b\c\b\win_clang\src\third_<wbr>party\WebKit\Source\core\dom\<wbr>custom\CustomElementRegistry.<wbr>cpp(229,7):  error: unknown type
 name 'definition'</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">  if (definition and definition->Descriptor().<wbr>LocalName() == desc.LocalName()) {</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">      ^</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">E:\b\c\b\win_clang\src\third_<wbr>party\WebKit\Source\core\dom\<wbr>custom\CustomElementRegistry.<wbr>cpp(229,18):  error: variable declaration
 in condition must have an initializer</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">  if (definition and definition->Descriptor().<wbr>LocalName() == desc.LocalName()) {</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">                 ^</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-family:"Courier New";background:white">2 errors generated.</span><u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On May 24, 2017 4:01 PM, "Blower, Melanie" <<a href="mailto:melanie.blower@intel.com" target="_blank">melanie.blower@intel.com</a>> wrote:<u></u><u></u></p>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Thanks for the feedback, working on it…</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<div>
<div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Keane, Erich
<br>
<b>Sent:</b> Wednesday, May 24, 2017 3:47 PM<br>
<b>To:</b> Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>>; Blower, Melanie <<a href="mailto:melanie.blower@intel.com" target="_blank">melanie.blower@intel.com</a>></span><u></u><u></u></p>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"><br>
<b>Cc:</b> cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>>; rnk <<a href="mailto:rnk@chromium.org" target="_blank">rnk@chromium.org</a>></span><u></u><u></u></p>
</div>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">Subject:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> RE: r303798 - For Microsoft
 compatibility, set fno_operator_names</span><u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Adding Melanie, the author of the patch.</span><u></u><u></u></p>
<p class="MsoNormal"><a name="m_-4040376343460842455_m_-5951069200774899918_m_842737365194708"></a><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">
<a href="mailto:thakis@google.com" target="_blank">thakis@google.com</a> [<a href="mailto:thakis@google.com" target="_blank">mailto:thakis@google.com</a>]
<b>On Behalf Of </b>Nico Weber<br>
<b>Sent:</b> Wednesday, May 24, 2017 12:43 PM<br>
<b>To:</b> Keane, Erich <<a href="mailto:erich.keane@intel.com" target="_blank">erich.keane@intel.com</a>><br>
<b>Cc:</b> cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>>; rnk <<a href="mailto:rnk@chromium.org" target="_blank">rnk@chromium.org</a>><br>
<b>Subject:</b> Re: r303798 - For Microsoft compatibility, set fno_operator_names</span><u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">Reviewed here: <a href="https://reviews.llvm.org/D33505" target="_blank">https://reviews.llvm.<wbr>org/D33505</a><u></u><u></u></p>
<div>
<p class="MsoNormal"><br>
Still, please make this warn.<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Wed, May 24, 2017 at 3:42 PM, Nico Weber <<a href="mailto:thakis@google.com" target="_blank">thakis@google.com</a>> wrote:<u></u><u></u></p>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">Was this reviewed somewhere?<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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).<u></u><u></u></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Wed, May 24, 2017 at 3:31 PM, Erich Keane via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<u></u><u></u></p>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal">Author: erichkeane<br>
Date: Wed May 24 14:31:19 2017<br>
New Revision: 303798<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=303798&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-<wbr>project?rev=303798&view=rev</a><br>
Log:<br>
For Microsoft compatibility, set fno_operator_names<br>
<br>
There's a Microsoft header in the Windows SDK which won't<br>
compile with clang because it uses an operator name (and)<br>
as a field name. This patch allows that file to compile by<br>
setting the option which disables operator names.<br>
The header which doesn't compile <Query.h> C:/Program Files (x86)/<br>
Windows Kits/10/include/10.0.14393.0/<wbr>um\Query.h:259:40:<br>
error: expected member name or ';' after declaration specifiers<br>
<br>
  /* [case()] */ NODERESTRICTION or;<br>
                   ~~~~~~~~~~~~~~~ ^<br>
<br>
                   1 error generated.<br>
<br>
Contributed for Melanie Blower<br>
<br>
Differential Revision:<a href="https://reviews.llvm.org/D33505" target="_blank">https://reviews.llvm.<wbr>org/D33505</a><br>
<br>
Modified:<br>
    cfe/trunk/lib/Frontend/<wbr>CompilerInvocation.cpp<br>
    cfe/trunk/test/Parser/<wbr>MicrosoftExtensions.cpp<br>
    cfe/trunk/test/Preprocessor/<wbr>cxx_oper_keyword.cpp<br>
<br>
Modified: cfe/trunk/lib/Frontend/<wbr>CompilerInvocation.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=303798&r1=303797&r2=303798&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/<wbr>Frontend/CompilerInvocation.<wbr>cpp?rev=303798&r1=303797&r2=<wbr>303798&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Frontend/<wbr>CompilerInvocation.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/<wbr>CompilerInvocation.cpp Wed May 24 14:31:19 2017<br>
@@ -1882,7 +1882,7 @@ static void ParseLangArgs(LangOptions &O<br>
   Opts.GNUKeywords = Args.hasFlag(OPT_fgnu_<wbr>keywords, OPT_fno_gnu_keywords,<br>
                                   Opts.GNUKeywords);<br>
<br>
-  if (Args.hasArg(OPT_fno_operator_<wbr>names))<br>
+  if (Args.hasArg(OPT_fno_operator_<wbr>names) || Args.hasArg(OPT_fms_<wbr>compatibility))<br>
     Opts.CXXOperatorNames = 0;<br>
<br>
   if (Args.hasArg(OPT_fcuda_is_<wbr>device))<br>
<br>
Modified: cfe/trunk/test/Parser/<wbr>MicrosoftExtensions.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=303798&r1=303797&r2=303798&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Parser/<wbr>MicrosoftExtensions.cpp?rev=<wbr>303798&r1=303797&r2=303798&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Parser/<wbr>MicrosoftExtensions.cpp (original)<br>
+++ cfe/trunk/test/Parser/<wbr>MicrosoftExtensions.cpp Wed May 24 14:31:19 2017<br>
@@ -261,9 +261,8 @@ int __identifier(else} = __identifier(fo<br>
 #define identifier_weird(x) __identifier(x<br>
 int k = identifier_weird(if)); // expected-error {{use of undeclared identifier 'if'}}<br>
<br>
-// This is a bit weird, but the alternative tokens aren't keywords, and this<br>
-// behavior matches MSVC. FIXME: Consider supporting this anyway.<br>
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token to an identifier}}<br>
+// 'and' is not an operator name with Microsoft compatibility.<br>
+extern int __identifier(and) r; // expected-error {{expected ';' after top level declarator}}<br>
<br>
 void f() {<br>
   __identifier(() // expected-error {{cannot convert '(' token to an identifier}}<br>
@@ -355,7 +354,6 @@ void TestProperty() {<br>
   ++sp.V11;<br>
 }<br>
<br>
-//expected-warning@+1 {{C++ operator 'and' (aka '&&') used as a macro name}}<br>
 #define and foo<br>
<br>
 struct __declspec(uuid("00000000-<wbr>0000-0000-C000-000000000046")) __declspec(novtable) IUnknown {};<br>
<br>
Modified: cfe/trunk/test/Preprocessor/<wbr>cxx_oper_keyword.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp?rev=303798&r1=303797&r2=303798&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Preprocessor/cxx_oper_keyword.<wbr>cpp?rev=303798&r1=303797&r2=<wbr>303798&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Preprocessor/<wbr>cxx_oper_keyword.cpp (original)<br>
+++ cfe/trunk/test/Preprocessor/<wbr>cxx_oper_keyword.cpp Wed May 24 14:31:19 2017<br>
@@ -1,5 +1,6 @@<br>
 // RUN: %clang_cc1 %s -E -verify -DOPERATOR_NAMES<br>
 // RUN: %clang_cc1 %s -E -verify -fno-operator-names<br>
+// RUN: %clang_cc1 %s    -verify -DTESTWIN -fms-compatibility<br>
<br>
 #ifndef OPERATOR_NAMES<br>
 //expected-error@+3 {{token is not a valid binary operator in a preprocessor subexpression}}<br>
@@ -29,3 +30,14 @@<br>
 #ifdef and<br>
 #warning and is defined<br>
 #endif<br>
+<br>
+#ifdef TESTWIN<br>
+// For cl compatibility, fno-operator-names is enabled by default.<br>
+int and;<br>
+int bitand;<br>
+int bitor;<br>
+int compl;<br>
+int not;<br>
+int or;<br>
+int xor;<br>
+#endif<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><u></u><u></u></p>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div></div></div>
</div>

</blockquote></div><br></div>