r303798 - For Microsoft compatibility, set fno_operator_names

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Thu May 25 13:22:54 PDT 2017


I agree with Nico in retrospect. Setting -fno-operator-names is too big a
hammer. Instead we should add some MSVCCompat hacks to pretend we saw an
identifier token instead of a keyword token when these keywords are used as
declarators.

On Thu, May 25, 2017 at 10:14 AM, Nico Weber <thakis at google.com> wrote:

> 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).
>
> The regular change plus header change plus warning would be one way to
> achieve this, yes.
>
> On Thu, May 25, 2017 at 1:01 PM, Keane, Erich <erich.keane at intel.com>
> wrote:
>
>> No problem, I definitely think it was the right choice.
>>
>>
>>
>> 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?
>>
>>
>>
>> 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.
>>
>>
>>
>> Thoughts?
>>
>>
>>
>>
>>
>> *From:* Nico Weber [mailto:thakis at google.com]
>> *Sent:* Thursday, May 25, 2017 9:56 AM
>> *To:* Keane, Erich <erich.keane at intel.com>
>> *Cc:* Blower, Melanie <melanie.blower at intel.com>; rnk <rnk at chromium.org>;
>> cfe-commits <cfe-commits at lists.llvm.org>; Hans Wennborg <
>> hans at chromium.org>
>>
>> *Subject:* Re: r303798 - For Microsoft compatibility, set
>> fno_operator_names
>>
>>
>>
>> On Thu, May 25, 2017 at 12:18 PM, Keane, Erich <erich.keane at 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:* thakis at google.com [mailto:thakis at google.com] *On Behalf Of *Nico
>> Weber
>> *Sent:* Thursday, May 25, 2017 9:16 AM
>> *To:* Blower, Melanie <melanie.blower at intel.com>
>> *Cc:* rnk <rnk at chromium.org>; Keane, Erich <erich.keane at intel.com>;
>> cfe-commits <cfe-commits at lists.llvm.org>; Hans Wennborg <
>> hans at 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/R
>> elease+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\cu
>> stom\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\cu
>> stom\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.blower at intel.com>
>> wrote:
>>
>> Thanks for the feedback, working on it…
>>
>>
>>
>> *From:* Keane, Erich
>> *Sent:* Wednesday, May 24, 2017 3:47 PM
>> *To:* Nico Weber <thakis at chromium.org>; Blower, Melanie <
>> melanie.blower at intel.com>
>>
>>
>> *Cc:* cfe-commits <cfe-commits at lists.llvm.org>; rnk <rnk at chromium.org>
>>
>> *Subject:* RE: r303798 - For Microsoft compatibility, set
>> fno_operator_names
>>
>>
>>
>> Adding Melanie, the author of the patch.
>>
>>
>>
>> *From:* thakis at google.com [mailto:thakis at google.com <thakis at google.com>] *On
>> Behalf Of *Nico Weber
>> *Sent:* Wednesday, May 24, 2017 12:43 PM
>> *To:* Keane, Erich <erich.keane at intel.com>
>> *Cc:* cfe-commits <cfe-commits at lists.llvm.org>; rnk <rnk at 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 <thakis at 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 at 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/Mi
>> crosoftExtensions.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 at +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/Preproces
>> sor/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 at +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 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/20170525/10a5cbae/attachment-0001.html>


More information about the cfe-commits mailing list