Effectively revert 33c3d8a916c / D33782

[rnk]

Summary

This change would treat the token or in system headers as an
identifier, and elsewhere as an operator. As reported in
llvm.org/pr42427, many users classify their third party library headers
as "system" headers to suppress warnings. There's no clean way to
separate Windows SDK headers from user headers.

Since D103773, clang-cl's default mode completely disables C++ operator
names. That means that query.h compiles out of the box without this hack
in the pre-processor. For users who are using /permissive-, they must
set the QUERY_H_RESTRICTION_PERMISSIVE macro to rename this problematic
or field.

By reverting this change, we get to a consistent state: C++ operator
names are either enabled or disabled globally. If that breaks
compilation because legacy code is using or as an identifier, the user
has flags to work around the problem (/permissive or
/clang:-fno-operator-names).

Fixes PR42427

[erichkeane]

This seems reasonable to me. The changes for /permissive and /permissive- I think are a better way about this, and now that there is a trivial workaround (QUERY_H_RESTRICTION_PERMISSIVE) I don't really see value in the original patch anymore.

Side note: @mibintc is now retired, so you won't see a response from her.

[zero9178]

Thanks a lot for the patch :) Wanted this bit of code to be removed for quite some time.

Since D103773, clang-cl's default mode completely disables C++ operator
names. That means that query.h compiles out of the box without this hack
in the pre-processor. For users who are using /permissive-, they must
set the QUERY_H_RESTRICTION_PERMISSIVE macro to rename this problematic
or field.

Just a heads up, the patch added /permissive and /permissive-, but made neither the default as to not be a breaking change. That means that clang-cl still parses operator keywords and also does delayed template parsing, as it did before the patch.

[rnk]

In D108720#2965642, @zero9178 wrote:

Just a heads up, the patch added /permissive and /permissive-, but made neither the default as to not be a breaking change. That means that clang-cl still parses operator keywords and also does delayed template parsing, as it did before the patch.

Oh, I see. So, my change will then be a breaking change: users of clang-cl with no flags will not be able to include <query.h> from the Windows SDK out of the box. But, such users can simply add /permissive and probably carry on their way. I think I can live with that. I may add a release note for it.

clang/docs/ReleaseNotes.rst

- Attributes loaded as clang plugins which are sensitive to LangOpts must
   with clang's built-in attributes.
   If plugins want to provide richer diagnostics, they can do so when the
   attribute is handled instead, e.g. in ``handleDeclAttribute``.
   (This was changed in order to better support attributes in code completion).
 
 Windows Support
 ---------------
 
+- An MSVC compatibility workaround for C++ operator names was removed. As a
+  result, the ``<query.h>`` Windows SDK header may not compile out of the box.
+  Users should use a recent SDK and pass ``-DQUERY_H_RESTRICTION_PERMISSIVE``
+  or pass ``/permissive`` to disable C++ operator names altogether. See
+  `PR42427 <https://llvm.org/pr42427>` for more info.
+
 C Language Changes in Clang
 ---------------------------
 
 - Wide multi-characters literals such as ``L'ab'`` that would previously be interpreted as ``L'b'``
   are now ill-formed in all language modes. The motivation for this change is outlined in
   `P2362 <wg21.link/P2362>`_.
 - Support for ``__attribute__((error("")))`` and
   ``__attribute__((warning("")))`` function attributes have been added.

clang/lib/Lex/Preprocessor.cpp

IdentifierInfo *Preprocessor::LookUpIdentifierInfo(Token &Identifier) const {
   // Update the token info (identifier info and appropriate token kind).
   // FIXME: the raw_identifier may contain leading whitespace which is removed
   // from the cleaned identifier token. The SourceLocation should be updated to
   // refer to the non-whitespace character. For instance, the text "\\\nB" (a
   // line continuation before 'B') is parsed as a single tok::raw_identifier and
   // is cleaned to tok::identifier "B". After cleaning the token's length is
   // still 3 and the SourceLocation refers to the location of the backslash.
   Identifier.setIdentifierInfo(II);
-  if (getLangOpts().MSVCCompat && II->isCPlusPlusOperatorKeyword() &&
-      getSourceManager().isInSystemHeader(Identifier.getLocation()))
-    Identifier.setKind(tok::identifier);
-  else
-    Identifier.setKind(II->getTokenID());
 
   return II;
 }
 
 void Preprocessor::SetPoisonReason(IdentifierInfo *II, unsigned DiagID) {
   PoisonReasons[II] = DiagID;
 }
 

clang/test/Headers/ms-cppoperkey.cpp

-// RUN: %clang_cc1 \
-// RUN:     -fms-compatibility -x c++-cpp-output \
-// RUN:     -ffreestanding -fsyntax-only -Werror \
-// RUN:     %s -verify
-// expected-no-diagnostics
-# 1 "t.cpp"
-# 1 "query.h" 1 3
-// MS header <query.h> uses operator keyword as field name.  
-// Compile without syntax errors.
-struct tagRESTRICTION
-  {
-   union _URes 
-     {
-       int or; // Note use of cpp operator token
-     } res;
-  };

clang/test/Headers/ms-cppoperkey1.cpp

-// RUN: %clang_cc1 \
-// RUN:     -fms-compatibility -x c++-cpp-output \
-// RUN:     -ffreestanding -fsyntax-only -Werror \
-// RUN:     %s -verify
-
-
-# 1 "t.cpp"
-# 1 "query.h" 1 3 4
-// MS header <query.h> uses operator keyword as field name.  
-// Compile without syntax errors.
-struct tagRESTRICTION
-  {
-   union _URes 
-     {
-       int or; // Note use of cpp operator token
-     } res;
-  };
-   ;
-
-int aa ( int x)
-{
-  // In system header code, treat operator keyword as identifier.
-  if ( // expected-note{{to match this '('}}
-    x>1 or x<0) return 1; // expected-error{{expected ')'}}
-  else return 0;  
-}
-

clang/test/Headers/ms-cppoperkey2.cpp

-// RUN: %clang_cc1  -triple x86_64-pc-win32 -fms-compatibility \
-// RUN:     -ffreestanding -fsyntax-only -Werror  %s -verify
-// RUN: %clang_cc1 \
-// RUN:     -ffreestanding -fsyntax-only -Werror  %s -verify
-// expected-no-diagnostics
-int bb ( int x)
-{
-  // In user code, treat operator keyword as operator keyword.
-  if ( x>1 or x<0) return 1;
-  else return 0;  
-}